diff --git a/deps/db/src/logseq/db/frontend/malli_schema.cljs b/deps/db/src/logseq/db/frontend/malli_schema.cljs index 1920de57a6..a91d2e6b93 100644 --- a/deps/db/src/logseq/db/frontend/malli_schema.cljs +++ b/deps/db/src/logseq/db/frontend/malli_schema.cljs @@ -118,10 +118,9 @@ page-attrs page-or-block-attrs))) -(def property-schema-attrs - [[:hide? {:optional true} :boolean] - [:description {:optional true} :string] - ;; For any types except for :checkbox :default :template +(def property-type-schema-attrs + "Property :schema attributes that vary by :type" + [;; For any types except for :checkbox :default :template [:cardinality {:optional true} [:enum :one :many]] ;; For closed values [:values {:optional true} [:vector :uuid]] @@ -130,28 +129,47 @@ ;; For :page and :template [:classes {:optional true} [:set [:or :uuid :keyword]]]]) +(def property-common-schema-attrs + "Property :schema attributes common to all properties" + [[:hide? {:optional true} :boolean] + [:description {:optional true} :string]]) + (def internal-property (vec (concat [:map [:block/schema - (into [:map - [:type (apply vector :enum (into db-property-type/internal-builtin-schema-types - db-property-type/user-builtin-schema-types))]] - property-schema-attrs)]] + (vec + (concat + [:map + [:type (apply vector :enum (into db-property-type/internal-builtin-schema-types + db-property-type/user-builtin-schema-types))]] + property-common-schema-attrs + property-type-schema-attrs))]] page-attrs page-or-block-attrs))) +(def user-property-schema + (into + [:multi {:dispatch :type}] + (map + (fn [prop-type] + [prop-type + (vec + (concat + [:map + ;; Once a schema is defined it must have :type as this is an irreversible decision + [:type :keyword]] + property-common-schema-attrs + (remove #(not (db-property-type/property-type-allows-schema-attribute? prop-type (first %))) + property-type-schema-attrs)))]) + db-property-type/user-builtin-schema-types))) + (def user-property (vec (concat [:map - [:block/schema - {:optional true} - (into [:map - ;; Once a schema is defined it must have :type as this is an irreversible decision - [:type (apply vector :enum db-property-type/user-builtin-schema-types)]] - property-schema-attrs)]] + [:block/schema {:optional true} user-property-schema]] page-attrs page-or-block-attrs))) diff --git a/deps/db/src/logseq/db/frontend/property/type.cljs b/deps/db/src/logseq/db/frontend/property/type.cljs index 01f0b0045d..f733cda013 100644 --- a/deps/db/src/logseq/db/frontend/property/type.cljs +++ b/deps/db/src/logseq/db/frontend/property/type.cljs @@ -1,8 +1,14 @@ (ns logseq.db.frontend.property.type - "Provides property types including fns to validate them" + "Provides property types and related helper fns e.g. property value validation + fns and their allowed schema attributes" (:require [datascript.core :as d] [clojure.set :as set])) +;; Config vars +;; =========== +;; These vars enumerate all known property types and their associated behaviors +;; except for validation which is in its own section + (def internal-builtin-schema-types "Valid schema :type only to be used by built-in-properties" #{:keyword :map :coll :any}) @@ -18,6 +24,22 @@ (assert (set/subset? closed-values-schema-types (set user-builtin-schema-types)) "All closed value types are valid property types") +(def ^:private user-built-in-allowed-schema-attributes + "Map of types to their set of allowed :schema attributes" + (merge-with into + (zipmap closed-values-schema-types (repeat #{:values :position})) + {:number #{:cardinality} + :date #{:cardinality} + :url #{:cardinality} + :page #{:cardinality :classes} + :template #{:classes} + :checkbox #{}})) + +(assert (= (set user-builtin-schema-types) (set (keys user-built-in-allowed-schema-attributes))) + "Each user built in type should have an allowed schema attribute") + +;; Property value validation +;; ========================= ;; TODO: ;; Validate && list fixes for non-validated values when updating property schema @@ -69,6 +91,7 @@ (type-validate-fn value)))) (def builtin-schema-types + "Map of types to malli fns that validate a property value for that type" {:default [:fn {:error/message "should be a text"} ;; uuid check needed for property block values @@ -97,9 +120,6 @@ :coll coll? :any some?}) -(def property-types-with-cardinality - #{:number :date :url :page}) - (def property-types-with-db "Property types whose validation fn requires a datascript db" #{:date :page :template}) @@ -108,3 +128,11 @@ (into internal-builtin-schema-types user-builtin-schema-types)) "Built-in schema types must be equal") + +;; Helper fns +;; ========== +(defn property-type-allows-schema-attribute? + "Returns boolean to indicate if property type allows the given :schema attribute" + [property-type schema-attribute] + (contains? (get user-built-in-allowed-schema-attributes property-type) + schema-attribute)) diff --git a/scripts/src/logseq/tasks/db_graph/create_graph_with_properties.cljs b/scripts/src/logseq/tasks/db_graph/create_graph_with_properties.cljs index 4970e0f993..9d5575500d 100644 --- a/scripts/src/logseq/tasks/db_graph/create_graph_with_properties.cljs +++ b/scripts/src/logseq/tasks/db_graph/create_graph_with_properties.cljs @@ -4,6 +4,7 @@ NOTE: This script is also used in CI to confirm graph creation works" (:require [logseq.tasks.db-graph.create-graph :as create-graph] [logseq.db.sqlite.util :as sqlite-util] + [logseq.db.frontend.property.type :as db-property-type] [clojure.string :as string] [datascript.core :as d] ["path" :as node-path] @@ -70,7 +71,7 @@ :properties (->> [:default :url :checkbox :number :page :date] (mapcat #(cond-> [[% {:block/schema {:type %}}]] - (not (#{:checkbox :default} %)) + (db-property-type/property-type-allows-schema-attribute? % :cardinality) (conj [(keyword (str (name %) "-many")) {:block/schema {:type % :cardinality :many}}]))) (into {}))})) diff --git a/src/main/frontend/components/property.cljs b/src/main/frontend/components/property.cljs index b956f19a8c..1cae60d03c 100644 --- a/src/main/frontend/components/property.cljs +++ b/src/main/frontend/components/property.cljs @@ -189,15 +189,17 @@ (ui/select schema-types (fn [_e v] (let [type (keyword (string/lower-case v)) - update-schema-fn (comp - (if (contains? db-property-type/property-types-with-cardinality type) - identity - #(dissoc % :cardinality)) - #(assoc % :type type))] + update-schema-fn (apply comp + #(assoc % :type type) + (keep + (fn [attr] + (when-not (db-property-type/property-type-allows-schema-attribute? type attr) + #(dissoc % attr))) + [:values :position :cardinality :classes]))] (swap! *property-schema update-schema-fn) (components-pu/update-property! property @*property-name @*property-schema))))]))] - (when (contains? db-property-type/property-types-with-cardinality (:type @*property-schema)) + (when (db-property-type/property-type-allows-schema-attribute? (:type @*property-schema) :cardinality) [:div.grid.grid-cols-4.gap-1.items-center.leading-8 [:label "Multiple values:"] (let [many? (boolean (= :many (:cardinality @*property-schema)))] @@ -208,27 +210,28 @@ (save-property-fn))}))]) - (case (:type @*property-schema) - :page - (when (empty? (:values @*property-schema)) - [:div.grid.grid-cols-4.gap-1.items-center.leading-8 - [:label "Specify classes:"] - (class-select *property-schema - (:classes @*property-schema) - (assoc opts - :disabled? disabled? - :save-property-fn save-property-fn))]) + (when (db-property-type/property-type-allows-schema-attribute? (:type @*property-schema) :classes) + (case (:type @*property-schema) + :page + (when (empty? (:values @*property-schema)) + [:div.grid.grid-cols-4.gap-1.items-center.leading-8 + [:label "Specify classes:"] + (class-select *property-schema + (:classes @*property-schema) + (assoc opts + :disabled? disabled? + :save-property-fn save-property-fn))]) - :template - [:div.grid.grid-cols-4.gap-1.items-center.leading-8 - [:label "Specify template:"] - (class-select *property-schema (:classes @*property-schema) - (assoc opts - :multiple-choices? false - :disabled? disabled? - :save-property-fn save-property-fn))] + :template + [:div.grid.grid-cols-4.gap-1.items-center.leading-8 + [:label "Specify template:"] + (class-select *property-schema (:classes @*property-schema) + (assoc opts + :multiple-choices? false + :disabled? disabled? + :save-property-fn save-property-fn))] - nil) + nil)) (when (and enable-closed-values? (empty? (:classes @*property-schema))) [:div.grid.grid-cols-4.gap-1.items-start.leading-8 diff --git a/src/test/frontend/handler/db_based/property_test.cljs b/src/test/frontend/handler/db_based/property_test.cljs index 62e576f681..68b96a073b 100644 --- a/src/test/frontend/handler/db_based/property_test.cljs +++ b/src/test/frontend/handler/db_based/property_test.cljs @@ -98,17 +98,17 @@ 2))) (testing "Add a multi-values property" - (db-property-handler/upsert-property! repo "property-3" {:type :default :cardinality :many} {}) - (db-property-handler/set-block-property! repo fbid "property-3" "value 1" {}) - (db-property-handler/set-block-property! repo fbid "property-3" "value 2" {}) - (db-property-handler/set-block-property! repo fbid "property-3" "value 3" {}) + (db-property-handler/upsert-property! repo "property-3" {:type :number :cardinality :many} {}) + (db-property-handler/set-block-property! repo fbid "property-3" 1 {}) + (db-property-handler/set-block-property! repo fbid "property-3" 2 {}) + (db-property-handler/set-block-property! repo fbid "property-3" 3 {}) (let [block (db/entity [:block/uuid fbid]) properties (:block/properties block) property (db/entity [:block/name "property-3"])] ;; ensure property exists (are [x y] (= x y) (:block/schema property) - {:type :default :cardinality :many} + {:type :number :cardinality :many} (:block/type property) #{"property"}) ;; check block's properties @@ -116,10 +116,10 @@ (count properties) 3 (get properties (:block/uuid property)) - #{"value 1" "value 2" "value 3"})) + #{1 2 3})) - ;; update property value from "value 1" to "value 4" - (db-property-handler/set-block-property! repo fbid "property-3" "value 4" {:old-value "value 1"}) + ;; update property value from 1 to 4 + (db-property-handler/set-block-property! repo fbid "property-3" 4 {:old-value 1}) (let [block (db/entity [:block/uuid fbid]) properties (:block/properties block) property (db/entity [:block/name "property-3"])] @@ -128,7 +128,7 @@ (count properties) 3 (get properties (:block/uuid property)) - #{"value 4" "value 2" "value 3"}) + #{4 2 3}) (db-property-handler/delete-property-value! repo block (:block/uuid property) "value 4") (let [properties (:block/properties block)]