mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
🔧 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 <noreply@anthropic.com>
This commit is contained in:
parent
8e306b8e24
commit
08f9fca672
@ -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
|
||||
|
||||
@ -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)))))))
|
||||
|
||||
@ -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))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user