From 08f9fca67232cefd138b84966fd26018d9f5ce05 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Tue, 29 Sep 2026 16:16:45 +0200 Subject: [PATCH] :wrench: Validate shapes after moving them into a variant generate-make-shapes-variant now validates the new variants and their variant container, and accepts a :skip-validation? option. generate-relocate calls it before the shapes are inside the container, so it passes :skip-validation?. Relocate already validates the new container once the shapes are moved. Add tests that corrupt the new variant or the container and expect a validation error, and a test for :skip-validation?. AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- common/src/app/common/logic/shapes.cljc | 3 +- .../app/common/logic/variant_properties.cljc | 60 +++++++++++------- .../logic/variant_properties_test.cljc | 63 +++++++++++++++++++ 3 files changed, 102 insertions(+), 24 deletions(-) diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 79123a12a2..c40e2290cd 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -630,8 +630,9 @@ (clvp/generate-make-shapes-no-variant variant-shapes :skip-validation? true)) ;; Add variant info and rename when moving into a different variant-container + ;; The shapes are not inside the container yet, so validate at the end (cond-> (ctk/is-variant-container? parent) - (clvp/generate-make-shapes-variant child-heads parent)) + (clvp/generate-make-shapes-variant child-heads parent :skip-validation? true)) ;; Move the shapes (pcb/change-parent parent-id diff --git a/common/src/app/common/logic/variant_properties.cljc b/common/src/app/common/logic/variant_properties.cljc index afc8a3334f..326f2edf92 100644 --- a/common/src/app/common/logic/variant_properties.cljc +++ b/common/src/app/common/logic/variant_properties.cljc @@ -283,8 +283,11 @@ (defn generate-make-shapes-variant "Introduce some components into a variant, adding the variant-id and variant-name to the - main instances and the variant-id and variant-properties to the components." - [changes shapes variant-container] + main instances and the variant-id and variant-properties to the components. + + The skip-validation? flag, if set, avoids validating the shapes. Use it when the shapes + are not inside the variant container yet, and validate after moving them." + [changes shapes variant-container & {:keys [skip-validation?]}] (let [data (pcb/get-library-data changes) objects (pcb/get-objects changes) variant-id (:id variant-container) @@ -331,24 +334,35 @@ changes (pcb/update-shapes changes (map :id shapes) #(assoc % :variant-id variant-id - :name (:name variant-container)))] - (reduce - (fn [changes shape] - (let [component (ctcl/get-component data (:component-id shape) true)] - (if (or (zero? num-shapes) ;; do nothing if there are no shapes - (and (= variant-id (:variant-id shape)) ;; or we are only moving the shape inside its parent (it is - (not (:deleted component)))) ;; the same parent and the component isn't deleted) - changes - (let [props (create-new-properties shape total-props) - variant-name (ctv/properties-to-name props)] - (-> (pcb/update-component changes - (:component-id shape) - #(assoc % :variant-id variant-id - :variant-properties props - :name cname - :path cpath) - {:apply-changes-local-library? true}) - (pcb/update-shapes [(:id shape)] - #(assoc % :variant-name variant-name))))))) - changes - shapes))) + :name (:name variant-container))) + + changes + (reduce + (fn [changes shape] + (let [component (ctcl/get-component data (:component-id shape) true)] + (if (or (zero? num-shapes) ;; do nothing if there are no shapes + (and (= variant-id (:variant-id shape)) ;; or we are only moving the shape inside its parent (it is + (not (:deleted component)))) ;; the same parent and the component isn't deleted) + changes + (let [props (create-new-properties shape total-props) + variant-name (ctv/properties-to-name props)] + (-> (pcb/update-component changes + (:component-id shape) + #(assoc % :variant-id variant-id + :variant-properties props + :name cname + :path cpath) + {:apply-changes-local-library? true}) + (pcb/update-shapes [(:id shape)] + #(assoc % :variant-name variant-name))))))) + changes + shapes)] + + (if (or skip-validation? (zero? num-shapes) (empty? shapes)) + changes + (let [ids-to-validate (conj (mapv :id shapes) variant-id)] + (pcb/validate-shapes changes + (pcb/get-page-id changes) + ids-to-validate + (str "generate-make-shapes-variant: " variant-id + " shapes: " (mapv :id shapes))))))) diff --git a/common/test/common_tests/logic/variant_properties_test.cljc b/common/test/common_tests/logic/variant_properties_test.cljc index ccbc28a0de..8f2235bc22 100644 --- a/common/test/common_tests/logic/variant_properties_test.cljc +++ b/common/test/common_tests/logic/variant_properties_test.cljc @@ -1020,3 +1020,66 @@ (t/is (str/includes? (:hint error) "generate-relocate")) (t/is (contains? (error-codes error) :main-instance-invalid-variant-id))))) + +#?(:cljs + (t/deftest test-make-shapes-variant-validates-shape + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (tho/add-simple-component :c03 :m03 :sh03 + :root-params {:parent-label :v01}) + (ths/update-shape :m03 :component-file (uuid/next))) + container (ths/get-shape file :v01) + m03 (ths/get-shape file :m03) + page (thf/current-page file) + + changes (-> (pcb/empty-changes nil) + (pcb/with-page-id (:id page)) + (pcb/with-library-data (:data file)) + (pcb/with-objects (:objects page)) + (clvp/generate-make-shapes-variant [m03] container)) + + error (validation-error file changes)] + + (t/is (str/includes? (:hint error) "generate-make-shapes-variant")) + (t/is (contains? (error-codes error) :component-main-external))))) + +#?(:cljs + (t/deftest test-make-shapes-variant-validates-container + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (tho/add-simple-component :c03 :m03 :sh03 + :root-params {:parent-label :v01}) + (ths/update-shape :m02 :variant-id (uuid/next))) + container (ths/get-shape file :v01) + m03 (ths/get-shape file :m03) + page (thf/current-page file) + + changes (-> (pcb/empty-changes nil) + (pcb/with-page-id (:id page)) + (pcb/with-library-data (:data file)) + (pcb/with-objects (:objects page)) + (clvp/generate-make-shapes-variant [m03] container)) + + error (validation-error file changes)] + + (t/is (str/includes? (:hint error) "generate-make-shapes-variant")) + (t/is (contains? (error-codes error) :main-instance-invalid-variant-id))))) + +#?(:cljs + (t/deftest test-make-shapes-variant-skip-validation + (let [file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (tho/add-simple-component :c03 :m03 :sh03 + :root-params {:parent-label :v01}) + (ths/update-shape :m02 :variant-id (uuid/next))) + container (ths/get-shape file :v01) + m03 (ths/get-shape file :m03) + page (thf/current-page file) + + changes (-> (pcb/empty-changes nil) + (pcb/with-page-id (:id page)) + (pcb/with-library-data (:data file)) + (pcb/with-objects (:objects page)) + (clvp/generate-make-shapes-variant [m03] container :skip-validation? true))] + + (t/is (nil? (validation-error file changes))))))