Browse Source

fix: validate property name when changed from menu

Fixes logseq/db-test#69
Gabriel Horner 1 year ago
parent
commit
5146e0da40

+ 1 - 0
.clj-kondo/config.edn

@@ -182,6 +182,7 @@
              logseq.outliner.core outliner-core
              logseq.outliner.op outliner-op
              logseq.outliner.pipeline outliner-pipeline
+             logseq.outliner.validate outliner-validate
              logseq.outliner.datascript-report ds-report
              logseq.shui.ui shui
              logseq.shui.popup.core shui-popup

+ 2 - 72
deps/outliner/src/logseq/outliner/core.cljs

@@ -7,6 +7,7 @@
             [logseq.db.frontend.schema :as db-schema]
             [logseq.outliner.datascript :as ds]
             [logseq.outliner.tree :as otree]
+            [logseq.outliner.validate :as outliner-validate]
             [logseq.common.util :as common-util]
             [malli.core :as m]
             [malli.util :as mu]
@@ -232,76 +233,6 @@
          (map (fn [tag]
                 [:db/retract (:db/id block) :block/tags (:db/id tag)])))))
 
-(defn- validate-unique-for-page
-  [db new-title {:block/keys [tags] :as entity}]
-  (if (seq tags)
-    (when-let [res (seq (d/q '[:find [?b ...]
-                               :in $ ?eid ?title [?tag-id ...]
-                               :where
-                               [?b :block/title ?title]
-                               [?b :block/tags ?tag-id]
-                               [(not= ?b ?eid)]]
-                             db
-                             (:db/id entity)
-                             new-title
-                             (map :db/id tags)))]
-      (throw (ex-info "Duplicate page by tag"
-                      {:type :notification
-                       :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 :warning}})))
-    (when-let [_res (seq (d/q '[:find [?b ...]
-                                :in $ ?eid ?type ?title
-                                :where
-                                [?b :block/title ?title]
-                                [?b :block/type ?type]
-                                [(not= ?b ?eid)]]
-                              db
-                              (:db/id entity)
-                              (:block/type entity)
-                              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}})))))
-
-(defn ^:api 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 by type e.g. their can be #Journal and Journal (normal page)
-   - Block names are unique by tag"
-  [db new-title {:block/keys [tags] :as entity}]
-  (cond
-    (ldb/page? entity)
-    (validate-unique-for-page db new-title entity)
-
-    (and (not (:block/type entity)) (seq tags))
-    (when-let [res (seq (d/q '[:find [?b ...]
-                               :in $ ?eid ?title [?tag-id ...]
-                               :where
-                               [?b :block/title ?title]
-                               [?b :block/tags ?tag-id]
-                               [(not= ?b ?eid)]
-                               [(missing? $ ?b :block/type)]]
-                             db
-                             (:db/id entity)
-                             new-title
-                             (map :db/id tags)))]
-      (throw (ex-info "Duplicate block by tag"
-                      {:type :notification
-                       :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 :warning}})))))
-
-(defn ^:api validate-built-in-pages
-  "Validates built-in pages shouldn't be modified"
-  [entity]
-  (when (ldb/built-in? entity)
-    (throw (ex-info "Rename built-in pages"
-                    {:type :notification
-                     :payload {:message "Built-in pages can't be edited"
-                               :type :warning}}))))
-
 (extend-type Entity
   otree/INode
   (-save [this txs-state conn repo _date-formatter {:keys [retract-attributes? retract-attributes]
@@ -345,8 +276,7 @@
                        (and (or (ldb/page? block-entity) (ldb/object? block-entity))
                             (:block/title m*)
                             (not= (:block/title m*) (:block/title block-entity))))
-              (validate-built-in-pages block-entity)
-              (validate-unique-by-name-tag-and-block-type db (:block/title m*) block-entity))
+              (outliner-validate/validate-block-title db (:block/title m*) block-entity))
           m (cond-> m*
               db-based?
               (dissoc :block/pre-block? :block/priority :block/marker :block/properties-order))]

+ 4 - 0
deps/outliner/src/logseq/outliner/property.cljs

@@ -14,6 +14,7 @@
             [logseq.db.frontend.entity-plus :as entity-plus]
             [logseq.db.sqlite.util :as sqlite-util]
             [logseq.outliner.core :as outliner-core]
+            [logseq.outliner.validate :as outliner-validate]
             [malli.error :as me]
             [malli.util :as mu]
             [clojure.set :as set]))
@@ -87,6 +88,9 @@
 
 (defn- update-property
   [conn db-ident property schema {:keys [property-name properties]}]
+  (when (and (some? property-name) (not= property-name (:block/title property)))
+    (outliner-validate/validate-block-title @conn property-name property))
+
   (let [changed-property-attrs
         ;; Only update property if something has changed as we are updating a timestamp
         (cond-> {}

+ 80 - 0
deps/outliner/src/logseq/outliner/validate.cljs

@@ -0,0 +1,80 @@
+(ns logseq.outliner.validate
+  "Reusable validations from outliner level and above"
+  (:require [datascript.core :as d]
+            [logseq.db :as ldb]))
+
+(defn ^:api validate-built-in-pages
+  "Validates built-in pages shouldn't be modified"
+  [entity]
+  (when (ldb/built-in? entity)
+    (throw (ex-info "Rename built-in pages"
+                    {:type :notification
+                     :payload {:message "Built-in pages can't be edited"
+                               :type :warning}}))))
+
+(defn- validate-unique-for-page
+  [db new-title {:block/keys [tags] :as entity}]
+  (if (seq tags)
+    (when-let [res (seq (d/q '[:find [?b ...]
+                               :in $ ?eid ?title [?tag-id ...]
+                               :where
+                               [?b :block/title ?title]
+                               [?b :block/tags ?tag-id]
+                               [(not= ?b ?eid)]]
+                             db
+                             (:db/id entity)
+                             new-title
+                             (map :db/id tags)))]
+      (throw (ex-info "Duplicate page by tag"
+                      {:type :notification
+                       :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 :warning}})))
+    (when-let [_res (seq (d/q '[:find [?b ...]
+                                :in $ ?eid ?type ?title
+                                :where
+                                [?b :block/title ?title]
+                                [?b :block/type ?type]
+                                [(not= ?b ?eid)]]
+                              db
+                              (:db/id entity)
+                              (:block/type entity)
+                              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}})))))
+
+(defn ^:api 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 by type e.g. their can be #Journal and Journal (normal page)
+   - Block names are unique by tag"
+  [db new-title {:block/keys [tags] :as entity}]
+  (cond
+    (ldb/page? entity)
+    (validate-unique-for-page db new-title entity)
+
+    (and (not (:block/type entity)) (seq tags))
+    (when-let [res (seq (d/q '[:find [?b ...]
+                               :in $ ?eid ?title [?tag-id ...]
+                               :where
+                               [?b :block/title ?title]
+                               [?b :block/tags ?tag-id]
+                               [(not= ?b ?eid)]
+                               [(missing? $ ?b :block/type)]]
+                             db
+                             (:db/id entity)
+                             new-title
+                             (map :db/id tags)))]
+      (throw (ex-info "Duplicate block by tag"
+                      {:type :notification
+                       :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 :warning}})))))
+
+(defn validate-block-title
+  "Validates a block title when it has changed"
+  [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))

+ 8 - 8
deps/outliner/test/logseq/outliner/core_test.cljs → deps/outliner/test/logseq/outliner/validate_test.cljs

@@ -1,10 +1,10 @@
-(ns logseq.outliner.core-test
+(ns logseq.outliner.validate-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]))
+            [logseq.outliner.validate :as outliner-validate]))
 
 (defn- create-conn-with-blocks [opts]
   (let [conn (d/create-conn db-schema/schema-for-db-based-graph)
@@ -30,13 +30,13 @@
       (is (thrown-with-msg?
            js/Error
            #"Duplicate page by tag"
-           (outliner-core/validate-unique-by-name-tag-and-block-type
+           (outliner-validate/validate-unique-by-name-tag-and-block-type
             @conn
             "Apple"
             (assoc (find-block-by-content conn "Apple") :db/id 10000)))
           "Disallow duplicate page with tag")
       (is (nil?
-           (outliner-core/validate-unique-by-name-tag-and-block-type
+           (outliner-validate/validate-unique-by-name-tag-and-block-type
             @conn
             "Apple"
             (find-block-by-content conn "Banana")))
@@ -45,7 +45,7 @@
       (is (thrown-with-msg?
            js/Error
            #"Duplicate page without tag"
-           (outliner-core/validate-unique-by-name-tag-and-block-type
+           (outliner-validate/validate-unique-by-name-tag-and-block-type
             @conn
             "page1"
             (assoc (find-block-by-content conn "page1") :db/id 10000)))
@@ -59,7 +59,7 @@
                            {:block/title "Chicago" :build/tags [:Musical]}]}])]
 
       (is (nil?
-           (outliner-core/validate-unique-by-name-tag-and-block-type
+           (outliner-validate/validate-unique-by-name-tag-and-block-type
             @conn
             "yahoo"
             (find-block-by-content conn "yahoo")))
@@ -68,13 +68,13 @@
       (is (thrown-with-msg?
            js/Error
            #"Duplicate block by tag"
-           (outliner-core/validate-unique-by-name-tag-and-block-type
+           (outliner-validate/validate-unique-by-name-tag-and-block-type
             @conn
             "Sing Sing"
             (assoc (find-block-by-content conn "Sing Sing") :db/id 10000)))
           "Disallow duplicate page with tag")
       (is (nil?
-           (outliner-core/validate-unique-by-name-tag-and-block-type
+           (outliner-validate/validate-unique-by-name-tag-and-block-type
             @conn
             "Sing Sing"
             (find-block-by-content conn "Chicago")))

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

@@ -7,7 +7,7 @@
             [frontend.handler.notification :as notification]
             [frontend.state :as state]
             [frontend.modules.outliner.ui :as ui-outliner-tx]
-            [logseq.outliner.core :as outliner-core]
+            [logseq.outliner.validate :as outliner-validate]
             [logseq.db.frontend.class :as db-class]
             [logseq.common.util :as common-util]
             [logseq.common.util.page-ref :as page-ref]
@@ -19,7 +19,7 @@
    When returning false, this fn also displays appropriate notifications to the user"
   [repo block tag-entity]
   (try
-    (outliner-core/validate-unique-by-name-tag-and-block-type
+    (outliner-validate/validate-unique-by-name-tag-and-block-type
      (db/get-db repo)
      (:block/title block)
      (update block :block/tags (fnil conj #{}) tag-entity))