From c48afa9f35092ed22949f2526bf33b678155b983 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Sat, 24 Oct 2026 00:54:58 +0200 Subject: [PATCH] :bug: Restore parent sync state when undoing child structure edits Add, remove and move inverses restore the copy parent's remote-synced flag with its touched groups. Swapping a child clears that flag during delete; undo must restore the original parent state (F157). Swap and add/remove regressions fail before the change. The focused swap suite passes 49 tests and 191 assertions; common lint and format pass. Benchmark ratios: mains 0.49/0.57/0.62; levels 0.53; chain 0.57; library 0.69; many shapes 0.62; switch 0.62, no regression flags. AI-assisted-by: GPT-6 --- .../memories/common/changes-architecture.md | 7 ++++- .../src/app/common/files/changes_builder.cljc | 28 ++++++++++++------- .../common_tests/logic/swap_group_test.cljc | 27 ++++++++++++++++++ 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/.serena/memories/common/changes-architecture.md b/.serena/memories/common/changes-architecture.md index 82da7bb71e..d5b182a33e 100644 --- a/.serena/memories/common/changes-architecture.md +++ b/.serena/memories/common/changes-architecture.md @@ -36,4 +36,9 @@ For component touched semantics and sync groups, read `mem:common/component-data ## Inspection -To inspect what a UI action emitted, use `mem:frontend/cljs-repl` with the snippets in `mem:common/component-debugging-recipes` rather than adding temporary source instrumentation. \ No newline at end of file +To inspect what a UI action emitted, use `mem:frontend/cljs-repl` with the snippets in `mem:common/component-debugging-recipes` rather than adding temporary source instrumentation. + +- Structural add/remove and change-parent inverses restore the copy parent's + `:remote-synced` as well as `:touched`. Removing or adding a child clears + the former even when touched groups do not change; restoring only the + groups loses prior state on undo (F157). diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index 442fb14660..e920a49cf8 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -458,6 +458,15 @@ ;; Shape tree changes +(defn- restore-parent-state-change + [page-id parent] + {:type :mod-obj + :page-id page-id + :id (:id parent) + :operations (cond-> [{:type :set-touched :touched (:touched parent)}] + (:remote-synced parent) + (conj {:type :set-remote-synced :remote-synced true}))}) + (defn add-object ([changes obj] (add-object changes obj nil)) @@ -489,11 +498,7 @@ :ignore-touched true} restore-touched-change - {:type :mod-obj - :page-id (::page-id (meta changes)) - :id (:id parent) - :operations [{:type :set-touched - :touched (:touched parent)}]}] + (restore-parent-state-change (::page-id (meta changes)) parent)] (-> changes (update :redo-changes conj add-change) @@ -552,11 +557,7 @@ (assoc :allow-altering-copies true))))) restore-touched-change - {:type :mod-obj - :page-id (::page-id (meta changes)) - :id (:id parent) - :operations [{:type :set-touched - :touched (:touched parent)}]}] + (restore-parent-state-change (::page-id (meta changes)) parent)] (-> changes (update :redo-changes conj set-parent-change) @@ -723,6 +724,13 @@ (-> changes (update :redo-changes #(reduce add-redo-change % ids)) (update :undo-changes #(as-> % $ + (reduce (fn [undo parent-id] + (let [parent (get objects parent-id)] + (cond-> undo + (and (not ignore-touched) (:shape-ref parent) + (:remote-synced parent)) + (conj (restore-parent-state-change page-id parent))))) + $ (distinct (keep (fn [id] (:parent-id (get objects id))) ids))) (reduce add-undo-change-parent $ ids) (reduce add-undo-change-shape $ ids)))))) diff --git a/common/test/common_tests/logic/swap_group_test.cljc b/common/test/common_tests/logic/swap_group_test.cljc index 93766cce1b..46d38cad27 100644 --- a/common/test/common_tests/logic/swap_group_test.cljc +++ b/common/test/common_tests/logic/swap_group_test.cljc @@ -821,3 +821,30 @@ undone (r/step moved (n/undo))] (t/is (nil? (:touched head))) (t/is (= (thn/normalize-file file) (thn/normalize-file (tm/file undone)))))) + +(t/deftest swapping-a-child-restores-its-parents-remote-sync-flag-on-undo + (let [initial (tm/make-situation (ths/update-shape (setup-sized) :copy :remote-synced true)) + swapped (-> initial (r/step (n/swap-shape :copy-star :heart)) r/settle) + undone (r/step swapped (n/undo))] + (t/is (:remote-synced (tm/resolve-shape initial :copy))) + (t/is (not (:remote-synced (tm/resolve-shape swapped :copy)))) + (t/is (:remote-synced (tm/resolve-shape undone :copy))) + (t/is (= (thn/normalize-file (tm/file initial)) + (thn/normalize-file (tm/file undone)))))) + +(t/deftest structural-child-changes-restore-the-parents-remote-sync-state + (doseq [operation [:add :remove]] + (let [file (ths/update-shape (setup-sized) :copy-star :remote-synced true) + page (thf/current-page file) + parent (ths/get-shape file :copy-star) + changes (-> (pcb/empty-changes nil (:id page)) + (pcb/with-objects (:objects page))) + changes (case operation + :add (pcb/add-object changes (ths/sample-shape :added + :parent-id (:id parent) + :frame-id (:id parent))) + :remove (pcb/remove-objects changes [(first (:shapes parent))])) + changed (thf/apply-changes file changes :validate? false) + undone (thf/apply-undo-changes changed changes)] + (t/is (not (:remote-synced (ths/get-shape changed :copy-star))) (name operation)) + (t/is (= (thn/normalize-file file) (thn/normalize-file undone)) (name operation)))))