Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import commonLocators from "../../../../locators/commonlocators.json";
import {
agHelper,
apiPage,
jsEditor,
} from "../../../../support/Objects/ObjectsCore";
import EditorNavigation, {
EditorViewMode,
PageLeftPane,
} from "../../../../support/Pages/EditorNavigation";
import apiWidgetLocator from "../../../../locators/apiWidgetslocator.json";

describe(
// https://github.com/appsmithorg/appsmith/issues/38547
"Validate if Show Bindings menu shows up in split pane mode for queries & JS Objects",
{ tags: ["@tag.IDE", "@tag.JS"] },
() => {
// Utility function to validate the bindings list against expected bindings
const validateBindings = (bindingsList, expectedBindings) => {
// Assert that the number of bindings matches the expected count
expect(bindingsList).to.have.length(expectedBindings.length);
expectedBindings.forEach((binding, index) => {
const element = bindingsList.eq(index); // Get the binding element by index
expect(element).to.contain(binding); // Assert the binding content matches the expected value
});
};

it("1. Validate 'Show bindings' gets displayed for JS Objects in split pane view", () => {
jsEditor.CreateJSObject("", { prettify: false, toRun: false });

// Switch to split view
EditorNavigation.SwitchScreenMode(EditorViewMode.SplitScreen);

// Switch to list view
cy.get(commonLocators.listToggle).click();
PageLeftPane.assertPresence("JSObject1");
agHelper.GetNClick(jsEditor._jsPageActions, 0, true);
cy.contains("Show bindings").click();

// Assert that the bindings menu is visible
cy.xpath(commonLocators.showBindingsMenu).should("be.visible");

/*
// There is a bug in which the order of bindings is incorrectly shown for JS Objects. Will enable the below validation once that is fixed.
// Expected bindings for the JavaScript Object
const expectedJSBindings = [
"{{JSObject1.myVar1}}",
"{{JSObject1.myVar2}}",
"{{JSObject1.myFun1()}}",
"{{JSObject1.myFun1.data}}",
"{{JSObject1.myFun2()}}",
"{{JSObject1.myFun2.data}}",
];

// Validate that the bindings in the menu match the expected bindings
cy.get(jsEditor._propertyList).then(($lis) => {
validateBindings($lis, expectedJSBindings);
});
*/
});
Comment on lines +28 to +60

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid using cy.wait and xpath selectors.

The test case violates Cypress best practices:

  1. agHelper.GetNClick implicitly uses cy.wait
  2. Using xpath selector commonLocators.showBindingsMenu

Consider:

  • Replace agHelper.GetNClick with proper assertions or intercepts
  • Use data-* attributes instead of xpath for showBindingsMenu
-cy.xpath(commonLocators.showBindingsMenu).should("be.visible");
+cy.get("[data-testid='entity-properties-container']").should("be.visible");

Committable suggestion skipped: line range outside the PR's diff.


it("2. Validate 'Show bindings' gets displayed for queries in split pane view", () => {
// Switch to standard view
EditorNavigation.SwitchScreenMode(EditorViewMode.FullScreen);
apiPage.CreateAndFillApi("www.google.com");

// Switch back to split view
EditorNavigation.SwitchScreenMode(EditorViewMode.SplitScreen);

// Switch to list view
cy.get(commonLocators.listToggle).click();
PageLeftPane.assertPresence("Api1");
agHelper.GetNClick(apiPage.splitPaneContextMenuTrigger, 0, true);
cy.contains("Show bindings").click();

// Assert that the bindings menu is visible
cy.xpath(commonLocators.showBindingsMenu).should("be.visible");

// Expected bindings for the API
const expectedApiBindings = [
"{{Api1.isLoading}}",
"{{Api1.data}}",
"{{Api1.responseMeta}}",
"{{Api1.run()}}",
"{{Api1.clear()}}",
];

// Validate that the bindings in the menu match the expected bindings
cy.get(apiWidgetLocator.propertyList).then(($lis) => {
validateBindings($lis, expectedApiBindings);
});
});
Comment on lines +62 to +92

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Similar issues with cy.wait and xpath selectors.

The second test case has the same violations of Cypress best practices.

Apply the same fixes as suggested for the first test case:

  • Replace agHelper.GetNClick
  • Use data-* attributes for selectors

},
);
4 changes: 3 additions & 1 deletion app/client/cypress/locators/commonlocators.json
Original file line number Diff line number Diff line change
Expand Up @@ -245,5 +245,7 @@
"enableClientSideSearch": ".t--property-control-enableclientsidesearch input[type='checkbox']",
"fixedFooterInput": ".t--property-control-fixedfooter input",
"tostifyIcon": ".Toastify--animate-icon > span",
"downloadFileType": "button[class*='t--open-dropdown-Select-file-type'] > span:first-of-type"
"downloadFileType": "button[class*='t--open-dropdown-Select-file-type'] > span:first-of-type",
"listToggle": "[data-testid='t--list-toggle']",
"showBindingsMenu": "//*[@id='entity-properties-container']"
Comment on lines +248 to +250

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace xpath with data- attribute.*

The showBindingsMenu locator uses xpath which violates the coding guidelines.

Update the locator to use a data-* attribute:

-"showBindingsMenu": "//*[@id='entity-properties-container']",
+"showBindingsMenu": "[data-testid='entity-properties-container']",
📝 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
"downloadFileType": "button[class*='t--open-dropdown-Select-file-type'] > span:first-of-type",
"listToggle": "[data-testid='t--list-toggle']",
"showBindingsMenu": "//*[@id='entity-properties-container']"
"downloadFileType": "button[class*='t--open-dropdown-Select-file-type'] > span:first-of-type",
"listToggle": "[data-testid='t--list-toggle']",
"showBindingsMenu": "[data-testid='entity-properties-container']"

}
1 change: 1 addition & 0 deletions app/client/cypress/support/Pages/ApiPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export class ApiPage {
private runOnPageLoadJSObject =
"input[name^='execute-on-page-load'][type='checkbox']";
public settingsTriggerLocator = "[data-testid='t--js-settings-trigger']";
public splitPaneContextMenuTrigger = ".entity-context-menu";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use data- attribute for selector.*

The selector uses a class which violates the coding guidelines.

Replace with a data-* attribute:

-public splitPaneContextMenuTrigger = ".entity-context-menu";
+public splitPaneContextMenuTrigger = "[data-testid='entity-context-menu']";
📝 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
public splitPaneContextMenuTrigger = ".entity-context-menu";
public splitPaneContextMenuTrigger = "[data-testid='entity-context-menu']";


CreateApi(
apiName = "",
Expand Down