mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
🔧 Validate shapes after adding a variant property
generate-add-new-property now validates the main instances and the variant container. Add a :skip-validation? option to it. generate-make-shapes-variant calls it several times before the new shapes become variants. Validating the container at that point reported :main-instance-not-a-variant on valid data, so it now passes :skip-validation? and will validate once the shapes are variants. Add tests that corrupt a main instance or the container and expect a validation error. Add a test that moves into a variant a component whose name needs more properties, which runs that intermediate path. Register variants-test in the runner so that it also runs on JS. AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
4cb36153af
commit
c02b75a37e
@ -173,8 +173,11 @@
|
||||
The value may also be given, or else it will be an empty value, unless fill-values? is true, in
|
||||
which case the value will be set to a default value based on the property number.
|
||||
|
||||
The editing? flag, if set, will be added to the metadata of the properties, for later use."
|
||||
[changes variant-id & {:keys [fill-values? editing? property-name property-value]}]
|
||||
The editing? flag, if set, will be added to the metadata of the properties, for later use.
|
||||
|
||||
The skip-validation? flag, if set, avoids validating the shapes. Use it when the caller
|
||||
leaves the variant in an intermediate state and validates at the end."
|
||||
[changes variant-id & {:keys [fill-values? editing? property-name property-value skip-validation?]}]
|
||||
(let [data (pcb/get-library-data changes)
|
||||
objects (pcb/get-objects changes)
|
||||
related-components (cfv/find-variant-components data objects variant-id)
|
||||
@ -214,7 +217,15 @@
|
||||
(pcb/update-shapes [main-id] #(update % :variant-name update-name)))]))
|
||||
[1 changes]
|
||||
related-components)]
|
||||
changes))
|
||||
|
||||
(if (or skip-validation? (empty? related-components))
|
||||
changes
|
||||
(let [ids-to-validate (conj (mapv :main-instance-id related-components) variant-id)]
|
||||
(pcb/validate-shapes changes
|
||||
(pcb/get-page-id changes)
|
||||
ids-to-validate
|
||||
(str "generate-add-new-property: " variant-id
|
||||
" name: " property-name))))))
|
||||
|
||||
(defn- generate-make-shape-no-variant
|
||||
[changes shape]
|
||||
@ -302,7 +313,8 @@
|
||||
(- total-props num-base-props))
|
||||
|
||||
changes (nth
|
||||
(iterate #(generate-add-new-property % variant-id) changes)
|
||||
;; The shapes are not variants yet, so validate at the end
|
||||
(iterate #(generate-add-new-property % variant-id :skip-validation? true) changes)
|
||||
num-new-props)
|
||||
|
||||
changes (pcb/update-shapes changes (map :id shapes)
|
||||
|
||||
@ -675,6 +675,36 @@
|
||||
(t/is (= "Frame1" (-> c03' :variant-properties first :value)))
|
||||
(t/is (= "Frame1" (-> m03' :variant-name)))))
|
||||
|
||||
;; 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
|
||||
(let [file (-> (thf/sample-file :file1)
|
||||
(thv/add-variant :v01 :c01 :m01 :c02 :m02))
|
||||
|
||||
container (ths/get-shape file :v01)
|
||||
|
||||
file (tho/add-simple-component file :c03 :m03 :sh03
|
||||
:root-params {:parent-label :v01
|
||||
:name "A / B / C"})
|
||||
|
||||
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))
|
||||
|
||||
file' (thf/apply-changes file changes :validate? false)
|
||||
|
||||
c01' (thc/get-component file' :c01)
|
||||
c03' (thc/get-component file' :c03)]
|
||||
|
||||
(t/is (= 3 (count (:variant-properties c03'))))
|
||||
(t/is (= (mapv :name (:variant-properties c01'))
|
||||
(mapv :name (:variant-properties c03'))))))
|
||||
|
||||
;; =============================================================================
|
||||
;; validate-shapes failure tests (only run on CLJS, where validation runs)
|
||||
;; =============================================================================
|
||||
@ -873,3 +903,58 @@
|
||||
|
||||
(t/is (str/includes? (:hint error) "generate-reorder-variant-poperties"))
|
||||
(t/is (contains? (error-codes error) :invalid-variant-properties)))))
|
||||
|
||||
#?(:cljs
|
||||
(t/deftest test-add-new-property-validates-main
|
||||
(let [file (-> (thf/sample-file :file1)
|
||||
(thv/add-variant-two-properties :v01 :c01 :m01 :c02 :m02)
|
||||
(ths/update-shape :m01 :variant-id (uuid/next)))
|
||||
v-id (-> (ths/get-shape file :v01) :id)
|
||||
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-add-new-property v-id))
|
||||
|
||||
error (validation-error file changes)]
|
||||
|
||||
(t/is (str/includes? (:hint error) "generate-add-new-property"))
|
||||
(t/is (contains? (error-codes error) :variant-component-bad-id)))))
|
||||
|
||||
#?(:cljs
|
||||
(t/deftest test-add-new-property-validates-container
|
||||
(let [file (-> (thf/sample-file :file1)
|
||||
(thv/add-variant-two-properties :v01 :c01 :m01 :c02 :m02)
|
||||
(thc/update-component :c02 {:variant-properties [{:name "Other" :value "p1v2"}
|
||||
{:name "Property 2" :value "p2v2"}]}))
|
||||
v-id (-> (ths/get-shape file :v01) :id)
|
||||
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-add-new-property v-id))
|
||||
|
||||
error (validation-error file changes)]
|
||||
|
||||
(t/is (str/includes? (:hint error) "generate-add-new-property"))
|
||||
(t/is (contains? (error-codes error) :invalid-variant-properties)))))
|
||||
|
||||
#?(:cljs
|
||||
(t/deftest test-add-new-property-skip-validation
|
||||
(let [file (-> (thf/sample-file :file1)
|
||||
(thv/add-variant-two-properties :v01 :c01 :m01 :c02 :m02)
|
||||
(ths/update-shape :m01 :variant-id (uuid/next)))
|
||||
v-id (-> (ths/get-shape file :v01) :id)
|
||||
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-add-new-property v-id :skip-validation? true))]
|
||||
|
||||
(t/is (nil? (validation-error file changes))))))
|
||||
|
||||
@ -65,6 +65,7 @@
|
||||
[common-tests.logic.token-test]
|
||||
[common-tests.logic.variant-properties-test]
|
||||
[common-tests.logic.variants-switch-test]
|
||||
[common-tests.logic.variants-test]
|
||||
[common-tests.math-test]
|
||||
[common-tests.media-test]
|
||||
[common-tests.path-names-test]
|
||||
@ -149,6 +150,7 @@
|
||||
'common-tests.logic.token-test
|
||||
'common-tests.logic.variant-properties-test
|
||||
'common-tests.logic.variants-switch-test
|
||||
'common-tests.logic.variants-test
|
||||
'common-tests.math-test
|
||||
'common-tests.types.variant-test
|
||||
'common-tests.media-test
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user