🐛 Fix detaching a nested copy inside a main component (#7304)

* 🐛 Fix detaching a nested copy inside a main component

* 💄 Rename functions for more semantic precission

common/src/app/common/files/repair.cljc

@@ -320,6 +320,31 @@
             (pcb/with-file-data file-data)
             (pcb/update-shapes shape-ids detach-shape))))))
 
+(defmethod repair-error :ref-shape-is-not-head
+  [_ {:keys [shape page-id] :as error} file-data _]
+  (let [repair-shape
+        (fn [shape]
+          ; Convert shape in a normal copy, removing nested copy status
+          (log/debug :hint "  -> unhead shape")
+          (ctk/unhead-shape shape))]
+
+    (log/dbg :hint "repairing shape :shape-ref-is-not-head" :id (:id shape) :name (:name shape) :page-id page-id)
+    (-> (pcb/empty-changes nil page-id)
+        (pcb/with-file-data file-data)
+        (pcb/update-shapes [(:id shape)] repair-shape))))
+
+(defmethod repair-error :ref-shape-is-head
+  [_ {:keys [shape page-id args] :as error} file-data _]
+  (let [repair-shape
+        (fn [shape]
+          ; Convert shape in a nested head, adding component info
+          (log/debug :hint "  -> reroot shape")
+          (ctk/rehead-shape shape (:component-file args) (:component-id args)))]
+
+    (log/dbg :hint "repairing shape :shape-ref-is-head" :id (:id shape) :name (:name shape) :page-id page-id)
+    (-> (pcb/empty-changes nil page-id)
+        (pcb/with-file-data file-data)
+        (pcb/update-shapes [(:id shape)] repair-shape))))
 
 (defmethod repair-error :shape-ref-cycle
   [_ {:keys [shape args] :as error} file-data _]

common/src/app/common/files/validate.cljc

@@ -47,6 +47,8 @@
     :should-be-component-root
     :should-not-be-component-root
     :ref-shape-not-found
+    :ref-shape-is-head
+    :ref-shape-is-not-head
     :shape-ref-in-main
     :root-main-not-allowed
     :nested-main-not-allowed
@@ -305,6 +307,28 @@
                   "Shape inside main instance should not have shape-ref"
                   shape file page)))
 
+(defn- check-ref-is-not-head
+  "Validate that the referenced shape is not a nested copy root."
+  [shape file page libraries]
+  (let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)]
+    (when (and (some? ref-shape)
+               (ctk/instance-head? ref-shape))
+      (report-error :ref-shape-is-head
+                    (str/ffmt "Referenced shape % is a component, so the copy must also be" (:shape-ref shape))
+                    shape file page))))
+
+(defn- check-ref-is-head
+  "Validate that the referenced shape is a nested copy root."
+  [shape file page libraries]
+  (let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)]
+    (when (and (some? ref-shape)
+               (not (ctk/instance-head? ref-shape)))
+      (report-error :ref-shape-is-not-head
+                    (str/ffmt "Referenced shape % of a head copy must also be a head" (:shape-ref shape))
+                    shape file page
+                    :component-file (:component-file ref-shape)
+                    :component-id (:component-id ref-shape)))))
+
 (defn- check-empty-swap-slot
   "Validate that this shape does not have any swap slot."
   [shape file page]
@@ -382,6 +406,7 @@
     (check-component-not-main-head shape file page libraries)
     (check-component-root shape file page)
     (check-component-ref shape file page libraries)
+    (check-ref-is-head shape file page libraries)
     (check-empty-swap-slot shape file page)
     (check-duplicate-swap-slot shape file page)
     (check-valid-touched shape file page)
@@ -399,7 +424,8 @@
   ;; We can have situations where the nested copy and the ancestor copy come from different libraries and some of them have been dettached
   ;; so we only validate the shape-ref if the ancestor is from a valid library
   (when library-exists
-    (check-component-ref shape file page libraries))
+    (check-component-ref shape file page libraries)
+    (check-ref-is-head shape file page libraries))
   (run! #(check-shape % file page libraries :context :copy-nested) (:shapes shape)))
 
 (defn- check-shape-main-not-root
@@ -417,6 +443,7 @@
   (check-component-not-main-not-head shape file page)
   (check-component-not-root shape file page)
   (check-component-ref shape file page libraries)
+  (check-ref-is-not-head shape file page libraries)
   (check-empty-swap-slot shape file page)
   (check-valid-touched shape file page)
   (run! #(check-shape % file page libraries :context :copy-any) (:shapes shape)))

common/src/app/common/logic/libraries.cljc

@@ -1734,6 +1734,17 @@
     [(conj roperations roperation)
      (conj uoperations uoperation)]))
 
+(defn- check-detached-main
+  [changes dest-shape origin-shape]
+  ;; Only for direct updates (from main to copy). Check if the main shape
+  ;; has been detached. If so, the copy shape must be unheaded (i.e. converted
+  ;; into a normal copy and not a nested instance).
+  (if (and (= (:shape-ref dest-shape) (:id origin-shape))
+           (ctk/subcopy-head? dest-shape)
+           (not (ctk/instance-head? origin-shape)))
+    (pcb/update-shapes changes [(:id dest-shape)] ctk/unhead-shape {:ignore-touched true})
+    changes))
+
 (defn- update-attrs
   "The main function that implements the attribute sync algorithm. Copy
   attributes that have changed in the origin shape to the dest shape.
@@ -1774,6 +1785,8 @@
             (seq roperations)
             (add-update-attr-changes dest-shape container roperations uoperations)
             :always
+            (check-detached-main dest-shape origin-shape)
+            :always
             (generate-update-tokens container dest-shape origin-shape touched omit-touched?))
 
           (let [attr-group        (get ctk/sync-attrs attr)
@@ -1797,7 +1810,6 @@
                  (= :content attr)
                  (touched attr-group))
 
-
                 skip-operations?
                 (or (= (get origin-shape attr) (get dest-shape attr))
                     (and (touched attr-group)

common/src/app/common/test_helpers/files.cljc

@@ -108,7 +108,8 @@
         page      (if (some? page-label)
                     (:id (get-page file page-label))
                     (current-page-id file))
-        libraries (or libraries {})]
+        libraries (or libraries
+                      {(:id file) file})]
 
     (ctf/dump-tree file page libraries params)))
 

common/src/app/common/types/component.cljc

@@ -145,9 +145,12 @@
 (defn component-attr?
   "Check if some attribute is one that is involved in component syncrhonization.
    Note that design tokens also are involved, although they go by an alternate
-   route and thus they are not part of :sync-attrs."
+   route and thus they are not part of :sync-attrs.
+   Also when detaching a nested copy it also needs to trigger a synchronization,
+   even though :shape-ref is not a synced attribute per se"
   [attr]
   (or (get sync-attrs attr)
+      (= :shape-ref attr)
       (= :applied-tokens attr)))
 
 (defn instance-root?
@@ -217,19 +220,16 @@
   (and (= shape-id (:main-instance-id component))
        (= page-id (:main-instance-page component))))
 
-
 (defn is-variant?
   "Check if this shape or component is a variant component"
   [item]
   (some? (:variant-id item)))
 
-
 (defn is-variant-container?
   "Check if this shape is a variant container"
   [shape]
   (:is-variant-container shape))
 
-
 (defn set-touched-group
   [touched group]
   (when group
@@ -310,6 +310,22 @@
           :shape-ref
           :touched))
 
+(defn unhead-shape
+  "Make the shape not be a component head, but keep its :shape-ref and :touched if it was a nested copy"
+  [shape]
+  (dissoc shape
+          :component-root
+          :component-file
+          :component-id
+          :main-instance))
+
+(defn rehead-shape
+  "Make the shape a component head, by adding component info"
+  [shape component-file component-id]
+  (assoc shape
+         :component-file component-file
+         :component-id component-id))
+
 (defn- extract-ids [shape]
   (if (map? shape)
     (let [current-id (:id shape)

common/test/common_tests/logic/comp_detach_with_nested_test.cljc

@@ -446,3 +446,35 @@
     (t/is (= (count fills') 1))
     (t/is (= (:fill-color fill') "#fabada"))
     (t/is (= (:fill-opacity fill') 1))))
+
+(t/deftest test-detach-copy-in-main
+  (let [;; ==== Setup
+        file (-> (setup-file)
+                 (thc/instantiate-component :c-big-board
+                                            :copy-big-board
+                                            :children-labels [:copy-h-board-with-ellipse
+                                                              :copy-nested-h-ellipse
+                                                              :copy-nested-ellipse]))
+
+        page (thf/current-page file)
+
+        ;; ==== Action
+        changes (cll/generate-detach-instance (-> (pcb/empty-changes nil)
+                                                  (pcb/with-page page)
+                                                  (pcb/with-objects (:objects page)))
+                                              page
+                                              {(:id file) file}
+                                              (thi/id :nested-h-ellipse))
+        file'   (-> (thf/apply-changes file changes)
+                    (tho/propagate-component-changes :c-board-with-ellipse)
+                    (tho/propagate-component-changes :c-big-board))
+
+        ;; ==== Get
+        nested2-h-ellipse (ths/get-shape file' :nested-h-ellipse)
+        copy-nested2-h-ellipse (ths/get-shape file' :copy-nested-h-ellipse)]
+
+    ;; ==== Check
+
+    ;; When the nested copy inside the main is detached, their copies are unheaded.
+    (t/is (not (ctk/subcopy-head? nested2-h-ellipse)))
+    (t/is (not (ctk/subcopy-head? copy-nested2-h-ellipse)))))