fix: properties stable sort, normalize :block/order of properties

1. When sorting properties, consider cases where values are nil or blocks/orders are identical.
2. If block/order are nil or duplicated in sorted-properties, recalculate and transact them

deps/db/src/logseq/db/frontend/property.cljs

@@ -6,6 +6,7 @@
             [flatland.ordered.map :refer [ordered-map]]
             [logseq.common.defkeywords :refer [defkeywords]]
             [logseq.common.uuid :as common-uuid]
+            [logseq.db.common.order :as db-order]
             [logseq.db.frontend.db-ident :as db-ident]
             [logseq.db.frontend.property.type :as db-property-type]))
 
@@ -715,10 +716,52 @@
   ([property-name user-namespace]
    (db-ident/create-db-ident-from-name user-namespace property-name)))
 
+(defn normalize-sorted-entities-block-order
+  "Return tx-data.
+  Generate appropriate :block/order values for sorted-blocks with :block/order value = nil or duplicated"
+  [sorted-entities]
+  (let [parts (partition-by :block/order sorted-entities)
+        [_ tx-data]
+        (reduce (fn [[start-order tx-data] ents]
+                  (let [n (count ents)]
+                    (if (> n 1)
+                      (let [orders (db-order/gen-n-keys n start-order (:block/order (first ents)))
+                            tx-data* (apply conj tx-data (map
+                                                          (fn [order ent]
+                                                            {:db/id (:db/id ent)
+                                                             :block/order order})
+                                                          orders ents))]
+                        [(last orders) tx-data*])
+                      [(:block/order (first ents)) tx-data])))
+                [nil []] parts)]
+    tx-data))
+
+(defn sort-properties
+  "Sort by :block/order and :block/uuid.
+  - nil is greater than non-nil
+  - When block/order is equal, sort by block/uuid"
+  [prop-entities]
+  (sort
+   (fn [a b]
+     (let [order-a (:block/order a)
+           order-b (:block/order b)]
+       (cond
+         (and (nil? order-a) (nil? order-b))
+         (compare (:block/uuid a) (:block/uuid b))
+
+         (nil? order-a) 1
+         (nil? order-b) -1
+
+         (= order-a order-b)
+         (compare (:block/uuid a) (:block/uuid b))
+
+         :else
+         (compare order-a order-b))))
+   prop-entities))
+
 (defn get-class-ordered-properties
   [class-entity]
-  (->> (:logseq.property.class/properties class-entity)
-       (sort-by :block/order)))
+  (sort-properties (:logseq.property.class/properties class-entity)))
 
 (defn property-created-block?
   "`block` has been created in a property and it's not a closed value."

deps/db/test/logseq/db/frontend/property_test.cljs

@@ -0,0 +1,45 @@
+(ns logseq.db.frontend.property-test
+  (:require [cljs.test :refer [deftest is testing are]]
+            [logseq.db.frontend.property :as db-property]))
+
+(deftest sort-properties
+  (let [p1 {:db/id 1, :block/order "a", :block/uuid "uuid-a"}
+        p2 {:db/id 2, :block/order "b", :block/uuid "uuid-b"}
+        p3 {:db/id 3, :block/order nil, :block/uuid "uuid-d"}
+        p4 {:db/id 4, :block/order "b", :block/uuid "uuid-c"}
+        p5 {:db/id 5, :block/order nil, :block/uuid "uuid-e"}]
+    (is (= [p1 p2 p4 p3 p5]
+           (db-property/sort-properties [p3 p1 p5 p2 p4])))))
+
+(deftest normalize-block-order-tx-data-test
+  (testing "Generate appropriate :block/order values for sorted-blocks with :block/order value = nil or duplicated"
+    (let [p1 {:db/id 1, :block/order "a0"}
+          p2 {:db/id 2, :block/order "bbb"}
+          p3 {:db/id 3, :block/order "bbb"}
+          p4 {:db/id 4, :block/order nil}
+          p5 {:db/id 5, :block/order nil}
+          sorted-entities [p1 p2 p3 p4 p5]
+          tx-data (db-property/normalize-sorted-entities-block-order sorted-entities)
+          ;; apply tx-data to entities
+          tx-map (into {} (map (juxt :db/id identity) tx-data))
+          updated-entities (map (fn [ent]
+                                  (if-let [tx (get tx-map (:db/id ent))]
+                                    (merge ent tx)
+                                    ent))
+                                sorted-entities)
+          ;; sort again and test
+          final-sorted (db-property/sort-properties updated-entities)]
+      (is (= 5 (count final-sorted)))
+      ;; Check that all orders are now strings
+      (is (every? string? (map :block/order final-sorted)))
+      ;; Check that all orders are unique
+      (is (= 5 (count (set (map :block/order final-sorted)))))
+      ;; Check that the final list is sorted correctly by the new orders
+      (is (= final-sorted (sort-by :block/order final-sorted)))))
+
+  (testing "No changes needed for already valid orders"
+    (let [p1 {:db/id 1, :block/order "b00"}
+          p2 {:db/id 2, :block/order "b01"}
+          sorted-entities [p1 p2]
+          tx-data (db-property/normalize-sorted-entities-block-order sorted-entities)]
+      (is (empty? tx-data)))))

src/main/frontend/components/property.cljs

@@ -506,7 +506,7 @@
                 (pv/property-value block property opts))]]])]))))
 
 (rum/defc ordered-properties
-  [block properties* opts]
+  [block properties* sorted-property-entities opts]
   (let [[properties set-properties!] (hooks/use-state properties*)
         [properties-order set-properties-order!] (hooks/use-state (mapv first properties))
         m (zipmap (map first properties*) (map second properties*))
@@ -530,15 +530,21 @@
                {:sort-by-inner-element? true
                 :on-drag-end (fn [properties-order {:keys [active-id over-id direction]}]
                                (set-properties-order! properties-order)
-                               (let [move-down? (= direction :down)
-                                     over (db/entity (keyword over-id))
-                                     active (db/entity (keyword active-id))
-                                     over-order (:block/order over)
-                                     new-order (if move-down?
-                                                 (let [next-order (db-order/get-next-order (db/get-db) nil (:db/id over))]
-                                                   (db-order/gen-key over-order next-order))
-                                                 (let [prev-order (db-order/get-prev-order (db/get-db) nil (:db/id over))]
-                                                   (db-order/gen-key prev-order over-order)))]
+                               (p/let [;; Before reordering properties,
+                                       ;; check if the :block/order of these properties is reasonable.
+                                       normalize-tx-data (db-property/normalize-sorted-entities-block-order
+                                                          sorted-property-entities)
+                                       _ (when (seq normalize-tx-data)
+                                           (db/transact! (state/get-current-repo) normalize-tx-data))
+                                       move-down? (= direction :down)
+                                       over (db/entity (keyword over-id))
+                                       active (db/entity (keyword active-id))
+                                       over-order (:block/order over)
+                                       new-order (if move-down?
+                                                   (let [next-order (db-order/get-next-order (db/get-db) nil (:db/id over))]
+                                                     (db-order/gen-key over-order next-order))
+                                                   (let [prev-order (db-order/get-prev-order (db/get-db) nil (:db/id over))]
+                                                     (db-order/gen-key prev-order over-order)))]
                                  (db/transact! (state/get-current-repo)
                                                [{:db/id (:db/id active)
                                                  :block/order new-order}
@@ -549,12 +555,10 @@
 (rum/defc properties-section < rum/static
   [block properties opts]
   (when (seq properties)
-      ;; Sort properties by :block/order
-    (let [properties' (sort-by (fn [[k _v]]
-                                 (if (= k :logseq.property.class/properties)
-                                   "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"
-                                   (:block/order (db/entity k)))) properties)]
-      (ordered-properties block properties' opts))))
+    (let [sorted-prop-entities (db-property/sort-properties (map (comp db/entity first) properties))
+          prop-kv-map (reduce (fn [m [p v]] (assoc m p v)) {} properties)
+          properties' (keep (fn [ent] (find prop-kv-map (:db/ident ent))) sorted-prop-entities)]
+      (ordered-properties block properties' sorted-prop-entities opts))))
 
 (rum/defc hidden-properties-cp
   [block hidden-properties {:keys [root-block? sidebar-properties?] :as opts}]