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

🔧 Validate shapes after removing them from a variant

Browse Source 
generate-make-shapes-no-variant now validates the shapes and their old
variant containers, and accepts a :skip-validation? option.

generate-relocate calls it before moving the shapes, while they are
still inside the container. Validating the container at that point
reported :main-instance-not-a-variant on valid data. Relocate now passes
:skip-validation? and validates at the end: the old and new variant
containers, and the moved variants when they end up outside any
component. Inside a component they are no longer roots, and the
validator can't check them on their own.

Add tests for the function and for moving a variant out of its
container, with valid and corrupted data.

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:13:40 +02:00
parent c02b75a37e
commit 8e306b8e24
3 changed files with 100 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
common/src/app/common/logic/shapes.cljc
View File

@ -585,7 +585,18 @@
(conj to-delete (:id parent))
to-delete)))
#{}
(remove #(= % parent-id) all-parents))]
(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)
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))]
(-> changes
;; Remove layout-item properties and tokens when moving a shape outside a layout
@ -614,8 +625,9 @@
(pcb/update-shapes child-heads-ids #(assoc % :component-root true)))
;; Remove variant info and rename when moving outside a variant-container
;; The shapes are still inside the container, so validate at the end
(cond-> (not (ctk/is-variant-container? parent))
(clvp/generate-make-shapes-no-variant variant-shapes))
(clvp/generate-make-shapes-no-variant variant-shapes :skip-validation? true))
;; Add variant info and rename when moving into a different variant-container
(cond-> (ctk/is-variant-container? parent)
@ -698,7 +710,13 @@
;; Remove parents when are a variant-container that becomes empty
(cond-> (seq empty-variant-cont)
(#(second (generate-delete-shapes % empty-variant-cont {})))))))
(#(second (generate-delete-shapes % empty-variant-cont {}))))
;; Validate the variants once the shapes are in their new parent
(cond-> (seq variant-ids-to-validate)
(pcb/validate-shapes (pcb/get-page-id changes)
variant-ids-to-validate
(str "generate-relocate: " (vec ids) " to parent " parent-id))))))
(defn change-show-in-viewer
[shape hide?]

20
common/src/app/common/logic/variant_properties.cljc
View File

@ -242,10 +242,22 @@
(assoc :name new-name))))))
(defn generate-make-shapes-no-variant
"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."
[changes shapes]
(reduce generate-make-shape-no-variant changes shapes))
"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."
[changes shapes & {:keys [skip-validation?]}]
(let [changes' (reduce generate-make-shape-no-variant changes shapes)]
(if (or skip-validation? (empty? shapes))
changes'
(let [ids-to-validate (into (mapv :id shapes)
(comp (keep :variant-id) (distinct))
shapes)]
(pcb/validate-shapes changes'
(pcb/get-page-id changes')
ids-to-validate
(str "generate-make-shapes-no-variant: " (mapv :id shapes)))))))
(defn- create-new-properties-from-variant
[shape min-props data container-name base-properties]

64
common/test/common_tests/logic/variant_properties_test.cljc
View File

@ -7,6 +7,7 @@
(ns common-tests.logic.variant-properties-test
(:require
[app.common.files.changes-builder :as pcb]
[app.common.logic.shapes :as cls]
[app.common.logic.variant-properties :as clvp]
[app.common.test-helpers.components :as thc]
[app.common.test-helpers.compositions :as tho]
@ -607,7 +608,9 @@
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page))
(clvp/generate-make-shapes-no-variant [main01]))
;; main01 is still inside the variant container, as happens in
;; generate-relocate before moving it, so this state is not valid
(clvp/generate-make-shapes-no-variant [main01] :skip-validation? true))
file' (thf/apply-changes file changes :validate? false)
@ -675,6 +678,27 @@
(t/is (= "Frame1" (-> c03' :variant-properties first :value)))
(t/is (= "Frame1" (-> m03' :variant-name)))))
(t/deftest test-relocate-variant-outside-container
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
page (thf/current-page file)
main01 (ths/get-shape file :m01)
changes (cls/generate-relocate (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page)))
uuid/zero 0 #{(:id main01)})
file' (thf/apply-changes file changes)
comp01' (thc/get-component file' :c01)
main01' (ths/get-shape file' :m01)]
(t/is (= uuid/zero (:parent-id main01')))
(t/is (nil? (:variant-id comp01')))
(t/is (nil? (:variant-id main01')))))
;; This makes generate-make-shapes-variant call generate-add-new-property
;; several times before the new shape joins the variant
(t/deftest test-make-shapes-variant-convert-adding-properties
@ -958,3 +982,41 @@
(clvp/generate-add-new-property v-id :skip-validation? true))]
(t/is (nil? (validation-error file changes))))))
#?(:cljs
(t/deftest test-make-shapes-no-variant-validates-shape
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(ths/update-shape :m01 :component-file (uuid/next)))
page (thf/current-page file)
main01 (ths/get-shape file :m01)
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-no-variant [main01]))
error (validation-error file changes)]
(t/is (str/includes? (:hint error) "generate-make-shapes-no-variant"))
(t/is (contains? (error-codes error) :component-main-external)))))
#?(:cljs
(t/deftest test-relocate-variant-outside-container-validates-container
(let [file (-> (thf/sample-file :file1)
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
(ths/update-shape :m02 :variant-id (uuid/next)))
page (thf/current-page file)
main01 (ths/get-shape file :m01)
changes (cls/generate-relocate (-> (pcb/empty-changes nil)
(pcb/with-page-id (:id page))
(pcb/with-library-data (:data file))
(pcb/with-objects (:objects page)))
uuid/zero 0 #{(:id main01)})
error (validation-error file changes)]
(t/is (str/includes? (:hint error) "generate-relocate"))
(t/is (contains? (error-codes error) :main-instance-invalid-variant-id)))))