test: Extract pageId with regex to cover more cases (#34521)

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

<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/9713573983>
> Commit: 49edbce5ae85ee7fe9f4d2df05e2933347ddb3f4
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9713573983&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: ``

<!-- end of auto-generated comment: Cypress test results  -->












<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## 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.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Shrikant Sharat Kandula 2024-07-02 06:31:59 +05:30 committed by GitHub
parent 5eada0aaed
commit 55b6e6d8d8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 78 additions and 103 deletions

View File

@ -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`,
);

View File

@ -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
);
}

View File

@ -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" : ""

View File

@ -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(),
);
}
});
};

View File

@ -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\/[^/]+/;

View File

@ -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);

View File

@ -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);
});
});

View File

@ -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`);
});