From e7e7bbba4d6934ee935e33aee2ed43c36557ea85 Mon Sep 17 00:00:00 2001 From: Oleg Isonen Date: Mon, 4 Mar 2024 16:27:30 +0100 Subject: [PATCH] fix: support wildcards when duplicating pages (#2962) ## Description Reverting back to the logic that during duplication uses '/test' to '/copy-1/test' because it also supports '/test/*' and '/test/name*' ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env.example` and the `builder/env-check.js` if mandatory --- .../panels/pages/page-utils.test.ts | 66 +++++++++++++++++-- .../sidebar-left/panels/pages/page-utils.ts | 22 +++++-- .../.webstudio/data.json | 3 +- .../.webstudio/data.json | 3 +- .../.webstudio/data.json | 3 +- .../.webstudio/data.json | 3 +- 6 files changed, 85 insertions(+), 15 deletions(-) diff --git a/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.test.ts b/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.test.ts index 775ff99eb5..a3c5353bb9 100644 --- a/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.test.ts +++ b/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.test.ts @@ -531,7 +531,7 @@ describe("duplicate page", () => { expect($pages.get()?.pages[0]).toEqual({ id: expect.not.stringMatching("pageId"), name: "My Name (1)", - path: "/1", + path: "/copy-1", title: `"My Title"`, meta: {}, rootInstanceId: expect.not.stringMatching("body"), @@ -554,7 +554,7 @@ describe("duplicate page", () => { ); }); - test("non-home page preserving old path and name with suffix", () => { + test("non-home page preserving old path and name with prefix", () => { $instances.set( toMap([{ type: "instance", id: "body", component: "Body", children: [] }]) ); @@ -583,7 +583,61 @@ describe("duplicate page", () => { expect($pages.get()?.pages[1]).toEqual({ id: expect.not.stringMatching("pageId"), name: "My Name (2)", - path: "/my-path-1", + path: "/copy-1/my-path", + title: `"My Title"`, + meta: {}, + rootInstanceId: expect.not.stringMatching("body"), + }); + }); + + test("handle wildcards", () => { + $instances.set( + toMap([{ type: "instance", id: "body", component: "Body", children: [] }]) + ); + $pages.set({ + homePage: { + id: "homeId", + name: "Home", + path: "/", + title: `"Home"`, + meta: {}, + rootInstanceId: "home", + }, + pages: [ + { + id: "pageId1", + name: "My Name 1", + path: "/my-path/*", + title: `"My Title"`, + meta: {}, + rootInstanceId: "body", + }, + { + id: "pageId2", + name: "My Name 2", + // Named wildcard + path: "/my-path/name*", + title: `"My Title"`, + meta: {}, + rootInstanceId: "body", + }, + ], + folders: [], + }); + duplicatePage("pageId1"); + duplicatePage("pageId2"); + expect($pages.get()?.pages[2]).toEqual({ + id: expect.not.stringMatching("pageId1"), + name: "My Name 1 (1)", + path: "/copy-1/my-path/*", + title: `"My Title"`, + meta: {}, + rootInstanceId: expect.not.stringMatching("body"), + }); + expect($pages.get()?.pages[3]).toEqual({ + id: expect.not.stringMatching("pageId2"), + name: "My Name 2 (1)", + path: "/copy-1/my-path/name*", title: `"My Title"`, meta: {}, rootInstanceId: expect.not.stringMatching("body"), @@ -626,7 +680,7 @@ describe("duplicate page", () => { expect($pages.get()?.pages[1]).toEqual({ id: expect.not.stringMatching("pageId"), name: "My Name (1)", - path: "/my-path-1", + path: "/copy-1/my-path", title: `"My Title"`, meta: {}, rootInstanceId: expect.not.stringMatching("body"), @@ -677,7 +731,7 @@ describe("duplicate page", () => { expect($pages.get()?.pages[0]).toEqual({ id: expect.not.stringMatching("pageId"), name: "My Name (1)", - path: "/1", + path: "/copy-1", title: `"Title: " + ${newVariableName}`, meta: { description: `"Description: " + ${newVariableName}`, @@ -727,7 +781,7 @@ describe("duplicate page", () => { expect($pages.get()?.pages[0]).toEqual({ id: expect.not.stringMatching("pageId"), name: "My Name (1)", - path: "/1", + path: "/copy-1", title: `"My Title"`, meta: {}, rootInstanceId: expect.not.stringMatching("body"), diff --git a/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.ts b/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.ts index 1dede277aa..f1bd115375 100644 --- a/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.ts +++ b/apps/builder/app/builder/features/sidebar-left/panels/pages/page-utils.ts @@ -337,16 +337,28 @@ export const $pageRootScope = computed( const deduplicatePath = (pages: Pages, page: Page) => { const fullPath = getPagePath(page.id, pages); - const matchedPage = findPageByIdOrPath(fullPath, pages); + let matchedPage = findPageByIdOrPath(fullPath, pages); + let { path } = page; if (matchedPage === undefined) { - return page.path; + return path; } - let counter = 1; - while (findPageByIdOrPath(`${fullPath}-${counter}`, pages) !== undefined) { + + if (path === "/") { + path = ""; + } + + let counter = 0; + const folder = findParentFolderByChildId(page.id, pages.folders); + const folderPath = folder ? getPagePath(folder.id, pages) : ""; + while (matchedPage !== undefined) { counter += 1; + matchedPage = findPageByIdOrPath( + `${folderPath}/copy-${counter}${path}`, + pages + ); } - return page.path === "/" ? `/${counter}` : `${page.path}-${counter}`; + return `/copy-${counter}${path}`; }; const replaceDataSources = ( diff --git a/fixtures/webstudio-custom-template/.webstudio/data.json b/fixtures/webstudio-custom-template/.webstudio/data.json index e3703aaf73..2b3d496b95 100644 --- a/fixtures/webstudio-custom-template/.webstudio/data.json +++ b/fixtures/webstudio-custom-template/.webstudio/data.json @@ -627,7 +627,8 @@ "deployment": { "domains": [], "projectDomain": "webstudio-custom-template-cochj" - } + }, + "marketplaceProduct": {} }, "page": { "id": "nfzls_SkTc9jKYyxcZ8Lw", diff --git a/fixtures/webstudio-remix-netlify-edge-functions/.webstudio/data.json b/fixtures/webstudio-remix-netlify-edge-functions/.webstudio/data.json index 8ccfc0c1a9..714ce1b10f 100644 --- a/fixtures/webstudio-remix-netlify-edge-functions/.webstudio/data.json +++ b/fixtures/webstudio-remix-netlify-edge-functions/.webstudio/data.json @@ -187,7 +187,8 @@ "deployment": { "domains": [], "projectDomain": "cli-basic-test-d0osr" - } + }, + "marketplaceProduct": {} }, "page": { "id": "9di_L14CzctvSruIoKVvE", diff --git a/fixtures/webstudio-remix-netlify-functions/.webstudio/data.json b/fixtures/webstudio-remix-netlify-functions/.webstudio/data.json index 8ccfc0c1a9..714ce1b10f 100644 --- a/fixtures/webstudio-remix-netlify-functions/.webstudio/data.json +++ b/fixtures/webstudio-remix-netlify-functions/.webstudio/data.json @@ -187,7 +187,8 @@ "deployment": { "domains": [], "projectDomain": "cli-basic-test-d0osr" - } + }, + "marketplaceProduct": {} }, "page": { "id": "9di_L14CzctvSruIoKVvE", diff --git a/fixtures/webstudio-remix-vercel/.webstudio/data.json b/fixtures/webstudio-remix-vercel/.webstudio/data.json index 9252bdb60b..34fec5bd9c 100644 --- a/fixtures/webstudio-remix-vercel/.webstudio/data.json +++ b/fixtures/webstudio-remix-vercel/.webstudio/data.json @@ -3284,7 +3284,8 @@ "deployment": { "domains": [], "projectDomain": "webstudio-fixture-project-a-0su3o" - } + }, + "marketplaceProduct": {} }, "page": { "id": "7Db64ZXgYiRqKSQNR-qTQ",