Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: removed old flags for airgap instances #36609

Merged
merged 19 commits into from
Oct 7, 2024
Merged

chore: removed old flags for airgap instances #36609

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9c009d8
chore: removed occurrences of old flags
AmanAgarwal041 Sep 11, 2024
972c1b9
fix: merge from release
AmanAgarwal041 Sep 25, 2024
c410ba8
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Sep 26, 2024
1786523
fix: failed test cases with feature flags removal
AmanAgarwal041 Sep 30, 2024
b92a9d0
fix: merge from release
AmanAgarwal041 Sep 30, 2024
63010c6
fix: failed tests
AmanAgarwal041 Sep 30, 2024
92ce628
fix: reduced limited test and other fixed test cases
AmanAgarwal041 Sep 30, 2024
7ffad1b
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Oct 1, 2024
b956497
fix: reverting limited test file
AmanAgarwal041 Oct 1, 2024
a02168b
fix: test cases
AmanAgarwal041 Oct 2, 2024
c2158d8
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Oct 2, 2024
2bdc8f5
fix: reverting limited test file
AmanAgarwal041 Oct 2, 2024
1089048
fix: test failure
AmanAgarwal041 Oct 2, 2024
90a54af
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Oct 2, 2024
19b10cf
fix: cypress fails
AmanAgarwal041 Oct 3, 2024
9461218
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Oct 3, 2024
b30c9bb
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Oct 4, 2024
126121f
fix: add new row spec fix
AmanAgarwal041 Oct 4, 2024
bfbf6bb
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
AmanAgarwal041 Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSlashCommand_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import { featureFlagIntercept } from "../../../../support/Objects/FeatureFlags";
import {
agHelper,
locators,
entityExplorer,
propPane,
draggableWidgets,
apiPage,
entityItems,
homePage,
assertHelper,
} from "../../../../support/Objects/ObjectsCore";
Expand All @@ -15,12 +10,6 @@ import EditorNavigation, {
} from "../../../../support/Pages/EditorNavigation";

describe("Property Pane Suggestions", { tags: ["@tag.JS"] }, () => {
before(() => {
featureFlagIntercept({
ab_learnability_ease_of_initial_use_enabled: true,
});
});

before(function () {
agHelper.ClearLocalStorageCache();
});
Expand Down
3 changes: 2 additions & 1 deletion app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const testdata = require("../../../../fixtures/testdata.json");
import {
agHelper,
entityExplorer,
table,
} from "../../../../support/Objects/ObjectsCore";

describe(
Expand All @@ -32,7 +33,7 @@ describe(

it("2. validation of data displayed in input widgets based on sorting", function () {
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget);

table.ExpandIfCollapsedSection("rowselection");
cy.testJsontext("defaultselectedrow", "0");
cy.get(".draggable-header").contains("id").click({ force: true });
cy.wait(1000);
Expand Down
1 change: 1 addition & 0 deletions .../cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe(

it("2. validation of data displayed in input widgets based on selected row", function () {
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget);
_.table.ExpandIfCollapsedSection("rowselection");
cy.testJsontext("defaultselectedrow", "2");
cy.readTableV2dataPublish("2", "0").then((tabData) => {
const tabValue = tabData;
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_ClientSide_Search_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe(
cy.readTableV2dataPublish("0", "0").then((tabData) => {
expect(tabData).to.eq("#2");
});
_.table.ExpandIfCollapsedSection("search\\&filters");
// Input onsearchtextchanged control
cy.get(".t--property-control-onsearchtextchanged .t--js-toggle")
.first()
Comment on lines 24 to 26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remember to use locator variables and data- attributes instead of CSS selectors*

As we've learned, using locator variables and data-* attributes enhances the maintainability and reliability of our tests. Relying on plain strings and CSS paths can make the tests brittle and harder to maintain. Let's replace the CSS selectors with appropriate locator variables.

Here's how you might adjust the code:

-cy.get(".t--property-control-onsearchtextchanged .t--js-toggle")
-  .first()
-  .click();
+cy.get(commonlocators.onSearchTextChangedToggle)
+  .click();

If commonlocators.onSearchTextChangedToggle doesn't exist, please consider adding it to your locators file.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Input onsearchtextchanged control
cy.get(".t--property-control-onsearchtextchanged .t--js-toggle")
.first()
// Input onsearchtextchanged control
cy.get(commonlocators.onSearchTextChangedToggle)
.click();

Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe(
);
// validation of data displayed in input widgets based on search value set
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget);
Comment on lines 27 to 29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Time for some homework revisions, students!

Let's improve our test by avoiding the use of cy.wait("@updateLayout"). Instead, we can use Cypress's built-in retry-ability to wait for elements to be in the correct state. Here's an example of how we could rewrite this:

cy.get(".t--property-control-allowsearching input").should('be.visible').click({ force: true });

This way, we're letting Cypress automatically wait for the element to be visible before clicking, which is more reliable than using a fixed wait time.

Also applies to: 33-34

_.table.ExpandIfCollapsedSection("search\\&filters");
cy.get(".t--property-control-allowsearching input").click({
force: true,
});
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe(
//Validate Table V2 with API data and then add a column
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget);
propPane.UpdatePropertyFieldValue("Table data", "{{Api1.data}}");
table.ExpandIfCollapsedSection("pagination");
cy.CheckWidgetProperties(commonlocators.serverSidePaginationCheckbox);
cy.get(`.t--widget-tablewidgetv2 .page-item`)
.first()
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ describe(
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget, {}, [
"Container3",
]);
table.ExpandIfCollapsedSection("rowselection");
cy.testJsontext("defaultselectedrow", "2");
cy.wait("@updateLayout");
cy.get(commonlocators.TableV2Row)
Expand Down
5 changes: 0 additions & 5 deletions ...nt/cypress/e2e/Regression/ClientSide/OneClickBinding/JSEnabledByDefaultExperiment_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import OneClickBindingLocator from "../../../../locators/OneClickBindingLocator";
import { featureFlagIntercept } from "../../../../support/Objects/FeatureFlags";
import {
agHelper,
apiPage,
Expand All @@ -23,10 +22,6 @@ describe(
() => {
let datasourceName: string;
before(() => {
featureFlagIntercept({
rollout_js_enabled_one_click_binding_enabled: true,
});

dataSources.CreateDataSource("Postgres");

cy.get("@dsName").then((dsName) => {
Expand Down
6 changes: 6 additions & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe(

it("1.1. should test that allow Add new row property is present", () => {
cy.openPropertyPane("tablewidgetv2");
table.ExpandIfCollapsedSection("addingarow");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use Locator Variables Instead of Plain Strings

It's important to use locator variables for selectors to enhance maintainability and readability. In the line:

table.ExpandIfCollapsedSection("addingarow");

Replace the plain string "addingarow" with a locator variable. This aligns with best practices for Cypress code.

Apply this change to use a locator variable:

-table.ExpandIfCollapsedSection("addingarow");
+table.ExpandIfCollapsedSection(propPane._addingARow);

Ensure that propPane._addingARow is defined appropriately in your locators file.

cy.get(".t--property-control-allowaddingarow").should("exist");
cy.get(".t--property-control-allowaddingarow input").should("exist");
cy.get(".t--add-new-row").should("not.exist");
Expand Down Expand Up @@ -151,6 +152,11 @@ describe(
it("1.7. should not hide the header section when add new row button is enabled and another header element is disabled", () => {
cy.get(".t--discard-new-row").click({ force: true });
//disable all header widgets for the table
["pagination", "search\\&filters", "general", "addingarow"].forEach(
(val) => {
table.ExpandIfCollapsedSection(val);
},
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Expand Sections Using Locator Variables

In the loop:

["pagination", "search\\&filters", "general", "addingarow"].forEach(
  (val) => {
    table.ExpandIfCollapsedSection(val);
  },
);

Ensure the val corresponds to locator variables rather than plain strings.

Refactor as:

-["pagination", "search\\&filters", "general", "addingarow"].forEach(
+const sections = [
+  propPane._paginationSection,
+  propPane._searchAndFiltersSection,
+  propPane._generalSection,
+  propPane._addingARowSection,
+];
+sections.forEach((section) => {
   table.ExpandIfCollapsedSection(section);
 });

Define the locator variables in your locators file.

[
"Show pagination",
"Allow searching",
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe(

it("1. Check if the selectedRowIndices does not contain 2d array", function () {
EditorNavigation.SelectEntityByName("Table1", EntityType.Widget);
table.ExpandIfCollapsedSection("rowselection");
propPane.TogglePropertyState("Enable multi-row selection", "On"); //Enable Multi row select

propPane.UpdatePropertyFieldValue("Default selected rows", "[1]"); //Change the value of default selected row
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe(
entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250);
//propPane.EnterJSContext("Table data", JSON.stringify(this.dataSet.TableInput));
// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use locator variables instead of plain strings for selectors

It's important to follow best practices in your test code. Using locator variables instead of plain strings enhances maintainability and readability. Let's replace the plain string with a predefined locator variable.

Apply this diff to make the improvement:

-table.ExpandIfCollapsedSection("search\\&filters");
+table.ExpandIfCollapsedSection(locators.searchAndFiltersSection);

Committable suggestion was skipped due to low confidence.

agHelper.GetNClick(".t--property-control-allowfiltering input");
table.AddSampleTableData();
//propPane.EnterJSContext("Table Data", JSON.stringify(this.dataSet.TableInput));
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe(
it("1. Verify Table Filter for 'empty'", function () {
entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250);
// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
agHelper.GetNClick(".t--property-control-allowfiltering input");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remember to use locator variables instead of plain strings for selectors

At line 20, we have:

agHelper.GetNClick(".t--property-control-allowfiltering input");

Using plain string selectors can make our tests brittle and harder to maintain. It's important to use locator variables, which promote reuse and make changes easier to manage.

Here's how you can update the code:

+const allowFilteringToggle = ".t--property-control-allowfiltering input";
...
-agHelper.GetNClick(".t--property-control-allowfiltering input");
+agHelper.GetNClick(allowFilteringToggle);

table.AddSampleTableData();
propPane.UpdatePropertyFieldValue(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using agHelper.Sleep();—wait for specific conditions instead

In line 22, the code uses:

agHelper.Sleep(2000); //table to filter & records to disappear

Relying on fixed delays can lead to flaky tests because the actual load time may vary. Instead, we should wait for a specific element or condition that indicates the table has finished filtering.

Consider replacing the sleep with a wait for the table to become empty:

table.WaitForTableEmpty("v2");

This ensures the test proceeds only after the table is fully filtered.

Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe(
entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250);
//propPane.EnterJSContext("Table data", JSON.stringify(this.dataSet.TableInput));
// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
agHelper.GetNClick(".t--property-control-allowfiltering input");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Let's improve selector usage by adopting locator variables

In line 21, you're directly using a CSS selector ".t--property-control-allowfiltering input". To make our tests more robust and maintainable, it's important to use locator variables and leverage data-* attributes for selectors, as highlighted in our coding guidelines. This practice enhances readability and eases future updates to the code.

To implement this improvement, let's define a locator variable using a data-* attribute:

Add the following in your locators file (e.g., locators.js):

export const allowFilteringToggle = '[data-cy="t--property-control-allowfiltering"] input';

Then, update line 21 as shown below:

- agHelper.GetNClick(".t--property-control-allowfiltering input");
+ agHelper.GetNClick(allowFilteringToggle);

table.AddSampleTableData();
//propPane.EnterJSContext("Table Data", JSON.stringify(this.dataSet.TableInput));
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe(
it("1. Verify Full table data - download csv and download Excel", function () {
entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250);
// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
agHelper.GetNClick(".t--property-control-allowfiltering input");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Let's use locator variables with data attributes instead of CSS selectors

Remember, to make our tests more maintainable and robust, we should use locator variables that utilize data-* attributes rather than CSS selectors like "search\\&filters" and ".t--property-control-allowfiltering input" used in lines 20-21.

table.AddSampleTableData();
propPane.UpdatePropertyFieldValue(
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe(
before(() => {
entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250);
// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
agHelper.GetNClick(".t--property-control-allowfiltering input");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remember to use locator variables instead of plain strings for selectors.

In line 32, you've used a plain string selector in agHelper.GetNClick(".t--property-control-allowfiltering input"). It's important to use locator variables for selectors to maintain consistency and ease future updates.

Let's define a locator variable for .t--property-control-allowfiltering input and replace the plain string. This practice enhances code readability and adherence to our coding standards.

propPane.EnterJSContext("Table data", JSON.stringify(data));
assertHelper.AssertNetworkStatus("@updateLayout");
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe(

it("1. Test multi select column shows when enable Multirowselection is true", function () {
cy.openPropertyPane("tablewidgetv2");
_.table.ExpandIfCollapsedSection("rowselection");
cy.get(widgetsPage.toggleEnableMultirowselection)
.first()
.click({ force: true });
Expand Down
4 changes: 4 additions & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ describe(
cy.get(commonlocators.editPropBackButton).click();
cy.openPropertyPane("tablewidgetv2");
// Confirm if isSortable is true
_.table.ExpandIfCollapsedSection("sorting");

cy.get(commonlocators.isSortable).should("be.checked");
// Publish App
_.deployMode.DeployApp();
Expand Down Expand Up @@ -124,6 +126,7 @@ describe(
"Table data",
`{{[{step: 1, task: 1}]}}`,
);
_.table.ExpandIfCollapsedSection("search\\&filters");
cy.get(".t--property-control-allowfiltering input").click();
cy.editColumn("step");
cy.get(".t--table-filter-toggle-btn").click();
Expand Down Expand Up @@ -237,6 +240,7 @@ describe(
it("7. should check that adding cyclic dependency in the table doesn't crash the app", () => {
//_.deployMode.NavigateBacktoEditor();
cy.openPropertyPane("tablewidgetv2");
_.table.ExpandIfCollapsedSection("rowselection");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider Using Locator Variables Instead of Plain Strings

Using plain strings like _.table.ExpandIfCollapsedSection("rowselection"); can make your tests harder to maintain, especially if the string is used in multiple places or may change in the future. By defining locator variables or constants, you centralize these values and make updates easier.

For example:

const rowSelectionSection = "rowselection";
_.table.ExpandIfCollapsedSection(rowSelectionSection);

This approach improves readability and maintainability of your tests.


cy.updateCodeInput(
".t--property-control-defaultselectedrow",
Expand Down
1 change: 1 addition & 0 deletions ...client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe(
before(() => {
_.entityExplorer.DragDropWidgetNVerify(_.draggableWidgets.TABLE);
// turn on filtering for the table - it is disabled by default in this PR(#34593)
_.table.ExpandIfCollapsedSection("search\\&filters");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remember to use locator variables instead of plain strings for locators

Dear student, to maintain code consistency and adhere to our best practices, please use locator variables instead of plain strings when specifying locators. This enhances the maintainability and readability of the code.

You can apply the following change to fix this:

- _.table.ExpandIfCollapsedSection("search\\&filters");
+ _.table.ExpandIfCollapsedSection(_.locators.searchAndFiltersSection);

If a locator variable for "search\\&filters" does not exist yet, kindly define it accordingly.

Committable suggestion was skipped due to low confidence.

_.agHelper.GetNClick(".t--property-control-allowfiltering input");
_.propPane.EnterJSContext("Table data", tableData);
cy.editColumn("completed");
Expand Down
2 changes: 2 additions & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
propPane,
deployMode,
draggableWidgets,
table,
} from "../../../../../../support/Objects/ObjectsCore";

const tableData = `[
Expand Down Expand Up @@ -41,6 +42,7 @@ describe(
before(() => {
entityExplorer.DragDropWidgetNVerify(draggableWidgets.TABLE);
// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
agHelper.GetNClick(".t--property-control-allowfiltering input");
propPane.EnterJSContext("Table data", tableData);
cy.editColumn("completed");
Expand Down
1 change: 1 addition & 0 deletions app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe(
entityExplorer.DragDropWidgetNVerify(draggableWidgets.TABLE, 300, 300);

// turn on filtering for the table - it is disabled by default in this PR(#34593)
table.ExpandIfCollapsedSection("search\\&filters");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remember to use locator variables instead of plain strings.

To ensure consistency and maintainability, it's important to use locator variables for selectors. Instead of passing a plain string "search\\&filters", consider defining and using a locator variable for the "search&filters" section.

agHelper.GetNClick(".t--property-control-allowfiltering input");

// Create SQL data-source
Expand Down
24 changes: 23 additions & 1 deletion app/client/cypress/limited-tests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
# To run only limited tests - give the spec names in below format:
cypress/e2e/Regression/ClientSide/Templates/Fork_Template_spec.js
# Not fixed
cypress/e2e/Regression/ClientSide/Binding/TableV2TextPagination_spec.js
cypress/e2e/Regression/ClientSide/DynamicHeight/JsonForm_spec.ts
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_PropertyPane_2_spec.js
#Fixed
cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js
cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js
cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js
cypress/e2e/Regression/ClientSide/Binding/TableV2_ClientSide_Search_spec.js
cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js
cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts

# For running all specs - uncomment below:
#cypress/e2e/**/**/*
Expand Down
1 change: 0 additions & 1 deletion app/client/cypress/support/Objects/FeatureFlags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import produce from "immer";

const defaultFlags = {
release_side_by_side_ide_enabled: true,
ab_learnability_discoverability_collapse_all_except_data_enabled: false, // remove this flag from here when it's removed from code
rollout_remove_feature_walkthrough_enabled: false, // remove this flag from here when it's removed from code
};

Expand Down
16 changes: 16 additions & 0 deletions app/client/cypress/support/Pages/Table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,4 +888,20 @@ export class Table {
isoFormat,
};
}

public ExpandIfCollapsedSection(sectionName: string) {
cy.get(`.t--property-pane-section-collapse-${sectionName}`)
.scrollIntoView()
.then(($element) => {
cy.wrap($element)
.siblings(".bp3-collapse")
.then(($sibling) => {
const siblingHeight = $sibling.height(); // Get the height of the sibling element

if (!siblingHeight) {
$element.click();
}
});
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Now, let's turn our attention to the ExpandIfCollapsedSection method.

This method is designed to expand a collapsed section in the property pane if it's not already expanded. While the intention is good, there are a few areas where we can improve:

  1. The method uses Cypress commands directly, which might not be consistent with the rest of the class that seems to use an agHelper for most Cypress interactions.

  2. The method doesn't handle potential errors, such as if the element is not found.

  3. The logic for determining if a section is collapsed (by checking the height of a sibling element) might be fragile if the DOM structure changes.

Let's consider refactoring this method to make it more robust and consistent with the rest of the class. Here's a suggested improvement:

- public ExpandIfCollapsedSection(sectionName: string) {
-   cy.get(`.t--property-pane-section-collapse-${sectionName}`)
-     .scrollIntoView()
-     .then(($element) => {
-       cy.wrap($element)
-         .siblings(".bp3-collapse")
-         .then(($sibling) => {
-           const siblingHeight = $sibling.height(); // Get the height of the sibling element
-
-           if (!siblingHeight) {
-             $element.click();
-           }
-         });
-     });
- }
+ public ExpandIfCollapsedSection(sectionName: string) {
+   return this.agHelper.GetElement(`.t--property-pane-section-collapse-${sectionName}`)
+     .then(($element) => {
+       if ($element.siblings(".bp3-collapse").height() === 0) {
+         this.agHelper.ScrollTo(`.t--property-pane-section-collapse-${sectionName}`);
+         this.agHelper.ClickButton($element);
+       }
+     })
+     .catch((error) => {
+       this.agHelper.LogErrorAndFail(`Failed to expand section ${sectionName}: ${error}`);
+     });
+ }

This refactored version:

  1. Uses the agHelper for consistency with the rest of the class.
  2. Handles potential errors with a catch block.
  3. Simplifies the logic for checking if the section is collapsed.
  4. Uses more descriptive method names from the agHelper.

Remember, class, consistency and error handling are key to maintaining a robust test suite!

Committable suggestion was skipped due to low confidence.

}
2 changes: 0 additions & 2 deletions app/client/cypress/support/e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ before(function () {
if (!Cypress.currentTest.titlePath[0].includes(WALKTHROUGH_TEST_PAGE)) {
// Adding key FEATURE_WALKTHROUGH (which is used to check if the walkthrough is already shown to the user or not) for non walkthrough cypress tests (to not show walkthrough)
addIndexedDBKey(FEATURE_WALKTHROUGH_INDEX_KEY, {
ab_ds_binding_enabled: true,
ab_ds_schema_enabled: true,
binding_widget: true,
});
}
Expand Down
9 changes: 0 additions & 9 deletions app/client/src/ce/entities/FeatureFlag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ export const FEATURE_FLAG = {
"release_drag_drop_building_blocks_enabled",
release_table_cell_label_value_enabled:
"release_table_cell_label_value_enabled",
rollout_js_enabled_one_click_binding_enabled:
"rollout_js_enabled_one_click_binding_enabled",
rollout_side_by_side_enabled: "rollout_side_by_side_enabled",
ab_learnability_ease_of_initial_use_enabled:
"ab_learnability_ease_of_initial_use_enabled",
ab_learnability_discoverability_collapse_all_except_data_enabled:
"ab_learnability_discoverability_collapse_all_except_data_enabled",
release_layout_conversion_enabled: "release_layout_conversion_enabled",
release_anvil_toggle_enabled: "release_anvil_toggle_enabled",
release_ide_animations_enabled: "release_ide_animations_enabled",
Expand Down Expand Up @@ -74,10 +68,7 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = {
ab_appsmith_ai_query: false,
release_actions_redesign_enabled: false,
rollout_remove_feature_walkthrough_enabled: true,
rollout_js_enabled_one_click_binding_enabled: true,
rollout_side_by_side_enabled: false,
ab_learnability_ease_of_initial_use_enabled: true,
ab_learnability_discoverability_collapse_all_except_data_enabled: true,
release_layout_conversion_enabled: false,
release_anvil_toggle_enabled: false,
release_ide_animations_enabled: false,
Expand Down
9 changes: 0 additions & 9 deletions app/client/src/components/editorComponents/CodeEditor/codeEditorUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ import type { WidgetEntity, ActionEntity } from "ee/entities/DataTree/types";
import { trim } from "lodash";
import { getDynamicStringSegments } from "utils/DynamicBindingUtils";
import { EditorSize } from "./EditorConfig";
import { selectFeatureFlagCheck } from "ee/selectors/featureFlagsSelectors";
import store from "store";
import { FEATURE_FLAG } from "ee/entities/FeatureFlag";
import { SlashCommandMenuOnFocusWidgetProps } from "./constants";

// TODO: Fix this the next time the file is edited
Expand Down Expand Up @@ -168,13 +165,7 @@ export function shouldShowSlashCommandMenu(
widgetType: string = "",
propertyPath: string = "",
) {
const isEaseOfUseFlagEnabled = selectFeatureFlagCheck(
store.getState(),
FEATURE_FLAG.ab_learnability_ease_of_initial_use_enabled,
);

return (
!!isEaseOfUseFlagEnabled &&
!!SlashCommandMenuOnFocusWidgetProps[widgetType] &&
SlashCommandMenuOnFocusWidgetProps[widgetType].includes(propertyPath)
);
Expand Down
4 changes: 0 additions & 4 deletions ...t/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12757,12 +12757,10 @@ export const defaultAppState = {
release_datasource_environments_enabled: false,
release_appnavigationlogoupload_enabled: false,
release_embed_hide_share_settings_enabled: false,
ab_gsheet_schema_enabled: true,
release_table_serverside_filtering_enabled: false,
license_branding_enabled: false,
license_sso_saml_enabled: false,
license_sso_oidc_enabled: false,
ab_mock_mongo_schema_enabled: true,
license_private_embeds_enabled: false,
release_show_publish_app_to_community_enabled: false,
license_gac_enabled: false,
Expand All @@ -12776,7 +12774,6 @@ export const defaultAppState = {
release_side_by_side_ide_enabled: false,
release_global_add_pane_enabled: false,
license_git_unlimited_repo_enabled: false,
ab_ds_binding_enabled: true,
ask_ai_js: false,
license_connection_pool_size_enabled: false,
ab_ai_js_function_completion_enabled: true,
Expand All @@ -12786,7 +12783,6 @@ export const defaultAppState = {
license_audit_logs_enabled: false,
ask_ai_sql: false,
release_query_module_enabled: false,
ab_ds_schema_enabled: true,
ab_onboarding_flow_start_with_data_dev_only_enabled: false,
license_session_limit_enabled: false,
rollout_datasource_test_rate_limit_enabled: false,
Expand Down
Loading
Loading