From 61db29cebaf2a36cfe76269a8943920c24f8d4f6 Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Thu, 9 May 2024 10:00:36 +0800 Subject: [PATCH] Remove [:block/schema :cardinality] Uses :db/cardinality instead. --- .../src/logseq/db/frontend/malli_schema.cljs | 14 ++-- deps/db/src/logseq/db/frontend/property.cljs | 4 + .../src/logseq/db/frontend/property/type.cljs | 4 +- deps/db/src/logseq/db/sqlite/util.cljs | 2 +- src/main/frontend/components/property.cljs | 2 +- .../frontend/components/property/value.cljs | 29 +++---- .../frontend/handler/db_based/property.cljs | 80 ++++--------------- .../handler/db_based/property_test.cljs | 5 +- 8 files changed, 50 insertions(+), 90 deletions(-) diff --git a/deps/db/src/logseq/db/frontend/malli_schema.cljs b/deps/db/src/logseq/db/frontend/malli_schema.cljs index c06764541..c60c4e1a7 100644 --- a/deps/db/src/logseq/db/frontend/malli_schema.cljs +++ b/deps/db/src/logseq/db/frontend/malli_schema.cljs @@ -77,8 +77,10 @@ (and (validate-fn' val) (contains? (set (map :db/id (:property/closed-values property))) val))) validate-fn')] - (if (= (:cardinality schema) :many) - (every? validate-fn'' property-val) + (if (db-property/many? property) + (if (coll? property-val) + (every? validate-fn'' property-val) + (validate-fn'' property-val)) (or (validate-fn'' property-val) (if (= :db.type/ref (:db/valueType property)) (and (integer? property-val) @@ -104,7 +106,7 @@ (update m :block/properties (fnil conj []) [(assoc (select-keys property [:db/ident :db/valueType]) :block/schema - (select-keys (:block/schema property) [:type :cardinality]) + (select-keys (:block/schema property) [:type]) :property/closed-values ;; use explicit call to be nbb compatible (entity-plus/lookup-kv-then-entity property :property/closed-values)) @@ -141,7 +143,7 @@ (map (fn [[k v]] (if-let [property (and (db-property/property? k) (entity-fn' k))] - (if (and (= :db.cardinality/many (:db/cardinality property)) + (if (and (db-property/many? property) (not (set? v))) ;; Fix :many property values that only had one value [k #{v}] @@ -245,9 +247,7 @@ (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 + [;; For closed values [:position {:optional true} :string]]) (def property-common-schema-attrs diff --git a/deps/db/src/logseq/db/frontend/property.cljs b/deps/db/src/logseq/db/frontend/property.cljs index cdf2cb242..36d5870fb 100644 --- a/deps/db/src/logseq/db/frontend/property.cljs +++ b/deps/db/src/logseq/db/frontend/property.cljs @@ -302,3 +302,7 @@ (:block/page block) ;; not closed value (not (some? (:block/content block))))) + +(defn many? + [property] + (= (:db/cardinality property) :db.cardinality/many)) diff --git a/deps/db/src/logseq/db/frontend/property/type.cljs b/deps/db/src/logseq/db/frontend/property/type.cljs index 4f16aa479..997d4d223 100644 --- a/deps/db/src/logseq/db/frontend/property/type.cljs +++ b/deps/db/src/logseq/db/frontend/property/type.cljs @@ -43,8 +43,8 @@ :number #{:cardinality} :date #{:cardinality} :url #{:cardinality} - :page #{:cardinality} - :template #{} + :page #{:cardinality :classes} + :template #{:classes} :checkbox #{}})) (assert (= (set user-built-in-property-types) (set (keys user-built-in-allowed-schema-attributes))) diff --git a/deps/db/src/logseq/db/sqlite/util.cljs b/deps/db/src/logseq/db/sqlite/util.cljs index 287346c77..aa36f7925 100644 --- a/deps/db/src/logseq/db/sqlite/util.cljs +++ b/deps/db/src/logseq/db/sqlite/util.cljs @@ -90,7 +90,7 @@ {:db/ident db-ident' :block/type "property" :block/format :markdown - :block/schema (merge {:type :default} (dissoc prop-schema :classes)) + :block/schema (merge {:type :default} (dissoc prop-schema :classes :cardinality)) :block/name (common-util/page-name-sanity-lc (name prop-name)) :block/uuid (or block-uuid (d/squuid)) :block/original-name (name prop-name) diff --git a/src/main/frontend/components/property.cljs b/src/main/frontend/components/property.cljs index 0df8a5349..52b129434 100644 --- a/src/main/frontend/components/property.cljs +++ b/src/main/frontend/components/property.cljs @@ -274,7 +274,7 @@ (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)))] + (let [many? (db-property/many? property)] (shui/checkbox {:checked many? :disabled disabled? diff --git a/src/main/frontend/components/property/value.cljs b/src/main/frontend/components/property/value.cljs index 4b6c42d4a..23167387b 100644 --- a/src/main/frontend/components/property/value.cljs +++ b/src/main/frontend/components/property/value.cljs @@ -73,6 +73,7 @@ ([block property-key property-value] (> selected-choices' @@ -457,7 +458,7 @@ [block property value-block block-cp editor-box & {:keys [closed-values?]}] (if (and (:block/uuid value-block) (state/sub-async-query-loading (:block/uuid value-block))) [:div.text-sm.opacity-70 "loading"] - (let [multiple-values? (= (:db/cardinality property) :db/cardinality.many)] + (let [multiple-values? (db-property/many? property)] (if value-block [:div.property-block-container.content (block-cp [value-block] {:id (str (if multiple-values? @@ -678,7 +679,7 @@ editor-box] :as opts}] (let [schema (:block/schema property) - multiple-values? (= :many (:cardinality schema)) + multiple-values? (db-property/many? property) class (str (when-not row? "flex flex-1 ") (when multiple-values? "property-value-content")) type (:type schema) @@ -840,7 +841,7 @@ editor-id (str dom-id "-editor") schema (:block/schema property) type (some-> schema (get :type :default)) - multiple-values? (= :many (:cardinality schema)) + multiple-values? (db-property/many? property) empty-value? (= :logseq.property/empty-placeholder v) editor-args {:block property :parent-block block diff --git a/src/main/frontend/handler/db_based/property.cljs b/src/main/frontend/handler/db_based/property.cljs index 08899f2de..53a961f6c 100644 --- a/src/main/frontend/handler/db_based/property.cljs +++ b/src/main/frontend/handler/db_based/property.cljs @@ -138,9 +138,10 @@ (cond-> [] (seq changed-property-attrs) (conj (outliner-core/block-with-updated-at - (merge {:db/ident db-ident} changed-property-attrs))) + (merge {:db/ident db-ident} + (common-util/dissoc-in changed-property-attrs [:block/schema :cardinality])))) (or (not= (:type schema) (get-in property [:block/schema :type])) - (not= (:cardinality schema) (get-in property [:block/schema :cardinality])) + (and (:cardinality schema) (not= (:cardinality schema) (keyword (name (:db/cardinality property))))) (and (= :default (:type schema)) (not= :db.type/ref (:db/valueType property))) (seq (:property/closed-values property))) (conj (update-datascript-schema property schema))) @@ -149,8 +150,7 @@ (mapcat (fn [[property-id v]] (build-property-value-tx-data property property-id v)) properties))) - many->one? (and (= (:db/cardinality property) :db.cardinality/many) - (= :one (:cardinality schema)))] + many->one? (and (db-property/many? property) (= :one (:cardinality schema)))] (when (seq tx-data) (db/transact! repo tx-data {:outliner-op :update-property :property-id (:db/id property) @@ -169,48 +169,6 @@ [schema value] (me/humanize (mu/explain-data schema value))) -(defn- set-block-property-multiple-values! - "Sets values for a :many property. Most calls to this fn come from components - that provide all existing values when updating. If this fn is called with a - single value it's because it came from a component that doesn't have existing - values. In this case the existing values are fetched and added to the new - single value e.g. adding a new date" - [repo block property one-or-many-values] - (let [property-id (:db/ident property) - values (if (coll? one-or-many-values) - one-or-many-values - (cond->> (some->> (:db/ident property) (get block)) - (= :db.type/ref (:db/valueType property)) - (mapv :db/id) - ;; single value means add to existing values - true - (into [one-or-many-values]) - true - (remove nil?)))] - (when (seq values) - (let [property-schema (:block/schema property) - property-type (:type property-schema) - schema (get-property-value-schema property-type property) - values' (try - (set (map #(convert-property-input-string property-type %) values)) - (catch :default e - (notification/show! (str e) :error false) - nil)) - old-values (get block (:db/ident property)) - deleted-values (remove values' old-values)] - (when (not= old-values values') - (if-let [msg (validate-property-value schema values')] - (let [msg' (str "\"" (:block/original-name property) "\"" " " (if (coll? msg) (first msg) msg))] - (notification/show! msg' :warning)) - (do - (upsert-property! repo property-id (assoc property-schema :type property-type) {}) - (let [tx-data (concat - (map (fn [v] - (let [v' (if (map? v) (:db/id v) v)] - [:db/retract (:db/id block) property-id v'])) deleted-values) - (build-property-value-tx-data block property-id values' false))] - (db/transact! repo tx-data {:outliner-op :save-block}))))))))) - (defn- resolve-tag "Change `v` to a tag's db id if v is a string tag, e.g. `#book`" [v] @@ -246,7 +204,7 @@ property (db/entity property-id) k-name (:block/original-name property) property-schema (:block/schema property) - {:keys [type cardinality]} property-schema + {:keys [type]} property-schema v' (or (resolve-tag v) v) db-attribute? (contains? db-property/db-attribute-properties property-id)] (cond @@ -254,9 +212,6 @@ (db/transact! repo [{:db/id (:db/id block) property-id v'}] {:outliner-op :save-block}) - (= cardinality :many) - (set-block-property-multiple-values! repo block property v') - :else (let [v'' (if property v' (or v' ""))] (when (some? v'') @@ -336,13 +291,13 @@ (let [type (:type (:block/schema property)) infer-schema (when-not type (infer-schema-from-input-string v)) property-type (or type infer-schema :default) - {:keys [cardinality]} (:block/schema property) + many? (db-property/many? property) status? (= :logseq.task/status (:db/ident property)) txs (->> (mapcat (fn [eid] (when-let [block (db/entity eid)] - (when (and (some? v) (not= cardinality :many)) + (when (and (some? v) (not many?)) (when-let [v* (try (convert-property-input-string property-type v) (catch :default e @@ -396,17 +351,16 @@ (when block (when (not= property-id (:db/ident block)) (when-let [property (db/entity property-id)] - (let [schema (:block/schema property)] - (if (= :many (:cardinality schema)) - (db/transact! repo - [[:db/retract (:db/id block) property-id property-value]] - {:outliner-op :save-block}) - (if (= :default (get-in property [:block/schema :type])) - (set-block-property! repo (:db/id block) - (:db/ident property) - "" - {}) - (remove-block-property! repo (:db/id block) property-id)))))))) + (if (db-property/many? property) + (db/transact! repo + [[:db/retract (:db/id block) property-id property-value]] + {:outliner-op :save-block}) + (if (= :default (get-in property [:block/schema :type])) + (set-block-property! repo (:db/id block) + (:db/ident property) + "" + {}) + (remove-block-property! repo (:db/id block) property-id))))))) (defn collapse-expand-property! "Notice this works only if the value itself if a block (property type should be either :default or :template)" diff --git a/src/test/frontend/handler/db_based/property_test.cljs b/src/test/frontend/handler/db_based/property_test.cljs index b1aa950b9..230d6c88f 100644 --- a/src/test/frontend/handler/db_based/property_test.cljs +++ b/src/test/frontend/handler/db_based/property_test.cljs @@ -5,7 +5,8 @@ [frontend.test.helper :as test-helper] [datascript.core :as d] [frontend.state :as state] - [frontend.handler.page :as page-handler])) + [frontend.handler.page :as page-handler] + [logseq.db.frontend.property :as db-property])) (def repo test-helper/test-db-name-db-version) @@ -222,7 +223,7 @@ (let [repo (state/get-current-repo)] (db-property-handler/upsert-property! repo nil {:type :default} {:property-name "p0"}) (db-property-handler/upsert-property! repo :user.property/p0 {:type :default :cardinality :many} {}) - (is (= :many (get-in (db/entity repo :user.property/p0) [:block/schema :cardinality]))))) + (is (db-property/many? (db/entity repo :user.property/p0))))) (testing "Multiple properties that generate the same initial :db/ident" (let [repo (state/get-current-repo)] (db-property-handler/upsert-property! repo nil {:type :default} {:property-name "p1"})