🐛 Fix error on validating file referential integrity when duplicating a page

CHANGES.md

@@ -32,10 +32,11 @@
 - Fix toggle focus mode did not restore viewport and selection upon exit [GitHub #6280](https://github.com/penpot/penpot/issues/6820)
 - Fix problem when creating a layout from an existing layout [Taiga #11554](https://tree.taiga.io/project/penpot/issue/11554)
 - Fix title button from Title Case to Capitalize [Taiga #11476](https://tree.taiga.io/project/penpot/issue/11476)
-- Fix Main component receives focus and is selected when using 'Show Main Component' [Taiga #11402](https://tree.taiga.io/project/penpot/issue/11402)
 - Fix touchpad swipe leading to navigating back/forth [GitHub #4246](https://github.com/penpot/penpot/issues/4246)
 - Keep color data when copying from info tab into CSS [Taiga #11144](https://tree.taiga.io/project/penpot/issue/11144)
 - Update HSL values to modern syntax as defined in W3C CSS Color Module Level 4 [Taiga #11144](https://tree.taiga.io/project/penpot/issue/11144)
+- Fix main component receives focus and is selected when using 'Show Main Component' [Taiga #11402](https://tree.taiga.io/project/penpot/issue/11402)
+- Fix duplicating pages with mainInstance shapes nested inside groups [Taiga #10774](https://tree.taiga.io/project/penpot/issue/10774)
 
 ## 2.8.0
 

common/src/app/common/types/container.cljc

@@ -294,8 +294,8 @@
   ([page component library-data position]
    (make-component-instance page component library-data position {}))
   ([page component library-data position
-    {:keys [main-instance? force-id force-frame-id keep-ids?]
-     :or {main-instance? false force-id nil force-frame-id nil keep-ids? false}}]
+    {:keys [main-instance? force-id force-frame-id keep-ids? force-parent-id]
+     :or {main-instance? false force-id nil force-frame-id nil keep-ids? false force-parent-id nil}}]
    (let [component-page  (ctpl/get-page library-data (:main-instance-page component))
 
          component-shape (-> (get-shape component-page (:main-instance-id component))
@@ -303,7 +303,6 @@
                              (assoc :frame-id uuid/zero)
                              (remove-swap-keep-attrs))
 
-
          orig-pos        (gpt/point (:x component-shape) (:y component-shape))
          delta           (gpt/subtract position orig-pos)
 
@@ -368,7 +367,7 @@
 
          [new-shape new-shapes _]
          (ctst/clone-shape component-shape
-                           frame-id
+                           (or force-parent-id frame-id)
                            (:objects component-page)
                            :update-new-shape update-new-shape
                            :force-id force-id

frontend/src/app/main/data/workspace/pages.cljs

@@ -174,12 +174,15 @@
             add-component-copy
             (fn [objs id shape]
               (let [component (ctkl/get-component fdata (:component-id shape))
+                    parent-id (when (not= (:parent-id shape) uuid/zero) (:parent-id shape))
                     [new-shape new-shapes]
                     (ctn/make-component-instance page
                                                  component
                                                  fdata
                                                  (gpt/point (:x shape) (:y shape))
-                                                 {:keep-ids? true :force-frame-id (:frame-id shape)})
+                                                 {:keep-ids? true
+                                                  :force-frame-id (:frame-id shape)
+                                                  :force-parent-id parent-id})
                     children (into {} (map (fn [shape] [(:id shape) shape]) new-shapes))
                     objs (assoc objs id new-shape)]
                 (merge objs children)))
@@ -201,10 +204,8 @@
                         (assoc :id id)
                         (assoc :objects
                                objects))
-
             changes (-> (pcb/empty-changes it)
                         (pcb/add-page id page))]
-
         (rx/of (dch/commit-changes changes))))))
 
 (s/def ::rename-page

frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs

@@ -11,6 +11,7 @@
    [app.common.test-helpers.files :as cthf]
    [app.common.test-helpers.ids-map :as cthi]
    [app.common.test-helpers.shapes :as cths]
+   [app.common.types.component :as ctk]
    [app.common.uuid :as uuid]
    [app.main.data.workspace :as dw]
    [app.main.data.workspace.colors :as dc]
@@ -394,3 +395,60 @@
              (t/is (= (count-shapes file' "rect-simple-1" "#111111") 10))
              (t/is (= (count-shapes file' "rect-simple-1" "#222222") 4))
              (t/is (= (count-shapes file' "rect-simple-1" "#333333") 0)))))))))
+
+(t/deftest duplicate-page-integrity-frame-group-component
+  ;; This test covers the bug fixed in 2.9.0: duplicating a page with a mainInstance inside a group
+  ;; must preserve parent/child referential integrity (parent-id and :shapes).
+  ;;
+  ;; Structure created:
+  ;;   Page
+  ;;     └─ Group ("group-1")
+  ;;         └─ Main component ("frame-1")
+  ;;             └─ Shape ("shape-1")
+  ;;   The frame is also promoted to a component (main).
+  ;;
+  ;; The test checks:
+  ;;   - The group, frame, and shape exist in the duplicated page.
+  ;;   - The parent/child relationships are correct (group:shapes contains frame, frame:shapes contains shape, etc).
+  ;;   - The duplicated page contains an instance of the component whose main is in the original page.
+  (t/async done
+    (with-redefs [uuid/next cthi/next-uuid]
+      (let [file (-> (cthf/sample-file :file1 :page-label :page-1)
+                     (ctho/add-group :group-1 {:name "group-1"})
+                     (ctho/add-frame :frame-1 :parent-label :group-1 {:name "frame-1"})
+                     (cths/add-sample-shape :shape-1 :parent-label :frame-1 {:name "shape-1"})
+                     (cthc/make-component :component-1 :frame-1))
+            page-id (cthf/current-page-id file)
+            store (ths/setup-store file)
+            events [(app.main.data.workspace.pages/duplicate-page page-id)]]
+        (ths/run-store
+         store done events
+         (fn [new-state]
+           (let [file' (ths/get-file-from-state new-state)
+                 pages-vec (get-in file' [:data :pages])
+                 pages-index (get-in file' [:data :pages-index])
+                 new-page-id (first (remove #(= page-id %) pages-vec))
+                 new-page (get pages-index new-page-id)
+                 new-objects (:objects new-page)
+                 group (some #(when (= (:name %) "group-1") %) (vals new-objects))
+                 frame (some #(when (= (:name %) "frame-1") %) (vals new-objects))
+                 shape (some #(when (= (:name %) "shape-1") %) (vals new-objects))
+                 component-ids (map :component-id (filter :component-root (vals new-objects)))]
+
+             (t/is group "Group exists in duplicated page")
+             (t/is frame "Frame exists in duplicated page")
+             (t/is shape "Shape exists in duplicated page")
+             (t/is (some #(= (:id frame) %) (:shapes group)) "Group's :shapes contains frame's id")
+             (t/is (some #(= (:id shape) %) (:shapes frame)) "Frame's :shapes contains shape's id")
+             (t/is (= (:parent-id frame) (:id group)) "Frame's parent is group")
+             (t/is (= (:parent-id shape) (:id frame)) "Shape's parent is frame")
+
+             ;; Check the duplicated page must contain an instance of the component whose main is in the original page
+             (let [original-page (get pages-index page-id)
+                   original-main (some #(when (:component-root %) %) (vals (:objects original-page)))
+                   instance (some #(when (:component-root %) %) (vals (:objects new-page)))
+                   component-id (:component-id original-main)]
+               (t/is (ctk/instance-of? instance (:id file) component-id)
+                     (str "Duplicated page contains an instance of the original main component (component-id: " component-id ")")))
+
+             (done))))))))