From fb2eeefbfec0d279f9a028088e2904b2d5f93bb8 Mon Sep 17 00:00:00 2001 From: Gabriel Horner Date: Thu, 29 Aug 2024 17:25:46 -0400 Subject: [PATCH] enhance: add duplicate detection for pages without tags Also added tests for duplicates and improved error ux. Fixes LOG-3197 --- deps/outliner/src/logseq/outliner/core.cljs | 39 ++++++--- .../test/logseq/outliner/core_test.cljs | 81 +++++++++++++++++++ 2 files changed, 108 insertions(+), 12 deletions(-) create mode 100644 deps/outliner/test/logseq/outliner/core_test.cljs diff --git a/deps/outliner/src/logseq/outliner/core.cljs b/deps/outliner/src/logseq/outliner/core.cljs index e302b1e48..59c17562e 100644 --- a/deps/outliner/src/logseq/outliner/core.cljs +++ b/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 diff --git a/deps/outliner/test/logseq/outliner/core_test.cljs b/deps/outliner/test/logseq/outliner/core_test.cljs new file mode 100644 index 000000000..2f94e94db --- /dev/null +++ b/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")))) \ No newline at end of file