Browse Source

enhance: add duplicate detection for pages without tags

Also added tests for duplicates and improved error ux.
Fixes LOG-3197
Gabriel Horner 1 year ago
parent
commit
fb2eeefbfe

+ 27 - 12
deps/outliner/src/logseq/outliner/core.cljs

@@ -232,12 +232,14 @@
          (map (fn [tag]
                 [:db/retract (:db/id block) :block/tags (:db/id tag)])))))
 
-;; TODO: Validate tagless pages and other block types
-(defn- validate-unique-by-name-tag-and-block-type
+(defn validate-unique-by-name-tag-and-block-type
+  "Validates uniqueness of blocks and pages for the following cases:
+   - Page names are unique by tag e.g. their can be Apple #Company and Apple #Fruit
+   - Page names are unique when they have no tag
+   - Block names are unique by tag"
   [db new-title {:block/keys [tags] :as entity}]
-  ;; (prn :entity entity)
   (cond
-    (ldb/page? entity)
+    (and (ldb/page? entity) (seq tags))
     (when-let [res (seq (d/q '[:find [?b ...]
                                :in $ ?title [?tag-id ...]
                                :where
@@ -247,12 +249,26 @@
                              db
                              new-title
                              (map :db/id tags)))]
-      (throw (ex-info "Duplicate tagged page"
+      (throw (ex-info "Duplicate page by tag"
                       {:type :notification
-                       :payload {:message (str "Another page with the new name already exists for tag "
+                       :payload {:message (str "Another page named " (pr-str new-title) " already exists for tag "
                                                (pr-str (->> res first (d/entity db) :block/tags first :block/title)))
-                                 :type :error}})))
-    :else
+                                 :type :warning}})))
+
+    (ldb/page? entity)
+    (when-let [_res (seq (d/q '[:find [?b ...]
+                                :in $ ?title
+                                :where
+                                [?b :block/title ?title]
+                                [?b :block/type "page"]]
+                              db
+                              new-title))]
+      (throw (ex-info "Duplicate page without tag"
+                      {:type :notification
+                       :payload {:message (str "Another page named " (pr-str new-title) " already exists")
+                                 :type :warning}})))
+
+    (and (not (:block/type entity)) (seq tags))
     (when-let [res (seq (d/q '[:find [?b ...]
                                :in $ ?title [?tag-id ...]
                                :where
@@ -262,12 +278,11 @@
                              db
                              new-title
                              (map :db/id tags)))]
-      (prn :res res)
-      (throw (ex-info "Duplicate tagged block"
+      (throw (ex-info "Duplicate block by tag"
                       {:type :notification
-                       :payload {:message (str "Another block with the new name already exists for tag "
+                       :payload {:message (str "Another block named " (pr-str new-title) " already exists for tag "
                                                (pr-str (->> res first (d/entity db) :block/tags first :block/title)))
-                                 :type :error}})))))
+                                 :type :warning}})))))
 
 (extend-type Entity
   otree/INode

+ 81 - 0
deps/outliner/test/logseq/outliner/core_test.cljs

@@ -0,0 +1,81 @@
+(ns logseq.outliner.core-test
+  (:require [cljs.test :refer [deftest is testing]]
+            [logseq.db.frontend.schema :as db-schema]
+            [datascript.core :as d]
+            [logseq.db.sqlite.create-graph :as sqlite-create-graph]
+            [logseq.db.sqlite.build :as sqlite-build]
+            [logseq.outliner.core :as outliner-core]))
+
+(defn- create-conn-with-blocks [opts]
+  (let [conn (d/create-conn db-schema/schema-for-db-based-graph)
+        _ (d/transact! conn (sqlite-create-graph/build-db-initial-data "{}"))
+        _ (sqlite-build/create-blocks conn opts)]
+    conn))
+
+(defn- find-block-by-content [conn content]
+  (->> content
+       (d/q '[:find [(pull ?b [*]) ...]
+              :in $ ?content
+              :where [?b :block/title ?content]]
+            @conn)
+       first))
+
+(deftest validate-unique-by-name-tag-and-block-type
+  (testing "pages"
+    (let [conn (create-conn-with-blocks
+                [{:page {:block/title "page1"}}
+                 {:page {:block/title "Apple" :build/tags [:Company]}}
+                 {:page {:block/title "Banana" :build/tags [:Fruit]}}])]
+
+      (is (thrown-with-msg?
+           js/Error
+           #"Duplicate page by tag"
+           (outliner-core/validate-unique-by-name-tag-and-block-type
+            @conn
+            "Apple"
+            (find-block-by-content conn "Apple")))
+          "Disallow duplicate page with tag")
+      (is (nil?
+           (outliner-core/validate-unique-by-name-tag-and-block-type
+            @conn
+            "Apple"
+            (find-block-by-content conn "Banana")))
+          "Allow page with same name for different tag")
+
+      (is (thrown-with-msg?
+           js/Error
+           #"Duplicate page without tag"
+           (outliner-core/validate-unique-by-name-tag-and-block-type
+            @conn
+            "page1"
+            (find-block-by-content conn "page1")))
+          "Disallow duplicate page without tag")))
+
+  (testing "blocks"
+    (let [conn (create-conn-with-blocks
+                [{:page {:block/title "page"}
+                  :blocks [{:block/title "yahoo"}
+                           {:block/title "Sing Sing" :build/tags [:Movie]}
+                           {:block/title "Chicago" :build/tags [:Musical]}]}])]
+
+      (is (nil?
+           (outliner-core/validate-unique-by-name-tag-and-block-type
+            @conn
+            "yahoo"
+            (find-block-by-content conn "yahoo")))
+          "Blocks without tags have no limits")
+
+      (is (thrown-with-msg?
+           js/Error
+           #"Duplicate block by tag"
+           (outliner-core/validate-unique-by-name-tag-and-block-type
+            @conn
+            "Sing Sing"
+            (find-block-by-content conn "Sing Sing")))
+          "Disallow duplicate page with tag")
+      (is (nil?
+           (outliner-core/validate-unique-by-name-tag-and-block-type
+            @conn
+            "Sing Sing"
+            (find-block-by-content conn "Chicago")))
+          "Allow block with same name for different tag"))))