From 3bc46a21dcd68f0436f5e9b4286d7f543c5aab11 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 9 Sep 2026 10:13:00 +0000 Subject: [PATCH] :recycle: Rename unfurl to link-preview and harden preview endpoint Rename the whole feature from unfurl to link-preview: handler namespace, template, /link-preview route, :link-preview flag, nginx variable and docs page. Make the flag opt-in by removing it from the default flags. Serve file-thumbnail objects with full public semantics while the flag is on, restrict the route to GET/HEAD, make a present file-id decisive over project/team context, preserve the query string on the human redirect and skip no-op replaceState writes. Cover the new behavior with backend and frontend tests. AI-assisted-by: muse-spark-1.3-contributor Signed-off-by: Andrey Antukh --- .../{unfurl.tmpl => link-preview.tmpl} | 2 +- backend/scripts/_env | 2 +- backend/src/app/http.clj | 6 +- backend/src/app/http/assets.clj | 20 +- .../app/http/{unfurl.clj => link_preview.clj} | 29 ++- backend/src/app/main.clj | 6 +- .../test/backend_tests/http_assets_test.clj | 12 +- .../backend_tests/http_link_preview_test.clj | 211 ++++++++++++++++++ .../test/backend_tests/http_unfurl_test.clj | 98 -------- common/src/app/common/flags.cljc | 5 +- docker/devenv/files/nginx.conf | 10 +- docker/images/files/nginx.conf.template | 10 +- .../{link-unfurl.md => link-preview.md} | 95 ++++---- frontend/src/app/main/router.cljs | 8 +- frontend/test/frontend_tests/router_test.cljs | 16 ++ 15 files changed, 339 insertions(+), 191 deletions(-) rename backend/resources/app/templates/{unfurl.tmpl => link-preview.tmpl} (91%) rename backend/src/app/http/{unfurl.clj => link_preview.clj} (71%) create mode 100644 backend/test/backend_tests/http_link_preview_test.clj delete mode 100644 backend/test/backend_tests/http_unfurl_test.clj rename docs/technical-guide/developer/subsystems/{link-unfurl.md => link-preview.md} (73%) diff --git a/backend/resources/app/templates/unfurl.tmpl b/backend/resources/app/templates/link-preview.tmpl similarity index 91% rename from backend/resources/app/templates/unfurl.tmpl rename to backend/resources/app/templates/link-preview.tmpl index fdbf3f083f..4b7753c595 100644 --- a/backend/resources/app/templates/unfurl.tmpl +++ b/backend/resources/app/templates/link-preview.tmpl @@ -17,6 +17,6 @@ - + diff --git a/backend/scripts/_env b/backend/scripts/_env index f806d4bde4..a0f819fa9e 100644 --- a/backend/scripts/_env +++ b/backend/scripts/_env @@ -62,7 +62,7 @@ export PENPOT_FLAGS="\ enable-file-validation \ enable-file-schema-validation \ enable-redis-cache \ - enable-link-unfurl \ + enable-link-preview \ enable-subscriptions"; # Uncomment for nexus integration testing diff --git a/backend/src/app/http.clj b/backend/src/app/http.clj index 94acc7bab2..22e436227f 100644 --- a/backend/src/app/http.clj +++ b/backend/src/app/http.clj @@ -17,11 +17,11 @@ [app.http.awsns :as-alias awsns] [app.http.debug :as-alias debug] [app.http.errors :as errors] + [app.http.link-preview :as-alias link-preview] [app.http.management :as mgmt] [app.http.middleware :as mw] [app.http.security :as sec] [app.http.session :as session] - [app.http.unfurl :as-alias unfurl] [app.http.websocket :as-alias ws] [app.main :as-alias main] [app.metrics :as mtx] @@ -150,7 +150,7 @@ [::rpc/routes schema:routes] [::oidc/routes schema:routes] [::assets/routes schema:routes] - [::unfurl/routes schema:routes] + [::link-preview/routes schema:routes] [::debug/routes schema:routes] [::mtx/routes schema:routes] [::awsns/routes schema:routes] @@ -179,7 +179,7 @@ (::mtx/routes cfg) (::assets/routes cfg) - (::unfurl/routes cfg) + (::link-preview/routes cfg) (::debug/routes cfg) ["/webhooks" diff --git a/backend/src/app/http/assets.clj b/backend/src/app/http/assets.clj index ce588731ca..9ae258d2a2 100644 --- a/backend/src/app/http/assets.clj +++ b/backend/src/app/http/assets.clj @@ -36,6 +36,14 @@ "file-data-fragment" "organization"}) +(defn- public-bucket? + [bucket] + (or (contains? public-buckets bucket) + ;; Dashboard file thumbnails become public when link previews + ;; are enabled, so link preview crawlers can fetch them. + (and (= "file-thumbnail" bucket) + (contains? cf/flags :link-preview)))) + (defn get-id [{:keys [path-params]}] (or (some-> path-params :id d/parse-uuid) @@ -57,7 +65,7 @@ (let [sig-max-age (or signature-max-age default-signature-max-age) cch-max-age (or cache-max-age default-cache-max-age) bucket (-> obj meta :bucket) - public? (contains? public-buckets bucket) + public? (public-bucket? bucket) ;; The disposition is also signed into the presigned url: this ;; response is a redirect, so the header below applies to the ;; redirect itself and not to the bytes the client then fetches @@ -85,7 +93,7 @@ headers (cond-> {"x-accel-redirect" (:path purl) "content-type" (:content-type mdata) "cache-control" (str "max-age=" (inst-ms cch-max-age))} - (not (contains? public-buckets bucket)) + (not (public-bucket? bucket)) (assoc "content-disposition" "attachment"))] {::yres/status 204 ::yres/headers headers})) @@ -98,14 +106,6 @@ (:s3 :assets-s3) (serve-object-from-s3 cfg obj) (:fs :assets-fs) (serve-object-from-fs cfg obj))) -(defn- public-bucket? - [bucket] - (or (contains? public-buckets bucket) - ;; Dashboard file thumbnails become public when link unfurling - ;; is enabled, so link preview crawlers can fetch them. - (and (= "file-thumbnail" bucket) - (contains? cf/flags :link-unfurl)))) - (defn- requires-auth? "Check if the storage object requires authentication based on its bucket." [obj] diff --git a/backend/src/app/http/unfurl.clj b/backend/src/app/http/link_preview.clj similarity index 71% rename from backend/src/app/http/unfurl.clj rename to backend/src/app/http/link_preview.clj index f1fa745d15..c415b2a99b 100644 --- a/backend/src/app/http/unfurl.clj +++ b/backend/src/app/http/link_preview.clj @@ -4,8 +4,8 @@ ;; ;; Copyright (c) KALEIDOS INC Sucursal en España SL -(ns app.http.unfurl - "Link unfurl (Open Graph metadata) related handlers. +(ns app.http.link-preview + "Link preview (Open Graph metadata) related handlers. Serves a minimal HTML page with Open Graph metadata used by link preview crawlers (Slack, Discord, Twitter, ...). The reverse proxy @@ -44,7 +44,7 @@ (str (cf/get :public-uri) "/images/penpot-link-preview.png")) (defn- get-file-context - "Return the unfurl context for a file link: the file name as title + "Return the link preview context for a file link: the file name as title and, when available, the last dashboard thumbnail as image." [pool file-id] (when-let [{:keys [name media-id]} (db/exec-one! pool [sql:get-file file-id])] @@ -54,24 +54,28 @@ (defn- get-context [pool params] - (let [file-id (some-> (:file-id params) d/parse-uuid) - project-id (some-> (:project-id params) d/parse-uuid) + (let [project-id (some-> (:project-id params) d/parse-uuid) team-id (some-> (:team-id params) d/parse-uuid)] - (cond - (some? file-id) (get-file-context pool file-id) - (some? project-id) (assoc default-context :title "Project | Penpot") - (some? team-id) (assoc default-context :title "Team dashboard | Penpot")))) + ;; A present file-id is decisive: file links never fall through to the + ;; project/team card, even when the value is malformed or unknown (both + ;; yield nil and the handler falls back to the default context). + (if (contains? params :file-id) + (when-some [file-id (d/parse-uuid (:file-id params))] + (get-file-context pool file-id)) + (cond + (some? project-id) (assoc default-context :title "Project | Penpot") + (some? team-id) (assoc default-context :title "Team dashboard | Penpot"))))) (defn- handler [{:keys [::db/pool]} request] - (let [context (when (contains? cf/flags :link-unfurl) + (let [context (when (contains? cf/flags :link-preview) (get-context pool (:query-params request))) context (-> (or context default-context) (update :image #(or % (resolve-default-image-uri))))] {::yres/status 200 ::yres/headers {"content-type" "text/html; charset=utf-8" "cache-control" "no-store, no-cache, max-age=0"} - ::yres/body (-> (io/resource "app/templates/unfurl.tmpl") + ::yres/body (-> (io/resource "app/templates/link-preview.tmpl") (tmpl/render context))})) ;; --- Initialization @@ -82,4 +86,5 @@ (defmethod ig/init-key ::routes [_ cfg] - ["/unfurl" {:handler (partial handler cfg)}]) + ["/link-preview" {:handler (partial handler cfg) + :allowed-methods #{:get :head}}]) diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 51c200c9be..f5346d9f48 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -20,10 +20,10 @@ [app.http.awsns :as http.awsns] [app.http.client :as-alias http.client] [app.http.debug :as-alias http.debug] + [app.http.link-preview :as-alias http.link-preview] [app.http.management :as mgmt] [app.http.session :as session] [app.http.session.tasks :as-alias session.tasks] - [app.http.unfurl :as-alias http.unfurl] [app.http.websocket :as http.ws] [app.loggers.webhooks :as-alias webhooks] [app.metrics :as-alias mtx] @@ -284,11 +284,11 @@ ::mgmt/routes (ig/ref ::mgmt/routes) ::http.debug/routes (ig/ref ::http.debug/routes) ::http.assets/routes (ig/ref ::http.assets/routes) - ::http.unfurl/routes (ig/ref ::http.unfurl/routes) + ::http.link-preview/routes (ig/ref ::http.link-preview/routes) ::http.ws/routes (ig/ref ::http.ws/routes) ::http.awsns/routes (ig/ref ::http.awsns/routes)} - ::http.unfurl/routes + ::http.link-preview/routes {::db/pool (ig/ref ::db/pool)} ::http.debug/routes diff --git a/backend/test/backend_tests/http_assets_test.clj b/backend/test/backend_tests/http_assets_test.clj index 8a157571db..6ebddef0f3 100644 --- a/backend/test/backend_tests/http_assets_test.clj +++ b/backend/test/backend_tests/http_assets_test.clj @@ -158,9 +158,9 @@ ;; Tests: objects-handler — non-public buckets (auth required) ;; ---------------------------------------------------------------- -(t/deftest objects-handler-file-thumbnail-bucket-link-unfurl-flag +(t/deftest objects-handler-file-thumbnail-bucket-link-preview-flag ;; Objects in the file-thumbnail bucket are public only when the - ;; link-unfurl flag is enabled. + ;; link-preview flag is enabled. (let [storage (-> (:app.storage/storage th/*system*) (configure-storage-backend)) cfg (make-handler-cfg storage) @@ -168,13 +168,13 @@ request {:path-params {:id (str (:id object))}}] (t/testing "flag enabled" - (with-redefs [cf/flags (conj cf/flags :link-unfurl)] + (with-redefs [cf/flags (conj cf/flags :link-preview)] (let [response (assets/objects-handler cfg request)] (t/is (not= 401 (::yres/status response))) (t/is (not= 404 (::yres/status response)))))) (t/testing "flag disabled" - (with-redefs [cf/flags (disj cf/flags :link-unfurl)] + (with-redefs [cf/flags (disj cf/flags :link-preview)] (let [response (assets/objects-handler cfg request)] (t/is (= 401 (::yres/status response)))))))) @@ -240,8 +240,8 @@ profile (th/create-profile* 1)] ;; NOTE: file-thumbnail is not included here because it is public - ;; when the link-unfurl flag is enabled; see - ;; objects-handler-file-thumbnail-bucket-link-unfurl-flag. + ;; when the link-preview flag is enabled; see + ;; objects-handler-file-thumbnail-bucket-link-preview-flag. (doseq [bucket ["profile" "tempfile" "file-data" diff --git a/backend/test/backend_tests/http_link_preview_test.clj b/backend/test/backend_tests/http_link_preview_test.clj new file mode 100644 index 0000000000..0272579bf5 --- /dev/null +++ b/backend/test/backend_tests/http_link_preview_test.clj @@ -0,0 +1,211 @@ +;; 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 INC Sucursal en España SL + +(ns backend-tests.http-link-preview-test + (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] + [app.http.link-preview :as link-preview] + [app.storage :as sto] + [backend-tests.helpers :as th] + [clojure.test :as t] + [cuerdas.core :as str] + [yetti.response :as-alias yres])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each (th/serial + th/database-reset + th/clean-storage)) + +(def ^:private default-title + "Penpot | Full-stack design") + +(defn- run-handler + [query-params] + (let [cfg {::db/pool (:app.db/pool th/*system*)}] + (#'link-preview/handler cfg {:query-params query-params}))) + +(defn- create-file-thumbnail! + [file-id] + (let [storage (::sto/storage th/*system*) + object (sto/put-object! storage {::sto/content (sto/content "thumbnail data") + :bucket "file-thumbnail" + :content-type "image/png"})] + (db/insert! (:app.db/pool th/*system*) :file-thumbnail + {:file-id file-id + :revn 1 + :media-id (:id object)}) + object)) + +(t/deftest link-preview-without-params + (let [response (run-handler {})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) default-title)) + (t/is (str/includes? (::yres/body response) "/images/penpot-link-preview.png")))) + +(t/deftest link-preview-file-without-thumbnail + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)})] + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) (str (:name file) " | Penpot"))) + (t/is (str/includes? (::yres/body response) "/images/penpot-link-preview.png")))))) + +(t/deftest link-preview-file-with-thumbnail + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + object (create-file-thumbnail! (:id file))] + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) (str (:name file) " | Penpot"))) + (t/is (str/includes? (::yres/body response) (str "/assets/by-id/" (:id object)))))))) + +(t/deftest link-preview-non-existent-file + (let [response (run-handler {:file-id (str (uuid/next))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) default-title)))) + +(t/deftest link-preview-invalid-file-id + (let [response (run-handler {:file-id "not-a-uuid"})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) default-title)))) + +(t/deftest link-preview-team-link + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:team-id (str (uuid/next))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) "Team dashboard | Penpot"))))) + +(t/deftest link-preview-project-link + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:team-id (str (uuid/next)) + :project-id (str (uuid/next))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) "Project | Penpot"))))) + +(t/deftest link-preview-flag-disabled + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)})] + (with-redefs [cf/flags (disj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) default-title)) + (t/is (not (str/includes? (::yres/body response) (:name file)))))))) + +(t/deftest link-preview-deleted-file + ;; A deleted file never leaks its name; crawlers get the generic card. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)})] + (th/mark-file-deleted* {:id (:id file)}) + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) default-title)) + (t/is (not (str/includes? (::yres/body response) (:name file)))))))) + +(t/deftest link-preview-file-with-only-deleted-thumbnail + ;; A file whose only thumbnail is deleted keeps its title but falls back + ;; to the default image. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + object (create-file-thumbnail! (:id file))] + (db/update! th/*system* :file-thumbnail + {:deleted-at (ct/now)} + {:file-id (:id file)}) + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) (str (:name file) " | Penpot"))) + (t/is (str/includes? (::yres/body response) "/images/penpot-link-preview.png")) + (t/is (not (str/includes? (::yres/body response) (str (:id object))))))))) + +(t/deftest link-preview-file-picks-latest-thumbnail + ;; With several thumbnail revisions, the latest non-deleted one wins. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile)}) + pool (:app.db/pool th/*system*) + storage (::sto/storage th/*system*) + old (sto/put-object! storage {::sto/content (sto/content "old thumbnail") + :bucket "file-thumbnail" + :content-type "image/png"}) + latest (sto/put-object! storage {::sto/content (sto/content "latest thumbnail") + :bucket "file-thumbnail" + :content-type "image/png"})] + (db/insert! pool :file-thumbnail + {:file-id (:id file) + :revn 1 + :media-id (:id old) + :deleted-at (ct/now)}) + (db/insert! pool :file-thumbnail + {:file-id (:id file) + :revn 2 + :media-id (:id latest)}) + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (str/includes? (::yres/body response) (str "/assets/by-id/" (:id latest)))) + (t/is (not (str/includes? (::yres/body response) (str (:id old))))))))) + +(t/deftest link-preview-escapes-file-name + ;; Hostile file names are HTML-escaped in the rendered meta tags. + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :name " & co"})] + (with-redefs [cf/flags (conj cf/flags :link-preview)] + (let [response (run-handler {:file-id (str (:id file))})] + (t/is (= 200 (::yres/status response))) + (t/is (not (str/includes? (::yres/body response) " + ``` -so that if a *human* somehow lands on `/unfurl` (e.g. some clients let users +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 fragment, and the app loads normally. Crawlers do not execute +keeping the query string and the fragment, and the app loads normally. Crawlers do not execute JavaScript, so they just read the meta tags. ### Making file thumbnails publicly accessible @@ -168,14 +175,14 @@ File: `backend/src/app/http/assets.clj`. Crawlers fetch `og:image` anonymously, so the thumbnail asset must be served without authentication. The assets handler decides per storage bucket whether auth is required; with this feature the `file-thumbnail` bucket is treated as -public **only while the `link-unfurl` flag is enabled**: +public **only while the `link-preview` flag is enabled**: ```clojure (defn- public-bucket? [bucket] (or (contains? public-buckets bucket) (and (= "file-thumbnail" bucket) - (contains? cf/flags :link-unfurl)))) + (contains? cf/flags :link-preview)))) ``` With the flag disabled, `file-thumbnail` objects keep requiring an @@ -183,12 +190,12 @@ authenticated profile with access to the file, as before. ## The feature flag -Defined in `common/src/app/common/flags.cljc` as `:link-unfurl`, listed in the +Defined in `common/src/app/common/flags.cljc` as `:link-preview`, listed in the `varia` set and **not** included in the default flags. Enable it on the backend with: ```bash -export PENPOT_FLAGS="$PENPOT_FLAGS enable-link-unfurl" +export PENPOT_FLAGS="$PENPOT_FLAGS enable-link-preview" ``` It is a backend-only decision point; the frontend URL mirroring is always @@ -198,10 +205,10 @@ metadata. ## Security considerations -Enabling `link-unfurl` deliberately trades some privacy for shareability: +Enabling `link-preview` deliberately trades some privacy for shareability: * **File names become readable by anyone who knows the file id** (the - `/unfurl` endpoint does no permission check). + `/link-preview` endpoint does no permission check). * **Dashboard thumbnails become downloadable by anyone who knows the media id** (the `file-thumbnail` bucket becomes public). @@ -210,7 +217,7 @@ knowledge-of-the-id access, not real authorization. This is the standard trade-off that link preview features make; it is the reason the flag is off by default and should be documented to self-hosters before they enable it. -The unfurl page also sets `robots: noindex` to keep search engines from +The preview page also sets `robots: noindex` to keep search engines from indexing these preview pages, and responses are marked non-cacheable. ## Testing it locally (devenv) @@ -221,7 +228,7 @@ indexing these preview pages, and responses are marked non-cacheable. 2. Enable the flag before starting the backend REPL: ```bash - export PENPOT_FLAGS="$PENPOT_FLAGS enable-link-unfurl" + export PENPOT_FLAGS="$PENPOT_FLAGS enable-link-preview" ``` 3. In the browser (`http://localhost:3449`), open a file in the workspace and @@ -232,7 +239,7 @@ indexing these preview pages, and responses are marked non-cacheable. 4. Hit the endpoint directly (bypasses the user-agent detection): ```bash - curl "http://localhost:3449/unfurl?file-id=" + curl "http://localhost:3449/link-preview?file-id=" ``` Expect HTML with `og:title` containing the file name and `og:image` @@ -264,24 +271,28 @@ indexing these preview pages, and responses are marked non-cacheable. ## Automated tests - * `backend/test/backend_tests/http_unfurl_test.clj` — endpoint behavior: + * `backend/test/backend_tests/http_link_preview_test.clj` — endpoint behavior: default context, file with/without thumbnail, non-existent and malformed - file ids, project and team links, and flag disabled. + file ids, deleted file, only-deleted thumbnail, latest-thumbnail revision + ordering, file-name HTML escaping, response headers, file-beats-project + priority, decisive file-id (malformed vs absent with a project id), + project and team links, and flag disabled. * `backend/test/backend_tests/http_assets_test.clj` - (`objects-handler-file-thumbnail-bucket-link-unfurl-flag`) — the + (`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` — `match->context-params` - priority and legacy path-params support. + priority (file > project > team), project link without team, and legacy + path-params support. ## Relevant files | File | Role | |---|---| -| `backend/src/app/http/unfurl.clj` | `/unfurl` handler: flag check, DB lookup, template rendering | -| `backend/resources/app/templates/unfurl.tmpl` | Open Graph HTML template + human redirect script | +| `backend/src/app/http/link_preview.clj` | `/link-preview` handler: flag check, DB lookup, template rendering | +| `backend/resources/app/templates/link-preview.tmpl` | Open Graph HTML template + human redirect script | | `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-unfurl` flag definition | +| `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 | -| `docker/devenv/files/nginx.conf` | Devenv crawler detection and `/unfurl` routing | +| `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/src/app/main/router.cljs b/frontend/src/app/main/router.cljs index a48dfa12e1..ccb3ec9aa8 100644 --- a/frontend/src/app/main/router.cljs +++ b/frontend/src/app/main/router.cljs @@ -70,7 +70,7 @@ They are mirrored on the query string (before the fragment) because the fragment is never sent to the server; this way shared links - carry enough context for rendering link preview (unfurl) metadata." + carry enough context for rendering link preview metadata." [match] (let [path-params (dm/get-in match [:params :path]) query-params (get match :query-params) @@ -111,7 +111,11 @@ href (dm/str (.-pathname globals/location) (if (some? query) (dm/str "?" query) "") (.-hash globals/location))] - (.replaceState js/history nil "" href))))) + ;; 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)) + (.replaceState js/history nil "" href)))))) (defn navigate [id params & {:keys [::replace ::new-window] :as options}] diff --git a/frontend/test/frontend_tests/router_test.cljs b/frontend/test/frontend_tests/router_test.cljs index 8ba9fa8ce7..07d796f886 100644 --- a/frontend/test/frontend_tests/router_test.cljs +++ b/frontend/test/frontend_tests/router_test.cljs @@ -39,3 +39,19 @@ (t/deftest match-context-params-no-context (let [match {:query-params {:token "some-token"}}] (t/is (nil? (rt/match->context-params match))))) + +(t/deftest match-context-params-project-link-without-team + ;; Without a team-id the raw map keeps a nil team-id; the nil is dropped + ;; later by the query-string serialization, not here. + (let [match {:query-params {:project-id "project-1"}}] + (t/is (= {:team-id nil + :project-id "project-1"} + (rt/match->context-params match))))) + +(t/deftest match-context-params-file-beats-project-and-team + ;; With file, project and team ids present, only the file-id is mirrored. + (let [match {:query-params {:team-id "team-1" + :project-id "project-1" + :file-id "file-1"}}] + (t/is (= {:file-id "file-1"} + (rt/match->context-params match)))))