github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 14:00:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Keep a page reorder on main visible to a bounded compare

Browse Source 
The page order lives in the document root, not in a page fragment, so
a reorder on main moves no fragment, and a compare bounded to the moved
pages compared the order of no page at all. moved-pages now leaves the
whole-file comparison in place when main orders the pages it shares
with the base differently from the base.

AI-assisted-by: claude-opus-5-5
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>
This commit is contained in:
Álvaro Tejero Cantero 2026-10-28 11:42:49 +01:00
parent 9d722cfb99
commit e53719f2b7
No known key found for this signature in database
2 changed files with 33 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
backend/src/app/rpc/commands/files_branch.clj
View File

@ -351,15 +351,20 @@
(defn- moved-pages
"The pages on which main can differ from a branch's base: every page
whose fragment main does not share with the base. Nil when fragment ids
cannot tell, because main or the base holds no pointers, which leaves
the whole-file comparison in place. `stored` is main's `:data` with its
pointers unrealized, and `base-ids` the `::bfc/base-page-pointers` of
the branch read."
[stored base-ids]
whose fragment main does not share with the base. Nil when that set
cannot bound a comparison, which leaves the whole-file comparison in
place: main or the base holds no pointers, or main ordered the pages
both hold differently from the base. The page order lives in the
document root rather than in a page's fragment, and a bounded
comparison only orders the pages it covers. `stored` is main's `:data`
with its pointers unrealized, `base` the comparison's base, and
`base-ids` the `::bfc/base-page-pointers` of the branch read."
[stored base base-ids]
(when (seq base-ids)
(let [main-ids (fdata/get-page-pointer-ids stored)]
(when (seq main-ids)
(let [main-ids (fdata/get-page-pointer-ids stored)
both (set/intersection (set (:pages stored)) (set (:pages base)))]
(when (and (seq main-ids)
(= (filterv both (:pages stored)) (filterv both (:pages base))))
(into #{}
(remove (fn [page-id]
(let [id (get main-ids page-id)]
@ -374,7 +379,7 @@
is the comparison's base, in main's id space."
[cfg source-file-id branch-file base-data]
(let [stored (bfc/get-file cfg source-file-id)
moved (moved-pages (:data stored) (::bfc/base-page-pointers branch-file))
moved (moved-pages (:data stored) base-data (::bfc/base-page-pointers branch-file))
shared (fn [pages page-id page]
(if (contains? moved page-id)
(assoc pages page-id page)
@ -770,7 +775,7 @@
;; the stored document is a handful of pointers, read only
;; when the base holds pointers to compare them with
(seq base-ids)
[main-data (moved-pages (:data (bfc/get-file cfg source-file-id)) base-ids)]
[main-data (moved-pages (:data (bfc/get-file cfg source-file-id)) base-data base-ids)]
:else
[main-data nil])

18
backend/test/backend_tests/branch_pointer_base_test.clj
View File

@ -282,3 +282,21 @@
(t/is (= 1 (count (:conflicts eager)))))
(t/testing (str dir " loads the base's six pages and main's two moved ones")
(t/is (= 8 @loads))))))))
(t/deftest compare-reports-a-page-reorder-on-main
;; the page order lives in the document root, not in a page's fragment,
;; so a reorder moves no fragment and must not bound the comparison
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [{:keys [profile-id file-id]} (setup)
page-b (uuid/random)
_ (update-file! profile-id file-id [{:type :add-page :id page-b :name "B"}])
branch (create-branch! profile-id file-id "reorder")]
(update-file! profile-id file-id [{:type :mov-page :id page-b :index 0}])
(let [out (th/command! {::th/type :get-branch-diff
::rpc/profile-id profile-id
:branch-id (:id branch)
:direction :main->branch})
eager (eager-compare file-id (:branch-file-id branch) :main->branch)]
(t/is (nil? (:error out)))
(t/is (seq (:changes eager)))
(t/is (= (:changes eager) (:changes (:result out))))))))