From b660ea9d53dae8a4fd9c1ca06f289ba144b3c4a0 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 14 Sep 2026 10:06:38 +0000 Subject: [PATCH] :sparkles: Route the app by query string with screen key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move SPA routing out of the URL fragment into the normal query string. The screen travels in a reserved `screen` key holding the route name (`?screen=workspace&team-id=…`); every other param keeps its name. `rt/nav` and `rt/resolve` keep their signatures. This deletes the fragment-mirroring URL surgery, simplifies link-preview (the server sees everything) and nginx (single path, no SPA fallback rules needed), and migrates OIDC redirects, email links, e2e helpers and plugin test utils to the new format. Legacy `#/…` URLs translate client-side for one Penpot version (`legacy-routes`, marked TODO(next-version)); non-SPA paths are untouched. AI-assisted-by: muse-spark-1.3-contributor --- .../frontend/routing-app-shell-subtleties.md | 5 +- .../resources/app/email/change-email/en.html | 2 +- .../resources/app/email/change-email/en.txt | 2 +- .../app/email/invite-to-organization/en.html | 2 +- .../app/email/invite-to-organization/en.txt | 2 +- .../app/email/invite-to-team/en.html | 2 +- .../resources/app/email/invite-to-team/en.txt | 2 +- backend/resources/app/email/join-team/en.html | 2 +- backend/resources/app/email/join-team/en.txt | 2 +- .../app/email/password-recovery/en.html | 2 +- .../app/email/password-recovery/en.txt | 2 +- backend/resources/app/email/register/en.html | 2 +- backend/resources/app/email/register/en.txt | 2 +- .../en.html | 2 +- .../en.txt | 2 +- .../request-file-access-yourpenpot/en.html | 2 +- .../request-file-access-yourpenpot/en.txt | 2 +- .../app/email/request-file-access/en.html | 4 +- .../app/email/request-file-access/en.txt | 4 +- .../app/email/request-team-access/en.html | 2 +- .../app/email/request-team-access/en.txt | 2 +- .../app/templates/graph-console.tmpl | 8 +- .../resources/app/templates/link-preview.tmpl | 2 +- backend/src/app/auth/oidc.clj | 14 +- backend/test/backend_tests/auth_oidc_test.clj | 8 +- .../developer/subsystems/link-preview.md | 99 +++++++------ .../playwright/helpers/MockWebSocketHelper.js | 4 + frontend/playwright/ui/pages/DashboardPage.js | 24 ++-- frontend/playwright/ui/pages/RegisterPage.js | 2 +- frontend/playwright/ui/pages/ShortcutsPage.js | 2 +- .../ui/pages/SubscriptionProfilePage.js | 2 +- frontend/playwright/ui/pages/ViewerPage.js | 20 ++- frontend/playwright/ui/pages/WorkspacePage.js | 35 ++++- .../playwright/ui/specs/dashboard.spec.js | 2 +- .../ui/specs/email-verification.spec.js | 6 +- .../ui/specs/legacy-hash-redirect.spec.js | 36 +++++ frontend/playwright/ui/specs/login.spec.js | 21 ++- frontend/playwright/ui/specs/register.spec.js | 2 +- .../playwright/ui/specs/tokens/crud.spec.js | 24 ++++ .../ui/visual-specs/visual-dashboard.spec.js | 2 +- .../ui/visual-specs/visual-login.spec.js | 4 +- frontend/src/app/main/data/nitrate.cljs | 2 +- frontend/src/app/main/data/profile.cljs | 3 +- frontend/src/app/main/data/team.cljs | 6 +- frontend/src/app/main/router.cljs | 120 +++++++--------- frontend/src/app/main/ui/auth.cljs | 2 +- .../app/main/ui/dashboard/subscription.cljs | 5 +- frontend/src/app/main/ui/routes.cljs | 99 +++++++++++-- .../src/app/main/ui/viewer/share_link.cljs | 5 +- .../sidebar/assets/file_library.cljs | 10 +- .../main/ui/workspace/sidebar/versions.cljs | 3 +- frontend/src/app/util/browser_history.js | 9 +- frontend/test/frontend_tests/router_test.cljs | 136 ++++-------------- plugins/apps/e2e/src/utils/agent.ts | 5 +- plugins/apps/e2e/src/utils/api.ts | 14 ++ plugins/apps/e2e/src/utils/get-file-url.ts | 5 +- 56 files changed, 447 insertions(+), 343 deletions(-) create mode 100644 frontend/playwright/ui/specs/legacy-hash-redirect.spec.js diff --git a/.serena/memories/frontend/routing-app-shell-subtleties.md b/.serena/memories/frontend/routing-app-shell-subtleties.md index cbdc75fcef..a340fb310f 100644 --- a/.serena/memories/frontend/routing-app-shell-subtleties.md +++ b/.serena/memories/frontend/routing-app-shell-subtleties.md @@ -2,8 +2,9 @@ ## Router, app shell, and errors -- Routing uses browser-history hash tokens, but `on-navigate` rejects navigation if the current origin/path does not match `cf/public-uri`. -- Route params are split into `:path` and `:query`; duplicate query params can become vectors, so use `rt/get-query-param` when a scalar is required. +- Routing uses browser-history query tokens (`?screen=¶ms`, single `/` path), but `on-navigate` rejects navigation if the current origin/path does not match `cf/public-uri`. +- Route params live entirely in the query map under the reserved `screen` key; duplicate query params can become vectors, so use `rt/get-query-param` when a scalar is required. +- Legacy `#/…` hash URLs translate client-side to the query format (one-version compat; see `legacy-routes` in `app.main.ui.routes`, TODO(next-version) to delete). - Unknown/empty routes trigger an extra `get-profile`/`get-teams` check before redirecting. This avoids invitation and root-route race conditions. - The root app renders an exception page from `:exception` state before the normal error boundary. `rt/navigated` clears `:exception`. - Frontend error handling treats stale cross-build JS chunk failures specially: messages containing `$cljs$cst$` or `$cljs$core$I` plus undefined/null/not-a-function signatures trigger throttled reload. diff --git a/backend/resources/app/email/change-email/en.html b/backend/resources/app/email/change-email/en.html index 238f9f401e..597f479176 100644 --- a/backend/resources/app/email/change-email/en.html +++ b/backend/resources/app/email/change-email/en.html @@ -205,7 +205,7 @@ - Confirm email change diff --git a/backend/resources/app/email/change-email/en.txt b/backend/resources/app/email/change-email/en.txt index eff98baf6f..6a59e9b629 100644 --- a/backend/resources/app/email/change-email/en.txt +++ b/backend/resources/app/email/change-email/en.txt @@ -4,7 +4,7 @@ We received a request to change your current email to {{ pending-email }}. Click the link below to confirm the change. -{{ public-uri }}/#/auth/verify-token?token={{token}} +{{ public-uri }}/?screen=auth-verify-token&token={{token}} If you did not request this change, consider changing your password for security reasons. diff --git a/backend/resources/app/email/invite-to-organization/en.html b/backend/resources/app/email/invite-to-organization/en.html index 2a23407f4b..4ef26818f6 100644 --- a/backend/resources/app/email/invite-to-organization/en.html +++ b/backend/resources/app/email/invite-to-organization/en.html @@ -243,7 +243,7 @@ - ACCEPT INVITE diff --git a/backend/resources/app/email/invite-to-organization/en.txt b/backend/resources/app/email/invite-to-organization/en.txt index 72c97eead7..2180bea363 100644 --- a/backend/resources/app/email/invite-to-organization/en.txt +++ b/backend/resources/app/email/invite-to-organization/en.txt @@ -11,7 +11,7 @@ If you can't get in, your account probably isn't in the directory yet. To get ac Accept invitation using this link: -{{ public-uri }}/#/auth/verify-token?token={{token}} +{{ public-uri }}/?screen=auth-verify-token&token={{token}} Enjoy! The Penpot team. diff --git a/backend/resources/app/email/invite-to-team/en.html b/backend/resources/app/email/invite-to-team/en.html index 02e148a2ec..7c9b7d5e31 100644 --- a/backend/resources/app/email/invite-to-team/en.html +++ b/backend/resources/app/email/invite-to-team/en.html @@ -220,7 +220,7 @@ - ACCEPT INVITE diff --git a/backend/resources/app/email/invite-to-team/en.txt b/backend/resources/app/email/invite-to-team/en.txt index ecbd5d0be1..49c0c3633e 100644 --- a/backend/resources/app/email/invite-to-team/en.txt +++ b/backend/resources/app/email/invite-to-team/en.txt @@ -11,7 +11,7 @@ If you can't get in, your account probably isn't in the directory yet. To get ac Accept invitation using this link: -{{ public-uri }}/#/auth/verify-token?token={{token}} +{{ public-uri }}/?screen=auth-verify-token&token={{token}} Enjoy! The Penpot team. diff --git a/backend/resources/app/email/join-team/en.html b/backend/resources/app/email/join-team/en.html index 7df193b444..3a85987bad 100644 --- a/backend/resources/app/email/join-team/en.html +++ b/backend/resources/app/email/join-team/en.html @@ -199,7 +199,7 @@ - GO TO THE TEAM diff --git a/backend/resources/app/email/join-team/en.txt b/backend/resources/app/email/join-team/en.txt index 78cba680e0..608e4106d6 100644 --- a/backend/resources/app/email/join-team/en.txt +++ b/backend/resources/app/email/join-team/en.txt @@ -4,7 +4,7 @@ As you requested, {{invited-by|abbreviate:25}} has added you to the team “{{ t Go to the team with this link: -{{ public-uri }}/#/dashboard/team/{{team-id}} +{{ public-uri }}/?screen=dashboard-members&team-id={{team-id}} Enjoy! The Penpot team. diff --git a/backend/resources/app/email/password-recovery/en.html b/backend/resources/app/email/password-recovery/en.html index ded10cf78f..e19679add5 100644 --- a/backend/resources/app/email/password-recovery/en.html +++ b/backend/resources/app/email/password-recovery/en.html @@ -199,7 +199,7 @@ - RESET PASSWORD diff --git a/backend/resources/app/email/password-recovery/en.txt b/backend/resources/app/email/password-recovery/en.txt index 3bac8f815f..dfcb8a190e 100644 --- a/backend/resources/app/email/password-recovery/en.txt +++ b/backend/resources/app/email/password-recovery/en.txt @@ -3,7 +3,7 @@ Hello {{name|abbreviate:25}}! We received a request to reset your password. Click the link below to choose a new one: -{{ public-uri }}/#/auth/recovery?token={{token}} +{{ public-uri }}/?screen=auth-recovery&token={{token}} If you received this email by mistake, you can safely ignore it. Your password won't be changed. diff --git a/backend/resources/app/email/register/en.html b/backend/resources/app/email/register/en.html index 6a5062b4de..aa50cad2aa 100644 --- a/backend/resources/app/email/register/en.html +++ b/backend/resources/app/email/register/en.html @@ -205,7 +205,7 @@ - VERIFY EMAIL diff --git a/backend/resources/app/email/register/en.txt b/backend/resources/app/email/register/en.txt index 30b91be484..5283f6414b 100644 --- a/backend/resources/app/email/register/en.txt +++ b/backend/resources/app/email/register/en.txt @@ -4,7 +4,7 @@ Welcome to Penpot! Please verify your email to get started with your first design and collaboration. -{{ public-uri }}/#/auth/verify-token?token={{token}} +{{ public-uri }}/?screen=auth-verify-token&token={{token}} Enjoy! diff --git a/backend/resources/app/email/request-file-access-yourpenpot-view/en.html b/backend/resources/app/email/request-file-access-yourpenpot-view/en.html index f26a4b65da..7256811632 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot-view/en.html +++ b/backend/resources/app/email/request-file-access-yourpenpot-view/en.html @@ -207,7 +207,7 @@ - SEND A VIEW-ONLY LINK diff --git a/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt b/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt index c52649dd37..22ba669bfe 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt +++ b/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt @@ -6,7 +6,7 @@ Since this file is in your Personal Projects, you can provide access by sending To proceed, please click the link below to generate and send the view-only link: -{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true +{{ public-uri }}/?screen=viewer&file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true diff --git a/backend/resources/app/email/request-file-access-yourpenpot/en.html b/backend/resources/app/email/request-file-access-yourpenpot/en.html index cf3161a6b6..08dd2a7891 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot/en.html +++ b/backend/resources/app/email/request-file-access-yourpenpot/en.html @@ -230,7 +230,7 @@ - SEND A VIEW-ONLY LINK diff --git a/backend/resources/app/email/request-file-access-yourpenpot/en.txt b/backend/resources/app/email/request-file-access-yourpenpot/en.txt index e33a0bf80a..154c0fab5b 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot/en.txt +++ b/backend/resources/app/email/request-file-access-yourpenpot/en.txt @@ -19,7 +19,7 @@ Alternatively, you can create and share a view-only link to the file. This will Click the link below to generate and send the link: -{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true +{{ public-uri }}/?screen=viewer&file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true diff --git a/backend/resources/app/email/request-file-access/en.html b/backend/resources/app/email/request-file-access/en.html index 5887027350..54d0fc45c3 100644 --- a/backend/resources/app/email/request-file-access/en.html +++ b/backend/resources/app/email/request-file-access/en.html @@ -214,7 +214,7 @@ - GIVE ACCESS TO “{{team-name|abbreviate:25}}” TEAM @@ -247,7 +247,7 @@ - SEND A VIEW-ONLY LINK diff --git a/backend/resources/app/email/request-file-access/en.txt b/backend/resources/app/email/request-file-access/en.txt index 60dcd853df..02e39bdc04 100644 --- a/backend/resources/app/email/request-file-access/en.txt +++ b/backend/resources/app/email/request-file-access/en.txt @@ -13,7 +13,7 @@ This will automatically include {{requested-by|abbreviate:25}} in the team, so t Click the link below to provide team access: -{{ public-uri }}/#/dashboard/members?team-id={{team-id}}&invite-email={{requested-by-email|urlescape}} +{{ public-uri }}/?screen=dashboard-members&team-id={{team-id}}&invite-email={{requested-by-email|urlescape}} @@ -23,7 +23,7 @@ Alternatively, you can create and share a view-only link to the file. This will Click the link below to generate and send the link: -{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true +{{ public-uri }}/?screen=viewer&file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true If you do not wish to grant access at this time, you can simply disregard this email. diff --git a/backend/resources/app/email/request-team-access/en.html b/backend/resources/app/email/request-team-access/en.html index 7d279f505b..ccc1c1dcd9 100644 --- a/backend/resources/app/email/request-team-access/en.html +++ b/backend/resources/app/email/request-team-access/en.html @@ -205,7 +205,7 @@ - GIVE ACCESS TO “{{team-name|abbreviate:25}}” TEAM diff --git a/backend/resources/app/email/request-team-access/en.txt b/backend/resources/app/email/request-team-access/en.txt index c05d2b8693..4228647bd9 100644 --- a/backend/resources/app/email/request-team-access/en.txt +++ b/backend/resources/app/email/request-team-access/en.txt @@ -4,7 +4,7 @@ Hello! To provide access, please click the link below: -{{ public-uri }}/#/dashboard/members?team-id={{team-id}}&invite-email={{requested-by-email|urlescape}} +{{ public-uri }}/?screen=dashboard-members&team-id={{team-id}}&invite-email={{requested-by-email|urlescape}} If you do not wish to grant access at this time, you can simply disregard this email. diff --git a/backend/resources/app/templates/graph-console.tmpl b/backend/resources/app/templates/graph-console.tmpl index 8746e65ec3..eb2f1d0389 100644 --- a/backend/resources/app/templates/graph-console.tmpl +++ b/backend/resources/app/templates/graph-console.tmpl @@ -246,9 +246,9 @@ Graph Console if (!tree) return; // Make the loaded-session file name a link into the Penpot workspace. - // The legacy /#/workspace// route resolves the team - // itself, and project-id is already in the files-tree payload; same - // origin as this page, so no base URL to configure. + // Query-string routing needs the team id up front; team, project and + // file ids are all in the files-tree payload; same origin as this + // page, so no base URL to configure. function linkLoadedFile(teams) { if (!penpotLink || !penpotLink.dataset.fileId) return; const fileId = penpotLink.dataset.fileId; @@ -256,7 +256,7 @@ Graph Console (team.projects || []).forEach(function (project) { (project.files || []).forEach(function (file) { if (file.id === fileId) { - penpotLink.href = "/#/workspace/" + project.id + "/" + file.id; + penpotLink.href = "/?screen=workspace&team-id=" + team.id + "&file-id=" + file.id; penpotLink.title = "Open in Penpot"; const crumbs = document.getElementById("graph-file-crumbs"); if (crumbs) crumbs.textContent = team.name + " › " + project.name + " › "; diff --git a/backend/resources/app/templates/link-preview.tmpl b/backend/resources/app/templates/link-preview.tmpl index 997e92267d..6602b7e4cc 100644 --- a/backend/resources/app/templates/link-preview.tmpl +++ b/backend/resources/app/templates/link-preview.tmpl @@ -17,6 +17,6 @@ - + diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index 4581e23c73..1cea8af046 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -685,10 +685,8 @@ (defn- redirect-with-error ([error] (redirect-with-error error nil)) ([error hint] - (let [params {:error error :hint hint} - params (d/without-nils params) + (let [params {:screen "auth-login" :error error :hint hint} uri (-> (u/uri (cf/get :public-uri)) - (assoc :path "/#/auth/login") (assoc :query (u/map->query-string params)))] (redirect-response uri)))) @@ -707,21 +705,19 @@ :iss :prepared-register :exp (ct/in-future {:hours 48})) - params {:token (tokens/generate cfg info) + params {:screen "auth-register-validate" + :token (tokens/generate cfg info) :provider (:provider (:id provider)) - :fullname (:fullname info)} - params (d/without-nils params)] + :fullname (:fullname info)}] (redirect-response (-> (u/uri (cf/get :public-uri)) - (assoc :path "/#/auth/register/validate") (assoc :query (u/map->query-string params)))))) (defn- redirect-to-verify-token [token] - (let [params {:token token} + (let [params {:screen "auth-verify-token" :token token} uri (-> (u/uri (cf/get :public-uri)) - (assoc :path "/#/auth/verify-token") (assoc :query (u/map->query-string params)))] (redirect-response uri))) diff --git a/backend/test/backend_tests/auth_oidc_test.clj b/backend/test/backend_tests/auth_oidc_test.clj index d95a12af8c..90e1aa18f4 100644 --- a/backend/test/backend_tests/auth_oidc_test.clj +++ b/backend/test/backend_tests/auth_oidc_test.clj @@ -147,7 +147,7 @@ (let [result (#'oidc/redirect-with-error "auth-error" "hint message") loc (get-in result [::yres/headers "location"])] (t/is (= 302 (::yres/status result))) - (t/is (.contains loc "http://localhost:3449/#/auth/login?")) + (t/is (.contains loc "http://localhost:3449?screen=auth-login&")) (t/is (.contains loc "error=auth-error")) (t/is (.contains loc "hint=hint")))) (t/testing "without hint omits hint param" @@ -161,7 +161,7 @@ (let [result (#'oidc/redirect-to-verify-token "test-token-value") loc (get-in result [::yres/headers "location"])] (t/is (= 302 (::yres/status result))) - (t/is (.contains loc "http://localhost:3449/#/auth/verify-token?")) + (t/is (.contains loc "http://localhost:3449?screen=auth-verify-token&")) (t/is (.contains loc "token=test-token-value"))))) (t/deftest build-redirect-uri-constructs-redirect @@ -485,7 +485,7 @@ app.auth.oidc/get-profile (constantly (assoc test-profile :is-active false))] (let [result (#'oidc/callback-handler cfg request) loc (redirect-location result)] - (t/is (.contains loc "http://localhost:3449/#/auth/register/validate?")) + (t/is (.contains loc "http://localhost:3449?screen=auth-register-validate&")) (t/is (.contains loc "token="))))))) (t/deftest callback-success-flow @@ -503,7 +503,7 @@ app.loggers.audit/submit (constantly nil)] (let [result (#'oidc/callback-handler cfg request) loc (redirect-location result)] - (t/is (.contains loc "http://localhost:3449/#/auth/verify-token?")) + (t/is (.contains loc "http://localhost:3449?screen=auth-verify-token&")) (t/is (.contains loc "token="))))))) (t/deftest callback-gracefully-handles-unable-to-retrieve-user-info diff --git a/docs/technical-guide/developer/subsystems/link-preview.md b/docs/technical-guide/developer/subsystems/link-preview.md index 50202b1c48..497aa23165 100644 --- a/docs/technical-guide/developer/subsystems/link-preview.md +++ b/docs/technical-guide/developer/subsystems/link-preview.md @@ -23,58 +23,50 @@ The whole feature is gated behind the `link-preview` flag (enabled with ## How it works, end to end -The main obstacle is that Penpot is a SPA and all the routing state lives in -the URL **fragment** (`#/workspace?file-id=...`). The fragment is never sent to -the server, so with a plain URL the backend has no way to know which file the -link points to. The feature is therefore built from three cooperating pieces: +Penpot routes by query string: the screen and its context travel as +normal query params (`/?screen=workspace&file-id=...`), so the backend +sees everything directly. The feature is therefore built from three +cooperating pieces: ```text - user shares URL crawler (Slackbot, ...) regular browser - │ │ │ - │ https://host/?file-id=X#/workspace?... │ - │ │ │ - ▼ ▼ ▼ + user shares URL crawler (Slackbot, ...) regular browser + │ │ │ + │ https://host/?screen=workspace&file-id=X │ + │ │ │ + ▼ ▼ ▼ [frontend] [nginx] [nginx] - mirrors context user-agent matches crawler user-agent is normal - params before the rewrite / -> /link-preview serve SPA index.html - fragment on every (query string preserved) - navigation │ - ▼ - [backend] - GET /link-preview?file-id=X - query DB, render Open - Graph HTML template + nothing special: user-agent matches crawler user-agent is normal + screen+ids are rewrite / -> /link-preview serve SPA index.html + already in the (query string preserved) + query │ + ▼ + [backend] + GET /link-preview?screen=workspace&file-id=X + query DB, render Open + Graph HTML template ``` -### 1. Frontend: mirroring context params on the query string +### 1. Frontend: nothing to do File: `frontend/src/app/main/router.cljs` -On every navigation, the `navigated` event reads the freshly stored -`(:route state)` and syncs its `file-id`/`team-id`/`project-id` fragment -params into the query string (before the fragment) using -`history.replaceState`. Every other param in the URL is left untouched, -so unrelated params owned by other code survive. The write is skipped -when the resulting href already matches the address bar. The backend -applies its own file > project > team priority, so no filtering happens -on the frontend. The resulting URLs look like: +No URL surgery is needed: navigation writes the query string +directly (`screen` plus the screen params), so when the user copies +the URL from the address bar and shares it, the context ids travel +in a part of the URL that *does* reach the server. The resulting +URLs look like: ```text -https://design.penpot.app/?file-id=#/workspace?team-id=...&file-id=...&page-id=... -https://design.penpot.app/?team-id=&project-id=#/dashboard/recent?... -https://design.penpot.app/?team-id=#/dashboard/recent?team-id=... +https://design.penpot.app/?screen=workspace&team-id=...&file-id=...&page-id=... +https://design.penpot.app/?screen=dashboard-recent&team-id=...&project-id=... +https://design.penpot.app/?screen=auth-login ``` -Routes without any of those ids (e.g. auth pages) clear them from the -query string; `replaceState` only writes when the computed href differs -from the current one, so no URL churn happens on navigation. - -This way, when the user copies the URL from the address bar and shares it, the -context ids travel in a part of the URL that *does* reach the server. - -Legacy hash routes (`/workspace/:project-id/:file-id`, `/view/:file-id`, -`/dashboard/team/:team-id/...`) were removed: those old URLs no longer -redirect and resolve to the not-found page instead. +Legacy hash URLs (`#/workspace?...`) from bookmarks and old emails +are translated client-side to the query format on load (one-version +compatibility window, see the query-string routing plan); the +fragment never reaches the server, so this translation can only +happen in the browser. ### 2. Nginx: detecting link preview crawlers @@ -106,8 +98,12 @@ location = /link-preview { ``` Regular browsers are not affected: they keep receiving the SPA `index.html`. -If you self-host behind a different reverse proxy, you need to replicate this -routing there. +No SPA fallback rules are needed for app screens because the app lives +on the single `/` path — the `try_files … /index.html` fallback and the +`^/[^/]+` deep-path rule already cover everything, and no new path +rules will ever be needed for routing. If you self-host behind a +different reverse proxy, you only need to replicate the crawler +rewrite on `/` there. ### 3. Backend: the `/link-preview` endpoint @@ -163,12 +159,12 @@ equivalent `twitter:*` card tags and ``. The body contains a single script: ```html - + ``` so that if a *human* somehow lands on `/link-preview` (e.g. some clients let users click through to the fetched URL), the browser bounces back to the SPA root -keeping the query string and the fragment, and the app loads normally. The +keeping the query string, and the app loads normally. The redirect strips only the trailing `link-preview` segment so subpath deployments keep their prefix. Crawlers do not execute JavaScript, so they just read the meta tags. @@ -240,8 +236,8 @@ indexing these preview pages, and responses are marked non-cacheable. 3. In the browser (`http://localhost:3449`), open a file in the workspace and go back to the dashboard — leaving the workspace is what generates the - dashboard thumbnail. Verify the address bar now shows `?file-id=...` - before the `#`. + dashboard thumbnail. Verify the address bar now shows + `?screen=workspace&file-id=...` (screen plus context, no fragment). 4. Hit the endpoint directly (bypasses the user-agent detection): @@ -257,7 +253,7 @@ indexing these preview pages, and responses are marked non-cacheable. nginx → rewrite → backend path: ```bash - curl -A "Slackbot-LinkExpanding 1.0" "http://localhost:3449/?file-id=" + curl -A "Slackbot-LinkExpanding 1.0" "http://localhost:3449/?screen=workspace&file-id=" ``` The same URL with a normal user-agent must return the SPA `index.html`. @@ -287,9 +283,9 @@ indexing these preview pages, and responses are marked non-cacheable. * `backend/test/backend_tests/http_assets_test.clj` (`objects-handler-file-thumbnail-bucket-link-preview-flag`) — the `file-thumbnail` bucket is public only while the flag is enabled. - * `frontend/test/frontend_tests/router_test.cljs` — the `navigated` URL - surgery (mirror, skip, stale-strip, clear, unrelated-param - preservation, every-present-id, repeated-key, subpath base). + * `frontend/test/frontend_tests/router_test.cljs` — the `screen` + match/resolve rules (token building, missing/unknown screen, + repeated keys). ## Relevant files @@ -300,6 +296,7 @@ indexing these preview pages, and responses are marked non-cacheable. | `backend/src/app/http/assets.clj` | Makes `file-thumbnail` bucket public under the flag | | `backend/src/app/http.clj`, `backend/src/app/main.clj` | Route registration and system wiring | | `common/src/app/common/flags.cljc` | `:link-preview` flag definition | -| `frontend/src/app/main/router.cljs` | Mirrors context ids on the query string on navigation | +| `frontend/src/app/main/router.cljs` | Screen match/resolve over the query string | +| `frontend/src/app/main/ui/routes.cljs` | Route table plus the one-version legacy hash translation | | `docker/devenv/files/nginx.conf` | Devenv crawler detection and `/link-preview` routing | | `docker/images/files/nginx.conf.template` | Same routing for the production image | diff --git a/frontend/playwright/helpers/MockWebSocketHelper.js b/frontend/playwright/helpers/MockWebSocketHelper.js index 8cf63f9737..375be4a83e 100644 --- a/frontend/playwright/helpers/MockWebSocketHelper.js +++ b/frontend/playwright/helpers/MockWebSocketHelper.js @@ -28,6 +28,10 @@ export class MockWebSocketHelper extends EventTarget { await page.addInitScript({ path: "playwright/scripts/MockWebSocket.js" }); } + static clear() { + this.#mocks = new Map(); + } + static waitForURL(url) { return new Promise((resolve) => { const intervalID = setInterval(() => { diff --git a/frontend/playwright/ui/pages/DashboardPage.js b/frontend/playwright/ui/pages/DashboardPage.js index eb4bea0b9f..8532455c88 100644 --- a/frontend/playwright/ui/pages/DashboardPage.js +++ b/frontend/playwright/ui/pages/DashboardPage.js @@ -235,63 +235,63 @@ export class DashboardPage extends BaseWebSocketPage { async goToDashboard() { await this.page.goto( - `#/dashboard/recent?team-id=${DashboardPage.anyTeamId}`, + `/?screen=dashboard-recent&team-id=${DashboardPage.anyTeamId}`, ); await expect(this.mainHeading).toBeVisible(); } async goToSecondTeamDashboard() { await this.page.goto( - `#/dashboard/recent?team-id=${DashboardPage.secondTeamId}`, + `/?screen=dashboard-recent&team-id=${DashboardPage.secondTeamId}`, ); } async goToSecondTeamMembersSection() { await this.page.goto( - `#/dashboard/members?team-id=${DashboardPage.secondTeamId}`, + `/?screen=dashboard-members&team-id=${DashboardPage.secondTeamId}`, ); } async goToSecondTeamInvitationsSection() { await this.page.goto( - `#/dashboard/invitations?team-id=${DashboardPage.secondTeamId}`, + `/?screen=dashboard-invitations&team-id=${DashboardPage.secondTeamId}`, ); } async goToSecondTeamWebhooksSection() { await this.page.goto( - `#/dashboard/webhooks?team-id=${DashboardPage.secondTeamId}`, + `/?screen=dashboard-webhooks&team-id=${DashboardPage.secondTeamId}`, ); } async goToSecondTeamWebhooksSection() { await this.page.goto( - `#/dashboard/webhooks?team-id=${DashboardPage.secondTeamId}`, + `/?screen=dashboard-webhooks&team-id=${DashboardPage.secondTeamId}`, ); } async goToSecondTeamSettingsSection() { await this.page.goto( - `#/dashboard/settings?team-id=${DashboardPage.secondTeamId}`, + `/?screen=dashboard-settings&team-id=${DashboardPage.secondTeamId}`, ); } async goToSearch() { await this.page.goto( - `#/dashboard/search?team-id=${DashboardPage.anyTeamId}`, + `/?screen=dashboard-search&team-id=${DashboardPage.anyTeamId}`, ); } async goToDrafts() { await this.page.goto( - `#/dashboard/files?team-id=${DashboardPage.anyTeamId}&project-id=${DashboardPage.draftProjectId}`, + `/?screen=dashboard-files&team-id=${DashboardPage.anyTeamId}&project-id=${DashboardPage.draftProjectId}`, ); await expect(this.mainHeading).toHaveText("Drafts"); } async goToFonts() { await this.page.goto( - `#/dashboard/fonts?team-id=${DashboardPage.anyTeamId}`, + `/?screen=dashboard-fonts&team-id=${DashboardPage.anyTeamId}`, ); await expect(this.mainHeading).toHaveText("Fonts"); } @@ -304,14 +304,14 @@ export class DashboardPage extends BaseWebSocketPage { async goToLibraries() { await this.page.goto( - `#/dashboard/libraries?team-id=${DashboardPage.anyTeamId}`, + `/?screen=dashboard-libraries&team-id=${DashboardPage.anyTeamId}`, ); await expect(this.mainHeading).toHaveText("Libraries"); } async goToDeleted() { await this.page.goto( - `#/dashboard/deleted?team-id=${DashboardPage.anyTeamId}`, + `/?screen=dashboard-deleted&team-id=${DashboardPage.anyTeamId}`, ); await expect(this.mainHeading).toHaveText("Projects"); } diff --git a/frontend/playwright/ui/pages/RegisterPage.js b/frontend/playwright/ui/pages/RegisterPage.js index 5045d34b88..ba89b519fe 100644 --- a/frontend/playwright/ui/pages/RegisterPage.js +++ b/frontend/playwright/ui/pages/RegisterPage.js @@ -78,7 +78,7 @@ export class RegisterPage extends BasePage { } async goToVerifyToken(token = "verify-email-token") { - await this.page.goto(`/#/auth/verify-token?token=${token}`); + await this.page.goto(`/?screen=auth-verify-token&token=${token}`); } static async init(page) { diff --git a/frontend/playwright/ui/pages/ShortcutsPage.js b/frontend/playwright/ui/pages/ShortcutsPage.js index d45bb1b970..763d3117d0 100644 --- a/frontend/playwright/ui/pages/ShortcutsPage.js +++ b/frontend/playwright/ui/pages/ShortcutsPage.js @@ -158,7 +158,7 @@ export class ShortcutsPage extends BaseWebSocketPage { } async goToShortcuts() { - await this.page.goto("#/settings/shortcuts"); + await this.page.goto("/?screen=settings-shortcuts"); await expect(this.shortcutsSection).toBeVisible(); } diff --git a/frontend/playwright/ui/pages/SubscriptionProfilePage.js b/frontend/playwright/ui/pages/SubscriptionProfilePage.js index b9769cfa52..e340e4a294 100644 --- a/frontend/playwright/ui/pages/SubscriptionProfilePage.js +++ b/frontend/playwright/ui/pages/SubscriptionProfilePage.js @@ -22,7 +22,7 @@ export class SubscriptionProfilePage extends DashboardPage { } async goToSubscriptions() { - await this.page.goto(`#/settings/subscriptions`); + await this.page.goto(`/?screen=settings-subscription`); await expect(this.mainHeading).toBeVisible(); } } diff --git a/frontend/playwright/ui/pages/ViewerPage.js b/frontend/playwright/ui/pages/ViewerPage.js index 034b25e2f3..4cec20fb51 100644 --- a/frontend/playwright/ui/pages/ViewerPage.js +++ b/frontend/playwright/ui/pages/ViewerPage.js @@ -1,4 +1,5 @@ import { BaseWebSocketPage } from "./BaseWebSocketPage"; +import { MockWebSocketHelper } from "../../helpers/MockWebSocketHelper"; export class ViewerPage extends BaseWebSocketPage { static anyFileId = "c7ce0794-0992-8105-8004-38f280443849"; @@ -119,9 +120,22 @@ export class ViewerPage extends BaseWebSocketPage { fileId = ViewerPage.anyFileId, pageId = ViewerPage.anyPageId, } = {}) { - await this.page.goto( - `/#/view?file-id=${fileId}&page-id=${pageId}§ion=interactions&index=0`, - ); + // Same as WorkspacePage.goToWorkspace: skip the reload when already + // on the target file so repeated setups keep the in-memory file + // state. Extra query params are ignored. + const currentParams = new URL(this.page.url()).searchParams; + const sameFile = + currentParams.get("screen") === "viewer" && + currentParams.get("file-id") === fileId && + currentParams.get("page-id") === pageId; + if (!sameFile) { + // Same as WorkspacePage.goToWorkspace: drop stale mocks from any + // previous document before reloading the app. + MockWebSocketHelper.clear(); + await this.page.goto( + `/?screen=viewer&file-id=${fileId}&page-id=${pageId}§ion=interactions&index=0`, + ); + } this.#ws = await this.waitForNotificationsWebSocket(); await this.#ws.mockOpen(); diff --git a/frontend/playwright/ui/pages/WorkspacePage.js b/frontend/playwright/ui/pages/WorkspacePage.js index 3ad06f251e..3e1758d491 100644 --- a/frontend/playwright/ui/pages/WorkspacePage.js +++ b/frontend/playwright/ui/pages/WorkspacePage.js @@ -1,5 +1,6 @@ import { expect } from "@playwright/test"; import { readFile } from "node:fs/promises"; +import { MockWebSocketHelper } from "../../helpers/MockWebSocketHelper"; import { BaseWebSocketPage } from "./BaseWebSocketPage"; import { Transit } from "../../helpers/Transit"; @@ -218,13 +219,39 @@ export class WorkspacePage extends BaseWebSocketPage { pageId = this.pageId ?? WorkspacePage.anyPageId, pageName = "Page 1", } = {}) { - await this.page.goto( - `/#/workspace?team-id=${WorkspacePage.anyTeamId}&file-id=${fileId}&page-id=${pageId}`, - ); + // Helpers often call setup (and this) several times per test with the + // same file. Re-navigating would reload the document and wipe the + // in-memory file state (e.g. tokens created by previous steps), so + // only navigate when the target file actually changes. Extra query + // params the app adds itself (e.g. layout=tokens) are ignored, and + // navigating away and back still reloads as before. + const currentParams = new URL(this.page.url()).searchParams; + const sameFile = + currentParams.get("screen") === "workspace" && + currentParams.get("team-id") === WorkspacePage.anyTeamId && + currentParams.get("file-id") === fileId && + currentParams.get("page-id") === pageId; + if (!sameFile) { + // Drop mocks from any previous document: page.goto reloads the app, + // so entries registered by the old document would otherwise resolve + // waitForNotificationsWebSocket immediately with a stale mock that + // no longer exists in the new document. + MockWebSocketHelper.clear(); + await this.page.goto( + `/?screen=workspace&team-id=${WorkspacePage.anyTeamId}&file-id=${fileId}&page-id=${pageId}`, + ); + } this.#ws = await this.waitForNotificationsWebSocket(); await this.#ws.mockOpen(); - await this.#waitForWebSocketReadiness(pageName); + if (!sameFile) { + await this.#waitForWebSocketReadiness(pageName); + } else { + // Already on the target file (e.g. Tokens tab open, where the + // sitemap page name is not rendered): just ensure the canvas is + // present instead of waiting for the page name. + await expect(this.viewport).toBeVisible({ timeout: 30000 }); + } } async #waitForWebSocketReadiness(pageName) { diff --git a/frontend/playwright/ui/specs/dashboard.spec.js b/frontend/playwright/ui/specs/dashboard.spec.js index 7b5df888d8..1fe01a3068 100644 --- a/frontend/playwright/ui/specs/dashboard.spec.js +++ b/frontend/playwright/ui/specs/dashboard.spec.js @@ -10,7 +10,7 @@ test("Dashboard page has title ", async ({ page }) => { await dashboardPage.goToDashboard(); - await expect(dashboardPage.page).toHaveURL(/dashboard/); + await expect(dashboardPage.page).toHaveURL(/screen=dashboard/); await expect(dashboardPage.mainHeading).toBeVisible(); }); diff --git a/frontend/playwright/ui/specs/email-verification.spec.js b/frontend/playwright/ui/specs/email-verification.spec.js index 6be76653ce..65c1a9cb99 100644 --- a/frontend/playwright/ui/specs/email-verification.spec.js +++ b/frontend/playwright/ui/specs/email-verification.spec.js @@ -18,7 +18,7 @@ test.describe("Email verification", () => { await registerPage.setupEmailVerificationSuccess(); await registerPage.goToVerifyToken(); - await page.waitForURL("**/dashboard/**"); + await page.waitForURL(/screen=dashboard/); // `default` is the body class applied for dark theme, `light` for // light theme (see app.util.theme/set-color-scheme). @@ -33,9 +33,9 @@ test.describe("Email verification", () => { await registerPage.setupEmailVerificationSuccess(); await registerPage.goToVerifyToken(); - await page.waitForURL("**/dashboard/**"); + await page.waitForURL(/screen=dashboard/); - await page.goto("/#/settings/options"); + await page.goto("/?screen=settings-options"); // The language select is the first combobox on the page, the theme // select is the second one. diff --git a/frontend/playwright/ui/specs/legacy-hash-redirect.spec.js b/frontend/playwright/ui/specs/legacy-hash-redirect.spec.js new file mode 100644 index 0000000000..64122dbe17 --- /dev/null +++ b/frontend/playwright/ui/specs/legacy-hash-redirect.spec.js @@ -0,0 +1,36 @@ +import { test, expect } from "@playwright/test"; +import { LoginPage } from "../pages/LoginPage"; + +// One-version compatibility: legacy `#/…` hash URLs translate to the +// query-string format client-side (the fragment never reaches the +// server). TODO(next-version): delete with the legacy hash shim. +test.beforeEach(async ({ page }) => { + await LoginPage.init(page); + + const login = new LoginPage(page); + await login.initWithLoggedOutUser(); +}); + +test("Legacy auth hash URL redirects to query-string format", async ({ + page, +}) => { + const loginPage = new LoginPage(page); + + await page.goto("/#/auth/login"); + + await expect(page).toHaveURL(/screen=auth-login/); + expect(new URL(page.url()).hash).toBe(""); + await expect(loginPage.initialHeading).toBeVisible(); +}); + +test("Unknown legacy hash falls through to the query flow", async ({ + page, +}) => { + const loginPage = new LoginPage(page); + // The unknown-route fallback rechecks profile AND teams. + await loginPage.mockRPC("get-teams", "logged-in-user/get-teams-default.json"); + + await page.goto("/?template=foo#/nope"); + + await expect(loginPage.initialHeading).toBeVisible(); +}); diff --git a/frontend/playwright/ui/specs/login.spec.js b/frontend/playwright/ui/specs/login.spec.js index 254e205b75..e08f8c6eee 100644 --- a/frontend/playwright/ui/specs/login.spec.js +++ b/frontend/playwright/ui/specs/login.spec.js @@ -7,7 +7,7 @@ test.beforeEach(async ({ page }) => { const login = new LoginPage(page); await login.initWithLoggedOutUser(); - await page.goto("/#/auth/login"); + await page.goto("/?screen=auth-login"); }); test("User is redirected to the login page when logged out", async ({ @@ -17,11 +17,22 @@ test("User is redirected to the login page when logged out", async ({ await loginPage.setupLoggedInUser(); - await expect(loginPage.page).toHaveURL(/auth\/login$/); + await expect(loginPage.page).toHaveURL(/screen=auth-login$/); await expect(loginPage.initialHeading).toBeVisible(); }); test.describe("Login form", () => { + test("User navigates to register by clicking the create account link", async ({ + page, + }) => { + await LoginPage.mockConfigFlags(page, ["registration"]); + await page.goto("/?screen=auth-login"); + + await page.getByTestId("register-submit").click(); + + await expect(page).toHaveURL(/screen=auth-register/); + }); + test("User logs in by filling the login form", async ({ page }) => { const loginPage = new LoginPage(page); await loginPage.setupLoginSuccess(); @@ -30,8 +41,8 @@ test.describe("Login form", () => { await loginPage.fillEmailAndPasswordInputs("foo@example.com", "loremipsum"); await loginPage.clickLoginButton(); - await page.waitForURL("**/dashboard/**"); - await expect(loginPage.page).toHaveURL(/dashboard/); + await page.waitForURL(/screen=dashboard/); + await expect(loginPage.page).toHaveURL(/screen=dashboard/); }); test("User gets error message when submitting an bad formatted email ", async ({ @@ -58,6 +69,6 @@ test.describe("Login form", () => { await loginPage.clickLoginButton(); await expect(loginPage.invalidCredentialsError).toBeVisible(); - await expect(loginPage.page).toHaveURL(/auth\/login$/); + await expect(loginPage.page).toHaveURL(/screen=auth-login$/); }); }); diff --git a/frontend/playwright/ui/specs/register.spec.js b/frontend/playwright/ui/specs/register.spec.js index aaaaf0f035..f13083672d 100644 --- a/frontend/playwright/ui/specs/register.spec.js +++ b/frontend/playwright/ui/specs/register.spec.js @@ -3,7 +3,7 @@ import { RegisterPage } from "../pages/RegisterPage"; test.beforeEach(async ({ page }) => { await RegisterPage.initWithLoggedOutUser(page); - await page.goto("/#/auth/register"); + await page.goto("/?screen=auth-register"); }); test.describe("Register form errors", () => { diff --git a/frontend/playwright/ui/specs/tokens/crud.spec.js b/frontend/playwright/ui/specs/tokens/crud.spec.js index 35922e0af1..3e90aca0e3 100644 --- a/frontend/playwright/ui/specs/tokens/crud.spec.js +++ b/frontend/playwright/ui/specs/tokens/crud.spec.js @@ -1936,6 +1936,9 @@ test.describe("User can't create groups that clash with token names", () => { const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" }); const { tokensUpdateCreateModal } = await setupTokensFileRender(page, { + // Same file as the outer setup: re-navigating would reload the + // app and wipe the fixture tokens this test clashes against. + file: "workspace/get-file-tokens-all-types.json", flags: ["enable-token-shadow"], }); @@ -1975,6 +1978,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -1990,6 +1994,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2005,6 +2010,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2020,6 +2026,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2035,6 +2042,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2050,6 +2058,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2065,6 +2074,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2080,6 +2090,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2095,6 +2106,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2110,6 +2122,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2125,6 +2138,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2140,6 +2154,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2155,6 +2170,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2177,6 +2193,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2186,6 +2203,9 @@ test.describe("User can't create groups that clash with token names", () => { const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" }); const { tokensUpdateCreateModal } = await setupTokensFileRender(page, { + // Same file as the outer setup: re-navigating would reload the + // app and wipe the fixture tokens this test clashes against. + file: "workspace/get-file-tokens-all-types.json", flags: ["enable-token-shadow"], }); @@ -2249,6 +2269,7 @@ test.describe("User can't create groups that clash with token names", () => { const { tokenThemesSetsSidebar, tokensSidebar } = await setupTokensFileRender(page, { file: "workspace/get-file-tokens-all-types.json", + flags: ["enable-token-shadow"], }); await expect(tokensSidebar).toBeVisible(); @@ -2258,6 +2279,9 @@ test.describe("User can't create groups that clash with token names", () => { const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" }); const { tokensUpdateCreateModal } = await setupTokensFileRender(page, { + // Same file as the outer setup: re-navigating would reload the + // app and wipe the fixture tokens this test clashes against. + file: "workspace/get-file-tokens-all-types.json", flags: ["enable-token-shadow"], }); diff --git a/frontend/playwright/ui/visual-specs/visual-dashboard.spec.js b/frontend/playwright/ui/visual-specs/visual-dashboard.spec.js index 9a61a8ae97..7388d5534c 100644 --- a/frontend/playwright/ui/visual-specs/visual-dashboard.spec.js +++ b/frontend/playwright/ui/visual-specs/visual-dashboard.spec.js @@ -9,7 +9,7 @@ test("User goes to an empty dashboard", async ({ page }) => { const dashboardPage = new DashboardPage(page); await dashboardPage.goToDashboard(); - await expect(dashboardPage.page).toHaveURL(/dashboard/); + await expect(dashboardPage.page).toHaveURL(/screen=dashboard/); await expect(dashboardPage.mainHeading).toBeVisible(); await expect(dashboardPage.page).toHaveScreenshot(); diff --git a/frontend/playwright/ui/visual-specs/visual-login.spec.js b/frontend/playwright/ui/visual-specs/visual-login.spec.js index 5ee1ba7a2b..6b53a2d5d9 100644 --- a/frontend/playwright/ui/visual-specs/visual-login.spec.js +++ b/frontend/playwright/ui/visual-specs/visual-login.spec.js @@ -6,7 +6,7 @@ test.beforeEach(async ({ page }) => { const login = new LoginPage(page); await login.initWithLoggedOutUser(); - await login.page.goto("/#/auth/login"); + await login.page.goto("/?screen=auth-login"); }); test.describe("Login form", () => { @@ -33,7 +33,7 @@ test.describe("Login form", () => { await login.clickLoginButton(); await expect(login.invalidCredentialsError).toBeVisible(); - await expect(login.page).toHaveURL(/auth\/login$/); + await expect(login.page).toHaveURL(/screen=auth-login$/); await expect(login.page).toHaveScreenshot(); }); }); diff --git a/frontend/src/app/main/data/nitrate.cljs b/frontend/src/app/main/data/nitrate.cljs index 4be62a2cbf..6908b39aaa 100644 --- a/frontend/src/app/main/data/nitrate.cljs +++ b/frontend/src/app/main/data/nitrate.cljs @@ -100,7 +100,7 @@ :profile-id profile-id :team-permissions team-permissions})) -(def go-to-subscription-url (dm/str (u/join cf/public-uri "#/settings/subscriptions"))) +(def go-to-subscription-url (dm/str cf/public-uri "?screen=settings-subscription")) (def go-to-ac-url (build-admin-console-url "")) diff --git a/frontend/src/app/main/data/profile.cljs b/frontend/src/app/main/data/profile.cljs index f2cae4b2bc..d68751ad78 100644 --- a/frontend/src/app/main/data/profile.cljs +++ b/frontend/src/app/main/data/profile.cljs @@ -77,8 +77,7 @@ (let [data (ex-data cause)] (if (and (= :authorization (:type data)) (= :challenge-required (:code data))) - (let [path (rt/get-current-path) - href (->> path + (let [href (->> (rt/get-current-href) (js/encodeURIComponent) (str "/challenge.html?redirect="))] (rx/of (rt/nav-raw :href href))) diff --git a/frontend/src/app/main/data/team.cljs b/frontend/src/app/main/data/team.cljs index bff83ff03a..081f41102d 100644 --- a/frontend/src/app/main/data/team.cljs +++ b/frontend/src/app/main/data/team.cljs @@ -683,11 +683,9 @@ (->> (rp/cmd! :get-team-invitation-token params) (rx/map (fn [params] - (rt/resolve router :auth-verify-token params))) - (rx/map (fn [fragment] - (assoc cf/public-uri :fragment fragment))) + (rt/resolve-uri router :auth-verify-token params))) (rx/tap (fn [uri] - (clipboard/to-clipboard (str uri)))) + (clipboard/to-clipboard uri))) (rx/tap on-success) (rx/ignore) (rx/catch on-error)))))) diff --git a/frontend/src/app/main/router.cljs b/frontend/src/app/main/router.cljs index cdaa6197cb..d459e81483 100644 --- a/frontend/src/app/main/router.cljs +++ b/frontend/src/app/main/router.cljs @@ -18,26 +18,35 @@ [beicon.v2.core :as rx] [cuerdas.core :as str] [goog.events :as e] - [potok.v2.core :as ptk] - [reitit.core :as r])) + [potok.v2.core :as ptk])) ;; --- Router API -(defn map->Match - [data] - (r/map->Match data)) - -(defn resolve - ([router id] - (resolve router id {})) - ([router id params] - (when router - (when-let [match (r/match-by-name router id)] - (r/match->path match params))))) +;; Query-string routing: `router` is the set of enabled route names +;; (see `app.main.ui.routes/routes`). The `screen` query param carries +;; the route name; every other param travels as a plain query param. (defn create [routes] - (r/router routes)) + routes) + +(defn resolve + "Build the history token (`?screen=¶ms`) for a route. + Returns nil when the route id is not enabled." + ([router id] + (resolve router id {})) + ([router id params] + (when (contains? router id) + ;; The target screen always wins: callers often forward the + ;; current query params (which carry the previous screen). + (str "?" (u/map->query-string + (into {:screen (name id)} (dissoc params :screen))))))) + +(defn resolve-uri + "Build the absolute URL string for a route under `cf/public-uri`." + [router id params] + (when-let [token (resolve router id params)] + (dm/str cf/public-uri "?" (subs token 1)))) (defn initialize-router [routes] @@ -50,19 +59,6 @@ [url] (js/encodeURIComponent url)) -(defn match - "Given routing tree and current path, return match with possibly - coerced parameters. Return nil if no match found." - [router path] - (let [uri (u/uri path)] - (when-let [match (r/match-by-path router (:path uri))] - (let [query-params (u/query-string->map (:query uri)) - params {:path (:path-params match) - :query query-params}] - (-> match - (assoc :params params) - (assoc :query-params query-params)))))) - ;; --- Navigate (Event) (defn get-query-param @@ -75,6 +71,22 @@ (let [v (get params k)] (if (sequential? v) (peek v) v))) +(defn match + "Given the enabled routes and the current history token (the query + string, `?screen=¶ms`), return a match shaped like the old + reitit one (`:data/:name`, `:params/:query`, `:query-params`). + Return nil when there is no usable `screen`." + [router token] + (let [query (if (str/starts-with? (or token "") "?") + (subs token 1) + (or token "")) + query-params (u/query-string->map query) + screen (some-> (get-query-param query-params :screen) keyword)] + (when (contains? router screen) + {:data {:name screen} + :params {:path {} :query query-params} + :query-params query-params}))) + (defn navigated [match send-event-info?] (ptk/reify ::navigated @@ -95,30 +107,7 @@ (update [_ state] (-> state (assoc :route match) - (dissoc :exception))) - - ptk/EffectEvent - (effect [_ state _] - ;; The route is read from the state the `update` above just stored: - ;; the effect always runs after the update. The sharing-context ids - ;; are synced into the pre-fragment query (the fragment never reaches - ;; the server, so shared links need them there); every other param is - ;; left untouched, except valueless ones (`?flag`), which the query - ;; codec cannot round-trip and are dropped. The backend applies its - ;; own file > project > team priority, so no filtering is needed - ;; here. - (let [params (:query-params (:route state)) - uri (u/uri (.-href globals/location)) - search (reduce (fn [m k] - (let [v (get-query-param params k)] - (if (some? v) - (assoc m k v) - (dissoc m k)))) - (u/query-string->map (:query uri)) - [:file-id :team-id :project-id]) - href (str (assoc uri :query (u/map->query-string search)))] - (when (not= href (.-href globals/location)) - (.replaceState js/history nil "" href)))))) + (dissoc :exception))))) (defn navigate [id params & {:keys [::replace ::new-window] :as options}] @@ -137,7 +126,7 @@ (if ^boolean new-window (let [name (or (::window-name options) "_blank") - uri (assoc cf/public-uri :fragment path)] + uri (assoc cf/public-uri :query (some-> path (subs 1)))] (dom/open-new-window uri name nil)) (ts/asap #(if ^boolean replace @@ -215,37 +204,26 @@ [] (.-href globals/location)) -(defn get-current-path - [] - (let [hash (.-hash globals/location)] - (if (str/starts-with? hash "#") - (subs hash 1) - hash))) - ;; --- History API ;; Check the urls to see if we need to send the navigated event. -;; If two paths are the same we only send the event when there is a -;; change in the parameters `file-id`, `page-id` or `team-id` +;; If two query strings select the same screen we only send the event +;; when there is a change in the parameters `screen`, `file-id`, +;; `page-id` or `team-id` (defn- send-event-info? [old-url new-url] - (let [params [:file-id :page-id :team-id] - new-uri (u/uri new-url) - new-path (:path new-uri) - new-params (-> new-uri :query u/query-string->map (select-keys params)) - old-uri (u/uri old-url) - old-path (:path old-uri) - old-params (-> old-uri :query u/query-string->map (select-keys params))] - (or (not= old-path new-path) - (not= new-params old-params)))) + (let [params [:screen :file-id :page-id :team-id] + new-params (-> (u/uri new-url) :query u/query-string->map (select-keys params)) + old-params (-> (u/uri old-url) :query u/query-string->map (select-keys params))] + (not= new-params old-params))) (defn initialize-history [on-change] (ptk/reify ::initialize-history ptk/UpdateEvent (update [_ state] - (let [history (bhistory/create)] + (let [history (bhistory/create (:path cf/public-uri))] (bhistory/enable! history) (assoc state :history history))) diff --git a/frontend/src/app/main/ui/auth.cljs b/frontend/src/app/main/ui/auth.cljs index 9ec094716d..d1ede92c33 100644 --- a/frontend/src/app/main/ui/auth.cljs +++ b/frontend/src/app/main/ui/auth.cljs @@ -47,7 +47,7 @@ :auth-section true :register is-register)} [:> heading* {:level 1 :typography "title-large" :class (stl/css :logo-container)} - [:a {:href "#/" :title "Penpot" :class (stl/css :logo-btn)} + [:a {:href "/" :title "Penpot" :class (stl/css :logo-btn)} [:> raw-svg* {:id raw-svg/penpot-logo :class (stl/css :logo)}]]] [:div {:class (stl/css :login-illustration)} diff --git a/frontend/src/app/main/ui/dashboard/subscription.cljs b/frontend/src/app/main/ui/dashboard/subscription.cljs index 6314349710..0643ba37c3 100644 --- a/frontend/src/app/main/ui/dashboard/subscription.cljs +++ b/frontend/src/app/main/ui/dashboard/subscription.cljs @@ -20,7 +20,6 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.keyboard :as kbd] [beicon.v2.core :as rx] - [lambdaisland.uri :as u] [rumext.v2 :as mf])) (defn get-subscription-type @@ -77,7 +76,7 @@ (let [subscription (:subscription (:props profile)) subscription-type (get-subscription-type subscription) subscription-is-trial (= (:status subscription) "trialing") - subscription-href (dm/str (u/join cf/public-uri "#/settings/subscriptions"))] + subscription-href (dm/str cf/public-uri "?screen=settings-subscription")] (case subscription-type "professional" @@ -341,7 +340,7 @@ [{:keys [profile]}] (let [subscription (-> profile :props :subscription) subscription-type (get-subscription-type subscription) - go-to-subscription (dm/str (u/join cf/public-uri "#/settings/subscriptions")) + go-to-subscription (dm/str cf/public-uri "?screen=settings-subscription") seats (:quantity subscription) editors (count (:editors subscription)) cta-title diff --git a/frontend/src/app/main/ui/routes.cljs b/frontend/src/app/main/ui/routes.cljs index 27f4ed8812..a4cb7fedab 100644 --- a/frontend/src/app/main/ui/routes.cljs +++ b/frontend/src/app/main/ui/routes.cljs @@ -20,7 +20,8 @@ [app.util.storage :as storage] [beicon.v2.core :as rx] [cuerdas.core :as str] - [potok.v2.core :as ptk])) + [potok.v2.core :as ptk] + [reitit.core :as r])) (def ^:private sso-authorization-max-age-ms (* 5 60 1000)) @@ -29,6 +30,52 @@ (atom {})) (def routes + "Enabled route names. Query-string routing: the `screen` query param + carries the route name (`?screen=¶ms`); it is router-owned + and reserved, every other param travels as a plain query param." + (into #{:auth-login + :auth-register + :auth-register-validate + :auth-register-success + :auth-recovery-request + :auth-recovery + :auth-verify-token + :settings-profile + :settings-password + :settings-feedback + :settings-options + :settings-subscription + :settings-integrations + :settings-notifications + :settings-shortcuts + :frame-preview + :viewer + :render-sprite + :dashboard-members + :dashboard-invitations + :dashboard-webhooks + :dashboard-settings + :dashboard-recent + :dashboard-search + :dashboard-fonts + :dashboard-font-providers + :dashboard-libraries + :dashboard-files + :dashboard-deleted + :workspace} + (concat + (when (contains? cf/flags :admin-console) + [:nitrate-entry]) + (when *assert* + [:debug-icons-preview + :debug-playground])))) + +;; TODO(next-version): delete the legacy hash table, `legacy-match` +;; and the hash branch of `on-navigate` below. Legacy `#/…` URLs stop +;; resolving after one Penpot version of compatibility. +(def ^:private legacy-routes + "Pre-query-string route table, kept only to translate legacy + `#/…` hash URLs during the compatibility window." [["/auth" ["/login" :auth-login] ["/register" :auth-register] @@ -79,6 +126,19 @@ ["/workspace" :workspace]]) +(defonce ^:private legacy-router + (r/router legacy-routes)) + +(defn- legacy-match + "Match a legacy hash path (`/workspace?...`, without the `#`) against + the pre-query-string table. Returns `{:name params}` or nil." + [hash-path] + (let [uri (u/uri hash-path)] + (when-let [match (r/match-by-path legacy-router (:path uri))] + {:name (get-in match [:data :name]) + :params (merge (:path-params match) + (u/query-string->map (:query uri)))}))) + (defn- store-session-params [{:keys [template plugin]}] @@ -157,24 +217,45 @@ :is-dashboard is-dashboard?})) (check-sso-and-navigate match send-event-info? url)))) +(declare on-query-navigate) + (defn on-navigate - [router path send-event-info?] + "Query-string routing entry point. `token` is the history token (the + query string, `?screen=¶ms`, or empty on bootstrap)." + [router token send-event-info?] (let [location (.-location js/document) - [base-path qs] (str/split path "?") location-path (dm/str (.-origin location) (.-pathname location)) valid-location? (= location-path (dm/str cf/public-uri)) - match (rt/match router path) - empty-path? (or (= base-path "") (= base-path "/")) - query-params (u/query-string->map qs)] + legacy-hash (.-hash location)] (cond (not valid-location?) (st/emit! (rt/assign-exception {:type :not-found})) - (some? match) - (handle-sso-error-and-navigate match send-event-info? (rt/get-current-href)) + ;; TODO(next-version): delete with `legacy-routes`. Legacy `#/…` + ;; URLs translate to the query format once (replace, no extra + ;; history entry); the fragment never reaches the server, so this + ;; can only run client-side. Untranslatable hashes fall through + ;; to the normal query flow below. + (str/starts-with? legacy-hash "#/") + (if-let [{:keys [name params]} (legacy-match (subs legacy-hash 1))] + (st/emit! (rt/nav name params {::rt/replace true})) + (on-query-navigate router token send-event-info?)) :else + (on-query-navigate router token send-event-info?)))) + +(defn- on-query-navigate + [router token send-event-info?] + (let [token-query (if (str/starts-with? (or token "") "?") + (subs token 1) + (or token "")) + query-params (u/query-string->map token-query) + empty-token? (str/blank? token-query) + match (rt/match router token)] + (if (some? match) + (handle-sso-error-and-navigate match send-event-info? (rt/get-current-href)) + ;; We just recheck with an additional profile request; this ;; avoids some race conditions that causes unexpected redirects ;; on invitations workflows (and probably other cases). @@ -190,7 +271,7 @@ (store-session-params query-params) (st/emit! (rt/nav :auth-login))) - empty-path? + empty-token? (let [default-team-id (:default-team-id profile) last-team-id (dtm/get-last-team-id) team-id (if (contains? teams last-team-id) diff --git a/frontend/src/app/main/ui/viewer/share_link.cljs b/frontend/src/app/main/ui/viewer/share_link.cljs index 1711997594..f0a4c3de22 100644 --- a/frontend/src/app/main/ui/viewer/share_link.cljs +++ b/frontend/src/app/main/ui/viewer/share_link.cljs @@ -11,7 +11,6 @@ [app.common.data.macros :as dm] [app.common.logging :as log] [app.common.uuid :as uuid] - [app.config :as cf] [app.main.data.common :as dc] [app.main.data.event :as ev] [app.main.data.modal :as modal] @@ -78,8 +77,8 @@ (dissoc params :zoom) (assoc params :zoom zoom-type)) - href (rt/resolve router :viewer params)] - (dm/str (assoc cf/public-uri :fragment href)))))) + href (rt/resolve-uri router :viewer params)] + href)))) on-close (fn [event] diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/file_library.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/file_library.cljs index b0d9747a6e..29a6190d12 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/file_library.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/file_library.cljs @@ -79,10 +79,10 @@ [{:keys [is-open is-local file-id page-id file-name]}] (let [router (mf/deref refs/router) team-id (mf/use-ctx ctx/current-team-id) - url (rt/resolve router :workspace - {:team-id team-id - :file-id file-id - :page-id page-id}) + url (rt/resolve-uri router :workspace + {:team-id team-id + :file-id file-id + :page-id page-id}) toggle-open (mf/use-fn (mf/deps file-id is-open) @@ -110,7 +110,7 @@ (when-not ^boolean is-local [:span {:title (tr "workspace.assets.open-library")} [:a {:class (stl/css :file-link) - :href (str "#" url) + :href url :target "_blank" :on-click on-click} deprecated-icon/open-link]])]])) diff --git a/frontend/src/app/main/ui/workspace/sidebar/versions.cljs b/frontend/src/app/main/ui/workspace/sidebar/versions.cljs index 5f3be0c953..107c4d0885 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/versions.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/versions.cljs @@ -29,7 +29,6 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.keyboard :as kbd] [cuerdas.core :as str] - [lambdaisland.uri :as u] [okulary.core :as l] [rumext.v2 :as mf])) @@ -55,7 +54,7 @@ is-owner? (-> team :permissions :is-owner) email-owner (:email (some #(when (:is-owner %) %) (:members team))) support-email "support@penpot.app" - go-to-subscription (dm/str (u/join cfg/public-uri "#/settings/subscriptions"))] + go-to-subscription (dm/str cfg/public-uri "?screen=settings-subscription")] (if (contains? cfg/flags :subscriptions) (if is-owner? diff --git a/frontend/src/app/util/browser_history.js b/frontend/src/app/util/browser_history.js index 9421aaad2a..2790376926 100644 --- a/frontend/src/app/util/browser_history.js +++ b/frontend/src/app/util/browser_history.js @@ -16,8 +16,10 @@ goog.scope(function() { const Html5History = goog.history.Html5History; class TokenTransformer { + // Query-string routing: the history token is the query string + // ("?screen=...&..."), the path always stays the application base. retrieveToken(pathPrefix, location) { - return location.pathname.substr(pathPrefix.length) + location.search; + return location.search; } createUrl(token, pathPrefix, location) { @@ -25,9 +27,10 @@ goog.scope(function() { } } - self.create = function() { + self.create = function(pathPrefix) { const instance = new Html5History(null, new TokenTransformer()); - instance.setUseFragment(true); + instance.setUseFragment(false); + instance.setPathPrefix(pathPrefix); return instance; }; diff --git a/frontend/test/frontend_tests/router_test.cljs b/frontend/test/frontend_tests/router_test.cljs index a3ba3763e7..10156e032d 100644 --- a/frontend/test/frontend_tests/router_test.cljs +++ b/frontend/test/frontend_tests/router_test.cljs @@ -7,115 +7,39 @@ (ns frontend-tests.router-test (:require [app.main.router :as rt] - [app.util.globals :as globals] - [cljs.test :as t :include-macros true] - [potok.v2.core :as ptk])) + [cljs.test :as t :include-macros true])) -(defn- with-stubbed-href - "Run `thunk` with the `globals/location` href replaced and a recording - `js/history.replaceState`. Only the href is stubbed: the effect parses - everything it needs out of it." - [href replace-calls thunk] - (let [loc globals/location - old-href (.-href loc) - old-history (.-history js/globalThis)] - (set! (.-href loc) href) - (set! (.-history js/globalThis) - #js {:replaceState (fn [_ _ url] (swap! replace-calls conj url))}) - (try - (thunk) - (finally - (set! (.-href loc) old-href) - (set! (.-history js/globalThis) old-history))))) +(def ^:private test-routes + #{:auth-login :auth-register :dashboard-recent :workspace}) -(defn- emit-navigated - "Run the `navigated` effect with `match` stored as the state route. - The closed-over match is deliberately empty to prove the effect reads - the route from the state, not from the closure." - [match] - (ptk/effect (rt/navigated {} false) {:route match} nil)) +(t/deftest resolve-target-screen-wins-over-forwarded-params + ;; Screens forward the current query params when navigating (e.g. + ;; login passes its params to the register link); the stale screen + ;; they carry must never override the destination. + (t/is (= "?screen=auth-register&foo=1" + (rt/resolve test-routes :auth-register {:screen "auth-login" + :foo "1"})))) -(t/deftest navigated-mirrors-context-on-change - ;; New context in the state route triggers exactly one mirrored write. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/#/workspace?file-id=file-1" - calls - (fn [] - (emit-navigated {:query-params {:file-id "file-1"}}) - (t/is (= ["http://localhost/?file-id=file-1#/workspace?file-id=file-1"] @calls)))))) +(t/deftest resolve-builds-screen-token + (t/is (= "?screen=dashboard-recent&team-id=team-1" + (rt/resolve test-routes :dashboard-recent {:team-id "team-1"}))) + (t/is (= "?screen=auth-login" + (rt/resolve test-routes :auth-login))) + (t/is (nil? (rt/resolve test-routes :unknown-screen {:team-id "team-1"})))) -(t/deftest navigated-skips-write-when-mirrored - ;; When the URL already carries the mirrored context, nothing is written. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/?file-id=file-1#/workspace?file-id=file-1" - calls - (fn [] - (emit-navigated {:query-params {:file-id "file-1"}}) - (t/is (= [] @calls)))))) +(t/deftest resolve-uri-builds-absolute-url + (let [uri (rt/resolve-uri test-routes :workspace {:file-id "file-1"})] + (t/is (string? uri)) + (t/is (re-find #"\?screen=workspace&file-id=file-1$" uri)))) -(t/deftest navigated-strips-stale-context - ;; A stale pre-fragment query is replaced with the current context. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/?file-id=old#/dashboard/recent?team-id=team-1" - calls - (fn [] - (emit-navigated {:query-params {:team-id "team-1"}}) - (t/is (= ["http://localhost/?team-id=team-1#/dashboard/recent?team-id=team-1"] @calls)))))) +(t/deftest match-resolves-screen-token + (let [match (rt/match test-routes "?screen=workspace&team-id=team-1&file-id=file-1")] + (t/is (= :workspace (get-in match [:data :name]))) + (t/is (= "team-1" (get-in match [:params :query :team-id]))) + (t/is (= "file-1" (get-in match [:query-params :file-id]))) + (t/is (= {} (get-in match [:params :path]))))) -(t/deftest navigated-clears-query-without-context - ;; Routes without context clear a stale pre-fragment query. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/?file-id=old#/auth/login" - calls - (fn [] - (emit-navigated {:query-params {:token "some-token"}}) - (t/is (= ["http://localhost/#/auth/login"] @calls)))))) - -(t/deftest navigated-preserves-unrelated-params - ;; Params owned by other code are kept as they are. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/?debug=1&file-id=old#/workspace?file-id=file-1" - calls - (fn [] - (emit-navigated {:query-params {:file-id "file-1"}}) - (t/is (= ["http://localhost/?debug=1&file-id=file-1#/workspace?file-id=file-1"] @calls)))))) - -(t/deftest navigated-mirrors-every-present-context-id - ;; Every present context id is mirrored; the backend applies its own - ;; file > project > team priority, so no filtering happens here. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/#/workspace?file-id=file-1&team-id=team-1&project-id=project-1&page-id=page-1" - calls - (fn [] - (emit-navigated {:query-params {:file-id "file-1" - :team-id "team-1" - :project-id "project-1" - :page-id "page-1"}}) - (t/is (= ["http://localhost/?file-id=file-1&team-id=team-1&project-id=project-1#/workspace?file-id=file-1&team-id=team-1&project-id=project-1&page-id=page-1"] - @calls)))))) - -(t/deftest navigated-repeated-key-last-wins - ;; A repeated query key arrives as a vector; the last value wins. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/#/workspace?file-id=file-1" - calls - (fn [] - (emit-navigated {:query-params {:file-id ["file-old" "file-1"]}}) - (t/is (= ["http://localhost/?file-id=file-1#/workspace?file-id=file-1"] @calls)))))) - -(t/deftest navigated-keeps-subpath-base - ;; Under a subpath deployment the prefix survives untouched. - (let [calls (atom [])] - (with-stubbed-href - "http://localhost/penpot/#/workspace?file-id=file-1" - calls - (fn [] - (emit-navigated {:query-params {:file-id "file-1"}}) - (t/is (= ["http://localhost/penpot/?file-id=file-1#/workspace?file-id=file-1"] @calls)))))) +(t/deftest match-rejects-missing-or-unknown-screen + (t/is (nil? (rt/match test-routes ""))) + (t/is (nil? (rt/match test-routes "?team-id=team-1"))) + (t/is (nil? (rt/match test-routes "?screen=nope&team-id=team-1")))) diff --git a/plugins/apps/e2e/src/utils/agent.ts b/plugins/apps/e2e/src/utils/agent.ts index c6a528c944..033da09703 100644 --- a/plugins/apps/e2e/src/utils/agent.ts +++ b/plugins/apps/e2e/src/utils/agent.ts @@ -1,7 +1,7 @@ import puppeteer, { ConsoleMessage } from 'puppeteer'; import { PenpotApi } from './api'; import { getFileUrl } from './get-file-url'; -import { idObjectToArray } from './clean-id'; +import { cleanId, idObjectToArray } from './clean-id'; import { Shape } from '../models/shape.model'; const screenshotsEnable = process.env['E2E_SCREENSHOTS'] === 'true'; @@ -52,7 +52,8 @@ export async function Agent() { const file = await penpotApi.createFile(); console.log('File created with id:', file['~:id']); - const fileUrl = getFileUrl(file); + const project = await penpotApi.getProject(cleanId(file['~:project-id'])); + const fileUrl = getFileUrl(file, cleanId(project['~:team-id'])); console.log('File URL:', fileUrl); console.log('Launching browser...'); diff --git a/plugins/apps/e2e/src/utils/api.ts b/plugins/apps/e2e/src/utils/api.ts index deeb1a4a95..6318dae48c 100644 --- a/plugins/apps/e2e/src/utils/api.ts +++ b/plugins/apps/e2e/src/utils/api.ts @@ -71,6 +71,20 @@ export async function PenpotApi() { console.log('File data received:', fileData); return fileData; }, + getProject: async (projectId: string) => { + const getProjectRequest = await fetch( + `${apiUrl}/api/main/methods/get-project?id=${projectId}`, + { + method: 'GET', + headers: { + cookie: authToken, + credentials: 'include', + }, + }, + ); + + return (await getProjectRequest.json()) as Record; + }, deleteFile: async (fileId: string) => { const deleteFileRequest = await fetch( `${apiUrl}/api/main/methods/delete-file`, diff --git a/plugins/apps/e2e/src/utils/get-file-url.ts b/plugins/apps/e2e/src/utils/get-file-url.ts index 14de946934..d7d579f7dc 100644 --- a/plugins/apps/e2e/src/utils/get-file-url.ts +++ b/plugins/apps/e2e/src/utils/get-file-url.ts @@ -1,10 +1,9 @@ import { FileRpc } from '../models/file-rpc.model'; import { cleanId } from './clean-id'; -export function getFileUrl(file: FileRpc) { - const projectId = cleanId(file['~:project-id']); +export function getFileUrl(file: FileRpc, teamId: string) { const fileId = cleanId(file['~:id']); const pageId = cleanId(file['~:data']['~:pages'][0]); - return `https://localhost:3449/#/workspace/${projectId}/${fileId}?page-id=${pageId}`; + return `https://localhost:3449/?screen=workspace&team-id=${teamId}&file-id=${fileId}&page-id=${pageId}`; }