From: Christian Heller 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/%7B%7B%20web_path%20%7D%7D/decks/%7B%7Bprefix%7D%7D/edit?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])