Parcourir la source

fix: storing and querying of :number property values

Gabriel Horner il y a 1 an
Parent
commit
13066001cd

+ 1 - 1
deps/db/src/logseq/db/frontend/malli_schema.cljs

@@ -326,7 +326,7 @@
 
 (def block-attrs
   "Common attributes for normal blocks"
-  [[:block/content :string]
+  [[:block/content [:or :string :double]]
    [:block/parent :int]
    [:block/order :string]
    ;; refs

+ 1 - 3
deps/db/src/logseq/db/frontend/property/type.cljs

@@ -88,9 +88,7 @@
   (if new-closed-value?
     (number? id-or-value)
     (when-let [entity (d/entity db id-or-value)]
-      (or (number? (:block/content entity))
-          ;; FIXME: UI concerns shouldn't be in this layer
-          (number? (some-> (:block/content entity) parse-double))))))
+      (number? (:block/content entity)))))
 
 
 (defn- text-entity?

+ 2 - 1
deps/db/src/logseq/db/frontend/rules.cljc

@@ -121,7 +121,8 @@
    :block-content
    '[(block-content ?b ?query)
      [?b :block/content ?content]
-     [(clojure.string/includes? ?content ?query)]]
+     [(str ?content) ?str-content]
+     [(clojure.string/includes? ?str-content ?query)]]
 
    :page
    '[(page ?b ?page-name)

+ 4 - 10
deps/db/src/logseq/db/sqlite/build.cljs

@@ -66,14 +66,6 @@
   "Provides the next temp :db/id to use in a create-graph transact!"
   #(swap! current-db-id dec))
 
-(defn- create-property-value
-  [block property-ident value]
-  (db-property-build/build-property-value-block
-   block
-   property-ident
-   ;; FIXME: Remove when fixed in UI
-   (str value)))
-
 ;; TODO: Use build-property-values-tx-m
 (defn- ->property-value-tx-m
   "Given a new block and its properties, creates a map of properties which have values of property value tx.
@@ -85,8 +77,10 @@
                          ;; TODO: Support translate-property-value without this hack
                          (not (vector? v)))
                 [k (if (set? v)
-                     (set (map #(create-property-value new-block (get-ident all-idents k) %) v))
-                     (create-property-value new-block (get-ident all-idents k) 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))])))
        (into {})))
 
 (defn- extract-content-refs

+ 5 - 6
deps/db/test/logseq/db/frontend/rules_test.cljs

@@ -74,21 +74,20 @@
                     set))
             "page-property can bind to property arg with bound property value"))
 
-    ;; FIXME: string uses of :number
     (testing "cardinality :many property"
       (is (= ["Page"]
-             (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (page-property ?b :user.property/number-many "5")]
+             (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (page-property ?b :user.property/number-many 5)]
                                 @conn)
                   (map (comp :block/original-name first))))
           "page-property returns result when page has property")
       (is (= []
-             (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (page-property ?b :user.property/number-many "20")]
+             (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (page-property ?b :user.property/number-many 20)]
                                 @conn)
                   (map (comp :block/original-name first))))
           "page-property returns no result when page doesn't have property value")
       (is (= #{:user.property/number-many}
              (->> (q-with-rules '[:find [?p ...]
-                                  :where (page-property ?b ?p "5") [?b :block/original-name "Page"]]
+                                  :where (page-property ?b ?p 5) [?b :block/original-name "Page"]]
                                 @conn)
                   set))
           "page-property can bind to property arg with bound property value"))
@@ -115,8 +114,8 @@
                                   @conn)
                     set))
             "page-property can bind to property arg with unbound property value")
-        (is (= #{[:user.property/number-many "10"]
-                 [:user.property/number-many "5"]
+        (is (= #{[:user.property/number-many 10]
+                 [:user.property/number-many 5]
                  [:user.property/foo "bar"]
                  [:user.property/page-many "Page A"]}
                (->> (q-with-rules '[:find ?p ?v

+ 6 - 6
deps/outliner/src/logseq/outliner/property.cljs

@@ -176,7 +176,8 @@
   (let [property (d/entity @conn property-id)
         block (when block-id (d/entity @conn block-id))]
     (when property
-      (let [new-value-block (cond-> (db-property-build/build-property-value-block (or block property) property value)
+      (let [value' (convert-property-input-string (get-in property [:block/schema :type]) value)
+            new-value-block (cond-> (db-property-build/build-property-value-block (or block property) property value')
                               new-block-id
                               (assoc :block/uuid new-block-id)
                               (int? template-id)
@@ -204,8 +205,8 @@
 (defn- find-or-create-property-value
   "Find or create a property value. Only to be used with properties that have ref types"
   [conn property-id v]
-  (or (get-property-value-eid @conn property-id (str v))
-      (let [v-uuid (create-property-text-block! conn nil property-id (str v) {})]
+  (or (get-property-value-eid @conn property-id v)
+      (let [v-uuid (create-property-text-block! conn nil property-id v {})]
         (:db/id (d/entity @conn [:block/uuid v-uuid])))))
 
 (defn- convert-ref-property-value
@@ -402,14 +403,13 @@
      (seq properties))))
 
 (defn- build-closed-value-tx
-  [db property property-type resolved-value {:keys [id icon description]
+  [db property resolved-value {:keys [id icon 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)
         description (string/trim description)
         description (when-not (string/blank? description) description)
-        resolved-value (if (= property-type :number) (str resolved-value) resolved-value)
         tx-data (if block
                   [(let [schema (:block/schema block)]
                      (cond->
@@ -476,7 +476,7 @@
 
           :else
           (ldb/transact! conn
-                       (build-closed-value-tx @conn property property-type resolved-value opts)
+                       (build-closed-value-tx @conn property resolved-value opts)
                        {:outliner-op :save-block}))))))
 
 (defn add-existing-values-to-closed-values!

+ 7 - 5
src/main/frontend/components/property/value.cljs

@@ -635,7 +635,8 @@
 
        (de/entity? value)
        (when-let [content (:block/content value)]
-         (inline-text-cp content))
+         ;; str for non-string values like :number
+         (inline-text-cp (str content)))
 
        :else
        (inline-text-cp (str value)))]))
@@ -744,8 +745,8 @@
          (inline-text {} :markdown (macro-util/expand-value-if-macro (str value) (state/get-macros)))))]))
 
 (rum/defcs property-scalar-value < rum/reactive db-mixins/query rum/static
-  [state block property value {:keys [container-id editing? on-chosen]
-                               :as opts}]
+  [state block property value* {:keys [container-id editing? on-chosen]
+                                :as opts}]
   (let [property (model/sub-block (:db/id property))
         schema (:block/schema property)
         type (get schema :type :default)
@@ -754,9 +755,10 @@
         select-type? (select-type? property type)
         closed-values? (seq (:property/closed-values property))
         select-opts {:on-chosen on-chosen}
-        value (if (and (de/entity? value) (= (:db/ident value) :logseq.property/empty-placeholder))
+        value (if (and (de/entity? value*) (= (:db/ident value*) :logseq.property/empty-placeholder))
                 nil
-                value)]
+                ;; str for non-string values like :number
+                (if (map? value*) (update value* :block/content str) value*))]
     (if (= :logseq.property/icon (:db/ident property))
       (icon-row block)
       (if (and select-type?

+ 11 - 11
src/test/frontend/handler/db_based/property_test.cljs

@@ -60,7 +60,7 @@
     (let [conn (db/get-db false)]
       (outliner-property/upsert-property! conn :user.property/property-2 {:type :number}
                                           {:property-name "property 2"})
-      (outliner-property/set-block-property! conn fbid :user.property/property-2 "1"))
+      (outliner-property/set-block-property! conn fbid :user.property/property-2 1))
 
     (let [block (db/entity [:block/uuid fbid])
           properties (:block/properties block)
@@ -78,7 +78,7 @@
         (every? keyword? (map first properties))
         true
         (:block/content (second (second properties)))
-        "1")))
+        1)))
 
   (testing "Update property value"
     (let [conn (db/get-db false)]
@@ -90,14 +90,14 @@
         (count properties)
         2
         (:block/content (second (second properties)))
-        "2")))
+        2)))
 
   (testing "Wrong type property value shouldn't transacted"
     (let [conn (db/get-db false)]
       (is
        (thrown-with-msg?
         js/Error
-        #"Schema validation failed"
+        #"Can't convert"
         (outliner-property/set-block-property! conn fbid :user.property/property-2 "Not a number"))))
     (let [block (db/entity [:block/uuid fbid])
           properties (:block/properties block)]
@@ -106,7 +106,7 @@
         (count properties)
         2
         (:block/content (second (second properties)))
-        "2")))
+        2)))
 
   (testing "Add a multi-values property"
     (let [conn (db/get-db false)]
@@ -128,7 +128,7 @@
         (are [x y] (= x y)
           3
           (count properties)
-          #{"1" "2" "3"}
+          #{1 2 3}
           (set (map :block/content (get properties :user.property/property-3)))))))
 
   (testing "Remove a property"
@@ -293,7 +293,7 @@
       (testing "Add existing values to closed values"
         (let [values (get-value-ids k)]
           (is (every? uuid? values))
-          (is (= #{"1" "2"} (get-closed-values values)))
+          (is (= #{1 2} (get-closed-values values)))
           (is (every? #(contains? (:block/type (db/entity [:block/uuid %])) "closed value")
                       values))))
       (testing "Add non-numbers shouldn't work"
@@ -303,7 +303,7 @@
           #"Can't convert"
           (outliner-property/upsert-closed-value! conn (:db/id property) {:value "not a number"})))
         (let [values (get-value-ids k)]
-          (is (= #{"1" "2"} (get-closed-values values)))))
+          (is (= #{1 2} (get-closed-values values)))))
 
       (testing "Add existing value"
         (is
@@ -314,10 +314,10 @@
 
       (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 :block/content 3]] @conn))]
           (is (contains? (set (:block/type b)) "closed value"))
           (let [values (get-value-ids k)]
-            (is (= #{"1" "2" "3"}
+            (is (= #{1 2 3}
                    (get-closed-values values))))
 
           (testing "Update closed value"
@@ -326,7 +326,7 @@
                                                                                     :value 4
                                                                                     :description "choice 4"})
                   b (db/entity [:block/uuid block-id])]
-              (is (= "4" (:block/content b)))
+              (is (= 4 (:block/content 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])))