mirror of
https://github.com/penpot/penpot.git
synced 2026-10-30 06:20:32 +00:00
🐛 Fix assertion when restoring a deleted component
The changes builder getters now assert that their context was given, but prepare-restore-component and generate-delete-shapes relied on them returning nil to fall back to other data. The workspace restores components with changes that carry no library data, so restoring a main component or a variant from one of its copies failed. Add has-library-data?, has-objects? and has-page? to the changes builder, and check them in those fallbacks before calling the getters. Add tests that restore a component, and a variant whose container was deleted, with changes that carry no library data, as the workspace does. AI-assisted-by: claude-opus-5-5
This commit is contained in:
parent
f2c026dd5d
commit
ae1b541208
@ -1264,6 +1264,21 @@
|
||||
(assert-page-id! changes)
|
||||
(::page-id (meta changes)))
|
||||
|
||||
;; The getters above assert that their context has been given. Use these
|
||||
;; predicates when a caller has a fallback for a missing context.
|
||||
|
||||
(defn has-library-data?
|
||||
[changes]
|
||||
(contains? (meta changes) ::library-data))
|
||||
|
||||
(defn has-objects?
|
||||
[changes]
|
||||
(contains? (meta changes) ::file-data))
|
||||
|
||||
(defn has-page?
|
||||
[changes]
|
||||
(contains? (meta changes) ::page))
|
||||
|
||||
(defn set-text-content
|
||||
[changes id content prev-content]
|
||||
(assert-page-id! changes)
|
||||
|
||||
@ -442,9 +442,13 @@
|
||||
(prepare-restore-component changes library-data component-id page nil nil nil nil)))
|
||||
|
||||
([changes library-data component-id page position old-id parent-id frame-id]
|
||||
(let [library-data (or (pcb/get-library-data changes) library-data)
|
||||
(let [library-data (or (when (pcb/has-library-data? changes)
|
||||
(pcb/get-library-data changes))
|
||||
library-data)
|
||||
component (ctkl/get-deleted-component library-data component-id)
|
||||
objects (or (pcb/get-objects changes) (:objects page))
|
||||
objects (or (when (pcb/has-objects? changes)
|
||||
(pcb/get-objects changes))
|
||||
(:objects page))
|
||||
parent (get objects parent-id)
|
||||
main-inst (get-in component [:objects (:main-instance-id component)])
|
||||
inside-component? (some? (ctn/get-instance-root (:objects page) parent))
|
||||
|
||||
@ -267,7 +267,8 @@
|
||||
(let [objects (pcb/get-objects changes)
|
||||
data (pcb/get-library-data changes)
|
||||
page-id (pcb/get-page-id changes)
|
||||
page (or (pcb/get-page changes)
|
||||
page (or (when (pcb/has-page? changes)
|
||||
(pcb/get-page changes))
|
||||
(ctpl/get-page data page-id))
|
||||
ids (cfh/clean-loops objects ids)
|
||||
|
||||
|
||||
@ -17,6 +17,7 @@
|
||||
[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]
|
||||
[app.common.types.component :as ctk]
|
||||
[app.common.types.components-list :as ctkl]
|
||||
[app.common.types.file :as ctf]
|
||||
@ -664,3 +665,70 @@
|
||||
(t/is (some? copy1-root'))
|
||||
(t/is (not (ctk/instance-head? copy1-root')))
|
||||
(t/is (not (ctk/in-component-copy? copy1-root')))))
|
||||
|
||||
(defn- delete-shapes
|
||||
[file labels]
|
||||
(let [page (thf/current-page file)
|
||||
[_ changes]
|
||||
(cls/generate-delete-shapes (pcb/empty-changes)
|
||||
file
|
||||
page
|
||||
(:objects page)
|
||||
(into #{} (map thi/id) labels)
|
||||
{:components-v2 true})]
|
||||
(thf/apply-changes file changes)))
|
||||
|
||||
(defn- restore-changes
|
||||
"Build the changes of restoring a deleted component as the workspace does,
|
||||
without any library data or objects in the changes builder."
|
||||
[file component-label]
|
||||
(let [page (thf/current-page file)]
|
||||
(cll/generate-restore-component (pcb/empty-changes)
|
||||
(:data file)
|
||||
(thi/id component-label)
|
||||
(:id file)
|
||||
page
|
||||
(:objects page))))
|
||||
|
||||
(t/deftest test-restore-component-without-library-data
|
||||
(let [;; ==== Setup
|
||||
file (-> (thf/sample-file :file1)
|
||||
(tho/add-simple-component-with-copy :component1
|
||||
:main1-root
|
||||
:main1-child
|
||||
:copy1-root)
|
||||
(delete-shapes [:main1-root]))
|
||||
|
||||
;; ==== Action
|
||||
changes (restore-changes file :component1)
|
||||
file' (thf/apply-changes file changes)
|
||||
|
||||
;; ==== Get
|
||||
component1' (thc/get-component file' :component1)
|
||||
main1-root' (ths/get-shape file' :main1-root)]
|
||||
|
||||
;; ==== Check
|
||||
(t/is (some? component1'))
|
||||
(t/is (some? main1-root'))))
|
||||
|
||||
(t/deftest test-restore-variant-after-deleting-its-container
|
||||
(let [;; ==== Setup
|
||||
file (-> (thf/sample-file :file1)
|
||||
(thv/add-variant :v01 :c01 :m01 :c02 :m02)
|
||||
(delete-shapes [:v01]))
|
||||
|
||||
;; ==== Action
|
||||
changes (restore-changes file :c01)
|
||||
file' (thf/apply-changes file changes)
|
||||
|
||||
;; ==== Get
|
||||
c01' (thc/get-component file' :c01)
|
||||
m01' (ths/get-shape file' :m01)]
|
||||
|
||||
;; ==== Check
|
||||
;; The container no longer exists, so the main is restored as a
|
||||
;; component that is not a variant
|
||||
(t/is (some? c01'))
|
||||
(t/is (some? m01'))
|
||||
(t/is (nil? (:variant-id c01')))
|
||||
(t/is (nil? (:variant-id m01')))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user