🐛 Fix name generation and handling on creating objects (files, tokens, ...) (#5745)

* ✨ Update function implementation

* ✨ Add tests for a new function implementation

* ✨ Update function usages according to new signature

* ✨ Update docstrings

* ✨ Use simpler assertion

* 💄 Replace concat with cons on name-seq

* 🐛 Fix incorrect error handling on legacy workspace redirect

---------

Co-authored-by: Andrey Fedorov <oran9e.red@gmail.com>

backend/src/app/features/components_v2.clj

@@ -1071,7 +1071,7 @@
         groups (d/group-by #(first (cfh/split-path (:path %))) assets)
         ;; If there is a group called as the generic-name we have to preserve it
         unames (into #{} (keep str) (keys groups))
-        groups (rename-keys groups {generic-name (cfh/generate-unique-name unames generic-name)})
+        groups (rename-keys groups {generic-name (cfh/generate-unique-name generic-name unames)})
 
         ;; Split large groups in chunks of max-group-size elements
         groups (loop [groups (seq groups)

common/src/app/common/files/helpers.cljc

@@ -9,7 +9,6 @@
    [app.common.data :as d]
    [app.common.data.macros :as dm]
    [app.common.geom.shapes.common :as gco]
-   [app.common.schema :as sm]
    [app.common.uuid :as uuid]
    [clojure.set :as set]
    [clojure.walk :as walk]
@@ -400,31 +399,51 @@
                    elements)]
     (into #{} (keep :name) elements)))
 
-(defn- extract-numeric-suffix
-  [basename]
-  (if-let [[_ p1 p2] (re-find #"(.*) ([0-9]+)$" basename)]
-    [p1 (+ 1 (d/parse-integer p2))]
-    [basename 1]))
+(defn- name-seq
+  "Creates a lazy, infinite sequence of names starting with `base-name`,
+   followed by variants with suffixes applied. The sequence follows this pattern:
+   - `base-name`
+   - `(str base-name (suffix-fn 1))`
+   - `(str base-name (suffix-fn 2))`
+   - `(str base-name (suffix-fn 3))`, etc."
+  [base-name suffix-fn]
+  (cons base-name
+        (map #(str/concat base-name (suffix-fn %))
+             (iterate inc 1))))
+
+(defn ^:private get-suffix
+  "Default suffix impelemtation"
+  [copy-count]
+  (str/concat " " copy-count))
 
 (defn generate-unique-name
-  "A unique name generator"
-  [used basename]
+  "Generates a unique name by selecting the first available name from a generated sequence.
+   The sequence consists of `base-name` and its variants, avoiding conflicts with `existing-names`.
+
+   Parameters:
+   - `base-name` - string used as the base for name generation.
+   - `existing-names` - a collection of existing names to check for uniqueness.
+   - Options:
+     - `:suffix-fn` - a function that generates suffixes, given an integer (default: `get-suffix`).
+     - `:immediate-suffix?` - if `true`, the base name is considered taken, and suffixing starts immediately.
+
+   Returns:
+   - A unique name not present in `existing-names`."
+  [base-name existing-names & {:keys [suffix-fn immediate-suffix?]
+                               :or {suffix-fn get-suffix}}]
   (dm/assert!
    "expected a set of strings"
-   (sm/check-set-of-strings! used))
+   (coll? existing-names))
 
   (dm/assert!
    "expected a string for `basename`."
-   (string? basename))
-
-  (if-not (contains? used basename)
-    basename
-    (let [[prefix initial] (extract-numeric-suffix basename)]
-      (loop [counter initial]
-        (let [candidate (str prefix " " counter)]
-          (if (contains? used candidate)
-            (recur (inc counter))
-            candidate))))))
+   (string? base-name))
+  (let [existing-name-set (cond-> (set existing-names)
+                            immediate-suffix? (conj base-name))
+        names (name-seq base-name suffix-fn)]
+    (->> names
+         (remove #(contains? existing-name-set %))
+         first)))
 
 (defn walk-pages
   "Go through all pages of a file and apply a function to each one"
@@ -724,7 +743,7 @@
 
 (defn split-by-last-period
   "Splits a string into two parts:
-   the text before and including the last period, 
+   the text before and including the last period,
    and the text after the last period."
   [s]
   (if-let [last-period (str/last-index-of s ".")]

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

@@ -2011,7 +2011,7 @@
         has-flow?        (partial ctp/get-frame-flow flows)]
 
     (reduce (fn [changes frame-id]
-              (let [name     (cfh/generate-unique-name @unames "Flow 1")
+              (let [name     (cfh/generate-unique-name "Flow" @unames :immediate-suffix? true)
                     frame-id (get ids-map frame-id)
                     flow-id  (uuid/next)
                     new-flow {:id flow-id

common/test/common_tests/files/helpers_test.cljc

@@ -0,0 +1,38 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC
+
+(ns common-tests.files.helpers-test
+  (:require
+   [app.common.files.helpers :as cfh]
+   [clojure.test :as t]))
+
+(t/deftest test-generate-unique-name
+  (t/testing "Test unique name generation"
+    (let [suffix-fn #(str "-copy-" %)]
+      (t/is (cfh/generate-unique-name "base-name"
+                                      #{"base-name" "base-name-copy-1"}
+                                      :suffix-fn suffix-fn)
+            "base-name-copy-2")
+      (t/is (cfh/generate-unique-name "base-name"
+                                      #{"base-name-copy-2"}
+                                      :suffix-fn suffix-fn)
+            "base-name-copy-1")
+      (t/is (cfh/generate-unique-name "base-name"
+                                      #{"base-namec-copy"}
+                                      :suffix-fn suffix-fn)
+            "base-name-copy-1")
+      (t/is (cfh/generate-unique-name "base-name"
+                                      #{"base-name"}
+                                      :suffix-fn suffix-fn)
+            "base-name-copy-1")))
+
+  (t/testing "Test unique name generation with immidate suffix and default suffix-fn"
+    (t/is (cfh/generate-unique-name "base-name" #{} :immediate-suffix? true)
+          "base-name 1")
+    (t/is (cfh/generate-unique-name "base-name"
+                                    #{"base-name 1" "base-name 2"}
+                                    :immediate-suffix? true)
+          "base-name 3")))

frontend/src/app/main/data/dashboard.cljs

@@ -25,6 +25,7 @@
    [app.util.time :as dt]
    [beicon.v2.core :as rx]
    [clojure.set :as set]
+   [cuerdas.core :as str]
    [potok.v2.core :as ptk]))
 
 (log/set-level! :warn)
@@ -247,9 +248,9 @@
   (ptk/reify ::create-project
     ptk/WatchEvent
     (watch [_ state _]
-      (let [projects (get state :projects)
-            unames   (cfh/get-used-names projects)
-            name     (cfh/generate-unique-name unames (str (tr "dashboard.new-project-prefix") " 1"))
+      (let [unames   (cfh/get-used-names (get state :projects))
+            base-name (tr "dashboard.new-project-prefix")
+            name     (cfh/generate-unique-name base-name unames :immediate-suffix? true)
             team-id  (:current-team-id state)
             params   {:name name
                       :team-id team-id}
@@ -280,13 +281,18 @@
        :name name})
 
     ptk/WatchEvent
-    (watch [_ _ _]
+    (watch [_ state _]
       (let [{:keys [on-success on-error]
              :or {on-success identity
                   on-error rx/throw}} (meta params)
-
-            new-name (str name " " (tr "dashboard.copy-suffix"))]
-
+            projects (get state :projects)
+            unames (cfh/get-used-names projects)
+            suffix-fn (fn [copy-count]
+                        (str/concat " "
+                                    (tr "dashboard.copy-suffix")
+                                    (when (> copy-count 1)
+                                      (str " " copy-count))))
+            new-name (cfh/generate-unique-name name unames :suffix-fn suffix-fn)]
         (->> (rp/cmd! :duplicate-project {:project-id id :name new-name})
              (rx/tap on-success)
              (rx/map project-duplicated)
@@ -477,15 +483,15 @@
       (let [{:keys [on-success on-error]
              :or {on-success identity
                   on-error rx/throw}} (meta params)
-
-            files    (get state :files)
-            unames   (cfh/get-used-names files)
-            name     (or name (cfh/generate-unique-name unames (str (tr "dashboard.new-file-prefix") " 1")))
-            features (-> (features/get-team-enabled-features state)
-                         (set/difference cfeat/frontend-only-features))
-            params   (-> params
-                         (assoc :name name)
-                         (assoc :features features))]
+            unames    (cfh/get-used-names (get state :files))
+            base-name (tr "dashboard.new-file-prefix")
+            name      (or name
+                          (cfh/generate-unique-name base-name unames :immediate-suffix? true))
+            features  (-> (features/get-team-enabled-features state)
+                          (set/difference cfeat/frontend-only-features))
+            params    (-> params
+                          (assoc :name name)
+                          (assoc :features features))]
 
         (->> (rp/cmd! :create-file params)
              (rx/tap on-success)
@@ -500,13 +506,17 @@
   (dm/assert! (string? name))
   (ptk/reify ::duplicate-file
     ptk/WatchEvent
-    (watch [_ _ _]
+    (watch [_ state _]
       (let [{:keys [on-success on-error]
              :or {on-success identity
                   on-error rx/throw}} (meta params)
-
-            new-name (str name " " (tr "dashboard.copy-suffix"))]
-
+            unames (cfh/get-used-names (get state :files))
+            suffix-fn (fn [copy-count]
+                        (str/concat " "
+                                    (tr "dashboard.copy-suffix")
+                                    (when (> copy-count 1)
+                                      (str " " copy-count))))
+            new-name (cfh/generate-unique-name name unames :suffix-fn suffix-fn)]
         (->> (rp/cmd! :duplicate-file {:file-id id :name new-name})
              (rx/tap on-success)
              (rx/map file-created)
@@ -589,10 +599,10 @@
             name          (if in-project?
                             (let [files  (get state :files)
                                   unames (cfh/get-used-names files)]
-                              (cfh/generate-unique-name unames (str (tr "dashboard.new-file-prefix") " 1")))
+                              (cfh/generate-unique-name (tr "dashboard.new-file-prefix") unames :immediate-suffix? true))
                             (let [projects (get state :projects)
                                   unames   (cfh/get-used-names projects)]
-                              (cfh/generate-unique-name unames (str (tr "dashboard.new-project-prefix") " 1"))))
+                              (cfh/generate-unique-name (tr "dashboard.new-project-prefix") unames :immediate-suffix? true)))
             params        (if in-project?
                             {:project-id (:project-id pparams)
                              :name name}

frontend/src/app/main/data/tokens.cljs

@@ -8,6 +8,7 @@
   (:require
    [app.common.data.macros :as dm]
    [app.common.files.changes-builder :as pcb]
+   [app.common.files.helpers :as cfh]
    [app.common.geom.point :as gpt]
    [app.common.logic.tokens :as clt]
    [app.common.types.shape :as cts]
@@ -286,10 +287,21 @@
   (ptk/reify ::duplicate-token
     ptk/WatchEvent
     (watch [_ state _]
-      (when-let [token (some-> (dwts/get-selected-token-set-token state token-name)
-                               (update :name #(str/concat % "-copy")))]
-        (rx/of
-         (update-create-token {:token token}))))))
+      (let [token-set (dwts/get-selected-token-set state)
+            token (some-> token-set (ctob/get-token token-name))
+            tokens (some-> token-set (ctob/get-tokens))
+            suffix-fn (fn [copy-count]
+                        (let [suffix (tr "workspace.token.duplicate-suffix")]
+                          (str/concat "-"
+                                      suffix
+                                      (when (> copy-count 1)
+                                        (str "-" copy-count)))))
+            unames (map :name tokens)
+            copy-name (cfh/generate-unique-name token-name unames :suffix-fn suffix-fn)]
+        (when token
+          (rx/of
+           (update-create-token
+            {:token (assoc token :name copy-name)})))))))
 
 (defn set-token-type-section-open
   [token-type open?]

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

@@ -507,10 +507,8 @@
       (watch [it state _]
         (let [pages   (-> (dsh/lookup-file-data state)
                           (get :pages-index))
-
               unames  (cfh/get-used-names pages)
-              name    (cfh/generate-unique-name unames "Page 1")
-
+              name    (cfh/generate-unique-name "Page" unames :immediate-suffix? true)
               changes (-> (pcb/empty-changes it)
                           (pcb/add-empty-page id name))]
 
@@ -527,7 +525,13 @@
             page               (get pages page-id)
 
             unames             (cfh/get-used-names pages)
-            name               (cfh/generate-unique-name unames (:name page))
+            suffix-fn          (fn [copy-count]
+                                 (str/concat " "
+                                             (tr "dashboard.copy-suffix")
+                                             (when (> copy-count 1)
+                                               (str " " copy-count))))
+            base-name          (:name page)
+            name               (cfh/generate-unique-name base-name unames :suffix-fn suffix-fn)
             objects            (update-vals (:objects page) #(dissoc % :use-for-thumbnail))
 
             main-instances-ids (set (keep #(when (ctk/main-instance? (val %)) (key %)) objects))

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

@@ -44,9 +44,9 @@
                        (dsh/lookup-page state page-id)
                        (dsh/lookup-page state))
 
-             flows   (get page :flows)
-             unames  (cfh/get-used-names (vals flows))
-             name    (or name (cfh/generate-unique-name unames "Flow 1"))
+             unames  (cfh/get-used-names (vals (get page :flows)))
+             name    (or name
+                         (cfh/generate-unique-name "Flow" unames :immediate-suffix? true))
 
              flow-id (or flow-id (uuid/next))
 

frontend/src/app/main/ui.cljs

@@ -10,6 +10,7 @@
    [app.config :as cf]
    [app.main.data.common :as dcm]
    [app.main.data.team :as dtm]
+   [app.main.errors :as errors]
    [app.main.refs :as refs]
    [app.main.repo :as rp]
    [app.main.router :as rt]
@@ -29,7 +30,6 @@
    [app.util.dom :as dom]
    [app.util.i18n :refer [tr]]
    [beicon.v2.core :as rx]
-   [potok.v2.core :as ptk]
    [rumext.v2 :as mf]))
 
 (def auth-page
@@ -61,8 +61,7 @@
                                                     :file-id file-id
                                                     :page-id page-id
                                                     :layout layout)))
-                   ptk/handle-error)))
-
+                   errors/on-error)))
   [:> loader*
    {:title (tr "labels.loading")
     :overlay true}])

frontend/translations/en.po

@@ -1143,6 +1143,10 @@ msgstr "Cannot drop a parent set to an own child path."
 msgid "errors.drag-drop.set-exists"
 msgstr "Cannot complete drop, a set with same name already exists at path %s."
 
+#: src/app/main/data/tokens.cljs:294
+msgid "workspace.token.duplicate-suffix"
+msgstr "copy"
+
 #: src/app/main/ui/auth/verify_token.cljs:84, src/app/main/ui/settings/change_email.cljs:29
 msgid "errors.email-already-exists"
 msgstr "Email already used"