From 4635eaa4679cbc89bb48edf93270c7d869890c5f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 19 Oct 2026 22:33:10 +0200 Subject: [PATCH] :bug: Show request access modal on viewer and dashboard urls (#12456) The exception page compared the screen query param against literals that never occur ("view", "dashboard"), so viewer and dashboard urls of inaccessible files and teams fell through to the plain not-found page instead of the request access modal. Classify the param with a tested helper that recognizes "viewer" and every "dashboard-*" route. Closes #11224 AI-assisted-by: muse-spark-1.3-contributor --- frontend/src/app/main/ui/static.cljs | 30 +++++++++-- frontend/test/frontend_tests/runner.cljs | 2 + .../test/frontend_tests/ui/static_test.cljs | 54 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 frontend/test/frontend_tests/ui/static_test.cljs diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index ec5e20a72a..fe75e1f7b2 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -34,6 +34,7 @@ [app.util.timers :as tm] [app.util.webapi :as wapi] [beicon.v2.core :as rx] + [cuerdas.core :as str] [rumext.v2 :as mf])) ;; FIXME: this is a workaround until we export this class on beicon library @@ -608,16 +609,37 @@ [:> internal-error* props]))) +(defn screen-kind + "Classify the `screen` query param for the exception page. + + The param carries the route name (`?screen=¶ms`), so the + viewer is \"viewer\" and every team dashboard is \"dashboard-*\". Returns + `:workspace`, `:dashboard`, `:viewer`, or nil when the screen is not a + request-access candidate. Reads the param with `rt/get-query-param` + because a repeated param arrives as a vector." + [query-params] + (let [screen (rt/get-query-param query-params :screen)] + (cond + (= screen "workspace") + :workspace + + (and (string? screen) + (str/starts-with? screen "dashboard")) + :dashboard + + (= screen "viewer") + :viewer))) + (mf/defc exception-page* [{:keys [data route] :as props}] (let [type (:type data) params (:query-params route) - screen (get params :screen) - workspace? (= screen "workspace") - dashboard? (= screen "dashboard") - view? (= screen "view") + kind (screen-kind params) + workspace? (= kind :workspace) + dashboard? (= kind :dashboard) + view? (= kind :viewer) ;; We store the request access info int this state info* (mf/use-state nil) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index ad9b96d56f..8cea58e9b6 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -125,6 +125,7 @@ [frontend-tests.ui.shape-filters-test] [frontend-tests.ui.shortcuts-labels-test] [frontend-tests.ui.sidebar-scroll-test] + [frontend-tests.ui.static-test] [frontend-tests.ui.stroke-menu-test] [frontend-tests.ui.workspace-libraries-test] [frontend-tests.util-clipboard-test] @@ -272,6 +273,7 @@ 'frontend-tests.ui.shape-filters-test 'frontend-tests.ui.shortcuts-labels-test 'frontend-tests.ui.sidebar-scroll-test + 'frontend-tests.ui.static-test 'frontend-tests.ui.stroke-menu-test 'frontend-tests.ui.workspace-libraries-test 'frontend-tests.util-clipboard-test diff --git a/frontend/test/frontend_tests/ui/static_test.cljs b/frontend/test/frontend_tests/ui/static_test.cljs new file mode 100644 index 0000000000..acdc1b0b2c --- /dev/null +++ b/frontend/test/frontend_tests/ui/static_test.cljs @@ -0,0 +1,54 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns frontend-tests.ui.static-test + (:require + [app.main.ui.static :as static] + [cljs.test :as t :include-macros true])) + +(t/deftest screen-kind-classifies-known-screens + ;; Regression test (penpot/penpot#11224): the exception page shows the + ;; REQUEST ACCESS modal only when it recognizes the screen the failing + ;; URL belongs to. The `screen` query param carries the route name, so + ;; the viewer is "viewer" and every team dashboard is "dashboard-*". + (t/testing "workspace" + (t/is (= :workspace (static/screen-kind {:screen "workspace"})))) + + (t/testing "bare dashboard prefix also classifies" + ;; No such route exists (routes are always "dashboard-*"), and the + ;; modal still needs loaded team/file info, so this is harmless and + ;; matches the prefix convention used in routes.cljs. + (t/is (= :dashboard (static/screen-kind {:screen "dashboard"})))) + + (t/testing "viewer" + (t/is (= :viewer (static/screen-kind {:screen "viewer"})))) + + (t/testing "dashboard variants" + (doseq [screen ["dashboard-recent" + "dashboard-members" + "dashboard-files" + "dashboard-settings"]] + (t/is (= :dashboard (static/screen-kind {:screen screen})) + (str "screen: " screen))))) + +(t/deftest screen-kind-ignores-unknown-screens + ;; Unknown, missing, or auth screens are not request-access candidates: + ;; the exception page must keep showing the plain not-found page. + (doseq [params [nil + {} + {:screen "auth-login"} + {:screen "settings-profile"} + ;; The "view" literal never was a real screen name; the + ;; exception page compared against it before the fix. + {:screen "view"}]] + (t/is (nil? (static/screen-kind params)) + (str "params: " (pr-str params))))) + +(t/deftest screen-kind-tolerates-duplicate-screen-params + ;; A repeated query param arrives as a vector; the classifier reads the + ;; scalar the same way the router does instead of crashing on it. + (t/is (= :dashboard + (static/screen-kind {:screen ["dashboard-recent" "dashboard-recent"]}))))