github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-09-10 05:58:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix link-preview replaceState guard and cover navigated effect

Browse Source 
Compare the mirrored href against the path-relative current location so the no-op guard actually skips unchanged URLs. Read context ids through get-query-param so repeated keys resolve to the last value. Cover the navigated effect with stubbed-globals tests (write on change, skip when mirrored, strip stale context) and surface enable-link-preview in the configuration flags list.

AI-assisted-by: muse-spark-1.3-contributor
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
Andrey Antukh 2026-09-09 12:34:09 +00:00
parent 3bc46a21dc
commit db84b77c7e
4 changed files with 99 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/technical-guide/configuration.md
View File

@ -730,6 +730,9 @@ __Since version 2.0.0__
- <code class="language-bash">enable-webhooks</code>: enables webhooks. More detail about this configuration in [webhooks section][6].
- <code class="language-bash">enable-access-tokens</code>: enables access tokens. More detail about this configuration in [access tokens section][7].
- <code class="language-bash">disable-google-fonts-provider</code>: disables the google fonts provider.
- <code class="language-bash">enable-link-preview</code>: 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/

4
docs/technical-guide/developer/subsystems/link-preview.md
View File

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

46
frontend/src/app/main/router.cljs
View File

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

67
frontend/test/frontend_tests/router_test.cljs
View File

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