enhance: property ref values can store their

original value optionally with :property/value.
Currently applies to :url and :number and not :default
as :default is meant to be a block and thus highly coupled
to :block/content. By introducing this, functionality that relies
on original property values automatically work e.g.
sorting by :number property for query results just works

deps/db/src/logseq/db/frontend/malli_schema.cljs

@@ -116,12 +116,13 @@
                (if-let [property (and (db-property/property? k)
                                       (d/entity db k))]
                  (update m :block/properties (fnil conj [])
-                         [(assoc (select-keys property [:db/ident :db/valueType :db/cardinality])
-                                 :block/schema
-                                 (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))
+                         ;; use explicit call to be nbb compatible
+                         [(let [closed-values (entity-plus/lookup-kv-then-entity property :property/closed-values)]
+                            (cond-> (assoc (select-keys property [:db/ident :db/valueType :db/cardinality])
+                                    :block/schema
+                                    (select-keys (:block/schema property) [:type]))
+                              (seq closed-values)
+                              (assoc :property/closed-values closed-values)))
                           v])
                  (assoc m k v)))
              {}
@@ -332,7 +333,7 @@
 
 (def block-attrs
   "Common attributes for normal blocks"
-  [[:block/content [:or :string :double]]
+  [[:block/content :string]
    [:block/parent :int]
    [:block/order block-order]
    ;; refs
@@ -354,15 +355,24 @@
      [:block/path-refs {:optional true} [:set :int]]]
     page-or-block-attrs)))
 
-(def closed-value-block
+(def property-value-block
   "A closed value for a property with closed/allowed values"
+  (vec
+   (concat
+    [:map]
+    [[:property/value [:or :string :double]]]
+    (remove #(#{:block/content} (first %)) block-attrs)
+    page-or-block-attrs)))
+
+(def closed-value-block*
   (vec
    (concat
     [:map]
     [[:block/type [:= #{"closed value"}]]
      ;; for built-in properties
      [:db/ident {:optional true} logseq-property-ident]
-     [:block/content [:or :string :double]]
+     [:block/content {:optional true} :string]
+     [:property/value {:optional true} [:or :string :double]]
      [:block/closed-value-property {:optional true} [:set :int]]
      [:block/schema {:optional true}
       [:map
@@ -370,6 +380,14 @@
     (remove #(#{:block/content} (first %)) block-attrs)
     page-or-block-attrs)))
 
+(def closed-value-block
+  "A closed value for a property with closed/allowed values"
+  [:and closed-value-block*
+   [:fn {:error/message ":block/content or :property/value required"
+         :error/path [:property/value]}
+    (fn [m]
+      (or (:block/content m) (:property/value m)))]])
+
 (def normal-block
   "A block with content and no special type or tag behavior"
   (vec
@@ -383,7 +401,8 @@
   [:or
    normal-block
    closed-value-block
-   whiteboard-block])
+   whiteboard-block
+   property-value-block])
 
 (def file-block
   [:map
@@ -460,7 +479,7 @@
 
 ;; Keep malli schema in sync with db schema
 ;; ========================================
-(let [malli-many-ref-attrs (->> (concat class-attrs property-attrs page-attrs block-attrs page-or-block-attrs (rest closed-value-block))
+(let [malli-many-ref-attrs (->> (concat class-attrs property-attrs page-attrs block-attrs page-or-block-attrs (rest closed-value-block*))
                                 (filter #(= (last %) [:set :int]))
                                 (map first)
                                 set)]
@@ -480,8 +499,8 @@
                     {}))))
 
 (let [malli-non-ref-attrs (->> (concat class-attrs property-attrs page-attrs block-attrs page-or-block-attrs (rest normal-page))
-                               (concat (rest file-block) (rest asset-block)
-                                        (rest db-ident-key-val) (rest class-page))
+                               (concat (rest file-block) (rest asset-block) (rest property-value-block)
+                                       (rest db-ident-key-val) (rest class-page))
                                (remove #(= (last %) [:set :int]))
                                (map first)
                                set)]

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

@@ -223,18 +223,19 @@
     (:property/closed-values property)))
 
 (defn closed-value-name
-  "Gets name of closed value given closed value ent/map. Works for all closed value types including pages, blocks"
+  "Gets name of closed value given closed value ent/map. Works for all closed value types"
   [ent]
-  (or (:block/original-name ent)
-      (:block/content ent)))
+  (or (:block/content ent)
+      (:property/value ent)))
 
 (defn get-property-value-name
   "Given an entity, gets a readable name for the property value of a ref type
   property. Different than closed-value-name as there implementation will likely
   differ"
   [ent]
-  (or (:block/original-name ent)
-      (:block/content ent)))
+  (or (:block/content ent)
+      (:property/value ent)
+      (:block/original-name ent)))
 
 (defn get-property-value-name-from-ref
   "Given a ref from a pulled query e.g. `{:db/id X}`, gets a readable name for
@@ -279,19 +280,12 @@
   (and (map? block)
        (:logseq.property/created-from-property block)
        (:block/page block)
-       ;; not closed value
-       (not (some? (:block/content block)))))
+       (not (some? (closed-value-name block)))))
 
 (defn many?
   [property]
   (= (:db/cardinality property) :db.cardinality/many))
 
-(defn property-value-when-closed
-  "Returns property value if the given entity is type 'closed value' or nil"
-  [ent]
-  (when (contains? (:block/type ent) "closed value")
-    (:block/content ent)))
-
 (defn properties-by-name
   "Given a block from a query result, returns a map of its properties indexed by
   property names"

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

@@ -2,19 +2,22 @@
   "Builds core property concepts"
   (:require [logseq.db.sqlite.util :as sqlite-util]
             [logseq.db.frontend.order :as db-order]
-            [datascript.core :as d]))
+            [datascript.core :as d]
+            [logseq.db.frontend.property.type :as db-property-type]))
 
 (defn- closed-value-new-block
   [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 (str value)
-     :block/closed-value-property property-id
-     :logseq.property/created-from-property property-id
-     :block/parent property-id}))
+    (merge {:block/type #{"closed value"}
+            :block/format :markdown
+            :block/uuid block-id
+            :block/page property-id
+            :block/closed-value-property property-id
+            :logseq.property/created-from-property property-id
+            :block/parent property-id}
+           (if (db-property-type/original-value-ref-property-types (get-in property [:block/schema :type]))
+             {:property/value value}
+             {:block/content value}))))
 
 (defn build-closed-value-block
   "Builds a closed value block to be transacted"
@@ -62,19 +65,23 @@
   "Builds a property value entity given a block map/entity, a property entity or
   ident and its property value"
   [block property value]
-  (-> {:block/uuid (d/squuid)
-       :block/format :markdown
-       :block/content value
-       :block/page (if (:block/page block)
-                     (:db/id (:block/page block))
+  (-> (merge
+       {:block/uuid (d/squuid)
+        :block/format :markdown
+        :block/page (if (:block/page block)
+                      (:db/id (:block/page block))
                      ;; page block
-                     (:db/id block))
-       :block/parent (:db/id block)
-       :logseq.property/created-from-property (or (:db/id property)
-                                                  (when (keyword? property) {:db/ident property}))
-       :block/order (db-order/gen-key)}
+                      (:db/id block))
+        :block/parent (:db/id block)
+        :logseq.property/created-from-property (or (:db/id property) (:db/ident property))
+        :block/order (db-order/gen-key)}
+       (if (db-property-type/original-value-ref-property-types (get-in property [:block/schema :type]))
+         {:property/value value}
+         {:block/content value}))
       sqlite-util/block-with-timestamps))
 
+;; TODO: Add support for types besides :default when needed by getting property types
+;; and passing them to build-property-value-block
 (defn build-property-values-tx-m
   "Builds a map of property names to their property value blocks to be transacted, given a block
    and a properties map with raw property values"
@@ -85,8 +92,8 @@
          (map (fn [[k v]]
                 [k
                  (if (set? v)
-                   (set (map #(build-property-value-block block' k %) v))
-                   (build-property-value-block block' k v))]))
+                   (set (map #(build-property-value-block block' {:db/ident k} %) v))
+                   (build-property-value-block block' {:db/ident k} v))]))
          (into {}))))
 
 (defn build-properties-with-ref-values

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

@@ -28,9 +28,17 @@
 (assert (set/subset? closed-value-property-types (set user-built-in-property-types))
         "All closed value types are valid property types")
 
+(def original-value-ref-property-types
+  "Property value ref types where the refed entity stores its value in
+  :property/value e.g. :number is stored as a number. new value-ref-property-types
+  should default to this as it allows for more querying power"
+  #{:number :url})
+
 (def value-ref-property-types
-  "Property value ref types"
-  #{:default :url :number :template})
+  "Property value ref types where the refed entities either store their value in
+  :property/value or block/content"
+  (into #{:default :template}
+        original-value-ref-property-types))
 
 (def ref-property-types
   "User facing ref types"
@@ -81,15 +89,14 @@
   (if new-closed-value?
     (url? val)
     (when-let [ent (d/entity db val)]
-      (url? (:block/content ent)))))
+      (url? (:property/value ent)))))
 
 (defn- number-entity?
   [db id-or-value {:keys [new-closed-value?]}]
   (if new-closed-value?
     (number? id-or-value)
     (when-let [entity (d/entity db id-or-value)]
-      (number? (:block/content entity)))))
-
+      (number? (:property/value entity)))))
 
 (defn- text-entity?
   [db s {:keys [new-closed-value?]}]

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

@@ -24,7 +24,7 @@
   [repo coll db-ident]
   (if (sqlite-util/db-based-graph? repo)
     (let [val (get coll db-ident)]
-      (if (built-in-has-ref-value? db-ident) (:block/content val) val))
+      (if (built-in-has-ref-value? db-ident) (db-property/get-property-value-name val) val))
     (get coll (get-pid repo db-ident))))
 
 (defn get-block-property-value

deps/db/src/logseq/db/frontend/schema.cljs

@@ -135,7 +135,7 @@
                                   :db/cardinality :db.cardinality/many}
     :property/schema.classes {:db/valueType :db.type/ref
                               :db/cardinality :db.cardinality/many}
-
+    :property/value {}
     :file/last-modified-at {}
     :asset/uuid {:db/unique :db.unique/identity}
     :asset/meta {}}))

deps/db/src/logseq/db/sqlite/build.cljs

@@ -85,11 +85,13 @@
               (when (and (db-property-type/value-ref-property-types (get-in properties-config [k :block/schema :type]))
                          ;; TODO: Support translate-property-value without this hack
                          (not (vector? v)))
-                [k (if (set? v)
-                     (->> v
-                          (map #(db-property-build/build-property-value-block new-block (get-ident all-idents k) %))
-                          set)
-                     (db-property-build/build-property-value-block new-block (get-ident all-idents k) v))])))
+                (let [property-map {:db/ident (get-ident all-idents k)
+                                    :block/schema {:type (get-in properties-config [k :block/schema :type])}}]
+                  [k (if (set? v)
+                      (->> v
+                           (map #(db-property-build/build-property-value-block new-block property-map %))
+                           set)
+                      (db-property-build/build-property-value-block new-block property-map v))]))))
        (into {})))
 
 (defn- extract-content-refs

deps/outliner/src/logseq/outliner/property.cljs

@@ -203,7 +203,8 @@
         raw-value)))
 
 (defn- find-or-create-property-value
-  "Find or create a property value. Only to be used with properties that have ref types"
+  "Find or create a property value. Only to be used with properties that have ref types.
+   Mainly used by :default property type"
   [conn property-id v]
   (or (get-property-value-eid @conn property-id v)
       (let [v-uuid (create-property-text-block! conn nil property-id v {})]
@@ -408,7 +409,7 @@
 
 (defn- build-closed-value-tx
   [db property resolved-value {:keys [id icon description]
-                                             :or {description ""}}]
+                               :or {description ""}}]
   (let [block (when id (d/entity db [:block/uuid id]))
         block-id (or id (ldb/new-block-id))
         icon (when-not (and (string? icon) (string/blank? icon)) icon)
@@ -418,12 +419,15 @@
                   [(let [schema (:block/schema block)]
                      (cond->
                       (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))})
+                       (merge
+                        {:block/uuid id
+                         :block/closed-value-property (:db/id property)
+                         :block/schema (if description
+                                         (assoc schema :description description)
+                                         (dissoc schema :description))}
+                        (if (db-property-type/original-value-ref-property-types (get-in property [:block/schema :type]))
+                          {:property/value resolved-value}
+                          {:block/content resolved-value})))
                        icon
                        (assoc :logseq.property/icon icon)))]
                   (let [max-order (:block/order (last (:property/closed-values property)))
@@ -455,7 +459,7 @@
                               resolved-value)]
         (cond
           (some (fn [b]
-                  (and (= (str resolved-value) (str (or (db-property/property-value-when-closed b)
+                  (and (= (str resolved-value) (str (or (db-property/closed-value-name b)
                                                         (:block/uuid b))))
                        (not= id (:block/uuid b))))
                 (:property/closed-values property))

deps/publishing/src/logseq/publishing/db.cljs

@@ -4,7 +4,8 @@
             [logseq.db.frontend.rules :as rules]
             [clojure.set :as set]
             [clojure.string :as string]
-            [logseq.db.frontend.entity-plus :as entity-plus]))
+            [logseq.db.frontend.entity-plus :as entity-plus]
+            [logseq.db.frontend.property :as db-property]))
 
 (defn ^:api get-area-block-asset-url
   "Returns asset url for an area block used by pdf assets. This lives in this ns
@@ -13,7 +14,7 @@
   (when-some [props (and block page (:block/properties block))]
     ;; Can't use db-property-util/lookup b/c repo isn't available
     (let [prop-lookup-fn (if (entity-plus/db-based-graph? db)
-                           #(:block/content (get %1 %2))
+                           #(db-property/get-property-value-name (get %1 %2))
                            #(get %1 (keyword (name %2))))]
       (when-some [uuid (:block/uuid block)]
         (when-some [stamp (prop-lookup-fn props :logseq.property.pdf/hl-stamp)]

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

@@ -186,8 +186,7 @@
     (for [value values]
       [:li (if (uuid? value)
              (let [result (db/entity [:block/uuid value])]
-               (or (:block/original-name result)
-                   (:block/content result)))
+               (db-property/closed-value-name result))
              (str value))])]
    (ui/button
     "Add choices"

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

@@ -454,8 +454,7 @@
                          (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))
+                                    {:label (db-property/get-property-value-name e)
                                      :value value})
                                   {:label value
                                    :value value})))
@@ -639,9 +638,11 @@
        (closed-value-item value opts)
 
        (de/entity? value)
-       (when-let [content (:block/content value)]
-         ;; str for non-string values like :number
-         (inline-text-cp (str content)))
+       (when-some [content (if (some? (:property/value value))
+                             ;; content needs to be a string for display purposes
+                             (str (:property/value value))
+                             (:block/content value))]
+         (inline-text-cp content))
 
        :else
        (inline-text-cp (str value)))]))
@@ -761,8 +762,7 @@
         select-opts {:on-chosen on-chosen}
         value (if (and (de/entity? value*) (= (:db/ident value*) :logseq.property/empty-placeholder))
                 nil
-                ;; str for non-string values like :number
-                (if (map? value*) (update value* :block/content str) value*))]
+                value*)]
     (if (= :logseq.property/icon (:db/ident property))
       (icon-row block)
       (if (and select-type?

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

@@ -14,8 +14,7 @@
   "Get a property's name given its id"
   [e]
   (if-let [e (if (number? e) (db-utils/pull e) e)]
-    (or (:block/content e)
-        (:block/original-name e))
+    (db-property/get-property-value-name e)
     e))
 
 (defn properties-by-name

src/test/frontend/handler/db_based/property_test.cljs

@@ -77,7 +77,7 @@
         2
         (every? keyword? (map first properties))
         true
-        (:block/content (second (second properties)))
+        (db-property/get-property-value-name (second (second properties)))
         1)))
 
   (testing "Update property value"
@@ -89,7 +89,7 @@
       (are [x y] (= x y)
         (count properties)
         2
-        (:block/content (second (second properties)))
+        (db-property/get-property-value-name (second (second properties)))
         2)))
 
   (testing "Wrong type property value shouldn't transacted"
@@ -105,7 +105,7 @@
       (are [x y] (= x y)
         (count properties)
         2
-        (:block/content (second (second properties)))
+        (db-property/get-property-value-name (second (second properties)))
         2)))
 
   (testing "Add a multi-values property"
@@ -129,7 +129,7 @@
           3
           (count properties)
           #{1 2 3}
-          (set (map :block/content (get properties :user.property/property-3)))))))
+          (set (map db-property/get-property-value-name (get properties :user.property/property-3)))))))
 
   (testing "Remove a property"
     (outliner-property/remove-block-property! (db/get-db false) fbid :user.property/property-3)
@@ -273,7 +273,7 @@
 (defn- get-closed-values
   "Get value from block ids"
   [values]
-  (set (map #(:block/content (db/entity [:block/uuid %])) values)))
+  (set (map #(db-property/closed-value-name (db/entity [:block/uuid %])) values)))
 
 ;; closed values related
 ;; upsert-closed-value
@@ -314,7 +314,7 @@
 
       (testing "Add new value"
         (let [_ (outliner-property/upsert-closed-value! conn (:db/id property) {:value 3})
-              b (first (d/q '[:find [(pull ?b [*]) ...] :where [?b :block/content 3]] @conn))]
+              b (first (d/q '[:find [(pull ?b [*]) ...] :where [?b :property/value 3]] @conn))]
           (is (contains? (set (:block/type b)) "closed value"))
           (let [values (get-value-ids k)]
             (is (= #{1 2 3}
@@ -326,7 +326,7 @@
                                                                                     :value 4
                                                                                     :description "choice 4"})
                   b (db/entity [:block/uuid block-id])]
-              (is (= 4 (:block/content b)))
+              (is (= 4 (db-property/closed-value-name b)))
               (is (= "choice 4" (:description (:block/schema b))))
               (is (contains? (:block/type b) "closed value"))
               (outliner-property/delete-closed-value! conn (:db/id property) (:db/id (db/entity [:block/uuid block-id])))