Browse Source

fix: tags can have duplicate names

Tags should not have duplicate names regardless of their
extends or if they're built-in or not.
Fixes https://github.com/logseq/db-test/issues/338 and fixes
https://github.com/logseq/db-test/issues/349
Gabriel Horner 6 months ago
parent
commit
e4ffd13340

+ 27 - 47
deps/outliner/src/logseq/outliner/validate.cljs

@@ -48,26 +48,8 @@
                      :payload {:message (or message "Built-in pages can't be edited")
                                :type :warning}}))))
 
-(defn- validate-unique-by-extends-and-name [db entity new-title]
-  (when-let [_res (seq (d/q '[:find [?b ...]
-                              :in $ ?eid ?type ?title
-                              :where
-                              [?b :block/title ?title]
-                              [?b :logseq.property.class/extends ?type]
-                              [(not= ?b ?eid)]]
-                            db
-                            (:db/id entity)
-                            (:db/id (:logseq.property.class/extends entity))
-                            new-title))]
-    (throw (ex-info "Duplicate page by parent"
-                    {:type :notification
-                     :payload {:message (str "Another page named " (pr-str new-title) " already exists for parents "
-                                             (pr-str (->> (ldb/get-class-extends entity)
-                                                          (map :block/title)
-                                                          (string/join ns-util/parent-char))))
-                               :type :warning}}))))
-
-(defn- another-id-q
+(defn- find-other-ids-with-title-and-tags
+  "Query that finds other ids given the id to ignore, title to look up and tags to consider"
   [entity]
   (cond
     (ldb/property? entity)
@@ -80,17 +62,6 @@
       [?b :block/tags ?tag-id]
       [(missing? $ ?b :logseq.property/built-in?)]
       [(not= ?b ?eid)]]
-    (:logseq.property.class/extends entity)
-    '[:find [?b ...]
-      :in $ ?eid ?title [?tag-id ...]
-      :where
-      [?b :block/title ?title]
-      [?b :block/tags ?tag-id]
-      [(not= ?b ?eid)]
-      ;; same extends
-      [?b :logseq.property.class/extends ?bp]
-      [?eid :logseq.property.class/extends ?ep]
-      [(= ?bp ?ep)]]
     (:block/parent entity)
     '[:find [?b ...]
       :in $ ?eid ?title [?tag-id ...]
@@ -112,10 +83,9 @@
 
 (defn- validate-unique-for-page
   [db new-title {:block/keys [tags] :as entity}]
-  (cond
-    (seq tags)
+  (when (seq tags)
     (when-let [another-id (first
-                           (d/q (another-id-q entity)
+                           (d/q (find-other-ids-with-title-and-tags entity)
                                 db
                                 (:db/id entity)
                                 new-title
@@ -127,20 +97,30 @@
         (when-not (and (= common-tag-ids #{:logseq.class/Page})
                        (> (count this-tags) 1)
                        (> (count another-tags) 1))
-          (throw (ex-info "Duplicate page"
-                          {:type :notification
-                           :payload {:message (str "Another page named " (pr-str new-title) " already exists for tags: "
-                                                   (string/join ", "
-                                                                (map (fn [id] (str "#" (:block/title (d/entity db id)))) common-tag-ids)))
-                                     :type :warning}})))))
-
-    (:logseq.property.class/extends entity)
-    (validate-unique-by-extends-and-name db entity new-title)))
-
-(defn ^:api validate-unique-by-name-tag-and-block-type
+          (cond
+            (ldb/property? entity)
+            (throw (ex-info "Duplicate property"
+                            {:type :notification
+                             :payload {:message (str "Another property named " (pr-str new-title) " already exists.")
+                                       :type :warning}}))
+            (ldb/class? entity)
+            (throw (ex-info "Duplicate class"
+                            {:type :notification
+                             :payload {:message (str "Another tag named " (pr-str new-title) " already exists.")
+                                       :type :warning}}))
+            :else
+            (throw (ex-info "Duplicate page"
+                            {:type :notification
+                             :payload {:message (str "Another page named " (pr-str new-title) " already exists for tags: "
+                                                     (string/join ", "
+                                                                  (map (fn [id] (str "#" (:block/title (d/entity db id)))) common-tag-ids)))
+                                       :type :warning}}))))))))
+
+(defn ^:api validate-unique-by-name-and-tags
   "Validates uniqueness of nodes for the following cases:
    - Page names are unique for a tag e.g. their can be Apple #Company and Apple #Fruit
-   - Page names are unique for a :logseq.property.class/extends"
+   - Property names are unique with user properties being allowed to have the same name as built-in ones
+   - Class names are unique regardless of their extends or if they're built-in"
   [db new-title entity]
   (when (entity-util/page? entity)
     (validate-unique-for-page db new-title entity)))
@@ -159,7 +139,7 @@
   "Validates a block title when it has changed for a entity-util/page? or tagged node"
   [db new-title existing-block-entity]
   (validate-built-in-pages existing-block-entity)
-  (validate-unique-by-name-tag-and-block-type db new-title existing-block-entity)
+  (validate-unique-by-name-and-tags db new-title existing-block-entity)
   (validate-disallow-page-with-journal-name new-title existing-block-entity))
 
 (defn validate-property-title

+ 31 - 9
deps/outliner/test/logseq/outliner/validate_test.cljs

@@ -11,7 +11,7 @@
                             :color2 {:logseq.property/type :default}}})]
 
     (is (nil?
-         (outliner-validate/validate-unique-by-name-tag-and-block-type
+         (outliner-validate/validate-unique-by-name-and-tags
           @conn
           (:block/title (d/entity @conn :logseq.property/background-color))
           (d/entity @conn :user.property/color)))
@@ -19,13 +19,35 @@
 
     (is (thrown-with-msg?
          js/Error
-         #"Duplicate page"
-         (outliner-validate/validate-unique-by-name-tag-and-block-type
+         #"Duplicate property"
+         (outliner-validate/validate-unique-by-name-and-tags
           @conn
           "color"
           (d/entity @conn :user.property/color2)))
         "Disallow duplicate user property")))
 
+(deftest ^:focus validate-block-title-unique-for-tags
+  (let [conn (db-test/create-conn-with-blocks
+              {:classes {:Class1 {}
+                         :Class2 {:logseq.property.class/extends :logseq.class/Task}}})]
+
+    (is (thrown-with-msg?
+         js/Error
+         #"Duplicate class"
+         (outliner-validate/validate-unique-by-name-and-tags
+          @conn
+          "Class1"
+          (d/entity @conn :user.class/Class2)))
+        "Disallow duplicate class names, regardless of extends")
+    (is (thrown-with-msg?
+         js/Error
+         #"Duplicate class"
+         (outliner-validate/validate-unique-by-name-and-tags
+          @conn
+          "Card"
+          (d/entity @conn :user.class/Class1)))
+        "Disallow duplicate class names even if it's built-in")))
+
 (deftest validate-block-title-unique-for-pages
   (let [conn (db-test/create-conn-with-blocks
               [{:page {:block/title "page1"}}
@@ -37,13 +59,13 @@
     (is (thrown-with-msg?
          js/Error
          #"Duplicate page"
-         (outliner-validate/validate-unique-by-name-tag-and-block-type
+         (outliner-validate/validate-unique-by-name-and-tags
           @conn
           "Apple"
           (db-test/find-page-by-title @conn "Another Company")))
         "Disallow duplicate page with tag")
     (is (nil?
-         (outliner-validate/validate-unique-by-name-tag-and-block-type
+         (outliner-validate/validate-unique-by-name-and-tags
           @conn
           "Apple"
           (db-test/find-page-by-title @conn "Banana")))
@@ -52,14 +74,14 @@
     (is (thrown-with-msg?
          js/Error
          #"Duplicate page"
-         (outliner-validate/validate-unique-by-name-tag-and-block-type
+         (outliner-validate/validate-unique-by-name-and-tags
           @conn
           "page1"
           (db-test/find-page-by-title @conn "another page")))
         "Disallow duplicate page without tag")
 
     (is (nil?
-         (outliner-validate/validate-unique-by-name-tag-and-block-type
+         (outliner-validate/validate-unique-by-name-and-tags
           @conn
           "Apple"
           (db-test/find-page-by-title @conn "Fruit")))
@@ -97,7 +119,7 @@
            js/Error
            #"Can't change.*built-in"
            (outliner-validate/validate-extends-property (entity-plus/entity-memoized @conn :logseq.class/Task)
-                                                       [(entity-plus/entity-memoized @conn :logseq.class/Cards)]))))))
+                                                        [(entity-plus/entity-memoized @conn :logseq.class/Cards)]))))))
 
 (deftest validate-tags-property
   (let [conn (db-test/create-conn-with-blocks
@@ -151,7 +173,7 @@
             page-errors (atom {})]
         (doseq [page pages]
           (try
-            (outliner-validate/validate-unique-by-name-tag-and-block-type @conn (:block/title page) page)
+            (outliner-validate/validate-unique-by-name-and-tags @conn (:block/title page) page)
             (outliner-validate/validate-page-title (:block/title page) {:node page})
             (outliner-validate/validate-page-title-characters (:block/title page) {:node page})
 

+ 1 - 1
src/main/frontend/handler/db_based/page.cljs

@@ -23,7 +23,7 @@
    When returning false, this fn also displays appropriate notifications to the user"
   [repo block tag-entity]
   (try
-    (outliner-validate/validate-unique-by-name-tag-and-block-type
+    (outliner-validate/validate-unique-by-name-and-tags
      (db/get-db repo)
      (:block/title block)
      (update block :block/tags (fnil conj #{}) tag-entity))