Fix: query table sorting (#7751)

* test: sorting for mixed type block properties

* test: edited integer sorting test

Wanted to ensure it included a case where the sorting would
behave different if the numbers were coerced to strings.

In that case, "77" would precede "8" whereas for integer sorting
we want "8" to precede "77".

Renamed `:rating` to `:integer` to reflect this new scenario.

* test: new decimal number test

* test: query table sort semver-style string block properties

* test: query table sort positive and negative integers

* fix: replace separate sort strategies with natural sorting strategy

* fix: comparing only numbers in query table sort

* test: consolidate query table sort integer tests

* format: removed line breaks and consolidated small comparison lines to pass large-vars lint

* Tweaks to PR

- use reader literal for js
- Bring back readability to test, each case should have whitespace
  between it. Split up deftest at a natural spot as deftest exceeds
  reasonable size of a large var

Co-authored-by: Junyi Du <[email protected]>
Co-authored-by: Gabriel Horner <[email protected]>
Co-authored-by: Gabriel Horner <[email protected]>

src/main/frontend/components/query_table.cljs

@@ -42,9 +42,9 @@
 (defn- locale-compare
   "Use locale specific comparison for strings and general comparison for others."
   [x y]
-  (if (and (string? x) (string? y))
-    (.localeCompare x y (state/sub :preferred-language))
-    (< x y)))
+    (if (and (number? x) (number? y))
+      (< x y)
+      (.localeCompare (str x) (str y) (state/sub :preferred-language) #js {:numeric true})))
 
 (defn- sort-result [result {:keys [sort-by-column sort-desc? sort-nlp-date?]}]
   (if (some? sort-by-column)

src/test/frontend/components/query_table_test.cljs

@@ -13,48 +13,78 @@
          (= (mapv #(hash-map :block/content %) sorted-result)
             (#'query-table/sort-result (mapv #(hash-map :block/content %) result) sort-state))
          {:sort-desc? true :sort-by-column :block}
-         ["abc" "cde"]
-         ["cde" "abc"]
+         ["abc" "cde"] ["cde" "abc"]
 
          {:sort-desc? false :sort-by-column :block}
-         ["abc" "cde"]
-         ["abc" "cde"]))
+         ["abc" "cde"] ["abc" "cde"]))
 
   (testing "sort by integer block property"
     (are [sort-state result sorted-result]
          (= (mapv #(hash-map :block/properties %) sorted-result)
             (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
-         {:sort-desc? true :sort-by-column :rating}
-         [{:rating 8} {:rating 7}]
-         [{:rating 8} {:rating 7}]
+         {:sort-desc? true :sort-by-column :integer}
+         [{:integer 8} {:integer 7} {:integer 77} {:integer 0} {:integer -8}]
+         [{:integer 77} {:integer 8} {:integer 7} {:integer 0} {:integer -8}]
 
-         {:sort-desc? false :sort-by-column :rating}
-         [{:rating 8} {:rating 7}]
-         [{:rating 7} {:rating 8}]))
+         {:sort-desc? false :sort-by-column :integer}
+         [{:integer 8} {:integer 7} {:integer 77} {:integer 0} {:integer -8}]
+         [{:integer -8} {:integer 0} {:integer 7} {:integer 8} {:integer 77}]))
 
   (testing "sort by boolean block property"
     (are [sort-state result sorted-result]
          (= (mapv #(hash-map :block/properties %) sorted-result)
             (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
          {:sort-desc? true :sort-by-column :funny?}
-         [{:funny? true} {:funny? false}]
-         [{:funny? true} {:funny? false}]
+         [{:funny? true} {:funny? false}] [{:funny? true} {:funny? false}]
 
          {:sort-desc? false :sort-by-column :funny?}
-         [{:funny? true} {:funny? false}]
-         [{:funny? false} {:funny? true}]))
+         [{:funny? true} {:funny? false}] [{:funny? false} {:funny? true}]))
 
   (testing "sort by string block property"
     (are [sort-state result sorted-result]
          (= (mapv #(hash-map :block/properties %) sorted-result)
             (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
          {:sort-desc? true :sort-by-column :title}
-         [{:title "abc"} {:title "cde"}]
-         [{:title "cde"} {:title "abc"}]
+         [{:title "abc"} {:title "cde"}] [{:title "cde"} {:title "abc"}]
 
          {:sort-desc? false :sort-by-column :title}
-         [{:title "abc"} {:title "cde"}]
-         [{:title "abc"} {:title "cde"}]))
+         [{:title "abc"} {:title "cde"}] [{:title "abc"} {:title "cde"}]))
+
+  (testing "sort by mixed type block property"
+    (are [sort-state result sorted-result]
+         (= (mapv #(hash-map :block/properties %) sorted-result)
+            (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
+         {:sort-desc? true :sort-by-column :title}
+         [{:title 1} {:title "A"} {:title 2} {:title "B"} {:title 11} {:title "C"}]
+         [{:title "C"} {:title "B"} {:title "A"} {:title 11} {:title 2} {:title 1}]
+
+         {:sort-desc? false :sort-by-column :title}
+         [{:title 1} {:title "A"} {:title 2} {:title "B"} {:title 11} {:title "C"}]
+         [{:title 1} {:title 2} {:title 11} {:title "A"} {:title "B"} {:title "C"}]))
+
+  (testing "sort by decimal number block property"
+    (are [sort-state result sorted-result]
+         (= (mapv #(hash-map :block/properties %) sorted-result)
+            (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
+         {:sort-desc? true :sort-by-column :title}
+         [{:title 1.1} {:title 1.2} {:title 1.11}]
+         [{:title 1.2} {:title 1.11} {:title 1.1}]
+
+         {:sort-desc? false :sort-by-column :title}
+         [{:title 1.1} {:title 1.2} {:title 1.11}]
+         [{:title 1.1} {:title 1.11} {:title 1.2}]))
+
+   (testing "sort by semver-style string block property"
+     (are [sort-state result sorted-result]
+          (= (mapv #(hash-map :block/properties %) sorted-result)
+             (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
+          {:sort-desc? true :sort-by-column :title}
+          [{:title "1.1.0"} {:title "1.2.0"} {:title "1.11.0"} {:title "1.1.1"} {:title "1.11.1"} {:title "1.2.1"}]
+          [{:title "1.11.1"} {:title "1.11.0"} {:title "1.2.1"} {:title "1.2.0"} {:title "1.1.1"} {:title "1.1.0"}]
+
+          {:sort-desc? false :sort-by-column :title}
+          [{:title "1.1.0"} {:title "1.2.0"} {:title "1.11.0"} {:title "1.1.1"} {:title "1.11.1"} {:title "1.2.1"}]
+          [{:title "1.1.0"} {:title "1.1.1"} {:title "1.2.0"} {:title "1.2.1"} {:title "1.11.0"} {:title "1.11.1"}]))
 
   (testing "sort by string block property for specific locale"
     (state/set-preferred-language! "zh-CN")
@@ -62,24 +92,21 @@
          (= (mapv #(hash-map :block/properties %) sorted-result)
             (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))
          {:sort-desc? true :sort-by-column :title}
-         [{:title "意志"} {:title "圆圈"}]
-         [{:title "圆圈"} {:title "意志"}]
+         [{:title "意志"} {:title "圆圈"}] [{:title "圆圈"} {:title "意志"}]
 
          {:sort-desc? false :sort-by-column :title}
-         [{:title "圆圈"} {:title "意志"}]
-         [{:title "意志"} {:title "圆圈"}])
-    (state/set-preferred-language! "en"))
+         [{:title "圆圈"} {:title "意志"}] [{:title "意志"} {:title "圆圈"}])
+    (state/set-preferred-language! "en")))
 
+(deftest sort-result-performance
   (testing "monitor time of sort by integer block property"
     (are [sort-state result _sorted-result timeout]
          (>= timeout (:time (util/with-time (#'query-table/sort-result (mapv #(hash-map :block/properties %) result) sort-state))))
       {:sort-desc? true :sort-by-column :rating}
-      [{:rating 8} {:rating 7}]
-      [{:rating 8} {:rating 7}]
-      2.0 ;; actual: ~0.05
-
+      [{:rating 8} {:rating 7}] [{:rating 8} {:rating 7}]
+         2.0 ;; actual: ~0.05
+ 
       {:sort-desc? false :sort-by-column :rating}
-      [{:rating 8} {:rating 7}]
-      [{:rating 7} {:rating 8}]
+      [{:rating 8} {:rating 7}] [{:rating 7} {:rating 8}]
       2.0 ;; actual: ~0.05
       )))