github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 22:10:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

🔧 Fix false validation error when pasting a variant

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Eva Marco 2026-09-29 16:38:31 +02:00
parent 08f9fca672
commit 26102fb412
2 changed files with 107 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
common/src/app/common/logic/libraries.cljc
View File

@ -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

100
common/test/common_tests/logic/variants_test.cljc
View File

@ -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")))))