Fix sort-by not working for blocks

- Also add sort-by support for pages which fixes #5643
- Add sort-by tests
- Fix sample producing weird queries e.g. mixed block and page clauses
- Clean up last, hard to read test setup in query-dsl

src/main/frontend/db/query_dsl.cljs

@@ -38,11 +38,7 @@
 ;; project (block, TBD)
 
 ;; Sort by (field, asc/desc):
-
-;; created_at
-;; last_modified_at
-
-;; (sort-by last_modified_at asc)
+;; (sort-by created-at asc)
 
 ;; (between -7d +7d)
 
@@ -151,13 +147,16 @@
 
       :else
       (->> clauses
-           (map (fn [result]
-                  (if (list? result)
-                    result
-                    (let [result (if (vector? (ffirst result))
-                                   (apply concat result)
-                                   result)]
-                      (cons 'and (seq result))))))
+           (mapcat (fn [result]
+                     (cond
+                       ;; rule like (task ?b #{"NOW"})
+                       (list? result)
+                       [result]
+                       ;; datalog clause like [[?b :block/uuid]]
+                       (vector? result)
+                       result
+                       :else
+                       [(cons 'and (seq result))])))
            (apply list fe)))
 
     :else
@@ -320,33 +319,24 @@
   (when-let [num (second e)]
     (when (integer? num)
       (reset! sample num)
-      {:query [['?p :block/uuid]]})))
+      ;; blank b/c this post-process filter doesn't effect query
+      {})))
 
 (defn- build-sort-by
   [e sort-by_]
-  (let [[k order] (rest e)
-             order (if (and order (contains? #{:asc :desc}
-                                             (keyword (string/lower-case (name order)))))
-                     (keyword (string/lower-case (name order)))
-                     :desc)
-             k (-> (string/lower-case (name k))
-                   (string/replace "_" "-"))
-             get-value (cond
-                         (= k "created-at")
-                         :block/created-at
-
-                         (= k "updated-at")
-                         :block/updated-at
-
-                         :else
-                         #(get-in % [:block/properties k]))
-             comp (if (= order :desc) >= <=)]
-         (reset! sort-by_
-                 (fn [result]
-                   (->> result
-                        flatten
-                        (sort-by get-value comp))))
-         nil))
+  (let [[k order*] (map keyword (rest e))
+        order (if (contains? #{:asc :desc} order*)
+                order*
+                :desc)
+        comp (if (= order :desc) >= <=)]
+    (reset! sort-by_
+            (fn sort-results [result]
+              ;; first because there is one binding result in query-wrapper
+              (sort-by #(-> % first (get-in [:block/properties k]))
+                       comp
+                       result)))
+    ;; blank b/c this post-process filter doesn't effect query
+    {}))
 
 (defn- build-page
   [e]
@@ -392,7 +382,7 @@ Some bindings in this fn:
          page-ref? (page-ref/page-ref? e)]
      (when (or (and page-ref?
                     (not (contains? #{'page-property 'page-tags} (:current-filter env))))
-               (contains? #{'between 'property 'todo 'task 'priority 'sort-by 'page} fe)
+               (contains? #{'between 'property 'todo 'task 'priority 'page} fe)
                (and (not page-ref?) (string? e)))
        (reset! blocks? true))
      (cond

src/test/frontend/db/query_dsl_test.cljs

@@ -235,10 +235,10 @@ prop-d:: nada"}])
 
   (is (= 1
          (count (dsl-query "(and (task todo) (sample 1))")))
-      "Correctly limits results")
-  (is (= 2
-         (count (dsl-query "(and (task todo) (sample blarg))")))
-      "Non-integer arg is ignored"))
+      "Correctly limits block results")
+  (is (= 1
+         (count (dsl-query "(and (page-property foo) (sample 1))")))
+      "Correctly limits page results"))
 
 (deftest priority-queries
   (load-test-files [{:file/path "pages/page1.md"
@@ -447,76 +447,30 @@ tags: other
          (map :block/content
               (dsl-query "(and [[Parent page]] (not [[Child page]]))")))))
 
-(defn- load-test-files-with-timestamps
-  []
-  (let [files [{:file/path "journals/2020_12_26.md"
-                :file/content "---
-title: Dec 26th, 2020
----
-- DONE 26-b1
+(deftest between-queries
+  (load-test-files [{:file/path "journals/2020_12_26.md"
+                     :file/content "- DONE 26-b1
 created-at:: 1608968448113
-last-modified-at:: 1608968448113
 - LATER 26-b2-modified-later
 created-at:: 1608968448114
-last-modified-at:: 1608968448120
 - DONE 26-b3
 created-at:: 1608968448115
-last-modified-at:: 1608968448115
-"}
-               {:file/path "journals/2020_12_27.md"
-                :file/content "---
-title: Dec 27th, 2020
----
-- NOW b1
-created-at:: 1609052958714
-last-modified-at:: 1609052958714
-- LATER b2-modified-later
-created-at:: 1609052959376
-last-modified-at:: 1609052974285
-- b3
-created-at:: 1609052959954
-last-modified-at:: 1609052959954
-- b4
-created-at:: 1609052961569
-last-modified-at:: 1609052961569
-- b5
-created-at:: 1609052963089
-last-modified-at:: 1609052963089"}
-               {:file/path "journals/2020_12_28.md"
-                :file/content "---
-title: Dec 28th, 2020
----
-- 28-b1
-created-at:: 1609084800000
-last-modified-at:: 1609084800000
-- 28-b2-modified-later
-created-at:: 1609084800001
-last-modified-at:: 1609084800020
-- 28-b3
-created-at:: 1609084800002
-last-modified-at:: 1609084800002"}]]
-    (load-test-files files)))
-
-(deftest between-queries
-  (load-test-files-with-timestamps)
+- 26-b4
+created-at:: 1608968448116
+"}])
 
   (are [x y] (= (count (dsl-query x)) y)
        "(and (task now later done) (between [[Dec 26th, 2020]] tomorrow))"
-       5
+       3
 
        ;; between with journal pages
-       "(and (task now later done) (between [[Dec 27th, 2020]] [[Dec 28th, 2020]]))"
-       2
+       "(and (task now later done) (between [[Dec 26th, 2020]] [[Dec 27th, 2020]]))"
+       3
 
        ;; ;; between with created-at
        ;; "(and (task now later done) (between created-at [[Dec 26th, 2020]] tomorrow))"
-       ;; 5
-
-       ;; ;; between with last-modified-at
-       ;; "(and (task now later done) (between last-modified-at [[Dec 26th, 2020]] tomorrow))"
-       ;; 5
-       )
-  )
+       ;; 3
+       ))
 
 (deftest custom-query-test
   (load-test-files [{:file/path "pages/page1.md"
@@ -533,63 +487,63 @@ last-modified-at:: 1609084800002"}]]
          (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
-    (load-test-files-with-timestamps)
-    ;; (testing "sort-by (created-at defaults to desc)"
-    ;;   (db/clear-query-state!)
-    ;;   (let [result (->> (dsl-query "(and (task now later done)
-    ;;                              (sort-by created-at))")
-    ;;                     (map #(get-in % [:block/properties "created-at"])))]
-    ;;     (is (= result
-    ;;            '(1609052959376 1609052958714 1608968448115 1608968448114 1608968448113)))))
-
-    ;; (testing "sort-by (created-at desc)"
-    ;;   (db/clear-query-state!)
-    ;;   (let [result (->> (dsl-query "(and (todo now later done)
-    ;;                              (sort-by created-at desc))")
-    ;;                     (map #(get-in % [:block/properties "created-at"])))]
-    ;;     (is (= result
-    ;;            '(1609052959376 1609052958714 1608968448115 1608968448114 1608968448113)))))
-
-    ;; (testing "sort-by (created-at asc)"
-    ;;   (db/clear-query-state!)
-    ;;   (let [result (->> (dsl-query "(and (todo now later done)
-    ;;                              (sort-by created-at asc))")
-    ;;                     (map #(get-in % [:block/properties "created-at"])))]
-    ;;     (is (= result
-    ;;            '(1608968448113 1608968448114 1608968448115 1609052958714 1609052959376)))))
-
-    ;; (testing "sort-by (last-modified-at defaults to desc)"
-    ;;   (db/clear-query-state!)
-    ;;   (let [result (->> (dsl-query "(and (todo now later done)
-    ;;                              (sort-by last-modified-at))")
-    ;;                     (map #(get-in % [:block/properties "last-modified-at"])))]
-    ;;     (is (= result
-    ;;            '(1609052974285 1609052958714 1608968448120 1608968448115 1608968448113)))))
-
-    ;; (testing "sort-by (last-modified-at desc)"
-    ;;   (db/clear-query-state!)
-    ;;   (let [result (->> (dsl-query "(and (todo now later done)
-    ;;                              (sort-by last-modified-at desc))")
-    ;;                     (map #(get-in % [:block/properties "last-modified-at"])))]
-    ;;     (is (= result
-    ;;            '(1609052974285 1609052958714 1608968448120 1608968448115 1608968448113)))))
-
-    ;; (testing "sort-by (last-modified-at desc)"
-    ;;   (db/clear-query-state!)
-    ;;   (let [result (->> (dsl-query "(and (todo now later done)
-    ;;                              (sort-by last-modified-at asc))")
-    ;;                     (map #(get-in % [:block/properties "last-modified-at"])))]
-    ;;     (is (= result
-    ;;            '(1608968448113 1608968448115 1608968448120 1609052958714 1609052974285)))))
-    )
-
-#_(cljs.test/run-tests)
+(deftest sort-by-queries
+  (load-test-files [{:file/path "journals/2020_02_25.md"
+                     :file/content "rating:: 10"}
+                    {:file/path "journals/2020_12_26.md"
+                     :file/content "rating:: 8
+- DONE 26-b1
+created-at:: 1608968448113
+fruit:: plum
+- LATER 26-b2-modified-later
+created-at:: 1608968448114
+fruit:: apple
+- DONE 26-b3 has no fruit to test sorting of absent property value
+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])))]
+      (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"))
+
+    (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")))
+
+  (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")))
+
+  (testing "user page property rating"
+    (is (= [10 8]
+           (->> (dsl-query "(and (page-property rating) (sort-by rating))")
+                (map #(get-in % [:block/properties :rating])))))))
 
 (comment
  (require '[clojure.pprint :as pprint])
  (test-helper/start-test-db!)
- (load-test-files-with-timestamps)
 
  (query-dsl/query test-db "(task done)")