From 26102fb41260c031c18d294468645257f20d6fa9 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Tue, 29 Sep 2026 16:38:31 +0200 Subject: [PATCH] :wrench: Fix false validation error when pasting a variant duplicate-variant calls generate-make-shapes-variant before it moves the new shape into the target container. When a variant is pasted into a different variant container, the new shape already has the target container's data but still sits in the old one, so the validation in generate-make-shapes-variant reported errors on valid data. Pass :skip-validation? to generate-make-shapes-variant, and validate the target container once the shape is inside it. That also covers the pasted shape, which the closing validation of generate-duplicate-changes missed. Add tests for pasting a variant into another container and for duplicating it inside the same container. AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- common/src/app/common/logic/libraries.cljc | 10 +- .../common_tests/logic/variants_test.cljc | 100 +++++++++++++++++- 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index ae683dd2ce..390f33c52a 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -2919,15 +2919,21 @@ [shape (cond-> changes + ;; The shape may not be inside the container yet, so validate at the end into-new-variant? - (clvp/generate-make-shapes-variant [shape] parent) + (clvp/generate-make-shapes-variant [shape] parent :skip-validation? true) ;; If it has the same parent, update the value of the last property (and into-new-variant? (= (:variant-id component) (:id parent))) (clvp/generate-update-property-value new-component-id (-> component :variant-properties count dec) value) :always - (pcb/change-parent (:id parent) [shape] 0))])) + (pcb/change-parent (:id parent) [shape] 0) + + into-new-variant? + (pcb/validate-shapes page-id + [(:id parent)] + (str "duplicate-variant: " (:id component) " into " (:id parent))))])) (defn generate-duplicate-component-change diff --git a/common/test/common_tests/logic/variants_test.cljc b/common/test/common_tests/logic/variants_test.cljc index 4cd237a818..858d623c21 100644 --- a/common/test/common_tests/logic/variants_test.cljc +++ b/common/test/common_tests/logic/variants_test.cljc @@ -10,14 +10,63 @@ [app.common.geom.point :as gpt] [app.common.logic.libraries :as cll] [app.common.logic.shapes :as cls] + [app.common.test-helpers.components :as thc] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] [app.common.test-helpers.variants :as thv] - [clojure.test :as t])) + [app.common.uuid :as uuid] + [clojure.test :as t] + [cuerdas.core :as str])) (t/use-fixtures :each thi/test-fixture) +#?(:cljs + (defn- validation-error + "Apply the changes and return the ex-data of the :validate-shapes + error they raise, or nil if they raise none." + [file changes] + (try + (thf/apply-changes file changes :validate? false) + nil + (catch :default e + (let [data (ex-data e)] + (when (= :referential-integrity (:code data)) + data)))))) + +(defn- add-two-variants + "Add two variant containers, :v01 named \"Board\" and :v02 named \"Other\"." + [file] + (-> file + (thv/add-variant :v01 :c01 :m01 :c02 :m02) + (thv/add-variant :v02 :c03 :m03 :c04 :m04) + (ths/update-shape :v02 :name "Other") + (ths/update-shape :m03 :name "Other") + (ths/update-shape :m04 :name "Other") + (thc/update-component :c03 {:name "Other"}) + (thc/update-component :c04 {:name "Other"}))) + +(defn- paste-changes + "Build the changes of pasting `shape-label` into `parent-label`, as the + workspace paste does: the pasted shape is re-parented before duplicating." + [file shape-label parent-label] + (let [page (thf/current-page file) + shape-id (thi/id shape-label) + parent-id (thi/id parent-label) + objects (-> (:objects page) + (update shape-id assoc :parent-id parent-id :frame-id parent-id))] + (-> (pcb/empty-changes nil) + (pcb/with-page-id (:id page)) + (pcb/with-library-data (:data file)) + (pcb/with-objects (:objects page)) + (cll/generate-duplicate-changes objects + page + #{shape-id} + (gpt/point 0 0) + {(:id file) file} + (:data file) + (:id file))))) + (t/deftest test-duplicate-variant-container (let [;; ==== Setup file (-> (thf/sample-file :file1) @@ -115,3 +164,52 @@ (thf/validate-file! file') (t/is (some? new-shape')) (t/is (not= (:parent-id new-shape') (:id container))))) + +(t/deftest test-paste-variant-into-another-container + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + add-two-variants) + + ;; ==== Action + changes (paste-changes file :m01 :v02) + file' (thf/apply-changes file changes) + + ;; ==== Get + v02' (ths/get-shape file' :v02) + new-id (last (:shapes v02')) + new' (ths/get-shape-by-id file' new-id) + c03' (thc/get-component file' :c03) + newc' (thc/get-component-by-id file' (:component-id new'))] + + ;; ==== Check + (t/is (= 3 (count (:shapes v02')))) + (t/is (= (:id v02') (:variant-id new'))) + (t/is (= (mapv :name (:variant-properties c03')) + (mapv :name (:variant-properties newc')))))) + +#?(:cljs + (t/deftest test-paste-variant-into-another-container-validates + (let [file (-> (thf/sample-file :file1) + add-two-variants) + changes (paste-changes file :m01 :v02)] + + (t/is (nil? (validation-error file changes)))))) + +#?(:cljs + (t/deftest test-duplicate-variant-in-same-container-validates + (let [file (-> (thf/sample-file :file1) + add-two-variants) + changes (paste-changes file :m01 :v01)] + + (t/is (nil? (validation-error file changes)))))) + +#?(:cljs + (t/deftest test-duplicate-variant-in-same-container-detects-errors + (let [file (-> (thf/sample-file :file1) + add-two-variants + (ths/update-shape :m02 :variant-id (uuid/next))) + changes (paste-changes file :m01 :v01) + error (validation-error file changes)] + + ;; Ctrl+D inside the same container also updates the last property value + (t/is (str/includes? (:hint error) "generate-update-property-value")))))