diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index c40e2290cd..a7567ea649 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -587,16 +587,22 @@ #{} (remove #(= % parent-id) all-parents)) - ;; The old and new variant containers, and the moved variants when they - ;; end up outside any component (inside one, they are not roots anymore - ;; and can't be validated on their own) + ;; The old and new variant containers, and the moved variants. Inside a + ;; component they are not roots anymore and can't be validated on their + ;; own, so validate the root of that component instead + parent-root + (ctn/get-instance-root objects parent) + variant-ids-to-validate (cond-> (into #{} (keep :variant-id) variant-shapes) (ctk/is-variant-container? parent) (conj parent-id) - (nil? (ctn/get-instance-root objects parent)) - (into (map :id) variant-shapes))] + (and (seq variant-shapes) (nil? parent-root)) + (into (map :id) variant-shapes) + + (and (seq variant-shapes) (some? parent-root)) + (conj (:id parent-root)))] (-> changes ;; Remove layout-item properties and tokens when moving a shape outside a layout diff --git a/common/src/app/common/logic/variant_properties.cljc b/common/src/app/common/logic/variant_properties.cljc index f2f056f660..35e2a9232e 100644 --- a/common/src/app/common/logic/variant_properties.cljc +++ b/common/src/app/common/logic/variant_properties.cljc @@ -249,8 +249,9 @@ "Extract some components from a variant, removing the variant-id and variant-name from the main instances and the variant-id and variant-properties from the components. - The skip-validation? flag, if set, avoids validating the shapes. Use it when the shapes - are still inside the variant container, and validate after moving them." + By default it validates the shapes and their old variant containers, so the shapes + must already be outside the container. If they are still inside it, set the + skip-validation? flag and validate after moving them." [changes shapes & {:keys [skip-validation?]}] (let [changes' (reduce generate-make-shape-no-variant changes shapes)] (if (or skip-validation? (empty? shapes)) diff --git a/common/test/common_tests/logic/variants_test.cljc b/common/test/common_tests/logic/variants_test.cljc index 858d623c21..e3a0abef77 100644 --- a/common/test/common_tests/logic/variants_test.cljc +++ b/common/test/common_tests/logic/variants_test.cljc @@ -11,6 +11,7 @@ [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] [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] @@ -21,6 +22,11 @@ (t/use-fixtures :each thi/test-fixture) +#?(:cljs + (defn- error-codes + [error] + (into #{} (map :code) (:details error)))) + #?(:cljs (defn- validation-error "Apply the changes and return the ex-data of the :validate-shapes @@ -213,3 +219,55 @@ ;; Ctrl+D inside the same container also updates the last property value (t/is (str/includes? (:hint error) "generate-update-property-value"))))) + +(defn- relocate-changes + [file shape-label parent-label] + (let [page (thf/current-page file)] + (cls/generate-relocate (-> (pcb/empty-changes nil) + (pcb/with-page-id (:id page)) + (pcb/with-library-data (:data file)) + (pcb/with-objects (:objects page))) + (thi/id parent-label) 0 #{(thi/id shape-label)}))) + +(t/deftest test-relocate-variant-into-another-container + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + add-two-variants) + + ;; ==== Action + changes (relocate-changes file :m01 :v02) + file' (thf/apply-changes file changes) + + ;; ==== Get + m01' (ths/get-shape file' :m01) + c01' (thc/get-component file' :c01)] + + ;; ==== Check + (t/is (= (thi/id :v02) (:parent-id m01'))) + (t/is (= (thi/id :v02) (:variant-id m01'))) + (t/is (= (thi/id :v02) (:variant-id c01'))))) + +#?(:cljs + (t/deftest test-relocate-variant-into-another-container-validates + (let [file (-> (thf/sample-file :file1) + add-two-variants) + changes (relocate-changes file :m01 :v02)] + + (t/is (nil? (validation-error file changes)))))) + +;; The workspace does not allow moving a main instance inside a component +;; (see ctn/invalid-structure-for-component?), but relocate may be called +;; directly. The moved variant is not a root anymore, so relocate validates +;; the root of the component that holds it. +#?(:cljs + (t/deftest test-relocate-variant-into-component-validates-root + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (tho/add-simple-component :c03 :m03 :b03 + :child-params {:type :frame}) + (ths/update-shape :m03 :component-file (uuid/next))) + changes (relocate-changes file :m01 :b03) + error (validation-error file changes)] + + (t/is (str/includes? (:hint error) "generate-relocate")) + (t/is (contains? (error-codes error) :component-main-external)))))