Browse Source

fix: property rules now work with property ref values

Also remove use of :string properties in tests
Gabriel Horner 1 year ago
parent
commit
d37df950fa

+ 24 - 0
deps/db/src/logseq/db/frontend/property/build.cljs

@@ -74,3 +74,27 @@
                                                   (when (keyword? property) {:db/ident property}))
        :block/order (db-order/gen-key)}
       sqlite-util/block-with-timestamps))
+
+(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"
+  [block properties]
+  ;; Build :db/id out of uuid if block doesn't have one for tx purposes
+  (let [block' (if (:db/id block) block (assoc block :db/id [:block/uuid (:block/uuid block)]))]
+    (->> properties
+         (map (fn [[k v]]
+                [k
+                 (if (set? v)
+                   (set (map #(build-property-value-block block' k %) v))
+                   (build-property-value-block block' k v))]))
+         (into {}))))
+
+(defn build-properties-with-ref-values
+  "Given a properties map with property values to be transacted e.g. from
+  build-property-values-tx-m, build a properties map to be transacted with the block"
+  [prop-vals-tx-m]
+  (update-vals prop-vals-tx-m
+               (fn [v]
+                 (if (set? v)
+                   (set (map #(vector :block/uuid (:block/uuid %)) v))
+                   (vector :block/uuid (:block/uuid v))))))

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

@@ -183,7 +183,9 @@
     :page-property
     '[[(page-property ?p ?prop ?val)
        [?p :block/name]
-       [?p ?prop ?val]
+       [?p ?prop ?pv]
+       (or [?pv :block/content ?val]
+           [?pv :block/original-name ?val])
        [?prop-e :db/ident ?prop]
        [?prop-e :block/type "property"]]
       ;; TODO: Delete when DB_GRAPH query-dsl tests are passing
@@ -226,7 +228,9 @@
 
     :property
     '[(property ?b ?prop ?val)
-      [?b ?prop ?val]
+      [?b ?prop ?pv]
+      (or [?pv :block/content ?val]
+          [?pv :block/original-name ?val])
       [(missing? $ ?b :block/name)]
       [?prop-e :db/ident ?prop]
       [?prop-e :block/type "property"]]

+ 29 - 18
deps/db/test/logseq/db/frontend/rules_test.cljs

@@ -5,7 +5,7 @@
             [logseq.db.frontend.rules :as rules]
             [logseq.db.sqlite.create-graph :as sqlite-create-graph]
             [logseq.db.sqlite.util :as sqlite-util]
-            [logseq.db :as ldb]))
+            [logseq.db.frontend.property.build :as db-property-build]))
 
 (defn- new-db-conn []
   (let [conn (d/create-conn db-schema/schema-for-db-based-graph)
@@ -21,10 +21,15 @@
 (deftest has-page-property-rule
   (let [conn (new-db-conn)
         page (sqlite-util/build-new-page "Page")
-        _ (d/transact! conn [(sqlite-util/build-new-property :user.property/foo {:type :string})
-                             (sqlite-util/build-new-property :user.property/foo2 {:type :string})
-                             page
-                             {:block/uuid (:block/uuid page) :user.property/foo "bar"}])]
+        prop-vals-tx-m (db-property-build/build-property-values-tx-m page {:user.property/foo "bar"})
+        _ (d/transact! conn
+                       (concat
+                        [(sqlite-util/build-new-property :user.property/foo {})
+                         (sqlite-util/build-new-property :user.property/foo2 {})
+                         page]
+                        (vals prop-vals-tx-m)
+                        [(merge {:block/uuid (:block/uuid page)}
+                                (db-property-build/build-properties-with-ref-values prop-vals-tx-m))]))]
     (is (= ["Page"]
            (->> (q-with-rules '[:find (pull ?b [:block/original-name]) :where (has-page-property ?b :user.property/foo)]
                               @conn)
@@ -45,16 +50,22 @@
   (let [conn (new-db-conn)
         page (sqlite-util/build-new-page "Page")
         page-a (sqlite-util/build-new-page "Page A")
-        _ (d/transact! conn [(sqlite-util/build-new-property :user.property/foo {:type :string})
-                             (sqlite-util/build-new-property :user.property/foo2 {:type :string})
-                             (sqlite-util/build-new-property :user.property/number-many {:type :number :cardinality :many})
-                             (sqlite-util/build-new-property :user.property/page-many {:type :page :cardinality :many})
-                             page
-                             page-a
-                             {:block/uuid (:block/uuid page) :user.property/foo "bar"}
-                             {:block/uuid (:block/uuid page) :user.property/number-many #{5 10}}
-                             {:block/uuid (:block/uuid page) :user.property/page-many #{[:block/uuid (:block/uuid page-a)]}}
-                             {:block/uuid (:block/uuid page-a) :user.property/foo "bar A"}])]
+        page-prop-vals-tx-m (db-property-build/build-property-values-tx-m page {:user.property/foo "bar" :user.property/number-many #{5 10}})
+        page-a-prop-vals-tx-m (db-property-build/build-property-values-tx-m page-a {:user.property/foo "bar A"})
+        _ (d/transact! conn
+                       (concat [(sqlite-util/build-new-property :user.property/foo {})
+                                (sqlite-util/build-new-property :user.property/foo2 {})
+                                (sqlite-util/build-new-property :user.property/number-many {:type :number :cardinality :many})
+                                (sqlite-util/build-new-property :user.property/page-many {:type :page :cardinality :many})
+                                page
+                                page-a]
+                               (mapcat #(if (set? %) % [%]) (vals page-prop-vals-tx-m))
+                               (mapcat #(if (set? %) % [%]) (vals page-a-prop-vals-tx-m))
+                               [(merge {:block/uuid (:block/uuid page)}
+                                       (db-property-build/build-properties-with-ref-values page-prop-vals-tx-m))
+                                {:block/uuid (:block/uuid page) :user.property/page-many #{[:block/uuid (:block/uuid page-a)]}}
+                                (merge {:block/uuid (:block/uuid page-a)}
+                                       (db-property-build/build-properties-with-ref-values page-a-prop-vals-tx-m))]))]
 
     (testing "cardinality :one property"
       (is (= ["Page"]
@@ -96,7 +107,7 @@
     (testing ":ref property"
       (is (= ["Page"]
              (->> (q-with-rules '[:find (pull ?b [:block/original-name])
-                                  :where (page-property ?b :user.property/page-many ?pv) [?pv :block/original-name "Page A"]]
+                                  :where (page-property ?b :user.property/page-many "Page A")]
                                 @conn)
                   (map (comp :block/original-name first))))
           "page-property returns result when page has property")
@@ -117,7 +128,7 @@
       (is (= #{[:user.property/number-many 10]
                [:user.property/number-many 5]
                [:user.property/foo "bar"]
-               [:user.property/page-many (:db/id (ldb/get-page @conn "Page A"))]}
+               [:user.property/page-many "Page A"]}
              (->> (q-with-rules '[:find ?p ?v
                                   :where (page-property ?b ?p ?v) [?b :block/original-name "Page"]]
                                 @conn)
@@ -126,7 +137,7 @@
       (is (= #{"Page"}
              (->> (q-with-rules '[:find (pull ?b [:block/original-name])
                                   :where
-                                  (page-property ?b :user.property/page-many ?pv)
+                                  [?b :user.property/page-many ?pv]
                                   (page-property ?pv :user.property/foo "bar A")]
                                 @conn)
                   (map (comp :block/original-name first))

+ 1 - 0
scripts/src/logseq/tasks/db_graph/create_graph.cljs

@@ -109,6 +109,7 @@
    ;; 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.
    This map is used for both creating the new property values and then adding them to a block"

+ 9 - 9
src/main/frontend/worker/handler/page.cljs

@@ -42,13 +42,14 @@
                                                               tags)}))]
       (if (sqlite-util/db-based-graph? repo)
         (let [property-vals-tx-m
-              ;; Builds property values for built-in :one properties like logseq.property.pdf/file
-              (->> properties
-                   (keep (fn [[k v]]
-                           (when (db-property-util/built-in-has-ref-value? k)
-                             [k
-                              (db-property-build/build-property-value-block {:db/id page-entity} k v)])))
-                   (into {}))]
+              ;; Builds property values for built-in properties like logseq.property.pdf/file
+              (db-property-build/build-property-values-tx-m
+               page'
+               (->> properties
+                    (keep (fn [[k v]]
+                            (when (db-property-util/built-in-has-ref-value? k)
+                              [k v])))
+                    (into {})))]
           (cond-> [(merge page'
                          (when class? {:block/type "class"}))]
            (seq property-vals-tx-m)
@@ -57,8 +58,7 @@
            (conj (merge {:block/uuid (:block/uuid page)}
                        ;; FIXME: Add refs for properties?
                         properties
-                       ;; Replace property values with their refs
-                        (update-vals property-vals-tx-m #(vector :block/uuid (:block/uuid %)))))))
+                        (db-property-build/build-properties-with-ref-values property-vals-tx-m)))))
         (let [file-page (merge page'
                                (when (seq properties) {:block/properties properties}))]
           (if (and (seq properties)