Fix sort-by for user properties with db graphs

sort-by-queries test passes except for timestamps.
Had to introduce another query to resolve property uuids.
Made it as efficient as possible with datoms and pull.
Refactored load-test-files to support journal files and
allow page and block properties from one file

src/main/frontend/db/query_dsl.cljs

@@ -9,6 +9,9 @@
             [frontend.date :as date]
             [frontend.db.model :as model]
             [frontend.db.query-react :as query-react]
+            [frontend.db.utils :as db-utils]
+            [frontend.db.conn :as conn]
+            [datascript.core :as d]
             [logseq.graph-parser.util.db :as db-util]
             [logseq.db.rules :as rules]
             [frontend.template :as template]
@@ -341,9 +344,9 @@
                 :desc)
         comp (if (= order :desc) >= <=)]
     (reset! sort-by_
-            (fn sort-results [result]
+            (fn sort-results [result block-attribute]
               ;; first because there is one binding result in query-wrapper
-              (sort-by #(-> % first (get-in [:block/properties k]))
+              (sort-by #(-> % first (get-in [block-attribute k]))
                        comp
                        result)))
     ;; blank b/c this post-process filter doesn't effect query
@@ -589,6 +592,26 @@ Some bindings in this fn:
   (let [q' (template/resolve-dynamic-template! q)]
     (pre-transform q')))
 
+(defn- sort-by-prep
+  "For a db graph, adds a block attribute :block/properties-by-name to be used
+  for sorting with its keys being property names"
+  [col]
+  ;; Only modify result shapes that we know of
+  (if (map? (ffirst col))
+    (let [prop-uuid-names (->> (d/datoms (conn/get-db) :avet :block/type "property")
+                               (map :e)
+                               (db-utils/pull-many '[:block/uuid :block/name])
+                               (map #(vector (:block/uuid %) (keyword (:block/name %))))
+                               (into {}))]
+      (map (fn [blocks]
+             (mapv (fn [block]
+                     (assoc block
+                            :block/properties-by-name
+                            (update-keys (:block/properties block) #(prop-uuid-names % %))))
+                   blocks))
+           col))
+    col))
+
 (defn query
   "Runs a dsl query with query as a string. Primary use is from '{{query }}'"
   ([repo query-string]
@@ -597,12 +620,16 @@ Some bindings in this fn:
    (when (and (string? query-string) (not= "\"\"" query-string))
      (let [{:keys [query rules sort-by blocks? sample]} (parse-query query-string {:db-graph? (config/db-based-graph? repo)})]
        (when-let [query' (some-> query (query-wrapper {:blocks? blocks?}))]
-         (let [sort-by (or sort-by identity)
-               random-samples (if @sample
+         (let [random-samples (if @sample
                                 (fn [col]
                                   (take @sample (shuffle col)))
                                 identity)
-               transform-fn (comp sort-by random-samples)]
+               sort-by' (if sort-by
+                          (if (config/db-based-graph? repo)
+                            (comp (fn [col] (sort-by col :block/properties-by-name)) sort-by-prep)
+                            #(sort-by % :block/properties))
+                          identity)
+               transform-fn (comp sort-by' random-samples)]
            (query-react/react-query repo
                                     {:query query'
                                      :query-string query-string
@@ -620,14 +647,16 @@ Some bindings in this fn:
           {:keys [query sort-by blocks? rules]} (parse query-string {:db-graph? (config/db-based-graph? repo)})]
       (when-let [query' (some-> query (query-wrapper {:blocks? blocks?}))]
         (query-react/react-query repo
-                           (merge
-                            query-m
-                            {:query query'
-                             :rules rules})
-                           (merge
-                            query-opts
-                            (when sort-by
-                              {:transform-fn sort-by})))))))
+                                 (merge
+                                  query-m
+                                  {:query query'
+                                   :rules rules})
+                                 (merge
+                                  query-opts
+                                  (when sort-by
+                                    {:transform-fn (if (config/db-based-graph? repo)
+                                                     (comp (fn [col] (sort-by col :block/properties-by-name)) sort-by-prep)
+                                                     #(sort-by % :block/properties))})))))))
 
 (defn query-contains-filter?
   [query filter-name]

src/test/frontend/db/query_dsl_test.cljs

@@ -517,7 +517,11 @@ created-at:: 1608968448116
          (map :block/content (custom-query {:query (list 'and '(task later) "b")})))
       "Query with rule that can't be derived from the form itself"))
 
-(deftest sort-by-queries
+(if js/process.env.DB_GRAPH
+  (def get-property-value  #(get-in %1 [:block/properties-by-name %2]))
+  (def get-property-value  #(get-in %1 [:block/properties %2])))
+
+(deftest ^:focus2 sort-by-queries
   (load-test-files [{:file/path "journals/2020_02_25.md"
                      :file/content "rating:: 10"}
                     {:file/path "journals/2020_12_26.md"
@@ -533,43 +537,43 @@ created-at:: 1608968448115
 - 26-b4
 created-at:: 1608968448116
 "}])
-
   (testing "sort-by user block property fruit"
     (let [result (->> (dsl-query "(and (task now later done) (sort-by fruit))")
-                      (map #(get-in % [:block/properties :fruit])))]
+                      (map #(get-property-value % :fruit))
+                      #_set)]
       (is (= ["plum" "apple" nil]
              result)
           "sort-by correctly defaults to desc"))
 
     (let [result (->> (dsl-query "(and (task now later done) (sort-by fruit desc))")
-                      (map #(get-in % [:block/properties :fruit])))]
-      (is (= ["plum" "apple" nil]
-             result)
-          "sort-by desc"))
+                        (map #(get-property-value % :fruit)))]
+        (is (= ["plum" "apple" nil]
+               result)
+            "sort-by desc"))
 
     (let [result (->> (dsl-query "(and (task now later done) (sort-by fruit asc))")
-                      (map #(get-in % [:block/properties :fruit])))]
-      (is (= ["apple" "plum" nil]
-             result)
-          "sort-by asc")))
+                        (map #(get-property-value % :fruit)))]
+        (is (= ["apple" "plum" nil]
+               result)
+            "sort-by asc")))
 
   (testing "sort-by hidden, built-in block property created-at"
-    (let [result (->> (dsl-query "(and (task now later done) (sort-by created-at desc))")
-                      (map #(get-in % [:block/properties :created-at])))]
-      (is (= [1608968448115 1608968448114 1608968448113]
-             result))
-      "sorted-by desc")
-
-    (let [result (->> (dsl-query "(and (todo now later done) (sort-by created-at asc))")
-                      (map #(get-in % [:block/properties :created-at])))]
-      (is (= [1608968448113 1608968448114 1608968448115]
-             result)
-          "sorted-by asc")))
+      (let [result (->> (dsl-query "(and (task now later done) (sort-by created-at desc))")
+                        (map #(get-property-value % :created-at)))]
+        (is (= [1608968448115 1608968448114 1608968448113]
+               result))
+        "sorted-by desc")
+
+      (let [result (->> (dsl-query "(and (todo now later done) (sort-by created-at asc))")
+                        (map #(get-property-value % :created-at)))]
+        (is (= [1608968448113 1608968448114 1608968448115]
+               result)
+            "sorted-by asc")))
 
   (testing "user page property rating"
-    (is (= [10 8]
-           (->> (dsl-query "(and (page-property rating) (sort-by rating))")
-                (map #(get-in % [:block/properties :rating])))))))
+      (is (= [10 8]
+             (->> (dsl-query "(and (page-property rating) (sort-by rating))")
+                  (map #(get-property-value % :rating)))))))
 
 (deftest simplify-query
   (are [x y] (= (query-dsl/simplify-query x) y)

src/test/frontend/test/helper.cljs

@@ -7,6 +7,7 @@
             [clojure.set :as set]
             [frontend.modules.outliner.core :as outliner-core]
             [frontend.db :as db]
+            [frontend.date :as date]
             [datascript.core :as d]
             [logseq.graph-parser.text :as text]))
 
@@ -33,6 +34,13 @@
       new-val
       value)))
 
+(defn- property-lines->properties
+  [property-lines]
+  (->> property-lines
+       (map #(let [[k v] (string/split % #"::\s*" 2)]
+               [(keyword k) (parse-property-value v)]))
+       (into {})))
+
 (defn- build-block-properties
   "Parses out properties from a file's content and associates it with the page name
    or block content"
@@ -42,16 +50,22 @@
      (->> (string/split file-content #"\n-\s*")
           (mapv (fn [s]
                   (let [[content & props] (string/split-lines s)]
-                    [content (->> props
-                                  (map #(let [[k v] (string/split % #"::\s*" 2)]
-                                          [(keyword k) (parse-property-value v)]))
-                                  (into {}))]))))}
+                    {:name-or-content content
+                     :properties (property-lines->properties props)}))))}
     {:page-properties
      (->> file-content
           string/split-lines
-          (map #(let [[k v] (string/split % #"::\s*" 2)]
-                  [(keyword k) (parse-property-value v)]))
-          (into {}))}))
+          property-lines->properties)}))
+
+(defn- file-path->page-name
+  [file-path]
+  (or (if (string/starts-with? file-path "journals")
+        (some-> (second (re-find #"([^/]+).md" file-path))
+                date/normalize-date
+                date/journal-name
+                string/lower-case)
+        (second (re-find #"([^/]+).md" file-path)))
+      (throw (ex-info "No page found" {}))))
 
 (defn- update-file-for-db-graph
   "Adds properties by block/page for a file and updates block content"
@@ -60,15 +74,22 @@
         (build-block-properties (:file/content file))]
     (if page-properties
       (merge file
-             {:file/block-properties [[(or (second (re-find #"([^/]+).md" (:file/path file)))
-                                           (throw (ex-info "No page found" {})))
-                                       page-properties]]
-              :page-properties? true})
+             {:file/block-properties [{:name-or-content (file-path->page-name (:file/path file))
+                                       :properties page-properties
+                                       :page-properties? true}]
+              :page-properties? false})
       (merge file
-             {:file/block-properties block-properties
-                                 ;; Rewrite content to strip it of properties which shouldn't be in content
+             {:file/block-properties (cond-> block-properties
+                                       ;; Optionally add page properties as a page block
+                                       (re-find #"^\s*[^-]+" (:name-or-content (first block-properties)))
+                                       (conj {:name-or-content (file-path->page-name (:file/path file))
+                                              :properties (->> (:name-or-content (first block-properties))
+                                                               string/split-lines
+                                                               property-lines->properties)
+                                              :page-properties? true}))
+              ;; Rewrite content to strip it of properties which shouldn't be in content
               :file/content (string/join "\n"
-                                         (map (fn [x] (str "- " (first x))) block-properties))}))))
+                                         (map (fn [x] (str "- " (:name-or-content x))) block-properties))}))))
 
 (defn- load-test-files-for-db-graph
   [files*]
@@ -91,14 +112,14 @@
                                     [?b :block/uuid ?uuid]]
                                   (db/get-db test-db)))
           property-uuids (->> files
-                              (mapcat #(->> % :file/block-properties (map second) (mapcat keys)))
+                              (mapcat #(->> % :file/block-properties (map :properties) (mapcat keys)))
                               set
                               ;; Property pages may be created by file graphs automatically,
                               ;; usually by page properties. Delete this if file graphs are long
                               ;; used to create datascript db
                               (map #(vector % (or (page-name-map (name %)) (random-uuid))))
                               (into {}))
-            ;; from upsert-property!
+          ;; from upsert-property!
           new-properties-tx (mapv (fn [[prop-name uuid]]
                                     (outliner-core/block-with-timestamps
                                      {:block/uuid uuid
@@ -110,7 +131,7 @@
           page-uuids (->> files
                           (mapcat #(->> %
                                         :file/block-properties
-                                        (map second)
+                                        (map :properties)
                                         (mapcat (fn [m]
                                                   (->> m vals (filter set?) (apply set/union))))))
                           set
@@ -122,19 +143,19 @@
                             :block/original-name page-name
                             :block/uuid uuid}))
                         page-uuids)
-            ;; from add-property!
+          ;; from add-property!
           block-properties-tx
           (mapcat
-           (fn [{:keys [page-properties?] :as file}]
+           (fn [file]
              (map
-              (fn [[page-name-or-content props]]
+              (fn [{:keys [name-or-content properties page-properties?]}]
                 {:block/uuid (if page-properties?
-                               (or (page-name-map page-name-or-content)
-                                   (throw (ex-info "No uuid for page" {:page-name page-name-or-content})))
-                               (or (content-uuid-map page-name-or-content)
-                                   (throw (ex-info "No uuid for content" {:content page-name-or-content}))))
+                               (or (page-name-map name-or-content)
+                                   (throw (ex-info "No uuid for page" {:page-name name-or-content})))
+                               (or (content-uuid-map name-or-content)
+                                   (throw (ex-info "No uuid for content" {:content name-or-content}))))
                  :block/properties
-                 (->> props
+                 (->> properties
                       (map
                        (fn [[prop-name val]]
                          [(or (property-uuids prop-name)