From 55b6e6d8d8dce7333c8219f84b1a9fc3d273ddc8 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Tue, 2 Jul 2024 06:31:59 +0530 Subject: [PATCH] test: Extract `pageId` with regex to cover more cases (#34521) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's a few places in Cypress tests that are trying to extract the page ID using `.split`, especially with just the path information, instead of the whole URL. So this PR changes the extraction implementation to use a regex, to support all three cases we need: 1. Full absolute application+page URL. 2. Just the path of an application+page URL. 3. Just the path of an application with a custom slug. 4. Full absolute application with a custom slug. (Supported, but we don't need this today). We've also fixed the URL parsing for the (very) old application URLs that didn't use slugs in the URL at all. This was making `FocusEntity.test.ts` fail. Fixed that test as well as improved it's error reporting. Before this PR: ![shot-2024-06-27-05-02-16](https://github.com/appsmithorg/appsmith/assets/120119/f3363376-a74d-4f3e-8196-5e72a9e758de) After this PR: ![shot-2024-06-27-05-03-15](https://github.com/appsmithorg/appsmith/assets/120119/8dc1be04-c60f-4251-acf0-e4fd962f4f00) No conflicts to EE. /test all > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 49edbce5ae85ee7fe9f4d2df05e2933347ddb3f4 > Cypress dashboard. > Tags: `` ## Summary by CodeRabbit - **Refactor** - Improved URL handling by centralizing page ID extraction logic across various tests and components. - Updated deprecated path constants to include ID extraction patterns for better consistency. - Enhanced code readability and maintainability by moving page ID extraction to a helper method. - **Tests** - Modified test cases to dynamically set `applicationId` and `pageId` instead of hardcoding values, ensuring more flexible and maintainable test scenarios. --- .../OtherUIFeatures/ApplicationURL_spec.js | 8 +- .../cypress/support/Pages/AggregateHelper.ts | 22 ++-- .../support/Pages/AppSettings/AppSettings.ts | 3 +- .../support/Pages/AppSettings/Utils.ts | 22 ---- .../src/ce/constants/routes/appRoutes.ts | 4 +- .../ce/pages/Editor/Explorer/helpers.test.ts | 7 +- app/client/src/navigation/FocusEntity.test.ts | 104 +++++++++--------- app/client/src/pages/tests/slug.test.tsx | 11 +- 8 files changed, 78 insertions(+), 103 deletions(-) delete mode 100644 app/client/cypress/support/Pages/AppSettings/Utils.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/ApplicationURL_spec.js b/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/ApplicationURL_spec.js index c678779251..b7116e288f 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/ApplicationURL_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/ApplicationURL_spec.js @@ -16,13 +16,13 @@ describe("Slug URLs", () => { it("1. Checks URL redirection from legacy URLs to slug URLs", () => { applicationId = localStorage.getItem("applicationId"); cy.location("pathname").then((pathname) => { - const pageId = pathname.split("/")[3]?.split("-").pop(); + const pageId = agHelper.extractPageIdFromUrl(pathname); cy.visit(`/applications/${applicationId}/pages/${pageId}/edit`, { timeout: Cypress.config().pageLoadTimeout, }).then(() => { agHelper.WaitUntilEleAppear(locators._sidebar); cy.location("pathname").then((pathname) => { - const pageId = pathname.split("/")[3]?.split("-").pop(); + const pageId = agHelper.extractPageIdFromUrl(pathname); const appName = localStorage .getItem("appName") .replace(/\s+/g, "-") @@ -38,7 +38,7 @@ describe("Slug URLs", () => { applicationName = appName; homePage.RenameApplication(applicationName); cy.location("pathname").then((pathname) => { - const pageId = pathname.split("/")[3]?.split("-").pop(); + const pageId = agHelper.extractPageIdFromUrl(pathname); expect(pathname).to.be.equal(`/app/${appName}/page1-${pageId}/edit`); }); }); @@ -53,7 +53,7 @@ describe("Slug URLs", () => { cy.url().then((url) => { const urlObject = new URL(url); const pathname = urlObject.pathname; - const pageId = pathname.split("/")[3]?.split("-").pop(); + const pageId = agHelper.extractPageIdFromUrl(pathname); expect(pathname).to.be.equal( `/app/${applicationName}/renamed-${pageId}/edit`, ); diff --git a/app/client/cypress/support/Pages/AggregateHelper.ts b/app/client/cypress/support/Pages/AggregateHelper.ts index 9f5bc761b5..18eb663181 100644 --- a/app/client/cypress/support/Pages/AggregateHelper.ts +++ b/app/client/cypress/support/Pages/AggregateHelper.ts @@ -133,19 +133,17 @@ export class AggregateHelper { }); } - public extractPageIdFromUrl(url: string): null | string { - const parts = url.split("/"); - - if (parts[3] !== "app") { - // Not a app URL. - return null; - } - - // Extract the page ID, either as an ObjectID or as a UUID. + /** + * Extract the pageId out of the URL, supporting both ObjectID and UUIDv4 values. This implementation is for tests + * only. Do NOT copy this over to production code. + * @param urlFragment can be either a full absolute URL (like https://dev.appsmith.com/app/name/page1-...) or just a + * path fragment (like /app/name/page1-...) or even a custom slug URL (like /app/custom-slug-...). + */ + public extractPageIdFromUrl(urlFragment: string): null | string { return ( - parts[5]?.match( - /[0-9a-f]{24}$|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/, - )?.[0] ?? null + urlFragment.match( + /\/app(?:\/[^/]+)?\/[^/]+-([0-9a-f]{24}|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\b/, + )?.[1] ?? null ); } diff --git a/app/client/cypress/support/Pages/AppSettings/AppSettings.ts b/app/client/cypress/support/Pages/AppSettings/AppSettings.ts index d2ff6fc676..d2cc928341 100644 --- a/app/client/cypress/support/Pages/AppSettings/AppSettings.ts +++ b/app/client/cypress/support/Pages/AppSettings/AppSettings.ts @@ -123,15 +123,14 @@ export class AppSettings { appName = appName.replace(/\s+/g, "-"); this.agHelper.AssertElementAbsence(this.locators._updateStatus, 10000); cy.location("pathname").then((pathname) => { + const pageId = this.agHelper.extractPageIdFromUrl(pathname); if (customSlug && customSlug.length > 0) { - const pageId = pathname.split("/")[2]?.split("-").pop(); expect(pathname).to.be.equal( `/app/${customSlug}-${pageId}${ editMode ? "/edit" : "" }${restOfUrl}`.toLowerCase(), ); } else { - const pageId = pathname.split("/")[3]?.split("-").pop(); expect(pathname).to.be.equal( `/app/${appName}/${pageName}-${pageId}${ editMode ? "/edit" : "" diff --git a/app/client/cypress/support/Pages/AppSettings/Utils.ts b/app/client/cypress/support/Pages/AppSettings/Utils.ts deleted file mode 100644 index d493af5e5e..0000000000 --- a/app/client/cypress/support/Pages/AppSettings/Utils.ts +++ /dev/null @@ -1,22 +0,0 @@ -export const checkUrl = ( - appName: string, - pageName: string, - customSlug?: string, - editMode = true, -) => { - cy.location("pathname").then((pathname) => { - if (customSlug && customSlug.length > 0) { - const pageId = pathname.split("/")[2]?.split("-").pop(); - expect(pathname).to.be.equal( - `/app/${customSlug}-${pageId}${editMode ? "/edit" : ""}`.toLowerCase(), - ); - } else { - const pageId = pathname.split("/")[3]?.split("-").pop(); - expect(pathname).to.be.equal( - `/app/${appName}/${pageName}-${pageId}${ - editMode ? "/edit" : "" - }`.toLowerCase(), - ); - } - }); -}; diff --git a/app/client/src/ce/constants/routes/appRoutes.ts b/app/client/src/ce/constants/routes/appRoutes.ts index bf0c97ab7d..58b3669e7b 100644 --- a/app/client/src/ce/constants/routes/appRoutes.ts +++ b/app/client/src/ce/constants/routes/appRoutes.ts @@ -28,8 +28,8 @@ export const getViewerPath = ( ) => `${BUILDER_VIEWER_PATH_PREFIX}${applicationSlug}/${pageSlug}-${pageId}`; export const getViewerCustomPath = (customSlug: string, pageId: string) => `${BUILDER_VIEWER_PATH_PREFIX}${customSlug}-${pageId}`; -export const BUILDER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId/edit`; -export const VIEWER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId`; +export const BUILDER_PATH_DEPRECATED = `/applications/:applicationId${ID_EXTRACTION_REGEX}/pages/:pageId${ID_EXTRACTION_REGEX}/edit`; +export const VIEWER_PATH_DEPRECATED = `/applications/:applicationId${ID_EXTRACTION_REGEX}/pages/:pageId${ID_EXTRACTION_REGEX}`; export const VIEWER_PATH_DEPRECATED_REGEX = /\/applications\/[^/]+\/pages\/[^/]+/; diff --git a/app/client/src/ce/pages/Editor/Explorer/helpers.test.ts b/app/client/src/ce/pages/Editor/Explorer/helpers.test.ts index b71712490e..8fcf6abbc9 100644 --- a/app/client/src/ce/pages/Editor/Explorer/helpers.test.ts +++ b/app/client/src/ce/pages/Editor/Explorer/helpers.test.ts @@ -3,7 +3,8 @@ import { getJSCollectionIdFromURL, } from "@appsmith/pages/Editor/Explorer/helpers"; -const pageId = "0123456789abcdef00000000"; +const applicationId = "a0123456789abcdef0000000"; +const pageId = "b0123456789abcdef0000000"; describe("getActionIdFromUrl", () => { it("getsApiId", () => { @@ -41,7 +42,7 @@ describe("getJSCollectionIdFromURL", () => { window.history.pushState( {}, "Query", - `/applications/appId/pages/${pageId}/edit/jsObjects/collectionId`, + `/applications/${applicationId}/pages/${pageId}/edit/jsObjects/collectionId`, ); const response = getJSCollectionIdFromURL(); expect(response).toBe("collectionId"); @@ -51,7 +52,7 @@ describe("getJSCollectionIdFromURL", () => { window.history.pushState( {}, "Query", - `/applications/appId/pages/${pageId}/edit/jsObjects`, + `/applications/${applicationId}/pages/${pageId}/edit/jsObjects`, ); const response = getJSCollectionIdFromURL(); expect(response).toBe(undefined); diff --git a/app/client/src/navigation/FocusEntity.test.ts b/app/client/src/navigation/FocusEntity.test.ts index befa2b4397..22ab15150e 100644 --- a/app/client/src/navigation/FocusEntity.test.ts +++ b/app/client/src/navigation/FocusEntity.test.ts @@ -7,123 +7,124 @@ interface TestCase { expected: FocusEntityInfo; } -const pageId = "0123456789abcdef00000000"; +const applicationId = "a0123456789abcdef0000000"; +const pageId = "b0123456789abcdef0000000"; describe("identifyEntityFromPath", () => { const oldUrlCases: TestCase[] = [ { - path: `/applications/myAppId/pages/${pageId}/edit`, + path: `/applications/${applicationId}/pages/${pageId}/edit`, expected: { entity: FocusEntity.CANVAS, id: "", appState: EditorState.EDITOR, params: { - applicationId: "myAppId", - pageId: pageId, + applicationId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/widgets/ryvc8i7oja`, + path: `/applications/${applicationId}/pages/${pageId}/edit/widgets/ryvc8i7oja`, expected: { entity: FocusEntity.PROPERTY_PANE, id: "ryvc8i7oja", appState: EditorState.EDITOR, params: { - applicationId: "myAppId", - pageId: pageId, + applicationId, + pageId, widgetIds: "ryvc8i7oja", }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/queries`, + path: `/applications/${applicationId}/pages/${pageId}/edit/queries`, expected: { entity: FocusEntity.QUERY_LIST, id: "", appState: EditorState.EDITOR, params: { - applicationId: "myAppId", + applicationId, entity: "queries", - pageId: pageId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/api/myApiId`, + path: `/applications/${applicationId}/pages/${pageId}/edit/api/myApiId`, expected: { entity: FocusEntity.QUERY, id: "myApiId", appState: EditorState.EDITOR, params: { apiId: "myApiId", - applicationId: "myAppId", - pageId: pageId, + applicationId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/queries/myQueryId`, + path: `/applications/${applicationId}/pages/${pageId}/edit/queries/myQueryId`, expected: { entity: FocusEntity.QUERY, id: "myQueryId", appState: EditorState.EDITOR, params: { queryId: "myQueryId", - applicationId: "myAppId", - pageId: pageId, + applicationId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/jsObjects`, + path: `/applications/${applicationId}/pages/${pageId}/edit/jsObjects`, expected: { entity: FocusEntity.JS_OBJECT_LIST, id: "", appState: EditorState.EDITOR, params: { - applicationId: "myAppId", + applicationId, entity: "jsObjects", - pageId: pageId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/jsObjects/myJSId`, + path: `/applications/${applicationId}/pages/${pageId}/edit/jsObjects/myJSId`, expected: { entity: FocusEntity.JS_OBJECT, id: "myJSId", appState: EditorState.EDITOR, params: { - applicationId: "myAppId", + applicationId, collectionId: "myJSId", - pageId: pageId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/datasource`, + path: `/applications/${applicationId}/pages/${pageId}/edit/datasource`, expected: { entity: FocusEntity.DATASOURCE_LIST, id: "", appState: EditorState.DATA, params: { - applicationId: "myAppId", + applicationId, entity: "datasource", - pageId: pageId, + pageId, }, }, }, { - path: `/applications/myAppId/pages/${pageId}/edit/datasource/myDatasourceId`, + path: `/applications/${applicationId}/pages/${pageId}/edit/datasource/myDatasourceId`, expected: { entity: FocusEntity.DATASOURCE, id: "myDatasourceId", appState: EditorState.DATA, params: { - applicationId: "myAppId", + applicationId, datasourceId: "myDatasourceId", - pageId: pageId, + pageId, }, }, }, @@ -137,20 +138,20 @@ describe("identifyEntityFromPath", () => { appState: EditorState.EDITOR, params: { applicationSlug: "eval", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, }, { - path: `/app/myAppId/page1-${pageId}/edit/widgets/ryvc8i7oja`, + path: `/app/app-slug/page1-${pageId}/edit/widgets/ryvc8i7oja`, expected: { entity: FocusEntity.PROPERTY_PANE, id: "ryvc8i7oja", appState: EditorState.EDITOR, params: { - applicationSlug: "myAppId", - pageId: pageId, + applicationSlug: "app-slug", + pageId, pageSlug: "page1-", widgetIds: "ryvc8i7oja", }, @@ -165,7 +166,7 @@ describe("identifyEntityFromPath", () => { params: { applicationSlug: "eval", entity: "queries", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, @@ -179,7 +180,7 @@ describe("identifyEntityFromPath", () => { params: { apiId: "myApiId", applicationSlug: "eval", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, @@ -192,7 +193,7 @@ describe("identifyEntityFromPath", () => { appState: EditorState.EDITOR, params: { applicationSlug: "eval", - pageId: pageId, + pageId, pageSlug: "page1-", queryId: "myQueryId", }, @@ -207,7 +208,7 @@ describe("identifyEntityFromPath", () => { params: { applicationSlug: "eval", entity: "jsObjects", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, @@ -221,7 +222,7 @@ describe("identifyEntityFromPath", () => { params: { applicationSlug: "eval", collectionId: "myJSId", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, @@ -235,7 +236,7 @@ describe("identifyEntityFromPath", () => { params: { applicationSlug: "eval", entity: "datasource", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, @@ -249,7 +250,7 @@ describe("identifyEntityFromPath", () => { params: { applicationSlug: "eval", datasourceId: "myDatasourceId", - pageId: pageId, + pageId, pageSlug: "page1-", }, }, @@ -263,7 +264,7 @@ describe("identifyEntityFromPath", () => { id: "", appState: EditorState.EDITOR, params: { - pageId: pageId, + pageId, customSlug: "myCustomSlug-", }, }, @@ -275,7 +276,7 @@ describe("identifyEntityFromPath", () => { id: "ryvc8i7oja", appState: EditorState.EDITOR, params: { - pageId: pageId, + pageId, customSlug: "myCustomSlug-", widgetIds: "ryvc8i7oja", }, @@ -288,7 +289,7 @@ describe("identifyEntityFromPath", () => { id: "", appState: EditorState.EDITOR, params: { - pageId: pageId, + pageId, customSlug: "myCustomSlug-", entity: "queries", }, @@ -301,7 +302,7 @@ describe("identifyEntityFromPath", () => { id: "myApiId", appState: EditorState.EDITOR, params: { - pageId: pageId, + pageId, customSlug: "myCustomSlug-", apiId: "myApiId", }, @@ -314,7 +315,7 @@ describe("identifyEntityFromPath", () => { id: "myQueryId", appState: EditorState.EDITOR, params: { - pageId: pageId, + pageId, customSlug: "myCustomSlug-", queryId: "myQueryId", }, @@ -327,7 +328,7 @@ describe("identifyEntityFromPath", () => { id: "", appState: EditorState.EDITOR, params: { - pageId: pageId, + pageId, entity: "jsObjects", customSlug: "myCustomSlug-", }, @@ -341,7 +342,7 @@ describe("identifyEntityFromPath", () => { appState: EditorState.EDITOR, params: { collectionId: "myJSId", - pageId: pageId, + pageId, customSlug: "myCustomSlug-", }, }, @@ -354,7 +355,7 @@ describe("identifyEntityFromPath", () => { appState: EditorState.DATA, params: { entity: "datasource", - pageId: pageId, + pageId, customSlug: "myCustomSlug-", }, }, @@ -366,7 +367,7 @@ describe("identifyEntityFromPath", () => { id: "myDatasourceId", appState: EditorState.DATA, params: { - pageId: pageId, + pageId, customSlug: "myCustomSlug-", datasourceId: "myDatasourceId", }, @@ -376,10 +377,7 @@ describe("identifyEntityFromPath", () => { const cases = oldUrlCases.concat(pageSlugCases.concat(customSlugCases)); - it("works", () => { - for (const testCase of cases) { - const actual = identifyEntityFromPath(testCase.path); - expect(actual).toStrictEqual(testCase.expected); - } + it.each(cases)("$# $path", ({ expected, path }) => { + expect(identifyEntityFromPath(path)).toStrictEqual(expected); }); }); diff --git a/app/client/src/pages/tests/slug.test.tsx b/app/client/src/pages/tests/slug.test.tsx index 0857e80b53..b1b1c7ecff 100644 --- a/app/client/src/pages/tests/slug.test.tsx +++ b/app/client/src/pages/tests/slug.test.tsx @@ -77,11 +77,12 @@ describe("URL slug names", () => { }); it("verifies that the baseURLBuilder uses applicationVersion", () => { - const pageId = "0123456789abcdef00000000"; + const applicationId = "a0123456789abcdef0000000"; + const pageId = "b0123456789abcdef0000000"; const params = { - applicationId: "appId", + applicationId, applicationSlug: "appSlug", - pageId: pageId, + pageId, pageSlug: "pageSlug", customSlug: "customSlug", }; @@ -113,9 +114,9 @@ describe("URL slug names", () => { payload: { applicationVersion: ApplicationVersion.SLUG_URL }, }); const url4 = builderURL({ pageId: params.pageId }); - expect(url1).toBe(`/applications/appId/pages/${pageId}/edit`); + expect(url1).toBe(`/applications/${applicationId}/pages/${pageId}/edit`); expect(url2).toBe(`/app/appSlug/pageSlug-${pageId}/edit`); - expect(url3).toBe(`/applications/appId/pages/${pageId}/edit`); + expect(url3).toBe(`/applications/${applicationId}/pages/${pageId}/edit`); expect(url4).toBe(`/app/appSlug/pageSlug-${pageId}/edit`); });