fix: closed value validation

A closed value must be a UUID and exists in the predefined schema
values from its property.

Related to LOG-2871.

deps/db/src/logseq/db/frontend/property/type.cljs

@@ -43,27 +43,45 @@
   (and (uuid? id)
        (some? (d/entity db [:block/uuid id]))))
 
-(defn- text-or-uuid?
-  [value]
-  (or (string? value) (uuid? value)))
+(defn- exist-closed-value?
+  [db property type-validate-fn value]
+  (boolean
+   (when-let [e (and (uuid? value)
+                     (d/entity db [:block/uuid value]))]
+     (let [values (get-in property [:block/schema :values])]
+       (and
+        (contains? (set values) value)
+        (contains? (:block/type e) "closed value")
+        (type-validate-fn (:value (:block/schema e))))))))
+
+(defn type-or-closed-value?
+  "The `value` could be either a closed value (when `property` has pre-defined values) or it can be validated by `type-validate-fn`.
+  Args:
+  `new-closed-value?`: a new value will be added, so we'll check it using `type-validate-fn`."
+  [type-validate-fn]
+  (fn [db property value new-closed-value?]
+    (if (and (seq (get-in property [:block/schema :values]))
+             (not new-closed-value?))
+      (exist-closed-value? db property type-validate-fn value)
+      (type-validate-fn value))))
 
 (def builtin-schema-types
   {:default  [:fn
-              {:error/message "should be a text or UUID"}
-              text-or-uuid?]                     ; refs/tags will not be extracted
+              {:error/message "should be a text"}
+              (type-or-closed-value? string?)]                     ; refs/tags will not be extracted
    :number   [:fn
-              {:error/message "should be a number or UUID"}
-              (some-fn number? uuid?)]
+              {:error/message "should be a number"}
+              (type-or-closed-value? number?)]
    :date     [:fn
               {:error/message "should be a journal date"}
-              logseq-page?]
+              (type-or-closed-value? logseq-page?)]
    :checkbox boolean?
    :url      [:fn
-              {:error/message "should be a URL or UUID"}
-              (some-fn url? uuid?)]
+              {:error/message "should be a URL"}
+              (type-or-closed-value? url?)]
    :page     [:fn
               {:error/message "should be a page"}
-              logseq-page?]
+              (type-or-closed-value? logseq-page?)]
    :template [:fn
               {:error/message "should has #template"}
               logseq-template?]
@@ -76,7 +94,7 @@
 
 (def property-types-with-db
   "Property types whose validation fn requires a datascript db"
-  #{:date :page :template})
+  #{:default :number :date :url :page :template})
 
 (assert (= (set (keys builtin-schema-types))
            (into internal-builtin-schema-types

src/main/frontend/handler/db_based/property.cljs

@@ -18,14 +18,16 @@
 ;; schema -> type, cardinality, object's class
 ;;           min, max -> string length, number range, cardinality size limit
 
-(def builtin-schema-types
+(defn builtin-schema-types
   "A frontend version of builtin-schema-types that adds the current database to
    schema fns"
+  [property & {:keys [new-closed-value?]
+               :or {new-closed-value? false}}]
   (into {}
         (map (fn [[property-type property-val-schema]]
                (if (db-property-type/property-types-with-db property-type)
                  (let [[_ schema-opts schema-fn] property-val-schema]
-                   [property-type [:fn schema-opts #(schema-fn (db/get-db) %)]])
+                   [property-type [:fn schema-opts #(schema-fn (db/get-db) property % new-closed-value?)]])
                  [property-type property-val-schema]))
              db-property-type/builtin-schema-types)))
 
@@ -93,13 +95,9 @@
                              (assoc :block/schema schema)))]
                     {:outliner-op :insert-blocks}))))
 
-(defn- validate-property-value
-  [property schema value]
-  (let [values (get-in property [:block/schema :values])]
-    (if (seq values)
-      (when-not (contains? (set values) value)
-        "Value is not included in the closed values.")
-      (me/humanize (mu/explain-data schema value)))))
+(defn validate-property-value
+  [schema value]
+  (me/humanize (mu/explain-data schema value)))
 
 (defn- reset-block-property-multiple-values!
   [repo block-id k-name values _opts]
@@ -114,7 +112,7 @@
     (when (and multiple-values? (seq values))
       (let [infer-schema (when-not type (infer-schema-from-input-string (first values)))
             property-type (or type infer-schema :default)
-            schema (get builtin-schema-types property-type)
+            schema (get (builtin-schema-types property) property-type)
             properties (:block/properties block)
             values' (try
                       (set (map #(convert-property-input-string property-type %) values))
@@ -139,7 +137,7 @@
                              {:block/uuid block-id
                               attribute property-value-ids}]
                             {:outliner-op :save-block}))
-            (if-let [msg (some #(validate-property-value property schema %) values')]
+            (if-let [msg (some #(validate-property-value schema %) values')]
               (let [msg' (str "\"" k-name "\"" " " (if (coll? msg) (first msg) msg))]
                 (notification/show! msg' :warning))
               (do
@@ -191,7 +189,7 @@
         (when (some? v)
           (let [infer-schema (when-not type (infer-schema-from-input-string v))
                 property-type (or type infer-schema :default)
-                schema (get builtin-schema-types property-type)
+                schema (get (builtin-schema-types property) property-type)
                 properties (:block/properties block)
                 value (get properties property-uuid)
                 v* (try
@@ -212,7 +210,7 @@
                               [[:db/add (:db/id block) attribute property-value-id]]
                               {:outliner-op :save-block}))
               (when-not (contains? (if (set? value) value #{value}) v*)
-                (if-let [msg (validate-property-value property schema v*)]
+                (if-let [msg (validate-property-value schema v*)]
                   (let [msg' (str "\"" k-name "\"" " " (if (coll? msg) (first msg) msg))]
                     (notification/show! msg' :warning))
                   (do

src/main/frontend/handler/property.cljs

@@ -244,83 +244,92 @@
   "id should be a block UUID or nil"
   [property {:keys [id value icon description]}]
   (assert (or (nil? id) (uuid? id)))
-  (when (contains? db-property-type/closed-values-schema-types (get-in property [:block/schema :type] :default))
-    (let [value (if (string? value) (string/trim value) value)
-          property-schema (:block/schema property)
-          closed-values (:values property-schema)
-          block-values (map (fn [id] (db/entity [:block/uuid id])) closed-values)
-          resolved-value (try
-                           (db-property-handler/convert-property-input-string (:type property-schema) value)
-                           (catch :default e
-                             (js/console.error e)
-                             (notification/show! (str e) :error false)
-                             nil))
-          block (when id (db/entity [:block/uuid id]))
-          value-block (when (uuid? value) (db/entity [:block/uuid value]))]
-      (cond
-        (nil? resolved-value)
-        nil
-
-        (some (fn [b] (and (= resolved-value (get-in b [:block/metadata :value]))
-                           (not= id (:block/uuid b)))) block-values)
-        (do
-          (notification/show! "Choice already exists" :warning)
-          :value-exists)
-
-        (:block/name value-block)             ; page
-        (let [new-values (vec (conj closed-values value))]
-          {:block-id value
-           :tx-data [{:db/id (:db/id property)
-                      :block/schema (assoc property-schema :values new-values)}]})
-
-        :else
-        (let [block-id (or id (db/new-block-id))
-              icon-id (pu/get-pid "icon")
-              icon (when-not (and (string? icon) (string/blank? icon)) icon)
-              description (string/trim description)
-              description (when-not (string/blank? description) description)
-              tx-data (if block
-                        [(let [properties (:block/properties block)
-                               schema (assoc (:block/schema block)
-                                             :value resolved-value)]
-                           {:block/uuid id
-                            :block/properties (if icon
-                                                (assoc properties icon-id icon)
-                                                (dissoc properties icon-id))
-                            :block/schema (if description
-                                            (assoc schema :description description)
-                                            (dissoc schema :description))})]
-                        (let [page-name (str "$$$" (:block/uuid property))
-                              page-entity (db/entity [:block/name page-name])
-                              page (or page-entity
-                                       (-> (block/page-name->map page-name true)
-                                           (assoc :block/type #{"hidden"}
-                                                  :block/format :markdown)))
-                              page-tx (when-not page-entity page)
-                              page-id [:block/uuid (:block/uuid page)]
-                              metadata {:created-from-property (:block/uuid property)}
-                              new-block (cond->
-                                         {:block/type #{"closed value"}
-                                          :block/uuid block-id
-                                          :block/page page-id
-                                          :block/metadata metadata
-                                          :block/schema {:value resolved-value}
-                                          :block/parent page-id}
-                                          icon
-                                          (assoc :block/properties {icon-id icon})
-
-                                          description
-                                          (update :block/schema assoc :description description)
-
-                                          true
-                                          outliner-core/block-with-timestamps)
-                              new-values (vec (conj closed-values block-id))]
-                          (->> (cons page-tx [new-block
-                                              {:db/id (:db/id property)
-                                               :block/schema (assoc property-schema :values new-values)}])
-                               (remove nil?))))]
-          {:block-id block-id
-           :tx-data tx-data})))))
+  (let [property-type (get-in property [:block/schema :type] :default)]
+    (when (contains? db-property-type/closed-values-schema-types property-type)
+     (let [value (if (string? value) (string/trim value) value)
+           property-schema (:block/schema property)
+           closed-values (:values property-schema)
+           block-values (map (fn [id] (db/entity [:block/uuid id])) closed-values)
+           resolved-value (try
+                            (db-property-handler/convert-property-input-string (:type property-schema) value)
+                            (catch :default e
+                              (js/console.error e)
+                              (notification/show! (str e) :error false)
+                              nil))
+           block (when id (db/entity [:block/uuid id]))
+           value-block (when (uuid? value) (db/entity [:block/uuid value]))
+           validate-message (db-property-handler/validate-property-value
+                             (get (db-property-handler/builtin-schema-types property {:new-closed-value? true}) property-type)
+                             value)]
+       (cond
+         (nil? resolved-value)
+         nil
+
+         (some (fn [b] (and (= resolved-value (get-in b [:block/metadata :value]))
+                            (not= id (:block/uuid b)))) block-values)
+         (do
+           (notification/show! "Choice already exists" :warning)
+           :value-exists)
+
+         validate-message
+         (do
+           (notification/show! validate-message :warning)
+           :value-invalid)
+
+         (:block/name value-block)             ; page
+         (let [new-values (vec (conj closed-values value))]
+           {:block-id value
+            :tx-data [{:db/id (:db/id property)
+                       :block/schema (assoc property-schema :values new-values)}]})
+
+         :else
+         (let [block-id (or id (db/new-block-id))
+               icon-id (pu/get-pid "icon")
+               icon (when-not (and (string? icon) (string/blank? icon)) icon)
+               description (string/trim description)
+               description (when-not (string/blank? description) description)
+               tx-data (if block
+                         [(let [properties (:block/properties block)
+                                schema (assoc (:block/schema block)
+                                              :value resolved-value)]
+                            {:block/uuid id
+                             :block/properties (if icon
+                                                 (assoc properties icon-id icon)
+                                                 (dissoc properties icon-id))
+                             :block/schema (if description
+                                             (assoc schema :description description)
+                                             (dissoc schema :description))})]
+                         (let [page-name (str "$$$" (:block/uuid property))
+                               page-entity (db/entity [:block/name page-name])
+                               page (or page-entity
+                                        (-> (block/page-name->map page-name true)
+                                            (assoc :block/type #{"hidden"}
+                                                   :block/format :markdown)))
+                               page-tx (when-not page-entity page)
+                               page-id [:block/uuid (:block/uuid page)]
+                               metadata {:created-from-property (:block/uuid property)}
+                               new-block (cond->
+                                          {:block/type #{"closed value"}
+                                           :block/uuid block-id
+                                           :block/page page-id
+                                           :block/metadata metadata
+                                           :block/schema {:value resolved-value}
+                                           :block/parent page-id}
+                                           icon
+                                           (assoc :block/properties {icon-id icon})
+
+                                           description
+                                           (update :block/schema assoc :description description)
+
+                                           true
+                                           outliner-core/block-with-timestamps)
+                               new-values (vec (conj closed-values block-id))]
+                           (->> (cons page-tx [new-block
+                                               {:db/id (:db/id property)
+                                                :block/schema (assoc property-schema :values new-values)}])
+                                (remove nil?))))]
+           {:block-id block-id
+            :tx-data tx-data}))))))
 
 (defn delete-closed-value
   [property item]