diff --git a/common/src/app/common/types/component.cljc b/common/src/app/common/types/component.cljc index ff07b66e2c..0e82eff410 100644 --- a/common/src/app/common/types/component.cljc +++ b/common/src/app/common/types/component.cljc @@ -313,24 +313,12 @@ :component-file component-file :component-id component-id)) -(defn- extract-ids [shape] - (if (map? shape) - (let [current-id (:id shape) - child-ids (mapcat extract-ids (:children shape))] - (cons current-id child-ids)) - [])) - (defn diff-components "Compare two components, and return a set of the keys with different values" [comp1 comp2] - (let [eq (fn [key val1 val2] - (if (= key :objects) - (= (extract-ids val1) (extract-ids val2)) - (= val1 val2)))] - (->> (concat (keys comp1) (keys comp2)) - (distinct) - (filter #(not (eq % (get comp1 %) (get comp2 %)))) - set))) + (into #{} + (filter #(not= (get comp1 %) (get comp2 %))) + (concat (keys comp1) (keys comp2)))) (defn allow-duplicate? [objects shape] diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index 9b569e04a7..3757b6c692 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -102,14 +102,14 @@ "Get the shape and their children recursively, but stopping when a component nested instance is found." [objects id] - (letfn [(get-children-rec [children id] + (letfn [(get-children-rec [result id top?] (let [shape (get objects id)] - (if (and (ctk/instance-head? shape) (seq children)) - children - (let [children' (conj children shape)] - (into children' - (mapcat #(get-children-rec children' %) (:shapes shape)))))))] - (get-children-rec [] id))) + (if (and (ctk/instance-head? shape) (not top?)) + result + (reduce #(get-children-rec %1 %2 false) + (conj result shape) + (:shapes shape)))))] + (get-children-rec [] id true))) (defn get-component-shape "Get the parent top shape linked to a component main for this shape, if any" @@ -347,7 +347,7 @@ (cond-> new-shape :always (-> (gsh/move delta) - (dissoc :touched :variant-id :variant-name)) + (dissoc :touched :remote-synced :variant-id :variant-name)) (and main-instance? root?) (assoc :main-instance true) @@ -355,9 +355,6 @@ (not main-instance?) (dissoc :main-instance) - main-instance? - (dissoc :shape-ref) - (not main-instance?) (assoc :shape-ref (:id original-shape)) ; shape-ref points to the near instance @@ -408,11 +405,10 @@ "Check if the shape has any parent that is a copy of a component." [objects shape] (let [parent (get objects (:parent-id shape))] - (if (nil? parent) - false - (if (ctk/in-component-copy? parent) - true - (has-any-copy-parent? objects (:parent-id shape)))))) + (cond + (or (nil? parent) (cfh/root? shape)) false + (ctk/in-component-copy? parent) true + :else (has-any-copy-parent? objects parent)))) (defn detachable? "Whether the shape can be detached from its component: a copy shape that @@ -451,7 +447,9 @@ (get-in libraries [(:component-file shape) :data :components (:component-id shape)])) (defn invalid-structure-for-component? - "Check if the structure generated nesting children in parent is invalid in terms of nested components" + "Check if the structure generated nesting children in parent is invalid in + terms of nested components: a change of a copy's structure, a main inside + a component, or a component nesting loop." [objects parent children pasting? libraries] (let [; If the original shapes had been cutted, and we are pasting them now, they aren't ; in objects. We can add them to locate later @@ -480,8 +478,9 @@ (has-any-copy-parent? objects parent) ;; If we are moving something containing a main instance the container can't be part of a component (neither main nor copy) (and selected-main-instance? parent-in-component?) - ;; Avoid placing a shape as a direct or indirect child of itself, - ;; or inside its main component if it's in a copy. + ;; Avoid a component nesting loop: a main inside itself, or a copy + ;; inside its own main. A plain shape moved into its own subtree passes; + ;; the move itself (`relocate-shapes`) refuses it. comps-nesting-loop?))) (defn parent-validation-cache diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index ae5eaa0332..549ac4a3c3 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -775,9 +775,12 @@ {:main-instance? true :keep-ids? true}) + ;; the heads of the library's components now point to the + ;; local ones; with the same ids, the nested copies keep + ;; their refs main-instance-shapes (map #(cond-> % - (some? (:component-file %)) + (= (:component-file %) (:id library-data)) (assoc :component-file (:id file-data))) main-instance-shapes) @@ -802,6 +805,7 @@ {:id (:id component) :name (:name component) :path (:path component) + :annotation (:annotation component) :main-instance-id (:id main-instance-shape) :main-instance-page page-id})) diff --git a/common/test/common_tests/types/absorb_assets_test.cljc b/common/test/common_tests/types/absorb_assets_test.cljc index 6d2e46e0db..d053fa6735 100644 --- a/common/test/common_tests/types/absorb_assets_test.cljc +++ b/common/test/common_tests/types/absorb_assets_test.cljc @@ -103,6 +103,27 @@ (t/is (some? (:objects card')) "the deleted component keeps its shapes") (t/is (empty? library-refs') "no shape points to the library") (t/is (nil? errors')))) +(t/deftest absorb-nested-components + ;; component2's main nests a copy of component1, and has an annotation; + ;; the file has a copy of component2 + (let [library (-> (thf/sample-file :library :is-shared true) + (tho/add-nested-component :component1 :main1-root :main1-child + :component2 :main2-root :nested-head) + (thc/update-component :component2 :annotation "Note")) + file (-> (thf/sample-file :file) + (thc/instantiate-component :component2 :copy2-root :library library + :children-labels [:copy-nested-head])) + file' (ctf/update-file-data file #(ctf/absorb-assets % (:data library))) + data' (ctf/file-data file') + page' (second (ctpl/pages-seq data')) + nested' (get-in page' [:objects (thi/id :nested-head)])] + (thf/validate-file! file') + (t/is (= 2 (count (ctkl/components-seq data')))) + (t/testing "the nested copy in the absorbed main keeps its link" + (t/is (= (thi/id :main1-root) (:shape-ref nested'))) + (t/is (= (:id file') (:component-file nested')))) + (t/testing "the component keeps its annotation" + (t/is (= "Note" (:annotation (ctkl/get-component data' (thi/id :component2)))))))) (t/deftest absorb-colors (let [;; Setup diff --git a/common/test/common_tests/types/container_test.cljc b/common/test/common_tests/types/container_test.cljc index be40472843..6857a0c0fc 100644 --- a/common/test/common_tests/types/container_test.cljc +++ b/common/test/common_tests/types/container_test.cljc @@ -8,7 +8,6 @@ (:require [app.common.geom.point :as gpt] [app.common.test-helpers.components :as thc] - [app.common.test-helpers.composable.core :as tm] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] @@ -332,8 +331,7 @@ labels #(set (map (comp label :id) (ctc/get-children-in-instance objects (thi/id %))))] (t/is (= #{:main1 :main1-child} (labels :main1))) (t/is (= #{:main2} (labels :main2))) - ;; each ancestor comes back once per child below it - (tm/known-failure "F59" 2 3 (count (ctc/get-children-in-instance objects (thi/id :main1)))))) + (t/is (= 2 (count (ctc/get-children-in-instance objects (thi/id :main1))))))) (t/deftest copy-parent-above-a-plain-parent ;; a rect in a plain group inside a copy: the copy is two levels up @@ -346,9 +344,7 @@ rect (make-shape (uuid/next) :parent-id group-id) objects (objects-map copy group rect)] (t/is (ctc/has-any-copy-parent? objects group)) - ;; the walk passes on the parent's id where it expects a shape, so it - ;; stops after the direct parent - (tm/known-failure "F63" true false (ctc/has-any-copy-parent? objects rect)))) + (t/is (ctc/has-any-copy-parent? objects rect)))) (t/deftest nesting-level-delta-of-a-move-between-copies ;; :copy2-root #--> [Component :component2] @@ -406,3 +402,24 @@ (t/is (= 2 (count shapes))) (t/testing "each shape references its main" (t/is (= #{(thi/id :main1) (thi/id :main1-child)} (set (map :shape-ref shapes))))))) + +(t/deftest a-new-copy-starts-clean + ;; the main of component2 nests a copy of component1, synced from a remote + ;; main and touched; a new copy of component2 has neither mark + (let [file (-> (thf/sample-file :file) + (tho/add-nested-component :c1 :main1 :main1-child + :c2 :main2 :nested-head) + (ths/update-shape :nested-head :remote-synced true) + (ths/update-shape :nested-head :touched #{:fill-group}) + (thc/instantiate-component :c2 :copy2 :children-labels [:copy-nested-head])) + shape (ths/get-shape file :copy-nested-head)] + (t/is (nil? (:touched shape))) + (t/is (nil? (:remote-synced shape))))) + +(t/deftest a-change-of-the-shapes-of-a-deleted-component-marks-it-modified + (let [component {:id (uuid/next) :name "C" :deleted true :modified-at 1 + :objects {uuid/zero {:id uuid/zero :name "Old"}}} + changed (assoc-in component [:objects uuid/zero :name] "New")] + (t/is (= #{:objects} (ctk/diff-components component changed))) + (t/is (= #{} (ctk/diff-components component component))))) +