🐛 Fix renaming a variant set saves half-finished steps

common/src/app/common/logic/variants.cljc

@@ -9,12 +9,35 @@
    [app.common.logic.libraries :as cll]
    [app.common.logic.shapes :as cls]
    [app.common.logic.variant-properties :as clvp]
+   [app.common.path-names :as cpn]
    [app.common.types.component :as ctk]
    [app.common.types.container :as ctn]
    [app.common.types.file :as ctf]
    [app.common.types.variant :as ctv]
    [app.common.uuid :as uuid]))
 
+(defn generate-rename-variant
+  "Rename a variant set in one change vector.
+
+   The builder already has the mains' page objects and library data.
+   The stored name is `clean-path` of `name`, or \"(empty)\" when that
+   path is blank. The board and every direct child take that name.
+   Each component's path and name come from `split-group-name`."
+  [changes variant-id name]
+  (let [objects     (pcb/get-objects changes)
+        data        (pcb/get-library-data changes)
+        clean-name  (cpn/clean-path name)
+        stored-name (if (empty? clean-name) "(empty)" clean-name)
+        shape-ids   (cons variant-id (:shapes (get objects variant-id)))
+        components  (cfv/find-variant-components data objects variant-id)]
+    (reduce (fn [changes component]
+              (let [[path item-name] (cpn/split-group-name stored-name)]
+                (pcb/update-component changes
+                                      (:id component)
+                                      #(assoc % :path path :name item-name))))
+            (pcb/update-shapes changes shape-ids #(assoc % :name stored-name))
+            components)))
+
 (defn generate-add-new-variant
   [changes shape variant-id new-component-id new-shape-id prop-num]
   (let [data                (pcb/get-library-data changes)

common/test/common_tests/logic/variants_test.cljc

@@ -11,6 +11,8 @@
    [app.common.logic.libraries :as cll]
    [app.common.logic.shapes :as cls]
    [app.common.logic.variant-properties :as clvp]
+   [app.common.logic.variants :as clv]
+   [app.common.path-names :as cpn]
    [app.common.test-helpers.components :as thc]
    [app.common.test-helpers.files :as thf]
    [app.common.test-helpers.ids-map :as thi]
@@ -563,3 +565,80 @@
     (t/is (= "crítica" (-> props' second :value)))
     (t/is (= (ctv/properties-to-name props') (:variant-name main')))
     (t/is (= "active, crítica" (:variant-name main')))))
+
+(defn- rename-variant-changes
+  [file name]
+  (let [page (thf/current-page file)]
+    (-> (pcb/empty-changes nil)
+        (pcb/with-page-id (:id page))
+        (pcb/with-library-data (:data file))
+        (pcb/with-objects (:objects page))
+        (clv/generate-rename-variant (thi/id :v01) name))))
+
+(defn- redo-ids
+  [changes change-type]
+  (into #{}
+        (comp (filter #(= change-type (:type %)))
+              (map :id))
+        (:redo-changes changes)))
+
+(t/deftest test-rename-variant-one-vector
+  (let [file     (-> (thf/sample-file :file1)
+                     (thv/add-variant :v01 :c01 :m01 :c02 :m02))
+        board    (ths/get-shape file :v01)
+        main1    (ths/get-shape file :m01)
+        main2    (ths/get-shape file :m02)
+        comp1    (thc/get-component file :c01)
+        comp2    (thc/get-component file :c02)
+        changes  (rename-variant-changes file "Group / Button")
+        file'    (thf/apply-changes file changes)
+        board'   (ths/get-shape file' :v01)
+        main1'   (ths/get-shape file' :m01)
+        main2'   (ths/get-shape file' :m02)
+        comp1'   (thc/get-component file' :c01)
+        comp2'   (thc/get-component file' :c02)
+        restored (thf/apply-undo-changes file' changes)]
+
+    (t/is (= #{(thi/id :v01) (thi/id :m01) (thi/id :m02)}
+             (redo-ids changes :mod-obj)))
+    (t/is (= #{(thi/id :c01) (thi/id :c02)}
+             (redo-ids changes :mod-component)))
+    (t/is (= "Group / Button" (:name board') (:name main1') (:name main2')))
+    (t/is (= "Group" (:path comp1') (:path comp2')))
+    (t/is (= "Button" (:name comp1') (:name comp2')))
+    (t/is (= "Group / Button"
+             (cpn/merge-path-item (:path comp1') (:name comp1'))
+             (cpn/merge-path-item (:path comp2') (:name comp2'))))
+    (t/is (= (:id board) (:variant-id comp1') (:variant-id comp2')))
+    (t/is (= (:variant-name main1) (:variant-name main1')))
+    (t/is (= (:variant-name main2) (:variant-name main2')))
+    (t/is (= (:variant-properties comp1) (:variant-properties comp1')))
+    (t/is (= (:variant-properties comp2) (:variant-properties comp2')))
+    (t/is (= (:name board) (:name (ths/get-shape restored :v01))))
+    (t/is (= (:name main1) (:name (ths/get-shape restored :m01))))
+    (t/is (= (:name main2) (:name (ths/get-shape restored :m02))))
+    (t/is (= (:path comp1) (:path (thc/get-component restored :c01))))
+    (t/is (= (:name comp1) (:name (thc/get-component restored :c01))))
+    (t/is (= (:path comp2) (:path (thc/get-component restored :c02))))
+    (t/is (= (:name comp2) (:name (thc/get-component restored :c02))))))
+
+(t/deftest test-rename-variant-slash-only-name
+  (let [file    (-> (thf/sample-file :file1)
+                    (thv/add-variant :v01 :c01 :m01 :c02 :m02))
+        changes (rename-variant-changes file "///")
+        file'   (thf/apply-changes file changes)
+        comp1'  (thc/get-component file' :c01)
+        comp2'  (thc/get-component file' :c02)]
+
+    (t/is (= "(empty)"
+             (:name (ths/get-shape file' :v01))
+             (:name (ths/get-shape file' :m01))
+             (:name (ths/get-shape file' :m02))
+             (:name comp1')
+             (:name comp2')))
+    (t/is (= "" (:path comp1') (:path comp2')))
+    (t/is (= "(empty)"
+             (cpn/merge-path-item (:path comp1') (:name comp1'))
+             (cpn/merge-path-item (:path comp2') (:name comp2'))))
+    (t/is (some? (:variant-id comp1')))
+    (t/is (some? (:variant-id comp2')))))

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

@@ -579,20 +579,19 @@
   (ptk/reify ::rename-variant
 
     ptk/WatchEvent
-    (watch [_ state _]
+    (watch [it state _]
       (let [data (dsh/lookup-file-data state)]
         (with-main-page data {:variant-id variant-id}
           (fn [page-id objects]
-            (let [variant-components (cfv/find-variant-components data objects variant-id)
-                  clean-name         (cpn/clean-path name)
-                  undo-id            (js/Symbol)]
-              (rx/concat
-               (rx/of (dwu/start-undo-transaction undo-id)
-                      (dwsh/update-shapes [variant-id] #(assoc % :name clean-name) {:page-id page-id}))
-               (rx/from (map
-                         #(dwl/rename-component-and-main-instance (:id %) clean-name)
-                         variant-components))
-               (rx/of (dwu/commit-undo-transaction undo-id))))))))))
+            (let [changes (-> (pcb/empty-changes it page-id)
+                              (pcb/with-library-data data)
+                              (pcb/with-objects objects)
+                              (clv/generate-rename-variant variant-id name))
+                  undo-id (js/Symbol)]
+              (rx/of
+               (dwu/start-undo-transaction undo-id)
+               (dch/commit-changes changes)
+               (dwu/commit-undo-transaction undo-id)))))))))
 
 (defn rename-comp-or-variant-and-main
   "If the component is in a variant, rename the variant.

frontend/test/frontend_tests/data/workspace_variant_rename_test.cljs

@@ -6,6 +6,7 @@
 
 (ns frontend-tests.data.workspace-variant-rename-test
   (:require
+   [app.common.path-names :as cpn]
    [app.common.test-helpers.components :as cthc]
    [app.common.test-helpers.files :as cthf]
    [app.common.test-helpers.ids-map :as cthi]
@@ -13,9 +14,13 @@
    [app.common.test-helpers.variants :as thv]
    [app.common.types.file :as ctf]
    [app.common.types.pages-list :as ctpl]
+   [app.main.data.changes :as dch]
    [app.main.data.workspace :as dw]
+   [app.main.data.workspace.variants :as dwv]
+   [beicon.v2.core :as rx]
    [cljs.test :as t :include-macros true]
-   [frontend-tests.helpers.state :as ths]))
+   [frontend-tests.helpers.state :as ths]
+   [potok.v2.core :as ptk]))
 
 (t/use-fixtures :each
   {:before cthi/reset-idmap!})
@@ -28,6 +33,15 @@
                        :variant2-params {:name "Button"})
       (cths/update-shape :variant :name "Button")))
 
+(defn- variant-file-on-other-page
+  []
+  (-> (cthf/sample-file :file1 :page-label :page1)
+      (thv/add-variant :variant :comp1 :root1 :comp2 :root2
+                       :variant1-params {:name "Button"}
+                       :variant2-params {:name "Button"})
+      (cths/update-shape :variant :name "Button")
+      (cthf/add-sample-page :page2)))
+
 (defn- without-variant-name
   [file label]
   (let [page-id  (cthf/current-page-id file)
@@ -47,16 +61,22 @@
   "Run workspace events, then validate the file before assertions.
 
    Pass `:validate? false` only when the fixture is intentionally incomplete
-   (e.g. a variant main without `:variant-name`)."
-  [file events done assert-fn & {:keys [validate?] :or {validate? true}}]
-  (ths/run-store
-   (ths/setup-store file)
-   done
-   events
-   (fn [state]
-     (when validate?
-       (cthf/validate-file! (ths/get-file-from-state state)))
-     (assert-fn state))))
+   (e.g. a variant main without `:variant-name`). Pass `:commits` as an atom
+   to collect each `dch/commit?` event from the store input."
+  [file events done assert-fn & {:keys [validate? commits] :or {validate? true}}]
+  (let [store (ths/setup-store file)]
+    (when commits
+      (->> (ptk/input-stream store)
+           (rx/filter dch/commit?)
+           (rx/subs! #(swap! commits conj %))))
+    (ths/run-store
+     store
+     done
+     events
+     (fn [state]
+       (when validate?
+         (cthf/validate-file! (ths/get-file-from-state state)))
+       (assert-fn state)))))
 
 (t/deftest end-rename-shape-does-not-apply-variant-formula
   (t/async
@@ -230,3 +250,61 @@
                (t/is (= "Renamed" (:name (cthc/get-component file :comp))))
                (t/is (= "Box" (:name (cths/get-shape file :rect))))
                (t/is (nil? (:shape-for-rename (:workspace-local state))))))))))
+
+(t/deftest end-rename-shape-renames-variant-container-in-one-commit
+  (t/async
+    done
+    (let [commits (atom [])]
+      (run (variant-file)
+           [(dw/end-rename-shape (cthi/id :variant) "Renamed set")]
+           done
+           (fn [state]
+             (let [file  (ths/get-file-from-state state)
+                   comp1 (cthc/get-component file :comp1)
+                   comp2 (cthc/get-component file :comp2)]
+               (t/is (= 1 (count @commits)))
+               (cthf/validate-file! file)
+               (t/is (= "Renamed set" (:name (cths/get-shape file :variant))))
+               (t/is (= "Renamed set" (:name (cths/get-shape file :root1))))
+               (t/is (= "Renamed set" (:name (cths/get-shape file :root2))))
+               (t/is (= "Renamed set"
+                        (cpn/merge-path-item (:path comp1) (:name comp1))
+                        (cpn/merge-path-item (:path comp2) (:name comp2))))))
+           :commits commits))))
+
+(t/deftest rename-variant-from-another-page-keeps-the-open-page
+  (t/async
+    done
+    (let [file         (variant-file-on-other-page)
+          open-page-id (cthf/current-page-id file)]
+      (run file
+           [(dwv/rename-variant (cthi/id :variant) "Renamed")]
+           done
+           (fn [state]
+             (let [file  (ths/get-file-from-state state)
+                   comp1 (cthc/get-component file :comp1)
+                   comp2 (cthc/get-component file :comp2)]
+               (t/is (= open-page-id (:current-page-id state)))
+               (t/is (= "Renamed" (:name (cths/get-shape file :variant :page-label :page1))))
+               (t/is (= "Renamed" (:name (cths/get-shape file :root1 :page-label :page1))))
+               (t/is (= "Renamed" (:name (cths/get-shape file :root2 :page-label :page1))))
+               (t/is (= "Renamed"
+                        (cpn/merge-path-item (:path comp1) (:name comp1))
+                        (cpn/merge-path-item (:path comp2) (:name comp2))))))))))
+
+(t/deftest rename-variant-stores-empty-for-a-slash-only-name
+  (t/async
+    done
+    (run (variant-file)
+         [(dwv/rename-variant (cthi/id :variant) "///")]
+         done
+         (fn [state]
+           (let [file  (ths/get-file-from-state state)
+                 comp1 (cthc/get-component file :comp1)
+                 comp2 (cthc/get-component file :comp2)]
+             (t/is (= "(empty)" (:name (cths/get-shape file :variant))))
+             (t/is (= "(empty)" (:name (cths/get-shape file :root1))))
+             (t/is (= "(empty)" (:name (cths/get-shape file :root2))))
+             (t/is (= "" (:path comp1) (:path comp2)))
+             (t/is (= "(empty)" (:name comp1) (:name comp2)))
+             (t/is (= (cthi/id :variant) (:variant-id comp1) (:variant-id comp2))))))))