refactor: make all user-facing property types refs

deps/db/src/logseq/db/frontend/entity_plus.cljc

@@ -48,7 +48,8 @@
 
      :block/_parent
      (->> (lookup-entity e k default-value)
-          (remove (fn [e] (:logseq.property/created-from-property e)))
+          (remove (fn [e] (or (:logseq.property/created-from-property e)
+                              (:block/closed-value-property e))))
           seq)
 
      :block/_raw-parent

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

@@ -7,21 +7,22 @@
 (defonce hidden-page-name-prefix "$$$")
 
 (defn- closed-value-new-block
-  [page-id block-id value property]
-  {:block/type #{"closed value"}
-   :block/format :markdown
-   :block/uuid block-id
-   :block/page page-id
-   :block/content value
-   :block/closed-value-property (:db/ident property)
-   :block/parent page-id})
+  [block-id value property]
+  (let [property-id (:db/ident property)]
+    {:block/type #{"closed value"}
+     :block/format :markdown
+     :block/uuid block-id
+     :block/page property-id
+     :block/content value
+     :block/closed-value-property property-id
+     :block/parent property-id}))
 
 (defn build-closed-value-block
   "Builds a closed value block to be transacted"
-  [block-uuid block-value page-id property {:keys [db-ident icon description]}]
+  [block-uuid block-value property {:keys [db-ident icon description]}]
   (assert block-uuid (uuid? block-uuid))
   (cond->
-   (closed-value-new-block page-id block-uuid block-value property)
+   (closed-value-new-block block-uuid block-value property)
     (and db-ident (keyword? db-ident))
     (assoc :db/ident db-ident)
 
@@ -38,13 +39,6 @@
     true
     sqlite-util/block-with-timestamps))
 
-(defn build-property-hidden-page
-  "Builds a hidden property page for closed values to be transacted"
-  [property]
-  (let [page-name (str hidden-page-name-prefix (:block/uuid property))]
-    (-> (sqlite-util/build-new-page page-name)
-        (assoc :block/type #{"hidden"}))))
-
 (defn build-closed-values
   "Builds all the tx needed for property with closed values including
    the hidden page and closed value blocks as needed"
@@ -59,18 +53,16 @@
         ;; closed ref types don't have hidden tx
         (if ref-type?
           []
-          (let [page-tx (build-property-hidden-page property-tx)
-                closed-value-blocks-tx
+          (let [closed-value-blocks-tx
                 (map (fn [{:keys [db-ident value icon description uuid]}]
                        (cond->
                         (build-closed-value-block
                          uuid
                          value
-                         [:block/uuid (:block/uuid page-tx)]
                          property
                          {:db-ident db-ident :icon icon :description description})
                          (not from-ui-thread?)
                          (assoc :block/order (db-order/gen-key))))
                      (:closed-values property))]
-            (into [page-tx] closed-value-blocks-tx)))]
+            closed-value-blocks-tx))]
     (into [property-tx] hidden-tx)))

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

@@ -2,8 +2,7 @@
   "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]
-            [logseq.common.util.macro :as macro-util]))
+            [clojure.set :as set]))
 
 ;; Config vars
 ;; ===========
@@ -27,7 +26,7 @@
 
 (def ref-property-types
   "User facing ref types"
-  #{:default :page :date})
+  #{:default :page :date :number :url})
 
 (assert (set/subset? ref-property-types
                      (set user-built-in-property-types))
@@ -65,21 +64,16 @@
          (catch :default _e
            false))))
 
-(defn macro-url?
-  [s]
-  ;; TODO: Confirm that macro expanded value is url when it's easier to pass data into validations
-  (macro-util/macro? s))
-
 (defn- entity?
   [db id]
   (some? (d/entity db id)))
 
-(defn- url-or-closed-url?
+(defn- url-entity?
   [db val]
-  (or (url? val)
-      (macro-url? val)
-      (when-let [ent (d/entity db val)]
-        (url? (:block/content ent)))))
+  (or
+   (= val :logseq.property/empty-placeholder)
+   (when-let [ent (d/entity db val)]
+     (url? (:block/content ent)))))
 
 (defn- property-value-block?
   [db s]
@@ -104,6 +98,14 @@
       (when-let [entity (d/entity db s)]
         (string? (:block/content entity)))))
 
+(defn- number-entity?
+  [db id]
+  (or
+   (= id :logseq.property/empty-placeholder)
+   (when-let [entity (d/entity db id)]
+     (number? (some-> (:block/content entity)
+                      parse-double)))))
+
 (def built-in-validation-schemas
   "Map of types to malli validation schemas that validate a property value for that type"
   {:default  [:fn
@@ -115,14 +117,14 @@
    :number   [:fn
               {:error/message "should be a number"}
               ;; Also handles entity? so no need to use it
-              number?]
+              number-entity?]
    :date     [:fn
               {:error/message "should be a journal date"}
               date?]
    :checkbox boolean?
    :url      [:fn
               {:error/message "should be a URL"}
-              url-or-closed-url?]
+              url-entity?]
    :page     [:fn
               {:error/message "should be a page"}
               page?]
@@ -148,7 +150,7 @@
 
 (def property-types-with-db
   "Property types whose validation fn requires a datascript db"
-  #{:default :string :url :date :page :template :entity})
+  #{:default :string :url :number :date :page :template :entity})
 
 ;; Helper fns
 ;; ==========

src/main/frontend/components/property/value.cljs

@@ -25,7 +25,8 @@
             [logseq.db :as ldb]
             [logseq.db.frontend.property :as db-property]
             [datascript.impl.entity :as de]
-            [frontend.handler.property.util :as pu]))
+            [frontend.handler.property.util :as pu]
+            [logseq.db.frontend.property.type :as db-property-type]))
 
 (rum/defc property-empty-value
   [& {:as opts}]
@@ -67,6 +68,19 @@
   (state/set-state! :editor/editing-property-value-id {})
   (state/clear-edit!))
 
+(defn <create-new-block!
+  [block property value & {:keys [edit-block?]
+                           :or {edit-block? true}}]
+  (p/let [{:keys [block-id result]} (db-property-handler/create-property-text-block!
+                                     block property value editor-handler/wrap-parse-block {})]
+    (p/do!
+     result
+     (exit-edit-property)
+     (let [block (db/entity [:block/uuid block-id])]
+       (when edit-block?
+         (editor-handler/edit-block! block :max {:container-id :unknown-container}))
+       block))))
+
 (defn <add-property!
   "If a class and in a class schema context, add the property to its schema.
   Otherwise, add a block's property and its value. Creates a new property as needed"
@@ -90,7 +104,13 @@
                       (db-property/create-user-property-ident-from-name property-key))
                      :logseq.property/empty-placeholder])
                   [property-key property-value])]
-            (property-handler/set-block-property! repo (:block/uuid block) property-id property-value'))))
+            (p/let [property (db/entity property-key)
+                    value (if (and (db-property-type/ref-property-types (get-in property [:block/schema :type]))
+                                   (not (int? property-value')))
+                            (p/let [result (<create-new-block! block (db/entity property-id) property-value' {:edit-block? false})]
+                              (:db/id result))
+                            property-value')]
+              (property-handler/set-block-property! repo (:block/uuid block) property-id value)))))
       (when exit-edit?
         (shui/popup-hide!)
         (exit-edit-property))))))
@@ -354,16 +374,6 @@
                      :input-opts input-opts)]
     (select-page property opts')))
 
-(defn <create-new-block!
-  [block property value]
-  (p/let [{:keys [block-id result]} (db-property-handler/create-property-text-block!
-                                     block property value editor-handler/wrap-parse-block {})]
-    (p/do!
-     result
-     (exit-edit-property)
-     (let [block (db/entity [:block/uuid block-id])]
-       (editor-handler/edit-block! block :max {:container-id :unknown-container})))))
-
 (defn <create-new-block-from-template!
   "`template`: tag block"
   [block property template]
@@ -389,6 +399,7 @@
             property (db/sub-block (:db/id property))
             type (:type schema)
             closed-values? (seq (:property/closed-values property))
+            ref-type? (db-property-type/ref-property-types type)
             items (if closed-values?
                     (keep (fn [block]
                             (let [icon (pu/get-block-property-value block :logseq.property/icon)
@@ -404,6 +415,14 @@
                                    (if (coll? value)
                                      (map (fn [v] {:value v}) value)
                                      [{:value value}])))
+                         (map (fn [{:keys [value]}]
+                                (if (and ref-type? (number? value))
+                                  (when-let [e (db/entity value)]
+                                    {:label (or (:block/content e)
+                                                (:block/original-name e))
+                                     :value value})
+                                  {:label value
+                                   :vaue value})))
                          (distinct)))
             items (->> (if (= :date type)
                          (map (fn [m] (let [label (:block/original-name (db/entity (:value m)))]
@@ -421,33 +440,31 @@
                                     (remove nil?))
                                [selected-choices'])]
         (select-aux block property
-                    (cond->
-                     {:multiple-choices? multiple-choices?
-                      :items items
-                      :selected-choices selected-choices
-                      :dropdown? dropdown?
-                      :show-new-when-not-exact-match? (not (or closed-values? (= :date type)))
-                      :input-default-placeholder "Select"
-                      :extract-chosen-fn :value
-                      :content-props content-props
-                      :on-chosen on-chosen
-                      :input-opts (fn [_]
-                                    {:on-blur (fn []
-                                                (exit-edit-property)
-                                                (when-let [f (:on-chosen select-opts)] (f)))
-                                     :on-click (fn []
-                                                 (when *show-new-property-config?
-                                                   (reset! *show-new-property-config? false)))
-                                     :on-key-down
-                                     (fn [e]
-                                       (case (util/ekey e)
-                                         "Escape"
-                                         (do
-                                           (exit-edit-property)
-                                           (when-let [f (:on-chosen select-opts)] (f)))
-                                         nil))})}
-                      closed-values?
-                      (assoc :extract-fn :label)))))))
+                    {:multiple-choices? multiple-choices?
+                     :items items
+                     :selected-choices selected-choices
+                     :dropdown? dropdown?
+                     :show-new-when-not-exact-match? (not (or closed-values? (= :date type)))
+                     :input-default-placeholder "Select"
+                     :extract-chosen-fn :value
+                     :extract-fn :label
+                     :content-props content-props
+                     :on-chosen on-chosen
+                     :input-opts (fn [_]
+                                   {:on-blur (fn []
+                                               (exit-edit-property)
+                                               (when-let [f (:on-chosen select-opts)] (f)))
+                                    :on-click (fn []
+                                                (when *show-new-property-config?
+                                                  (reset! *show-new-property-config? false)))
+                                    :on-key-down
+                                    (fn [e]
+                                      (case (util/ekey e)
+                                        "Escape"
+                                        (do
+                                          (exit-edit-property)
+                                          (when-let [f (:on-chosen select-opts)] (f)))
+                                        nil))})})))))
 
 (rum/defc property-normal-block-value < rum/reactive db-mixins/query
   {:init (fn [state]
@@ -578,8 +595,9 @@
        closed-values?
        (closed-value-item value opts)
 
-       (= type :number)
-       [:span.number (str value)]
+       (de/entity? value)
+       (when-let [content (:block/content value)]
+         (inline-text {} :markdown (macro-util/expand-value-if-macro content (state/get-macros))))
 
        :else
        (inline-text {} :markdown (macro-util/expand-value-if-macro (str value) (state/get-macros))))]))
@@ -701,9 +719,7 @@
       :style {:min-height 24}
       :on-click (fn []
                   (when (and (= type :default) (nil? value))
-                    (<create-new-block! block property ""))
-                  (when (= type :string)
-                    (set-editing! block property editor-id dom-id value opts)))}
+                    (<create-new-block! block property "")))}
      (if (and (string/blank? value) template?)
        (when-let [template (first (:property/schema.classes schema))]
          [:a.fade-link.pointer.text-sm.jtrigger

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

@@ -29,9 +29,9 @@
 ;;           min, max -> string length, number range, cardinality size limit
 
 (defn- build-property-value-tx-data
-  ([block property-id value]
-   (build-property-value-tx-data block property-id value (= property-id :logseq.task/status)))
-  ([block property-id value status?]
+  ([db block property-id value]
+   (build-property-value-tx-data db block property-id value (= property-id :logseq.task/status)))
+  ([db block property-id value status?]
    (when (some? value)
      (let [block (assoc (outliner-core/block-with-updated-at {:db/id (:db/id block)})
                         property-id value)
@@ -86,16 +86,12 @@
     v-str))
 
 (defn- update-datascript-schema
-  [property {:keys [type cardinality values]}]
+  [property {:keys [cardinality]}]
   (let [ident (:db/ident property)
-        cardinality (if (= cardinality :many) :db.cardinality/many :db.cardinality/one)
-        type-data (when (and type (or (db-property-type/ref-property-types type) (seq values))) ; type changes or closed values
-                    {:db/ident ident
-                     :db/valueType :db.type/ref
-                     :db/cardinality cardinality})]
-    (or type-data
-        {:db/ident ident
-         :db/cardinality cardinality})))
+        cardinality (if (= cardinality :many) :db.cardinality/many :db.cardinality/one)]
+    {:db/ident ident
+     :db/valueType :db.type/ref
+     :db/cardinality cardinality}))
 
 (defn ensure-unique-db-ident
   "Ensures the given db-ident is unique. If a db-ident conflicts, it is made
@@ -124,7 +120,8 @@
    with the given property-id or :property-name option. When a property is created
    it is ensured to have a unique :db/ident"
   [repo property-id schema {:keys [property-name properties]}]
-  (let [db-ident (or property-id (db-property/create-user-property-ident-from-name property-name))]
+  (let [db-ident (or property-id (db-property/create-user-property-ident-from-name property-name))
+        db (db/get-db repo)]
     (assert (qualified-keyword? db-ident))
     (if-let [property (and (qualified-keyword? property-id) (db/entity db-ident))]
       (let [changed-property-attrs
@@ -149,7 +146,7 @@
                             (when (seq properties)
                               (mapcat
                                (fn [[property-id v]]
-                                 (build-property-value-tx-data property property-id v)) properties)))
+                                 (build-property-value-tx-data db property property-id v)) properties)))
             many->one? (and (db-property/many? property) (= :one (:cardinality schema)))]
         (when (seq tx-data)
           (db/transact! repo tx-data {:outliner-op :update-property
@@ -206,7 +203,8 @@
         property-schema (:block/schema property)
         {:keys [type]} property-schema
         v' (or (resolve-tag v) v)
-        db-attribute? (contains? db-property/db-attribute-properties property-id)]
+        db-attribute? (contains? db-property/db-attribute-properties property-id)
+        db (db/get-db repo)]
     (cond
       db-attribute?
       (db/transact! repo [{:db/id (:db/id block) property-id v'}]
@@ -245,7 +243,7 @@
                                     (vec (remove string/blank? new-value*))
                                     (set (remove string/blank? new-value*)))
                                   new-value*)
-                      tx-data (build-property-value-tx-data block property-id new-value status?)]
+                      tx-data (build-property-value-tx-data db block property-id new-value status?)]
                   (db/transact! repo tx-data {:outliner-op :save-block}))))))))))
 
 (defn class-add-property!
@@ -297,6 +295,7 @@
             property-type (or type infer-schema :default)
             many? (db-property/many? property)
             status? (= :logseq.task/status (:db/ident property))
+            db (db/get-db repo)
             txs (->>
                  (mapcat
                   (fn [eid]
@@ -307,7 +306,7 @@
                                         (catch :default e
                                           (notification/show! (str e) :error false)
                                           nil))]
-                          (build-property-value-tx-data block property-id v* status?)))))
+                          (build-property-value-tx-data db block property-id v* status?)))))
                   block-eids)
                  (remove nil?))]
         (when (seq txs)
@@ -481,12 +480,6 @@
     {:page page-tx
      :blocks [new-block]}))
 
-(defn- get-property-hidden-page
-  [property]
-  (let [page-name (str db-property-build/hidden-page-name-prefix (:block/uuid property))]
-    (or (db/get-case-page page-name)
-        (db-property-build/build-property-hidden-page property))))
-
 (defn re-init-commands!
   "Update commands after task status and priority's closed values has been changed"
   [property]
@@ -524,7 +517,6 @@
                                  (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 (validate-property-value
                                 (get-property-value-schema property-type property {:new-closed-value? true})
                                 resolved-value)]
@@ -545,10 +537,6 @@
           (nil? resolved-value)
           nil
 
-          (db/page? value-block)             ; page
-          {:block-id value
-           :tx-data [[:db/add [:block/uuid value] :property/value-property (:db/id property)]]}
-
           :else
           (let [block-id (or id (db/new-block-id))
                 icon (when-not (and (string? icon) (string/blank? icon)) icon)
@@ -560,29 +548,20 @@
                               (outliner-core/block-with-updated-at
                                {:block/uuid id
                                 :block/content resolved-value
+                                :block/closed-value-property (:db/id property)
                                 :block/schema (if description
                                                 (assoc schema :description description)
                                                 (dissoc schema :description))})
                                icon
                                (assoc :logseq.property/icon icon)))]
-                          (let [hidden-tx
-                                (if (contains? db-property-type/ref-property-types (:type property-schema))
-                                  []
-                                  (let [page (get-property-hidden-page property)
-                                        max-order (:block/order (last (:property/closed-values property)))
-                                        new-block (-> (db-property-build/build-closed-value-block block-id resolved-value [:block/uuid (:block/uuid page)]
-                                                                                                  property {:icon icon
-                                                                                                            :description description})
-                                                      (assoc :block/order (db-order/gen-key max-order nil)))]
-                                    (cond-> []
-                                      (not (e/entity? page))
-                                      (conj page)
-                                      true
-                                      (conj new-block))))]
-                            (conj hidden-tx
-                                  (outliner-core/block-with-updated-at
-                                   {:db/id (:db/id property)
-                                    :block/schema (merge {:type property-type} property-schema)}))))]
+                          (let [max-order (:block/order (last (:property/closed-values property)))
+                                new-block (-> (db-property-build/build-closed-value-block block-id resolved-value
+                                                                                                property {:icon icon
+                                                                                                          :description description})
+                                              (assoc :block/order (db-order/gen-key max-order nil)))]
+                            [new-block
+                             (outliner-core/block-with-updated-at
+                              {:db/id (:db/id property)})]))]
             {:block-id block-id
              :tx-data tx-data}))))))
 
@@ -591,64 +570,18 @@
   [property values]
   (assert (e/entity? property))
   (when (seq values)
-    (let [values' (remove string/blank? values)
-          property-schema (:block/schema property)]
-      (if (every? uuid? values')
-        (p/let [values (keep #(db/entity [:block/uuid %]) values')]
-          (when (seq values)
-            (let [property-tx {:db/ident (:db/ident property)
-                               :db/valueType :db.type/ref
-                               :db/cardinality (:db/cardinality property)
-                               :block/schema property-schema}
-                  value-property-tx (map (fn [id]
-                                           {:db/id id
-                                            :block/type "closed value"
-                                            :block/closed-value-property (:db/id property)})
-                                         (map :db/id values))]
-              (db/transact! (state/get-current-repo) (cons property-tx value-property-tx)
-                            {:outliner-op :insert-blocks}))))
-        (p/let [property-id (:db/ident property)
-                page (get-property-hidden-page property)
-                page-tx (when-not (e/entity? page) page)
-                page-id (:block/uuid page)
-                *max-key (atom nil)
-                closed-value-blocks (doall
-                                     (map (fn [value]
-                                            (let [b (db-property-build/build-closed-value-block
-                                                     (db/new-block-id)
-                                                     value
-                                                     [:block/uuid page-id]
-                                                     property
-                                                     {})]
-                                              (assoc b :block/order (db-order/gen-key @*max-key nil))))
-                                          values'))
-                value->block-id (zipmap
-                                 (map :block/content closed-value-blocks)
-                                 (map :block/uuid closed-value-blocks))
-                property-tx {:db/ident (:db/ident property)
-                             :db/valueType :db.type/ref
-                             :db/cardinality (:db/cardinality property)
-                             :block/schema property-schema}
-                property-values (db-async/<get-block-property-values (state/get-current-repo) (:db/ident property))
-                block-values (->> property-values
-                                  (remove #(uuid? (first %))))
-                id-values (filter second block-values)
-                ;; Order matters here, retract old property values first and then update the property schema
-                ;; Otherwise, the UI Datascript will throw tempid errors
-                tx-data (concat
-                         ;; retract old property values
-                         (map (fn [[id _value]] [:db/retract id property-id]) id-values)
-                         (when page-tx [page-tx])
-                         closed-value-blocks
-                         [property-tx]
-                         (map (fn [[id value]]
-                                (let [value' (if (set? value)
-                                               (set (map #(vector :block/uuid (value->block-id %)) value))
-                                               [:block/uuid (value->block-id value)])]
-                                  {:db/id id property-id value'}))
-                              id-values))]
-          (db/transact! (state/get-current-repo) tx-data
-                        {:outliner-op :insert-blocks}))))))
+    (let [values' (remove string/blank? values)]
+      (assert (every? uuid? values') "existing values should all be UUIDs")
+      (p/let [values (keep #(db/entity [:block/uuid %]) values')]
+        (when (seq values)
+          (let [value-property-tx (map (fn [id]
+                                         {:db/id id
+                                          :block/type "closed value"
+                                          :block/closed-value-property (:db/id property)})
+                                       (map :db/id values))
+                property-tx (outliner-core/block-with-updated-at {:db/id (:db/id property)})]
+            (db/transact! (state/get-current-repo) (cons property-tx value-property-tx)
+                          {:outliner-op :save-blocks})))))))
 
 (defn delete-closed-value!
   "Returns true when deleted or if not deleted displays warning and returns false"