From: Christian Heller <c.heller@plomlompom.de>
Date: Fri, 12 Jul 2024 00:31:25 +0000 (+0200)
Subject: On BaseModel.sort_by, always sort by .id_ first, for predictability.
X-Git-Url: https://plomlompom.com/repos/%7B%7Bdb.prefix%7D%7D/static/%7B%7B%20web_path%20%7D%7D/%7B%7Bprefix%7D%7D/todos?a=commitdiff_plain;h=c27144c198e6ca0b84870a90085fedf620c4242e;p=plomtask

On BaseModel.sort_by, always sort by .id_ first, for predictability.
---

diff --git a/plomtask/db.py b/plomtask/db.py
index f1169c3..dac3e39 100644
--- a/plomtask/db.py
+++ b/plomtask/db.py
@@ -345,13 +345,18 @@ class BaseModel(Generic[BaseModelId]):
     @classmethod
     def sort_by(cls, seq: list[Any], sort_key: str, default: str = 'title'
                 ) -> str:
-        """Sort cls list by cls.sorters[sort_key] (reverse if '-'-prefixed)."""
+        """Sort cls list by cls.sorters[sort_key] (reverse if '-'-prefixed).
+
+        Before cls.sorters[sort_key] is applied, seq is sorted by .id_, to
+        ensure predictability where parts of seq are of same sort value.
+        """
         reverse = False
         if len(sort_key) > 1 and '-' == sort_key[0]:
             sort_key = sort_key[1:]
             reverse = True
         if sort_key not in cls.sorters:
             sort_key = default
+        seq.sort(key=lambda x: x.id_, reverse=reverse)
         sorter: Callable[..., Any] = cls.sorters[sort_key]
         seq.sort(key=sorter, reverse=reverse)
         if reverse:
diff --git a/tests/conditions.py b/tests/conditions.py
index 69dcc66..1a6b08e 100644
--- a/tests/conditions.py
+++ b/tests/conditions.py
@@ -166,16 +166,15 @@ class TestsWithServer(TestCaseWithServer):
         expected = self.GET_conditions_dict([cond2, cond3, cond1])
         self.check_json_get('/conditions', expected)
         # test other sortings
-        # (NB: by .is_active has two items of =False, their order currently
-        # is not explicitly made predictable, so _may_ fail until we do)
         expected['sort_by'] = '-title'
-        expected['conditions'] = self.as_id_list([cond1, cond3, cond2])
+        assert isinstance(expected['conditions'], list)
+        expected['conditions'].reverse()
         self.check_json_get('/conditions?sort_by=-title', expected)
         expected['sort_by'] = 'is_active'
         expected['conditions'] = self.as_id_list([cond1, cond2, cond3])
         self.check_json_get('/conditions?sort_by=is_active', expected)
         expected['sort_by'] = '-is_active'
-        expected['conditions'] = self.as_id_list([cond3, cond1, cond2])
+        expected['conditions'].reverse()
         self.check_json_get('/conditions?sort_by=-is_active', expected)
         # test pattern matching on title
         expected = self.GET_conditions_dict([cond2, cond3])