From ad7fc402c0a957c1557bf32eb37b979e76178a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Tejero=20Cantero?= Date: Wed, 28 Oct 2026 16:37:24 +0100 Subject: [PATCH] :bug: Open the first page in view mode when the URL has no page id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A view URL without a page-id crashed builds with assertions enabled: the viewer passes a nil page-id to `initialize`, whose schema accepted a missing key but not a nil value. Release builds skip that check and draw the first page, but every viewer event reads the page from the route, so zoom to fit and fill, the next-board arrow, Navigate To interactions, and comment pins did not work. `initialize` now accepts a nil page-id, `fetch-bundle` makes it optional, and `bundle-fetched` replaces a route without page-id by one naming the file's first page. The viewer then initializes again on the completed route, as it does when the user changes page. A file with no pages keeps its route. AI-assisted-by: claude-opus-5-5 Signed-off-by: Álvaro Tejero Cantero --- frontend/src/app/main/data/viewer.cljs | 36 +++++++----- .../test/frontend_tests/data/viewer_test.cljs | 56 ++++++++++++++++++- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/frontend/src/app/main/data/viewer.cljs b/frontend/src/app/main/data/viewer.cljs index 683bbcf8b9..89a867355c 100644 --- a/frontend/src/app/main/data/viewer.cljs +++ b/frontend/src/app/main/data/viewer.cljs @@ -57,7 +57,7 @@ [:map {:title "initialize"} [:file-id ::sm/uuid] [:share-id {:optional true} [:maybe ::sm/uuid]] - [:page-id {:optional true} ::sm/uuid]]) + [:page-id {:optional true} [:maybe ::sm/uuid]]]) (defn initialize [{:keys [file-id share-id] :as params}] @@ -116,7 +116,7 @@ (def ^:private schema:fetch-bundle [:map {:title "fetch-bundle"} - [:page-id ::sm/uuid] + [:page-id {:optional true} ::sm/uuid] [:file-id ::sm/uuid] [:share-id {:optional true} ::sm/uuid]]) @@ -305,19 +305,27 @@ index (some-> (rt/get-query-param qparams :index) parse-long) frame-id (some-> (:frame-id qparams) uuid/parse) page-id (some-> (rt/get-query-param qparams :page-id) uuid/parse) - file-id (some-> (rt/get-query-param qparams :file-id) uuid/parse)] + file-id (some-> (rt/get-query-param qparams :file-id) uuid/parse) + first-page-id (-> file :data :pages first)] - (rx/merge - (rx/of (case (:zoom qparams) - "fit" zoom-to-fit - "fill" zoom-to-fill - nil)) - (rx/of - (update-page-position-data file-id page-id) - (cond - (some? frame-id) (go-to-frame frame-id) - (some? index) (go-to-frame-by-index index) - :else (go-to-frame-auto))))))))) + (if (and (nil? page-id) (some? first-page-id)) + ;; Every viewer event reads the page from the route, so a URL + ;; without page-id names the first page; the viewer then + ;; initializes again on the completed route. + (rx/of (rt/nav :viewer + (assoc (rt/get-params state) :page-id first-page-id) + {::rt/replace true})) + (rx/merge + (rx/of (case (:zoom qparams) + "fit" zoom-to-fit + "fill" zoom-to-fill + nil)) + (rx/of + (update-page-position-data file-id page-id) + (cond + (some? frame-id) (go-to-frame frame-id) + (some? index) (go-to-frame-by-index index) + :else (go-to-frame-auto)))))))))) (defn fetch-comment-threads [{:keys [file-id page-id share-id] :as params}] diff --git a/frontend/test/frontend_tests/data/viewer_test.cljs b/frontend/test/frontend_tests/data/viewer_test.cljs index 5c3b0ca128..94f43904b2 100644 --- a/frontend/test/frontend_tests/data/viewer_test.cljs +++ b/frontend/test/frontend_tests/data/viewer_test.cljs @@ -131,9 +131,13 @@ ;; because update-zoom-querystring navigated unconditionally. ;; At HEAD the guard breaks the cycle, so a bundle fetch must not ;; emit any navigation. - (let [state (-> (base-state {:frames [{:selrect {:width 100 :height 100}}] - :index 0}) - (assoc-in [:route :query-params :zoom] "fill"))] + (let [state (as-> (base-state {:frames [{:selrect {:width 100 :height 100}}] + :index 0}) state + ;; The router stores the same query map under both keys. + (let [query (assoc (rt/get-params state) :zoom "fill")] + (-> state + (assoc-in [:route :params :query] query) + (assoc-in [:route :query-params] query))))] (let [events (watch-events (dv/bundle-fetched {:file {:id page-id :data {:pages [page-id] @@ -151,3 +155,49 @@ ;; ones that can restart the zoom cycle. navigations (filter #(= :app.main.router/navigate (ptk/type %)) events)] (t/is (empty? navigations))))) + +(defn- minimal-bundle + [page-ids] + {:file {:id (uuid/custom 2 2) + :data {:pages (vec page-ids) + :pages-index (into {} (map (fn [id] [id {:objects {}}])) page-ids)}} + :project {} + :team {:features []} + :share-links [] + :libraries [] + :users [] + :permissions {} + :thumbnails {}}) + +(defn- route-without-page-id + [] + (let [query {:file-id (str (uuid/custom 2 2)) :section "interactions" :index "0"}] + {:route {:params {:query query} :query-params query}})) + +(t/deftest initialize-without-page-id-fetches-the-bundle + ;; The viewer route passes `:page-id nil` when the URL has none; the + ;; params check used to reject it before any fetch. + (let [events (watch-events (dv/initialize {:file-id (uuid/custom 2 2) + :page-id nil + :share-id nil}) + {})] + (t/is (= [::dv/fetch-bundle] (map ptk/type (remove nil? events)))))) + +(t/deftest bundle-fetched-without-page-id-names-the-first-page + (t/testing "the route is replaced by one naming the first page" + (let [first-page (uuid/custom 3 1) + events (watch-events (dv/bundle-fetched + (minimal-bundle [first-page (uuid/custom 3 2)])) + (route-without-page-id)) + {:keys [id params options]} (some-> (first events) deref)] + (t/is (= 1 (count events))) + (t/is (= :viewer id)) + (t/is (= first-page (:page-id params))) + (t/is (= "interactions" (:section params))) + (t/is (true? (::rt/replace options))))) + + (t/testing "a file without pages keeps the route" + (let [events (watch-events (dv/bundle-fetched (minimal-bundle [])) + (route-without-page-id)) + navigations (filter #(= :app.main.router/navigate (ptk/type %)) events)] + (t/is (empty? navigations)))))