diff --git a/docs/technical-guide/configuration.md b/docs/technical-guide/configuration.md index 76426c22e5..98601290b9 100644 --- a/docs/technical-guide/configuration.md +++ b/docs/technical-guide/configuration.md @@ -730,6 +730,9 @@ __Since version 2.0.0__ - enable-webhooks: enables webhooks. More detail about this configuration in [webhooks section][6]. - enable-access-tokens: enables access tokens. More detail about this configuration in [access tokens section][7]. - disable-google-fonts-provider: disables the google fonts provider. +- enable-link-preview: enables Open Graph link previews for shared links. + File names and dashboard thumbnails become readable by anyone holding the link, so only enable + it if you accept that trade-off. More detail in the [link previews page][9]. [1]: /technical-guide/getting-started#configure-penpot-with-elestio [2]: /technical-guide/getting-started#configure-penpot-with-docker @@ -739,3 +742,4 @@ __Since version 2.0.0__ [6]: /technical-guide/integration/#webhooks [7]: /technical-guide/integration/#access-tokens [8]: /mcp/ +[9]: /technical-guide/developer/subsystems/link-preview/ diff --git a/docs/technical-guide/developer/subsystems/link-preview.md b/docs/technical-guide/developer/subsystems/link-preview.md index 92eafba030..2bfd75ce43 100644 --- a/docs/technical-guide/developer/subsystems/link-preview.md +++ b/docs/technical-guide/developer/subsystems/link-preview.md @@ -225,7 +225,9 @@ indexing these preview pages, and responses are marked non-cacheable. 1. Make sure the devenv nginx picked up the config (restart the devenv, or `nginx -s reload` inside the container, if it predates these changes). -2. Enable the flag before starting the backend REPL: +2. The flag already ships enabled in devenv via `backend/scripts/_env`, so + no export is needed there; outside devenv, enable it before starting + the backend: ```bash export PENPOT_FLAGS="$PENPOT_FLAGS enable-link-preview" diff --git a/frontend/src/app/main/router.cljs b/frontend/src/app/main/router.cljs index ccb3ec9aa8..5fd748f51b 100644 --- a/frontend/src/app/main/router.cljs +++ b/frontend/src/app/main/router.cljs @@ -65,6 +65,16 @@ ;; --- Navigate (Event) +(defn get-query-param + "Safely extracts a scalar value for a query param key from a params + map. When the same key appears multiple times in a URL, + query-string->map returns a vector for that key; this function + always returns a single (last) element in that case, so downstream + consumers such as parse-long always receive a plain string or nil." + [params k] + (let [v (get params k)] + (if (sequential? v) (peek v) v))) + (defn match->context-params "Extract the params that give sharing context to the current URL. @@ -74,9 +84,12 @@ [match] (let [path-params (dm/get-in match [:params :path]) query-params (get match :query-params) - file-id (or (get query-params :file-id) (get path-params :file-id)) - team-id (or (get query-params :team-id) (get path-params :team-id)) - project-id (or (get query-params :project-id) (get path-params :project-id))] + file-id (or (get-query-param query-params :file-id) + (get-query-param path-params :file-id)) + team-id (or (get-query-param query-params :team-id) + (get-query-param path-params :team-id)) + project-id (or (get-query-param query-params :project-id) + (get-query-param path-params :project-id))] (cond (some? file-id) {:file-id file-id} (some? project-id) {:team-id team-id :project-id project-id} @@ -106,15 +119,18 @@ ptk/EffectEvent (effect [_ _ _] - (let [query (some-> (match->context-params match) - (u/map->query-string)) - href (dm/str (.-pathname globals/location) - (if (some? query) (dm/str "?" query) "") - (.-hash globals/location))] + (let [query (some-> (match->context-params match) + (u/map->query-string)) + href (dm/str (.-pathname globals/location) + (if (some? query) (dm/str "?" query) "") + (.-hash globals/location)) + current (dm/str (.-pathname globals/location) + (.-search globals/location) + (.-hash globals/location))] ;; The pre-fragment query string is owned by this mirroring: skip ;; the write when nothing changed to avoid URL churn and dropping - ;; unrelated params set by other code. - (when (not= href (.-href globals/location)) + ;; unrelated params set by other code. Both sides are path-relative. + (when (not= href current) (.replaceState js/history nil "" href)))))) (defn navigate @@ -165,16 +181,6 @@ [state] (dm/get-in state [:route :params :query])) -(defn get-query-param - "Safely extracts a scalar value for a query param key from a params - map. When the same key appears multiple times in a URL, - query-string->map returns a vector for that key; this function - always returns a single (last) element in that case, so downstream - consumers such as parse-long always receive a plain string or nil." - [params k] - (let [v (get params k)] - (if (sequential? v) (peek v) v))) - (defn nav-back [] (ptk/reify ::nav-back diff --git a/frontend/test/frontend_tests/router_test.cljs b/frontend/test/frontend_tests/router_test.cljs index 07d796f886..e72f3a49e4 100644 --- a/frontend/test/frontend_tests/router_test.cljs +++ b/frontend/test/frontend_tests/router_test.cljs @@ -7,7 +7,9 @@ (ns frontend-tests.router-test (:require [app.main.router :as rt] - [cljs.test :as t :include-macros true])) + [app.util.globals :as globals] + [cljs.test :as t :include-macros true] + [potok.v2.core :as ptk])) (t/deftest match-context-params-file-link ;; Workspace and viewer links only mirror the file-id. @@ -55,3 +57,66 @@ :file-id "file-1"}}] (t/is (= {:file-id "file-1"} (rt/match->context-params match))))) + +(t/deftest match-context-params-repeated-key + ;; A repeated query key arrives as a vector; the last value wins. + (let [match {:query-params {:file-id ["file-old" "file-1"]}}] + (t/is (= {:file-id "file-1"} + (rt/match->context-params match))))) + +(defn- with-stubbed-browser + "Run `thunk` with the `globals/location` mock props and a recording + `js/history.replaceState`. Restores both originals afterwards: the + location mock is shared across tests and `js/history` may not exist + in the test environment at all." + [{:keys [pathname search hash href]} replace-calls thunk] + (let [loc globals/location + old-pathname (.-pathname loc) + old-search (.-search loc) + old-hash (.-hash loc) + old-href (.-href loc) + old-history (.-history js/globalThis)] + (set! (.-pathname loc) pathname) + (set! (.-search loc) search) + (set! (.-hash loc) hash) + (set! (.-href loc) href) + (set! (.-history js/globalThis) + #js {:replaceState (fn [_ _ url] (swap! replace-calls conj url))}) + (try + (thunk) + (finally + (set! (.-pathname loc) old-pathname) + (set! (.-search loc) old-search) + (set! (.-hash loc) old-hash) + (set! (.-href loc) old-href) + (set! (.-history js/globalThis) old-history))))) + +(t/deftest navigated-mirrors-context-on-change + ;; New context in the match triggers exactly one mirrored write. + (let [calls (atom [])] + (with-stubbed-browser + {:pathname "/" :search "" :hash "#/workspace?file-id=file-1" :href "http://localhost/"} + calls + (fn [] + (ptk/effect (rt/navigated {:query-params {:file-id "file-1"}} false) nil nil) + (t/is (= ["/?file-id=file-1#/workspace?file-id=file-1"] @calls)))))) + +(t/deftest navigated-skips-write-when-mirrored + ;; When the URL already carries the mirrored context, nothing is written. + (let [calls (atom [])] + (with-stubbed-browser + {:pathname "/" :search "?file-id=file-1" :hash "#/workspace?file-id=file-1" :href "http://localhost/?file-id=file-1#/workspace?file-id=file-1"} + calls + (fn [] + (ptk/effect (rt/navigated {:query-params {:file-id "file-1"}} false) nil nil) + (t/is (= [] @calls)))))) + +(t/deftest navigated-strips-stale-context + ;; A stale pre-fragment query is replaced with the current context. + (let [calls (atom [])] + (with-stubbed-browser + {:pathname "/" :search "?file-id=old" :hash "#/dashboard/recent?team-id=team-1" :href "http://localhost/?file-id=old#/dashboard/recent?team-id=team-1"} + calls + (fn [] + (ptk/effect (rt/navigated {:query-params {:team-id "team-1"}} false) nil nil) + (t/is (= ["/?team-id=team-1#/dashboard/recent?team-id=team-1"] @calls))))))