✨ Avoid duplicated property names adding a number (#6681)

* ✨ Avoid repeated property names appending a number

* 📎 PR changes

* 🐛 Adjust rules for incrementing numbers in prop names

common/src/app/common/logic/variant_properties.cljc

@@ -18,7 +18,12 @@
   [changes variant-id pos new-name]
   (let [data               (pcb/get-library-data changes)
         objects            (pcb/get-objects changes)
-        related-components (cfv/find-variant-components data objects variant-id)]
+        related-components (cfv/find-variant-components data objects variant-id)
+
+        props              (-> related-components last :variant-properties)
+        prop-names         (mapv :name props)
+        prop-names         (concat (subvec prop-names 0 pos) (subvec prop-names (inc pos)))
+        new-name           (ctv/update-number-in-repeated-item prop-names new-name)]
     (reduce (fn [changes component]
               (pcb/update-component
                changes (:id component)
@@ -81,6 +86,9 @@
         next-prop-num      (ctv/next-property-number props)
         property-name      (or property-name (str ctv/property-prefix next-prop-num))
 
+        prop-names         (mapv :name props)
+        property-name      (ctv/update-number-in-repeated-item prop-names property-name)
+
         [_ changes]
         (reduce (fn [[num changes] component]
                   (let [main-id      (:main-instance-id component)

common/src/app/common/types/variant.cljc

@@ -139,7 +139,6 @@
                      (< (count (first %)) property-max-length)
                      (< (count (second %)) property-max-length)))))
 
-
 (defn find-properties-to-remove
   "Compares two property maps to find which properties should be removed"
   [prev-props upd-props]
@@ -161,6 +160,46 @@
     (filterv #(not (contains? prev-names (:name %))) upd-props)))
 
 
+(defn- split-base-name-and-number
+  "Extract the number in parentheses from an item, if present, and return both the base name and the number"
+  [item]
+  (let [pattern-num-parens #"\(\d+\)$"
+        pattern-num        #"\d+"
+        base (-> item (str/replace pattern-num-parens "") (str/trim))
+        num  (some->> item (re-find pattern-num-parens) (re-find pattern-num) (d/parse-integer))]
+    [base (d/nilv num 0)]))
+
+(defn- group-numbers-by-base-name
+  "Return a map with a set of numbers associated to each base name"
+  [items]
+  (reduce (fn [acc item]
+            (let [[base num] (split-base-name-and-number item)]
+              (update acc base (fnil conj #{}) num)))
+          {}
+          items))
+
+(defn update-number-in-repeated-item
+  "Add, keep or update a number in parentheses for a given item, if necessary, depending on the items
+   already present in a list, to avoid repetitions"
+  [items item]
+  (let [names      (group-numbers-by-base-name items)
+        [base num] (split-base-name-and-number item)
+        nums-taken (get names base #{})]
+    (loop [n num]
+      (if (nums-taken n)
+        (recur (inc n))
+        (str base (when (pos? n) (str " (" n ")")))))))
+
+(defn update-number-in-repeated-prop-names
+  "Add, keep or update a number for each prop name depending on the previous ones"
+  [props]
+  (->> props
+       (reduce (fn [acc prop]
+                 (conj acc {:name (update-number-in-repeated-item (mapv :name acc) (:name prop))
+                            :value (:value prop)}))
+               [])))
+
+
 (defn find-index-for-property-name
   "Finds the index of a name in a property map"
   [props name]

common/test/common_tests/variant_test.cljc

@@ -15,13 +15,13 @@
         map-with-two-props-dashes    [{:name "border" :value "no"} {:name "color" :value "--"}]
         map-with-one-prop            [{:name "border" :value "no"}]
         map-with-equal               [{:name "border" :value "yes color=yes"}]
-        map-with-spaces              [{:name "border 1" :value "of course"}
+        map-with-spaces              [{:name "border (1)" :value "of course"}
-                                      {:name "color 2" :value "dark gray"}
+                                      {:name "color (2)" :value "dark gray"}
-                                      {:name "background 3" :value "anoth€r co-lor"}]
+                                      {:name "background (3)" :value "anoth€r co-lor"}]
 
         string-valid-with-two-props  "border=yes, color=gray"
         string-valid-with-one-prop   "border=no"
-        string-valid-with-spaces     "border 1=of course, color 2=dark gray, background 3=anoth€r co-lor"
+        string-valid-with-spaces     "border (1)=of course, color (2)=dark gray, background (3)=anoth€r co-lor"
         string-valid-with-no-value   "border=no, color="
         string-valid-with-dashes     "border=no, color=--"
         string-valid-with-equal      "border=yes color=yes"
@@ -131,3 +131,31 @@
       (t/is (= (ctv/same-variant? components-2) false))
       (t/is (= (ctv/same-variant? components-3) false))
       (t/is (= (ctv/same-variant? components-4) false)))))
+
+
+(t/deftest update-number-in-repeated-item
+  (let [names ["border" "color" "color 1" "color 2" "color (1)" "color (7)" "area 51"]]
+
+    (t/testing "update-number-in-repeated-item"
+      (t/is (= (ctv/update-number-in-repeated-item names "background") "background"))
+      (t/is (= (ctv/update-number-in-repeated-item names "border")     "border (1)"))
+      (t/is (= (ctv/update-number-in-repeated-item names "color")      "color (2)"))
+      (t/is (= (ctv/update-number-in-repeated-item names "color 1")    "color 1 (1)"))
+      (t/is (= (ctv/update-number-in-repeated-item names "color (1)")  "color (2)"))
+      (t/is (= (ctv/update-number-in-repeated-item names "area 51")    "area 51 (1)")))))
+
+
+(t/deftest update-number-in-repeated-prop-names
+  (let [props          [{:name "color"      :value "yellow"}
+                        {:name "color"      :value "blue"}
+                        {:name "color"      :value "red"}
+                        {:name "border (1)" :value "no"}
+                        {:name "border (1)" :value "yes"}]
+        numbered-props [{:name "color"      :value "yellow"}
+                        {:name "color (1)"  :value "blue"}
+                        {:name "color (2)"  :value "red"}
+                        {:name "border (1)" :value "no"}
+                        {:name "border (2)" :value "yes"}]]
+
+    (t/testing "update-number-in-repeated-prop-names"
+      (t/is (= (ctv/update-number-in-repeated-prop-names props) numbered-props)))))

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

@@ -49,6 +49,8 @@
             objects (-> (dsh/get-page data page-id)
                         (get :objects))
 
+            updated-properties   (ctv/update-number-in-repeated-prop-names updated-properties)
+
             properties-to-remove (ctv/find-properties-to-remove previous-properties updated-properties)
             properties-to-add    (ctv/find-properties-to-add previous-properties updated-properties)
             properties-to-update (ctv/find-properties-to-update previous-properties updated-properties)