diff --git a/games/sorting.py b/games/sorting.py index 05ea9b9..6f53845 100644 --- a/games/sorting.py +++ b/games/sorting.py @@ -13,10 +13,16 @@ from django.http import HttpRequest from games.filters import FindFilter -type SortKey = str # public column key in a *_SORTS map and in a URL term ("playtime", "name") -type SortString = str # comma-list of signed SortKeys: the URL ?sort= value and *_DEFAULT_SORT ("-date,created") -type AnnotationName = str # an alias added via .annotate(), then referenced by SortSpec.expression -type OrderField = str # SortSpec.expression: a real model field path OR an AnnotationName +type SortKey = ( + str # public column key in a *_SORTS map and in a URL term ("playtime", "name") +) +type SortString = str # comma-list of signed SortKeys: the URL ?sort= value and *_DEFAULT_SORT ("-date,created") +type AnnotationName = ( + str # an alias added via .annotate(), then referenced by SortSpec.expression +) +type OrderField = ( + str # SortSpec.expression: a real model field path OR an AnnotationName +) # alias name -> the ORM aggregate that computes it, applied via queryset.annotate() # e.g. {"total_playtime": Sum("sessions__duration_total")} @@ -25,13 +31,13 @@ type Annotations = dict[AnnotationName, Aggregate] @dataclass(frozen=True) class SortSpec: - expression: OrderField # unsigned; a real column path or an AnnotationName + expression: OrderField # unsigned; a real column path or an AnnotationName annotate: Annotations | None = None class SortTerm(NamedTuple): key: SortKey - descending: bool # True = "-key" (desc), False = bare key (asc) + descending: bool # True = "-key" (desc), False = bare key (asc) type SortMap = dict[SortKey, SortSpec] @@ -39,7 +45,7 @@ type SortMap = dict[SortKey, SortSpec] class ParsedSort(NamedTuple): terms: list[SortTerm] - unknown: list[SortKey] # keys not in the map — the view turns these into warnings + unknown: list[SortKey] # keys not in the map — the view turns these into warnings def parse_sort_terms(raw: SortString, sort_map: SortMap) -> ParsedSort: @@ -69,7 +75,9 @@ GAME_SORTS: SortMap = { "status": SortSpec("status"), "wikidata": SortSpec("wikidata"), "created": SortSpec("created_at"), - "playtime": SortSpec("total_playtime", {"total_playtime": Sum("sessions__duration_total")}), + "playtime": SortSpec( + "total_playtime", {"total_playtime": Sum("sessions__duration_total")} + ), "finished": SortSpec("last_finished", {"last_finished": Max("playevents__ended")}), } GAME_DEFAULT_SORT: SortString = "-created" @@ -91,7 +99,9 @@ PURCHASE_SORTS: SortMap = { "purchased": SortSpec("date_purchased"), "refunded": SortSpec("date_refunded"), "created": SortSpec("created_at"), - "finished": SortSpec("last_finished", {"last_finished": Max("games__playevents__ended")}), + "finished": SortSpec( + "last_finished", {"last_finished": Max("games__playevents__ended")} + ), } PURCHASE_DEFAULT_SORT: SortString = "-purchased,-created" @@ -101,8 +111,8 @@ PURCHASE_DEFAULT_SORT: SortString = "-purchased,-created" class SortResult(NamedTuple): queryset: QuerySet - terms: list[SortTerm] # the order actually applied — #73's header UI consumes this - unknown: list[SortKey] # rejected keys — the view turns these into warning toasts + terms: list[SortTerm] # the order actually applied — #73's header UI consumes this + unknown: list[SortKey] # rejected keys — the view turns these into warning toasts def apply_sort( @@ -125,4 +135,6 @@ def apply_sort( def parse_find_filter(request: HttpRequest) -> FindFilter: - return FindFilter(sort=request.GET.get("sort") or None) # FindFilter.sort holds a SortString + return FindFilter( + sort=request.GET.get("sort") or None + ) # FindFilter.sort holds a SortString diff --git a/games/views/purchase.py b/games/views/purchase.py index f72fa5b..cac0ee0 100644 --- a/games/views/purchase.py +++ b/games/views/purchase.py @@ -135,7 +135,9 @@ def list_purchases(request: HttpRequest) -> HttpResponse: if pf is not None: purchases = purchases.filter(pf.to_q()) - sort = apply_sort(purchases, parse_find_filter(request), PURCHASE_SORTS, PURCHASE_DEFAULT_SORT) + sort = apply_sort( + purchases, parse_find_filter(request), PURCHASE_SORTS, PURCHASE_DEFAULT_SORT + ) purchases = sort.queryset for key in sort.unknown: messages.warning(request, f"Unknown sort field '{key}' was ignored.") diff --git a/games/views/session.py b/games/views/session.py index aeff163..091c48a 100644 --- a/games/views/session.py +++ b/games/views/session.py @@ -152,7 +152,9 @@ def list_sessions(request: HttpRequest, search_string: str = "") -> HttpResponse last_session = sessions.latest() except Session.DoesNotExist: last_session = None - sort = apply_sort(sessions, parse_find_filter(request), SESSION_SORTS, SESSION_DEFAULT_SORT) + sort = apply_sort( + sessions, parse_find_filter(request), SESSION_SORTS, SESSION_DEFAULT_SORT + ) sessions = sort.queryset for key in sort.unknown: messages.warning(request, f"Unknown sort field '{key}' was ignored.") diff --git a/tests/test_sorting.py b/tests/test_sorting.py index f21cdc6..3312957 100644 --- a/tests/test_sorting.py +++ b/tests/test_sorting.py @@ -82,23 +82,31 @@ def two_games(db): class TestApplySortGames: def test_name_ascending(self, two_games): alpha, beta = two_games - result = apply_sort(Game.objects.all(), _find("name"), GAME_SORTS, GAME_DEFAULT_SORT) + result = apply_sort( + Game.objects.all(), _find("name"), GAME_SORTS, GAME_DEFAULT_SORT + ) assert list(result.queryset) == [alpha, beta] assert result.terms[0].key == "name" assert result.unknown == [] def test_name_descending(self, two_games): alpha, beta = two_games - result = apply_sort(Game.objects.all(), _find("-name"), GAME_SORTS, GAME_DEFAULT_SORT) + result = apply_sort( + Game.objects.all(), _find("-name"), GAME_SORTS, GAME_DEFAULT_SORT + ) assert list(result.queryset) == [beta, alpha] def test_default_sort_when_absent_is_created_desc(self, two_games): alpha, beta = two_games # beta created after alpha - result = apply_sort(Game.objects.all(), _find(None), GAME_SORTS, GAME_DEFAULT_SORT) + result = apply_sort( + Game.objects.all(), _find(None), GAME_SORTS, GAME_DEFAULT_SORT + ) assert list(result.queryset) == [beta, alpha] def test_unknown_key_reported_and_falls_back(self, two_games): - result = apply_sort(Game.objects.all(), _find("bogus"), GAME_SORTS, GAME_DEFAULT_SORT) + result = apply_sort( + Game.objects.all(), _find("bogus"), GAME_SORTS, GAME_DEFAULT_SORT + ) assert result.unknown == ["bogus"] assert result.queryset.count() == 2 # still returns rows (default order) @@ -114,13 +122,14 @@ class TestApplySortGames: timestamp_start=datetime(2022, 1, 2, 10, tzinfo=ZONEINFO), timestamp_end=datetime(2022, 1, 2, 11, tzinfo=ZONEINFO), ) - result = apply_sort(Game.objects.all(), _find("-playtime"), GAME_SORTS, GAME_DEFAULT_SORT) + result = apply_sort( + Game.objects.all(), _find("-playtime"), GAME_SORTS, GAME_DEFAULT_SORT + ) # two sessions on alpha must not duplicate the alpha row assert result.queryset.count() == 2 assert list(result.queryset)[0] == alpha # most playtime first - class TestParseFindFilter: def test_reads_sort_param(self): request = RequestFactory().get("/x", {"sort": "-playtime,name"}) @@ -182,22 +191,30 @@ class TestListSessionsSort: Session.objects.create( game=alpha, timestamp_start=datetime(2022, 1, 1, 10, tzinfo=ZONEINFO), - timestamp_end=datetime(2022, 1, 1, 13, tzinfo=ZONEINFO), # 3 h, earlier date + timestamp_end=datetime( + 2022, 1, 1, 13, tzinfo=ZONEINFO + ), # 3 h, earlier date ) Session.objects.create( game=beta, timestamp_start=datetime(2022, 1, 2, 10, tzinfo=ZONEINFO), - timestamp_end=datetime(2022, 1, 2, 10, 30, tzinfo=ZONEINFO), # 30 min, later date + timestamp_end=datetime( + 2022, 1, 2, 10, 30, tzinfo=ZONEINFO + ), # 30 min, later date ) # default order is -date (beta first); -duration must override it (alpha's 3h first) - response = logged_client.get(reverse("games:list_sessions"), {"sort": "-duration"}) + response = logged_client.get( + reverse("games:list_sessions"), {"sort": "-duration"} + ) assert response.status_code == 200 body = response.content.decode() # Extract only the table body to avoid finding "Beta" in header's last-session button tbody_match = re.search(r"]*>(.*?)", body, re.DOTALL) assert tbody_match tbody = tbody_match.group(1) - assert tbody.index("Alpha") < tbody.index("Beta") # longer session first, despite earlier date + assert tbody.index("Alpha") < tbody.index( + "Beta" + ) # longer session first, despite earlier date def test_unknown_sort_emits_warning(self, logged_client, two_games): response = logged_client.get(reverse("games:list_sessions"), {"sort": "nope"}) @@ -229,7 +246,9 @@ class TestListPurchasesSort: def test_sort_by_price_descending(self, logged_client, two_purchases): # default -purchased puts Alpha (later date) first; -price must override to show Beta (90) first - response = logged_client.get(reverse("games:list_purchases"), {"sort": "-price"}) + response = logged_client.get( + reverse("games:list_purchases"), {"sort": "-price"} + ) assert response.status_code == 200 body = response.content.decode() tbody = re.search(r"]*>(.*?)", body, re.DOTALL).group(1) @@ -238,7 +257,9 @@ class TestListPurchasesSort: def test_name_aggregate_sort_no_duplicate_rows(self, logged_client, two_purchases): # a multi-game purchase must still render exactly one row cheap, _ = two_purchases - extra = Game.objects.create(name="Aaa", sort_name="Aaa", platform=cheap.platform) + extra = Game.objects.create( + name="Aaa", sort_name="Aaa", platform=cheap.platform + ) cheap.games.add(extra) response = logged_client.get(reverse("games:list_purchases"), {"sort": "name"}) body = response.content.decode() @@ -248,3 +269,34 @@ class TestListPurchasesSort: response = logged_client.get(reverse("games:list_purchases"), {"sort": "nope"}) warnings = [str(m) for m in get_messages(response.wsgi_request)] assert any("nope" in w for w in warnings) + + +class TestDefaultOrderUnchanged: + """The default sort strings must reproduce the pre-#68 hardcoded order.""" + + def test_games_default_is_created_descending(self, logged_client, two_games): + alpha, beta = two_games # beta newer + response = logged_client.get(reverse("games:list_games")) + body = response.content.decode() + tbody_match = re.search(r"]*>(.*?)", body, re.DOTALL) + assert tbody_match + tbody = tbody_match.group(1) + assert tbody.index("Beta") < tbody.index("Alpha") + + +class TestEverySortKeyReturns200: + def test_all_game_keys(self, logged_client, two_games): + for key in GAME_SORTS: + for raw in (key, f"-{key}"): + response = logged_client.get(reverse("games:list_games"), {"sort": raw}) + assert response.status_code == 200, raw + + def test_all_session_keys(self, logged_client, two_games): + for key in SESSION_SORTS: + response = logged_client.get(reverse("games:list_sessions"), {"sort": key}) + assert response.status_code == 200, key + + def test_all_purchase_keys(self, logged_client, two_games): + for key in PURCHASE_SORTS: + response = logged_client.get(reverse("games:list_purchases"), {"sort": key}) + assert response.status_code == 200, key