diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 1a1efd807d..3c4408f149 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -1766,6 +1766,59 @@ (update :pages-index d/update-vals update-container) (d/update-when :components d/update-vals update-container)))) +(defmethod migrate-data "0017-remove-unneeded-objects-from-components" + [data _] + ;; Some components have an `:objects` attribute, despite not being + ;; deleted. This migration removes it. + (letfn [(check-component [component] + (if (and (not (:deleted component)) + (contains? component :objects)) + (dissoc component :objects) + component))] + (d/update-when data :components d/update-vals check-component))) + +(defmethod migrate-data "0018-sync-component-id-with-near-main" + [data _] + (let [libs (some-> (:libs data) deref)] + (letfn [(fix-shape + [data page shape] + (if (and (ctk/subcopy-head? shape) + (nil? (ctk/get-swap-slot shape))) + (let [file {:id (:id data) :data data} + ref-shape (ctf/find-ref-shape file page libs shape {:include-deleted? true :with-context? true})] + (if (and (some? ref-shape) + (or (not= (:component-id shape) (:component-id ref-shape)) + (not= (:component-file shape) (:component-file ref-shape)))) + (cond-> shape + (some? (:component-id ref-shape)) + (assoc :component-id (:component-id ref-shape)) + + (nil? (:component-id ref-shape)) + (dissoc :component-id) + + (some? (:component-file ref-shape)) + (assoc :component-file (:component-file ref-shape)) + + (nil? (:component-file ref-shape)) + (dissoc :component-file)) + shape)) + shape)) + + (update-page + [data page] + (d/update-when page :objects d/update-vals (partial fix-shape data page))) + + (fix-data [data] + (loop [current-data data + iteration 0] + (let [next-data (update current-data :pages-index d/update-vals (partial update-page current-data))] + (if (or (= current-data next-data) + (> iteration 20)) ;; safety bound + next-data + (recur next-data (inc iteration))))))] + (fix-data data)))) + + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -1839,4 +1892,6 @@ "0014-clear-components-nil-objects" "0015-fix-text-attrs-blank-strings" "0015-clean-shadow-color" - "0016-copy-fills-from-position-data-to-text-node"])) + "0016-copy-fills-from-position-data-to-text-node" + "0017-remove-unneeded-objects-from-components" + "0018-sync-component-id-with-near-main"])) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index eaa70209a2..c8e3d172e3 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -333,6 +333,31 @@ (pcb/with-file-data file-data) (pcb/update-shapes [(:id shape)] repair-shape)))) +(defmethod repair-error :component-id-mismatch + [_ {:keys [shape page-id args] :as error} file-data _] + (let [repair-shape + (fn [shape] + ; Set the component-id and component-file to the ones of the near main + (log/debug :hint (str " -> set component-id to " (:component-id args))) + (log/debug :hint (str " -> set component-file to " (:component-file args))) + (cond-> shape + (some? (:component-id args)) + (assoc :component-id (:component-id args)) + + (nil? (:component-id args)) + (dissoc :component-id) + + (some? (:component-file args)) + (assoc :component-file (:component-file args)) + + (nil? (:component-file args)) + (dissoc :component-file)))] + + (log/dbg :hint "repairing shape :component-id-mismatch" :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 @@ -499,7 +524,7 @@ (pcb/update-shapes [(:id shape)] repair-shape)))) (defmethod repair-error :component-nil-objects-not-allowed - [_ {:keys [shape] :as error} file-data _] + [_ {component :shape} file-data _] ; in this error the :shape argument is the component (let [repair-component (fn [component] ; Remove the objects key, or set it to {} if the component is deleted @@ -511,10 +536,26 @@ (log/debug :hint " -> remove :objects") (dissoc component :objects))))] - (log/dbg :hint "repairing component :component-nil-objects-not-allowed" :id (:id shape) :name (:name shape)) + (log/dbg :hint "repairing component :component-nil-objects-not-allowed" :id (:id component) :name (:name component)) (-> (pcb/empty-changes nil) (pcb/with-library-data file-data) - (pcb/update-component (:id shape) repair-component)))) + (pcb/update-component (:id component) repair-component)))) + +(defmethod repair-error :non-deleted-component-cannot-have-objects + [_ {component :shape} file-data _] ; in this error the :shape argument is the component + (let [repair-component + (fn [component] + ; Remove the :objects field + (if-not (:deleted component) + (do + (log/debug :hint " -> remove :objects") + (dissoc component :objects)) + component))] + + (log/dbg :hint "repairing component :non-deleted-component-cannot-have-objects" :id (:id component) :name (:name component)) + (-> (pcb/empty-changes nil) + (pcb/with-library-data file-data) + (pcb/update-component (:id component) repair-component)))) (defmethod repair-error :invalid-text-touched [_ {:keys [shape page-id] :as error} file-data _] diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 5b0e1d74d4..59f1f5fad0 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -51,6 +51,7 @@ :ref-shape-is-head :ref-shape-is-not-head :shape-ref-in-main + :component-id-mismatch :root-main-not-allowed :nested-main-not-allowed :root-copy-not-allowed @@ -59,6 +60,7 @@ :not-head-copy-not-allowed :not-component-not-allowed :component-nil-objects-not-allowed + :non-deleted-component-cannot-have-objects :instance-head-not-frame :invalid-text-touched :misplaced-slot @@ -326,6 +328,20 @@ :component-file (:component-file ref-shape) :component-id (:component-id ref-shape))))) +(defn- check-ref-component-id + "Validate that if the copy has not been swwpped, the component-id and component-file are + the same as in the referenced shape in the near main." + [shape file page libraries] + (when (nil? (ctk/get-swap-slot shape)) + (when-let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (when (or (not= (:component-id shape) (:component-id ref-shape)) + (not= (:component-file shape) (:component-file ref-shape))) + (report-error :component-id-mismatch + "Nested copy component-id and component-file must be the same as the near main" + shape file page + :component-id (:component-id ref-shape) + :component-file (:component-file ref-shape)))))) + (defn- check-empty-swap-slot "Validate that this shape does not have any swap slot." [shape file page] @@ -418,6 +434,7 @@ (check-component-not-main-head shape file page libraries) (check-component-not-root shape file page) (check-valid-touched shape file page) + (check-ref-component-id shape file page libraries) ;; 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 @@ -648,6 +665,13 @@ "Component main not allowed inside other component" main-instance file component-page)))) +(defn- check-not-objects + [component file] + (when (d/not-empty? (:objects component)) + (report-error :non-deleted-component-cannot-have-objects + "A non-deleted component cannot have shapes inside" + component file nil))) + (defn- check-component "Validate semantic coherence of a component. Report all errors found." [component file] @@ -656,7 +680,8 @@ "Objects list cannot be nil" component file nil)) (when-not (:deleted component) - (check-main-inside-main component file)) + (check-main-inside-main component file) + (check-not-objects component file)) (when (:deleted component) (check-component-duplicate-swap-slot component file) (check-ref-cycles component file)) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 0d291a94f4..f476910670 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -1769,6 +1769,23 @@ (pcb/update-shapes changes [(:id dest-shape)] ctk/unhead-shape {:ignore-touched true}) changes)) +(defn- check-swapped-main + [changes dest-shape origin-shape] + ;; Only for direct updates (from main to copy). Check if the main shape + ;; has been swapped. If so, the new component-id and component-file must + ;; be put into the copy. + (if (and (= (:shape-ref dest-shape) (:id origin-shape)) + (ctk/instance-head? dest-shape) + (ctk/instance-head? origin-shape) + (or (not= (:component-id dest-shape) (:component-id origin-shape)) + (not= (:component-file dest-shape) (:component-file origin-shape)))) + (pcb/update-shapes changes [(:id dest-shape)] + #(assoc % + :component-id (:component-id origin-shape) + :component-file (:component-file origin-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. @@ -1811,6 +1828,8 @@ :always (check-detached-main dest-shape origin-shape) :always + (check-swapped-main dest-shape origin-shape) + :always (generate-update-tokens container dest-shape origin-shape touched omit-touched? nil)) (let [attr-group (get ctk/sync-attrs attr) diff --git a/common/src/app/common/test_helpers/compositions.cljc b/common/src/app/common/test_helpers/compositions.cljc index f5c9b5a1ca..b230e342d9 100644 --- a/common/src/app/common/test_helpers/compositions.cljc +++ b/common/src/app/common/test_helpers/compositions.cljc @@ -274,7 +274,7 @@ file-id {file-id file} file-id))] - (thf/apply-changes file changes))) + (thf/apply-changes file changes :validate? false))) (defn swap-component "Swap the specified shape by the component specified by component-tag" @@ -305,12 +305,13 @@ [changes nil]) - file' (thf/apply-changes file changes)] + file' (thf/apply-changes file changes :validate? (not propagate-fn))] (when new-shape-label (thi/rm-id! (:id new-shape)) (thi/set-id! new-shape-label (:id new-shape))) (if propagate-fn - (propagate-fn file') + (-> (propagate-fn file') + (thf/validate-file!)) file'))) (defn swap-component-in-shape [file shape-tag component-tag & {:keys [page-label propagate-fn]}] @@ -339,9 +340,10 @@ (assoc shape :fills (ths/sample-fills-color :fill-color color))) (:objects page) {}) - file' (thf/apply-changes file changes)] + file' (thf/apply-changes file changes :validate? (not propagate-fn))] (if propagate-fn - (propagate-fn file') + (-> (propagate-fn file') + (thf/validate-file!)) file'))) (defn update-bottom-color @@ -357,9 +359,10 @@ (assoc shape :fills (ths/sample-fills-color :fill-color color))) (:objects page) {}) - file' (thf/apply-changes file changes)] + file' (thf/apply-changes file changes :validate? (not propagate-fn))] (if propagate-fn - (propagate-fn file') + (-> (propagate-fn file') + (thf/validate-file!)) file'))) (defn reset-overrides [file shape & {:keys [page-label propagate-fn]}] @@ -374,9 +377,10 @@ {file-id file} (ctn/make-container container :page) (:id shape))) - file' (thf/apply-changes file changes)] + file' (thf/apply-changes file changes :validate? (not propagate-fn))] (if propagate-fn - (propagate-fn file') + (-> (propagate-fn file') + (thf/validate-file!)) file'))) (defn reset-overrides-in-first-child [file shape-tag & {:keys [page-label propagate-fn]}] @@ -398,9 +402,10 @@ #{(-> (ths/get-shape file shape-tag :page-label page-label) :id)} {}) - file' (thf/apply-changes file changes)] + file' (thf/apply-changes file changes :validate? (not propagate-fn))] (if propagate-fn - (propagate-fn file') + (-> (propagate-fn file') + (thf/validate-file!)) file'))) (defn duplicate-shape [file shape-tag & {:keys [page-label propagate-fn]}] @@ -419,8 +424,9 @@ (:id file)) ;; file-id (cll/generate-duplicate-changes-update-indices (:objects page) ;; objects #{(:id shape)})) - file' (thf/apply-changes file changes)] + file' (thf/apply-changes file changes :validate? (not propagate-fn))] (if propagate-fn - (propagate-fn file') + (-> (propagate-fn file') + (thf/validate-file!)) file'))) diff --git a/common/src/app/common/test_helpers/files.cljc b/common/src/app/common/test_helpers/files.cljc index a80675b65a..6357ab555b 100644 --- a/common/src/app/common/test_helpers/files.cljc +++ b/common/src/app/common/test_helpers/files.cljc @@ -54,12 +54,14 @@ ([file] (validate-file! file {})) ([file libraries] (cfv/validate-file-schema! file) - (cfv/validate-file! file libraries))) + (cfv/validate-file! file libraries) + file)) (defn apply-changes - [file changes] + [file changes & {:keys [validate?] :or {validate? true}}] (let [file' (ctf/update-file-data file #(cfc/process-changes % (:redo-changes changes) true))] - (validate-file! file') + (when validate? + (validate-file! file')) file')) (defn apply-undo-changes diff --git a/common/src/app/common/types/component.cljc b/common/src/app/common/types/component.cljc index 130bece565..dd2e2f738b 100644 --- a/common/src/app/common/types/component.cljc +++ b/common/src/app/common/types/component.cljc @@ -146,12 +146,15 @@ "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. - Also when detaching a nested copy it also needs to trigger a synchronization, - even though :shape-ref is not a synced attribute per se" + Also when detaching a nested copy or it also needs to trigger a synchronization, + even though :shape-ref or :component-id are not synced attribute per se" [attr] (or (get sync-attrs attr) (= :shape-ref attr) - (= :applied-tokens attr))) + (= :applied-tokens attr) + (= :component-id attr) + (= :component-file attr) + (= :component-root attr))) (defn instance-root? "Check if this shape is the head of a top instance." diff --git a/common/src/app/common/types/components_list.cljc b/common/src/app/common/types/components_list.cljc index c4f3a66063..be92b16999 100644 --- a/common/src/app/common/types/components_list.cljc +++ b/common/src/app/common/types/components_list.cljc @@ -60,6 +60,9 @@ (some? objects) (assoc :objects objects) + (nil? objects) + (dissoc :objects) + (some? modified-at) (assoc :modified-at modified-at) 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 9460a3b91c..143221a4d3 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 @@ -465,9 +465,10 @@ page {(:id file) file} (thi/id :nested-h-ellipse)) - file' (-> (thf/apply-changes file changes) + file' (-> (thf/apply-changes file changes :validate? false) (tho/propagate-component-changes :c-board-with-ellipse) - (tho/propagate-component-changes :c-big-board)) + (tho/propagate-component-changes :c-big-board) + (thf/validate-file!)) ;; ==== Get nested2-h-ellipse (ths/get-shape file' :nested-h-ellipse) diff --git a/common/test/common_tests/logic/duplicated_pages_test.cljc b/common/test/common_tests/logic/duplicated_pages_test.cljc index d1bafb88d7..57dd490143 100644 --- a/common/test/common_tests/logic/duplicated_pages_test.cljc +++ b/common/test/common_tests/logic/duplicated_pages_test.cljc @@ -64,9 +64,8 @@ (reset-all-overrides [file] (-> file - (tho/reset-overrides-in-first-child :frame-board-1 :page-label :page-1) - (tho/reset-overrides-in-first-child :copy-board-1 :page-label :page-2) - (propagate-all-component-changes))) + (tho/reset-overrides-in-first-child :frame-board-1 :page-label :page-1 :propagate-fn propagate-all-component-changes) + (tho/reset-overrides-in-first-child :copy-board-1 :page-label :page-2 :propagate-fn propagate-all-component-changes))) (fill-colors [file] [(tho/bottom-fill-color file :frame-ellipse-1 :page-label :page-1) diff --git a/common/test/common_tests/logic/multiple_nesting_levels_test.cljc b/common/test/common_tests/logic/multiple_nesting_levels_test.cljc index 43b7c7ef0e..d0337feb8f 100644 --- a/common/test/common_tests/logic/multiple_nesting_levels_test.cljc +++ b/common/test/common_tests/logic/multiple_nesting_levels_test.cljc @@ -56,10 +56,9 @@ (reset-all-overrides [file] (-> file - (tho/reset-overrides (ths/get-shape file :copy-simple-1)) - (tho/reset-overrides (ths/get-shape file :copy-frame-composed-1)) - (tho/reset-overrides (ths/get-shape file :composed-1-composed-2-copy)) - (propagate-all-component-changes))) + (tho/reset-overrides (ths/get-shape file :copy-simple-1 :propagate-fn propagate-all-component-changes)) + (tho/reset-overrides (ths/get-shape file :copy-frame-composed-1 :propagate-fn propagate-all-component-changes)) + (tho/reset-overrides (ths/get-shape file :composed-1-composed-2-copy :propagate-fn propagate-all-component-changes)))) (fill-colors [file] [(tho/bottom-fill-color file :frame-simple-1) diff --git a/common/test/common_tests/logic/swap_as_override_test.cljc b/common/test/common_tests/logic/swap_as_override_test.cljc index a4a1b5a632..519b24e48f 100644 --- a/common/test/common_tests/logic/swap_as_override_test.cljc +++ b/common/test/common_tests/logic/swap_as_override_test.cljc @@ -6,20 +6,12 @@ (ns common-tests.logic.swap-as-override-test (:require - [app.common.files.changes :as ch] - [app.common.files.changes-builder :as pcb] - [app.common.logic.libraries :as cll] - [app.common.logic.shapes :as cls] - [app.common.pprint :as pp] + [app.common.data :as d] [app.common.test-helpers.components :as thc] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] - [app.common.types.component :as ctk] - [app.common.types.container :as ctn] - [app.common.types.file :as ctf] - [app.common.uuid :as uuid] [clojure.test :as t])) (t/use-fixtures :each thi/test-fixture) @@ -27,23 +19,40 @@ (defn- setup [] (-> (thf/sample-file :file1) - (tho/add-simple-component :component-1 :frame-component-1 :child-component-1 :child-params {:name "child-component-1" :type :rect :fills (ths/sample-fills-color :fill-color "#111111")}) - (tho/add-simple-component :component-2 :frame-component-2 :child-component-2 :child-params {:name "child-component-2" :type :rect :fills (ths/sample-fills-color :fill-color "#222222")}) - (tho/add-simple-component :component-3 :frame-component-3 :child-component-3 :child-params {:name "child-component-3" :type :rect :fills (ths/sample-fills-color :fill-color "#333333")}) + (tho/add-simple-component :component-1 :frame-component-1 :child-component-1 + :root-params {:name "component-1"} + :child-params {:name "child-component-1" + :type :rect + :fills (ths/sample-fills-color :fill-color "#111111")}) + (tho/add-simple-component :component-2 :frame-component-2 :child-component-2 + :root-params {:name "component-2"} + :child-params {:name "child-component-2" + :type :rect + :fills (ths/sample-fills-color :fill-color "#222222")}) + (tho/add-simple-component :component-3 :frame-component-3 :child-component-3 + :root-params {:name "component-3"} + :child-params {:name "child-component-3" + :type :rect + :fills (ths/sample-fills-color :fill-color "#333333")}) - (tho/add-frame :frame-icon-and-text) - (thc/instantiate-component :component-1 :copy-component-1 :parent-label :frame-icon-and-text :children-labels [:component-1-icon-and-text]) + (tho/add-frame :frame-icon-and-text :name "copy-component-1") + (thc/instantiate-component :component-1 :copy-component-1 + :parent-label :frame-icon-and-text + :children-labels [:component-1-icon-and-text]) (ths/add-sample-shape :text {:type :text :name "icon+text" :parent-label :frame-icon-and-text}) (thc/make-component :icon-and-text :frame-icon-and-text) - (tho/add-frame :frame-panel) - (thc/instantiate-component :icon-and-text :copy-icon-and-text :parent-label :frame-panel :children-labels [:icon-and-text-panel]) + (tho/add-frame :frame-panel :name "icon-and-text") + (thc/instantiate-component :icon-and-text :copy-icon-and-text + :parent-label :frame-panel + :children-labels [:icon-and-text-panel]) (thc/make-component :panel :frame-panel) - (thc/instantiate-component :panel :copy-panel :children-labels [:copy-icon-and-text-panel]))) + (thc/instantiate-component :panel :copy-panel + :children-labels [:copy-icon-and-text-panel]))) (defn- propagate-all-component-changes [file] (-> file