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.

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

chore: Removing the feature flag for using Entity Item component from ADS templates #39093

Open
wants to merge 24 commits into
base: release
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1682ff2
removing feature flag for using new Entity Item component from ADS te…
ankitakinger Feb 6, 2025
75b36b7
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
ankitakinger Feb 6, 2025
304b6d7
fixing failing unit tests
ankitakinger Feb 6, 2025
e068b56
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
ankitakinger Feb 6, 2025
edbba40
fixing another unit test
ankitakinger Feb 6, 2025
5c13d0e
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
ankitakinger Feb 7, 2025
345fd93
updating locators
ankitakinger Feb 7, 2025
a5b55a1
fixing unit tests
ankitakinger Feb 7, 2025
a4551fa
updating selected entity item selector
ankitakinger Feb 7, 2025
e320299
fixing some more tests
ankitakinger Feb 7, 2025
672e056
fixing some more tests
ankitakinger Feb 7, 2025
bdc0de9
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
ankitakinger Feb 11, 2025
0182006
fixing a couple of integration tests
ankitakinger Feb 11, 2025
e864601
minor change
ankitakinger Feb 11, 2025
500002f
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
ankitakinger Feb 11, 2025
675a44e
fixing more tests
ankitakinger Feb 11, 2025
dc62804
fix for failing tests
ankitakinger Feb 12, 2025
29ad43c
fix for failing tests
ankitakinger Feb 12, 2025
f1452fb
reverting double click changes on entity item
ankitakinger Feb 12, 2025
7e84052
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
ankitakinger Feb 12, 2025
521dc3e
updating a test
ankitakinger Feb 12, 2025
73bfc57
fixing remaining tests
ankitakinger Feb 12, 2025
76f393a
fixing another set of failing tests
ankitakinger Feb 12, 2025
8acb96f
fixing client build
ankitakinger Feb 12, 2025
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
Expand Up @@ -92,7 +92,7 @@ describe(
cy.get(appPage.closeButton).closest("div").click({ force: true });
PageLeftPane.switchSegment(PagePaneSegment.UI);
PageLeftPane.switchSegment(PagePaneSegment.Queries);
cy.xpath(appPage.postApi).click({ force: true });
cy.get(appPage.postApi).click({ force: true });
cy.ResponseCheck("Test");
// cy.ResponseCheck("Task completed");
cy.ResponseCheck("[email protected]");
Comment on lines 96 to 98
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

Use structured assertions instead of plain strings.

Direct string assertions should be replaced with more structured expectations.

-cy.ResponseCheck("Test");
-cy.ResponseCheck("[email protected]");
+cy.ResponseCheck({
+  subject: "Test",
+  recipient: "[email protected]"
+});
📝 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
cy.ResponseCheck("Test");
// cy.ResponseCheck("Task completed");
cy.ResponseCheck("[email protected]");
cy.ResponseCheck({
subject: "Test",
recipient: "[email protected]"
});
// cy.ResponseCheck("Task completed");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe(
entityNameinLeftSidebar: "Tab2",
action: "Rename",
});
agHelper.TypeText(locators._entityNameEditing("Tab2"), tabname);
agHelper.TypeText(locators._entityNameEditing, tabname, { clear: true });
agHelper.Sleep(2000);
entityExplorer.ValidateDuplicateMessageToolTip(tabname);
cy.get(explorer.editEntity)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe(
entityExplorer.RenameEntityFromExplorer(
"Page1",
pageName,
false,
true,
EntityItems.Page,
);
PageList.ClonePage(pageName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe(
entityExplorer.RenameEntityFromExplorer(
"Page2",
"ParentPage1",
false,
true,
EntityItems.Page,
);
dataSources.NavigateToDSCreateNew();
Expand All @@ -101,7 +101,7 @@ describe(
entityExplorer.RenameEntityFromExplorer(
"Page2",
"ChildPage1",
false,
true,
EntityItems.Page,
);
dataSources.NavigateToDSCreateNew();
Expand Down Expand Up @@ -132,7 +132,7 @@ describe(
entityExplorer.RenameEntityFromExplorer(
"ParentPage1",
"ParentPageRenamed",
false,
true,
EntityItems.Page,
);
agHelper.RemoveUIElement(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ describe("Canvas context Property Pane", { tags: ["@tag.IDE"] }, function () {

cy.get(".t--widget-imagewidget").eq(0).click();
//check if the entities are expanded
cy.get(`[data-guided-tour-id="explorer-entity-Image1"]`).should("exist");
cy.get(`[data-testid="t--entity-item-Image1"]`).should("exist");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,20 @@ describe("Fork application with Jsobjects", {}, function () {
for (let i = 1; i <= 11; i++) {
agHelper.GetNClick(locators._entityTestId(`JS${i}`));
agHelper.FailIfErrorToast("");
agHelper.AssertClassExists(locators._entityTestId(`JS${i}`), "active");
agHelper.AssertAttribute(
locators._entityTestId(`JS${i}`),
"data-selected",
"true",
);
}
for (let i = 12; i <= 17; i++) {
agHelper.GetNClick(locators._entityTestId(`J${i}`));
agHelper.FailIfErrorToast("");
agHelper.AssertClassExists(locators._entityTestId(`J${i}`), "active");
agHelper.AssertAttribute(
locators._entityTestId(`J${i}`),
"data-selected",
"true",
);
}

jsEditor.CreateJSObject('"MiddleName": "Test",\n', {
Expand All @@ -51,9 +59,17 @@ describe("Fork application with Jsobjects", {}, function () {
lineNumber: 5,
});
agHelper.GetNClick(locators._entityTestId("J16"));
agHelper.AssertClassExists(locators._entityTestId("J16"), "active");
agHelper.AssertAttribute(
locators._entityTestId("J16"),
"data-selected",
"true",
);
agHelper.GetNClick(locators._entityTestId("J17"));
agHelper.AssertClassExists(locators._entityTestId("J17"), "active");
agHelper.AssertAttribute(
locators._entityTestId("J17"),
"data-selected",
"true",
);
agHelper.GetNAssertContains(".CodeMirror-line ", '"MiddleName": "Test"');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("Slug URLs", { tags: ["@tag.AppUrl"] }, () => {
entityExplorer.RenameEntityFromExplorer(
"Page1",
"Renamed",
false,
true,
EntityItems.Page,
);
assertHelper.AssertNetworkStatus("updatePage");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ describe("Check Page Actions Menu", {}, function () {
PageList.ShowList();
agHelper.AssertAttribute(
locators._entityTestId("NewPage Copy"),
"disabled",
"disabled",
"data-disabled",
"true",
);
PageList.DeletePage("NewPage Copy");
PageList.assertAbsence("NewPage Copy");
Expand Down Expand Up @@ -92,8 +92,8 @@ describe("Check Page Actions Menu", {}, function () {
PageList.ShowList();
agHelper.AssertAttribute(
locators._entityTestId("Page2 Copy"),
"disabled",
"disabled",
"data-disabled",
"true",
);
PageList.DeletePage("Page2 Copy");
PageList.assertAbsence("Page2 Copy");
Expand All @@ -113,8 +113,8 @@ describe("Check Page Actions Menu", {}, function () {
PageList.ShowList();
agHelper.AssertAttribute(
locators._entityTestId("HomePage Copy"),
"disabled",
"disabled",
"data-disabled",
"true",
);
PageList.DeletePage("HomePage Copy");
PageList.assertAbsence("HomePage Copy");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe(
entityExplorer.RenameEntityFromExplorer(
"Page1",
"Home",
false,
true,
entityItems.Page,
);
});
Expand Down
2 changes: 1 addition & 1 deletion app/client/cypress/locators/CMSApplocators.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export default {
confirmButton: "span:contains('Confirm')",
closeButton: "span:contains('Close')",
datasourcesbutton: "//div[text()='Datasources']",
postApi: "//div[text()='send_mail']",
postApi: "[data-testid='t--ide-tab-send_mail']",
deleteButton: "//span[text()='Delete Proposal']",
deleteTaskText: "//span[text()='Delete this task']",
};
2 changes: 1 addition & 1 deletion app/client/cypress/locators/apiWidgetslocator.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"searchApi": ".t--sidebar input[type=text]",
"createapi": ".t--createBlankApiCard",
"createAuthApiDatasource": ".t--createAuthApiDatasource",
"popover": "//button[contains(@class, 'entity-context-menu')]",
"popover": "button[data-testid='t--more-action-trigger']",
"moveTo": ".single-select >div:contains('Move to')",
"copyTo": ".single-select >div:contains('Copy to page')",
"home": ".single-select >div:contains('Page1')",
Expand Down
3 changes: 1 addition & 2 deletions app/client/cypress/locators/commonlocators.json
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@
"filepickerv2": ".t--draggable-filepickerwidgetv2",
"dashboardItemName": ".uppy-Dashboard-Item-name",
"mapChartShowLabels": ".t--property-control-showlabels input",
"widgetSection": ".t--entity.widgets > .t--entity-item > span.t--entity-collapse-toggle",
"changeThemeBtn": ".t--change-theme-btn",
"editThemeBtn": ".t--edit-theme-btn",
"themeCard": ".t--theme-card",
Expand Down Expand Up @@ -248,4 +247,4 @@
"downloadFileType": "button[class*='t--open-dropdown-Select-file-type'] > span:first-of-type",
"listToggle": "[data-testid='t--list-toggle']",
"showBindingsMenu": "//*[@id='entity-properties-container']"
}
}
2 changes: 1 addition & 1 deletion app/client/cypress/support/ApiCommands.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ Cypress.Commands.add("validateMessage", (value) => {
});

Cypress.Commands.add("DeleteWidgetFromSideBar", () => {
cy.xpath(apiwidget.popover).last().click({ force: true });
cy.get(apiwidget.popover).last().click({ force: true });
cy.get(apiwidget.delete).click({ force: true });
cy.wait("@updateLayout").should(
"have.nested.property",
Expand Down
13 changes: 8 additions & 5 deletions app/client/cypress/support/Objects/CommonLocators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,7 @@ export class CommonLocators {
"//div[text()='" +
entityNameinLeftSidebar +
"']/ancestor::div[contains(@class, 't--entity-item')]/following-sibling::div//div[contains(@class, 't--entity-property')]//code";
_entityNameEditing = (entityNameinLeftSidebar: string) =>
"//span[text()='" +
entityNameinLeftSidebar +
"']/parent::div[contains(@class, 't--entity-name editing')]/input";
_entityNameEditing = ".t--entity-name.editing input";
_jsToggle = (controlToToggle: string) =>
`.t--property-control-${controlToToggle} .t--js-toggle, [data-guided-tour-iid='${controlToToggle}']`;
_buttonByText = (btnVisibleText: string) =>
Expand Down Expand Up @@ -334,9 +331,14 @@ export class CommonLocators {
_exitFullScreen = ".application-demo-new-dashboard-control-exit-fullscreen";
_menuItem = ".bp3-menu-item";
_slashCommandHintText = ".slash-command-hint-text";
_selectionItem = ".rc-select-selection-item";
errorPageTitle = ".t--error-page-title";
errorPageDescription = ".t--error-page-description";
_moduleInstanceEntity = (module: string) =>
`[data-testid=t--entity-item-${module}1]`;
_codeEditor = "[data-testid=code-editor-target]";
_selectionItem = ".rc-select-selection-item";
_moduleInputEntity = (inputName: string) =>
`[data-testid=t--module-instance-input-field-wrapper-${inputName}]`;
_selectClearButton_testId = "selectbutton.btn.cancel";
_selectClearButton_dataTestId = `[data-testid="${this._selectClearButton_testId}"]`;
_saveDatasource = `[data-testid='t--store-as-datasource']`;
Expand All @@ -347,6 +349,7 @@ export class CommonLocators {
_showBoundary = ".show-boundary";
_entityItem = "[data-testid='t--entity-item-Api1']";
_rowData = "[data-colindex='0'][data-rowindex='0']";
_visualNonIdeaState = ".bp3-non-ideal-state";
_editorTab = ".editor-tab";
_entityTestId = (entity: string) =>
`[data-testid="t--entity-item-${entity}"]`;
Expand Down
11 changes: 11 additions & 0 deletions app/client/cypress/support/Pages/AggregateHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -986,12 +986,14 @@ export class AggregateHelper {
parseSpecialCharSeq: boolean;
shouldFocus: boolean;
delay: number;
clear: boolean;
}> = 0,
) {
let index: number;
let shouldFocus = true;
let parseSpecialCharSeq = false;
let delay = 10;
let clear = false;

if (typeof indexOrOptions === "number") {
index = indexOrOptions;
Expand All @@ -1003,6 +1005,7 @@ export class AggregateHelper {
indexOrOptions.shouldFocus !== undefined
? indexOrOptions.shouldFocus
: true;
clear = indexOrOptions.clear || false;
}

const element = this.GetElement(selector).eq(index);
Expand All @@ -1013,6 +1016,14 @@ export class AggregateHelper {

if (value === "") return element;

if (clear) {
return element.wait(100).clear().type(value, {
parseSpecialCharSequences: parseSpecialCharSeq,
delay: delay,
force: true,
});
}

return element.wait(100).type(value, {
parseSpecialCharSequences: parseSpecialCharSeq,
delay: delay,
Expand Down
4 changes: 2 additions & 2 deletions app/client/cypress/support/Pages/EditorNavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ export enum EditorViewMode {
}

const pagePaneListItemSelector = (name: string) =>
"//div[contains(@class, 't--entity-name')][text()='" + name + "']";
`//span[contains(@class, 't--entity-name')][text()='${name}']`;

export const PageLeftPane = new LeftPane(
pagePaneListItemSelector,
".ide-editor-left-pane",
".ide-editor-left-pane__content .active.t--entity-item",
".ide-editor-left-pane__content .t--entity-item[data-selected='true']",
Object.values(PagePaneSegment),
);

Expand Down
22 changes: 11 additions & 11 deletions app/client/cypress/support/Pages/EntityExplorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,9 @@ export class EntityExplorer {
private assertHelper = ObjectsRegistry.AssertHelper;

public _contextMenu = (entityNameinLeftSidebar: string) =>
"//div[text()='" +
"//span[text()='" +
entityNameinLeftSidebar +
"']/ancestor::div[1]/following-sibling::div//button[contains(@class, 'entity-context-menu')]";
_entityNameInExplorer = (entityNameinLeftSidebar: string) =>
"//div[contains(@class, 't--entity-explorer')]//div[contains(@class, 't--entity-name')][text()='" +
entityNameinLeftSidebar +
"']";
"']/parent::div/following-sibling::div//button";
Comment on lines 54 to +57
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

Update selector to use data- attribute.*

The selector uses XPath with hardcoded HTML tags. This is fragile and may break if the UI structure changes.

Replace the XPath selector with a data-* attribute:

-    "//span[text()='" +
-    entityNameinLeftSidebar +
-    "']/parent::div/following-sibling::div//button";
+    `[data-cy="entity-context-menu-${entityNameinLeftSidebar}"]`;
📝 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 _contextMenu = (entityNameinLeftSidebar: string) =>
"//div[text()='" +
"//span[text()='" +
entityNameinLeftSidebar +
"']/ancestor::div[1]/following-sibling::div//button[contains(@class, 'entity-context-menu')]";
_entityNameInExplorer = (entityNameinLeftSidebar: string) =>
"//div[contains(@class, 't--entity-explorer')]//div[contains(@class, 't--entity-name')][text()='" +
entityNameinLeftSidebar +
"']";
"']/parent::div/following-sibling::div//button";
public _contextMenu = (entityNameinLeftSidebar: string) =>
`[data-cy="entity-context-menu-${entityNameinLeftSidebar}"]`;


private _visibleTextSpan = (spanText: string) =>
"//span[text()='" + spanText + "']";
Expand All @@ -72,6 +68,7 @@ export class EntityExplorer {
_widgetTagSuggestedWidgets = ".widget-tag-collapsible-suggested";
_widgetTagBuildingBlocks = ".widget-tag-collapsible-building-blocks";
_widgetSeeMoreButton = "[data-testid='t--explorer-ui-entity-tag-see-more']";
_entityAddButton = ".t--entity-add-btn";
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 data-testid for entity add button selector.

The selector uses a class which is more brittle than data attributes.

Apply this diff:

-  _entityAddButton = ".t--entity-add-btn";
+  _entityAddButton = "[data-testid='t--entity-add-btn']";
📝 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
_entityAddButton = ".t--entity-add-btn";
_entityAddButton = "[data-testid='t--entity-add-btn']";

_entityName = ".t--entity-name";

public ActionContextMenuByEntityName({
Expand Down Expand Up @@ -103,7 +100,7 @@ export class EntityExplorer {
toastToValidate: toastToValidate,
});
}
if (entityType === EntityItems.Page) {
if (entityType === EntityItems.Page && action !== "Rename") {
PageList.HideList();
}
}
Expand All @@ -121,16 +118,18 @@ export class EntityExplorer {
}

public ValidateDuplicateMessageToolTip(tooltipText: string) {
this.agHelper.AssertTooltip(tooltipText.concat(" is already being used."));
this.agHelper.AssertTooltip(
tooltipText.concat(" is already being used or is a restricted keyword."),
);
}

public DeleteAllQueriesForDB(dsName: string) {
AppSidebar.navigate(AppSidebarButton.Editor);
PageLeftPane.switchSegment(PagePaneSegment.Queries);
this.agHelper
.GetElement(this._visibleTextSpan(dsName))
.parent()
.siblings()
.children()
.each(($el: any) => {
cy.wrap($el)
.find(".t--entity-name")
Expand Down Expand Up @@ -275,8 +274,9 @@ export class EntityExplorer {
action: "Rename",
entityType,
});
else cy.xpath(PageLeftPane.listItemSelector(entityName)).dblclick();
cy.xpath(this.locator._entityNameEditing(entityName))
else cy.get(this.locator._entityTestId(entityName)).dblclick();
cy.get(this.locator._entityNameEditing)
.clear()
.type(renameVal)
.wait(500)
.type("{enter}")
Expand Down
11 changes: 5 additions & 6 deletions app/client/cypress/support/Pages/IDE/LeftPane.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { ObjectsRegistry } from "../../Objects/Registry";
import { PagePaneSegment } from "../EditorNavigation";
import AddView from "./AddView";
import FileTabs from "./FileTabs";
import ListView from "./ListView";

export class LeftPane {
Expand All @@ -11,11 +9,10 @@ export class LeftPane {
locators = {
segment: (name: string) => "//span[text()='" + name + "']/ancestor::div",
expandCollapseArrow: (name: string) =>
"//div[text()='" +
name +
"']/ancestor::div/span[contains(@class, 't--entity-collapse-toggle')]",
`//span[contains(@class, 't--entity-name')][text()="${name}"]/ancestor::div//*[@data-testid="t--entity-collapse-toggle"]`,
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
activeItemSelector: "",
selector: "",
entityItem: (name: string) => `div[data-testid='t--entity-item-${name}']`,
};
public listView: ListView;

Expand All @@ -41,7 +38,7 @@ export class LeftPane {

public assertPresence(name: string) {
ObjectsRegistry.AggregateHelper.AssertElementLength(
this.listItemSelector(name),
this.locators.entityItem(name),
1,
);
}
Expand Down Expand Up @@ -83,13 +80,15 @@ export class LeftPane {
.then((state) => {
const closed = state === "arrow-right-s-line";
const opened = state === "arrow-down-s-line";

if ((expand && closed) || (!expand && opened)) {
ObjectsRegistry.AggregateHelper.GetNClick(
this.locators.expandCollapseArrow(itemName),
);
}
});
}

public selectedItem(
exists?: "exist" | "not.exist" | "noVerify",
): Cypress.Chainable {
Expand Down
Loading
Loading