From ed3fc5b8b27846362c103e46aa071234cafd7f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Fri, 12 Sep 2025 16:00:01 +0200 Subject: [PATCH] :bug: Fix detaching a nested copy inside a main component (#7304) * :bug: Fix detaching a nested copy inside a main component * :lipstick: Rename functions for more semantic precission --- common/src/app/common/files/repair.cljc | 25 +++++++++++++++ common/src/app/common/files/validate.cljc | 29 ++++++++++++++++- common/src/app/common/logic/libraries.cljc | 14 +++++++- common/src/app/common/test_helpers/files.cljc | 3 +- common/src/app/common/types/component.cljc | 24 +++++++++++--- .../logic/comp_detach_with_nested_test.cljc | 32 +++++++++++++++++++ 6 files changed, 120 insertions(+), 7 deletions(-) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 945b0c1f57..eaa70209a2 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -320,6 +320,31 @@ (pcb/with-file-data file-data) (pcb/update-shapes shape-ids detach-shape)))))) +(defmethod repair-error :ref-shape-is-not-head + [_ {:keys [shape page-id] :as error} file-data _] + (let [repair-shape + (fn [shape] + ; Convert shape in a normal copy, removing nested copy status + (log/debug :hint " -> unhead shape") + (ctk/unhead-shape shape))] + + (log/dbg :hint "repairing shape :shape-ref-is-not-head" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-shape)))) + +(defmethod repair-error :ref-shape-is-head + [_ {:keys [shape page-id args] :as error} file-data _] + (let [repair-shape + (fn [shape] + ; Convert shape in a nested head, adding component info + (log/debug :hint " -> reroot shape") + (ctk/rehead-shape shape (:component-file args) (:component-id args)))] + + (log/dbg :hint "repairing shape :shape-ref-is-head" :id (:id shape) :name (:name shape) :page-id page-id) + (-> (pcb/empty-changes nil page-id) + (pcb/with-file-data file-data) + (pcb/update-shapes [(:id shape)] repair-shape)))) (defmethod repair-error :shape-ref-cycle [_ {:keys [shape args] :as error} file-data _] diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 348db903f5..0d68b7dcd2 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -47,6 +47,8 @@ :should-be-component-root :should-not-be-component-root :ref-shape-not-found + :ref-shape-is-head + :ref-shape-is-not-head :shape-ref-in-main :root-main-not-allowed :nested-main-not-allowed @@ -305,6 +307,28 @@ "Shape inside main instance should not have shape-ref" shape file page))) +(defn- check-ref-is-not-head + "Validate that the referenced shape is not a nested copy root." + [shape file page libraries] + (let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (when (and (some? ref-shape) + (ctk/instance-head? ref-shape)) + (report-error :ref-shape-is-head + (str/ffmt "Referenced shape % is a component, so the copy must also be" (:shape-ref shape)) + shape file page)))) + +(defn- check-ref-is-head + "Validate that the referenced shape is a nested copy root." + [shape file page libraries] + (let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (when (and (some? ref-shape) + (not (ctk/instance-head? ref-shape))) + (report-error :ref-shape-is-not-head + (str/ffmt "Referenced shape % of a head copy must also be a head" (:shape-ref shape)) + shape file page + :component-file (:component-file ref-shape) + :component-id (:component-id ref-shape))))) + (defn- check-empty-swap-slot "Validate that this shape does not have any swap slot." [shape file page] @@ -382,6 +406,7 @@ (check-component-not-main-head shape file page libraries) (check-component-root shape file page) (check-component-ref shape file page libraries) + (check-ref-is-head shape file page libraries) (check-empty-swap-slot shape file page) (check-duplicate-swap-slot shape file page) (check-valid-touched shape file page) @@ -399,7 +424,8 @@ ;; We can have situations where the nested copy and the ancestor copy come from different libraries and some of them have been dettached ;; so we only validate the shape-ref if the ancestor is from a valid library (when library-exists - (check-component-ref shape file page libraries)) + (check-component-ref shape file page libraries) + (check-ref-is-head shape file page libraries)) (run! #(check-shape % file page libraries :context :copy-nested) (:shapes shape))) (defn- check-shape-main-not-root @@ -417,6 +443,7 @@ (check-component-not-main-not-head shape file page) (check-component-not-root shape file page) (check-component-ref shape file page libraries) + (check-ref-is-not-head shape file page libraries) (check-empty-swap-slot shape file page) (check-valid-touched shape file page) (run! #(check-shape % file page libraries :context :copy-any) (:shapes shape))) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index d7f4d602c0..72e35ab1fe 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -1734,6 +1734,17 @@ [(conj roperations roperation) (conj uoperations uoperation)])) +(defn- check-detached-main + [changes dest-shape origin-shape] + ;; Only for direct updates (from main to copy). Check if the main shape + ;; has been detached. If so, the copy shape must be unheaded (i.e. converted + ;; into a normal copy and not a nested instance). + (if (and (= (:shape-ref dest-shape) (:id origin-shape)) + (ctk/subcopy-head? dest-shape) + (not (ctk/instance-head? origin-shape))) + (pcb/update-shapes changes [(:id dest-shape)] ctk/unhead-shape {:ignore-touched true}) + changes)) + (defn- update-attrs "The main function that implements the attribute sync algorithm. Copy attributes that have changed in the origin shape to the dest shape. @@ -1774,6 +1785,8 @@ (seq roperations) (add-update-attr-changes dest-shape container roperations uoperations) :always + (check-detached-main dest-shape origin-shape) + :always (generate-update-tokens container dest-shape origin-shape touched omit-touched?)) (let [attr-group (get ctk/sync-attrs attr) @@ -1797,7 +1810,6 @@ (= :content attr) (touched attr-group)) - skip-operations? (or (= (get origin-shape attr) (get dest-shape attr)) (and (touched attr-group) diff --git a/common/src/app/common/test_helpers/files.cljc b/common/src/app/common/test_helpers/files.cljc index 5420a0c377..a80675b65a 100644 --- a/common/src/app/common/test_helpers/files.cljc +++ b/common/src/app/common/test_helpers/files.cljc @@ -108,7 +108,8 @@ page (if (some? page-label) (:id (get-page file page-label)) (current-page-id file)) - libraries (or libraries {})] + libraries (or libraries + {(:id file) file})] (ctf/dump-tree file page libraries params))) diff --git a/common/src/app/common/types/component.cljc b/common/src/app/common/types/component.cljc index a97c1d3b78..a9331f7a61 100644 --- a/common/src/app/common/types/component.cljc +++ b/common/src/app/common/types/component.cljc @@ -145,9 +145,12 @@ (defn component-attr? "Check if some attribute is one that is involved in component syncrhonization. Note that design tokens also are involved, although they go by an alternate - route and thus they are not part of :sync-attrs." + route and thus they are not part of :sync-attrs. + Also when detaching a nested copy it also needs to trigger a synchronization, + even though :shape-ref is not a synced attribute per se" [attr] (or (get sync-attrs attr) + (= :shape-ref attr) (= :applied-tokens attr))) (defn instance-root? @@ -217,19 +220,16 @@ (and (= shape-id (:main-instance-id component)) (= page-id (:main-instance-page component)))) - (defn is-variant? "Check if this shape or component is a variant component" [item] (some? (:variant-id item))) - (defn is-variant-container? "Check if this shape is a variant container" [shape] (:is-variant-container shape)) - (defn set-touched-group [touched group] (when group @@ -310,6 +310,22 @@ :shape-ref :touched)) +(defn unhead-shape + "Make the shape not be a component head, but keep its :shape-ref and :touched if it was a nested copy" + [shape] + (dissoc shape + :component-root + :component-file + :component-id + :main-instance)) + +(defn rehead-shape + "Make the shape a component head, by adding component info" + [shape component-file component-id] + (assoc shape + :component-file component-file + :component-id component-id)) + (defn- extract-ids [shape] (if (map? shape) (let [current-id (:id shape) diff --git a/common/test/common_tests/logic/comp_detach_with_nested_test.cljc b/common/test/common_tests/logic/comp_detach_with_nested_test.cljc index ac7d62d116..9460a3b91c 100644 --- a/common/test/common_tests/logic/comp_detach_with_nested_test.cljc +++ b/common/test/common_tests/logic/comp_detach_with_nested_test.cljc @@ -446,3 +446,35 @@ (t/is (= (count fills') 1)) (t/is (= (:fill-color fill') "#fabada")) (t/is (= (:fill-opacity fill') 1)))) + +(t/deftest test-detach-copy-in-main + (let [;; ==== Setup + file (-> (setup-file) + (thc/instantiate-component :c-big-board + :copy-big-board + :children-labels [:copy-h-board-with-ellipse + :copy-nested-h-ellipse + :copy-nested-ellipse])) + + page (thf/current-page file) + + ;; ==== Action + changes (cll/generate-detach-instance (-> (pcb/empty-changes nil) + (pcb/with-page page) + (pcb/with-objects (:objects page))) + page + {(:id file) file} + (thi/id :nested-h-ellipse)) + file' (-> (thf/apply-changes file changes) + (tho/propagate-component-changes :c-board-with-ellipse) + (tho/propagate-component-changes :c-big-board)) + + ;; ==== Get + nested2-h-ellipse (ths/get-shape file' :nested-h-ellipse) + copy-nested2-h-ellipse (ths/get-shape file' :copy-nested-h-ellipse)] + + ;; ==== Check + + ;; When the nested copy inside the main is detached, their copies are unheaded. + (t/is (not (ctk/subcopy-head? nested2-h-ellipse))) + (t/is (not (ctk/subcopy-head? copy-nested2-h-ellipse))))) \ No newline at end of file