From ae1b541208ef4d9c350eeb534bf2ec1cb7192042 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Wed, 30 Sep 2026 16:30:38 +0200 Subject: [PATCH] :bug: Fix assertion when restoring a deleted component The changes builder getters now assert that their context was given, but prepare-restore-component and generate-delete-shapes relied on them returning nil to fall back to other data. The workspace restores components with changes that carry no library data, so restoring a main component or a variant from one of its copies failed. Add has-library-data?, has-objects? and has-page? to the changes builder, and check them in those fallbacks before calling the getters. Add tests that restore a component, and a variant whose container was deleted, with changes that carry no library data, as the workspace does. AI-assisted-by: claude-opus-5-5 --- .../src/app/common/files/changes_builder.cljc | 15 ++++ common/src/app/common/logic/libraries.cljc | 8 ++- common/src/app/common/logic/shapes.cljc | 3 +- .../logic/comp_creation_test.cljc | 68 +++++++++++++++++++ 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index 4587c3416c..cb0b2b06fd 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -1264,6 +1264,21 @@ (assert-page-id! changes) (::page-id (meta changes))) +;; The getters above assert that their context has been given. Use these +;; predicates when a caller has a fallback for a missing context. + +(defn has-library-data? + [changes] + (contains? (meta changes) ::library-data)) + +(defn has-objects? + [changes] + (contains? (meta changes) ::file-data)) + +(defn has-page? + [changes] + (contains? (meta changes) ::page)) + (defn set-text-content [changes id content prev-content] (assert-page-id! changes) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 1cc1a638f2..05dac8ca49 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -442,9 +442,13 @@ (prepare-restore-component changes library-data component-id page nil nil nil nil))) ([changes library-data component-id page position old-id parent-id frame-id] - (let [library-data (or (pcb/get-library-data changes) library-data) + (let [library-data (or (when (pcb/has-library-data? changes) + (pcb/get-library-data changes)) + library-data) component (ctkl/get-deleted-component library-data component-id) - objects (or (pcb/get-objects changes) (:objects page)) + objects (or (when (pcb/has-objects? changes) + (pcb/get-objects changes)) + (:objects page)) parent (get objects parent-id) main-inst (get-in component [:objects (:main-instance-id component)]) inside-component? (some? (ctn/get-instance-root (:objects page) parent)) diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 020f92d0a8..2ee3d54f55 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -267,7 +267,8 @@ (let [objects (pcb/get-objects changes) data (pcb/get-library-data changes) page-id (pcb/get-page-id changes) - page (or (pcb/get-page changes) + page (or (when (pcb/has-page? changes) + (pcb/get-page changes)) (ctpl/get-page data page-id)) ids (cfh/clean-loops objects ids) diff --git a/common/test/common_tests/logic/comp_creation_test.cljc b/common/test/common_tests/logic/comp_creation_test.cljc index 9b5a7bf5f6..7cd5acc67c 100644 --- a/common/test/common_tests/logic/comp_creation_test.cljc +++ b/common/test/common_tests/logic/comp_creation_test.cljc @@ -17,6 +17,7 @@ [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] + [app.common.test-helpers.variants :as thv] [app.common.types.component :as ctk] [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] @@ -664,3 +665,70 @@ (t/is (some? copy1-root')) (t/is (not (ctk/instance-head? copy1-root'))) (t/is (not (ctk/in-component-copy? copy1-root'))))) + +(defn- delete-shapes + [file labels] + (let [page (thf/current-page file) + [_ changes] + (cls/generate-delete-shapes (pcb/empty-changes) + file + page + (:objects page) + (into #{} (map thi/id) labels) + {:components-v2 true})] + (thf/apply-changes file changes))) + +(defn- restore-changes + "Build the changes of restoring a deleted component as the workspace does, + without any library data or objects in the changes builder." + [file component-label] + (let [page (thf/current-page file)] + (cll/generate-restore-component (pcb/empty-changes) + (:data file) + (thi/id component-label) + (:id file) + page + (:objects page)))) + +(t/deftest test-restore-component-without-library-data + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + (tho/add-simple-component-with-copy :component1 + :main1-root + :main1-child + :copy1-root) + (delete-shapes [:main1-root])) + + ;; ==== Action + changes (restore-changes file :component1) + file' (thf/apply-changes file changes) + + ;; ==== Get + component1' (thc/get-component file' :component1) + main1-root' (ths/get-shape file' :main1-root)] + + ;; ==== Check + (t/is (some? component1')) + (t/is (some? main1-root')))) + +(t/deftest test-restore-variant-after-deleting-its-container + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (delete-shapes [:v01])) + + ;; ==== Action + changes (restore-changes file :c01) + file' (thf/apply-changes file changes) + + ;; ==== Get + c01' (thc/get-component file' :c01) + m01' (ths/get-shape file' :m01)] + + ;; ==== Check + ;; The container no longer exists, so the main is restored as a + ;; component that is not a variant + (t/is (some? c01')) + (t/is (some? m01')) + (t/is (nil? (:variant-id c01'))) + (t/is (nil? (:variant-id m01')))))