Merge pull request #78 from KucharczykL/feat/issue-68-list-view-sort-param

feat(sorting): honor ?sort= query param on list views (#68)
This commit is contained in:
2026-06-21 14:52:54 +02:00
committed by GitHub
8 changed files with 1570 additions and 3 deletions
+1
View File
@@ -195,3 +195,4 @@ Pytest settings are in `pyproject.toml` under `[tool.pytest.ini_options]` (`DJAN
- **Disabling composite widgets**: a composite widget (e.g. `SearchSelect`) carries its `id` on a wrapper `<div>`, which has no `disabled` state — setting `.disabled` on it is a no-op. Disable the inner control (for `SearchSelect`, the `[data-search-select-search]` input); the wrapper fades itself via `DISABLED_WITHIN_CLASS`, so callers toggle only the control's `disabled`, never styles.
- **Platform icons** are SVG snippets in `games/templates/icons/<slug>.html`. Add new ones there and reference them by slug in `Platform.icon`.
- **Name compound types explicitly** — if a `tuple`, `dict`, or other compound value is passed between functions or appears in multiple signatures, give it a named type (`TypedDict`, `NamedTuple`, or a `type` alias) rather than repeating the structural annotation. This applies even to small types used in only a few places; the name carries intent that the structure cannot. Examples: `LabeledOption = tuple[str, str]` instead of repeating `tuple[str, str]` for (value, label) pairs; `RangeValues(min, max)` instead of `tuple[str, str]` for range bounds.
- **Name primitive roles too** — when a bare `str`/`int` stands for a domain concept (an id, a key, a token, a field name), give it a PEP 695 transparent alias (`type SortKey = str`) so signatures say *which* string/int goes where instead of a wall of `str`. These are zero-cost and need no wrapping (unlike `NewType`); reach for them especially when several distinct string roles meet in one function (e.g. a `dict[SortKey, SortSpec]` whose values reference an `AnnotationName`). Add a trailing comment on the alias noting an example value. Use `NewType` only when you actually want the checker to reject cross-assignment and are willing to wrap every literal.
@@ -0,0 +1,780 @@
# List-view `sort` query param Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Make `list_games`, `list_sessions`, `list_purchases` honor a signed comma-list `?sort=` query param (e.g. `?sort=-playtime,name`), with every visible column sortable plus stats-parity aggregates (playtime, finish date).
**Architecture:** A new `games/sorting.py` defines a per-model whitelist (`SortKey → SortSpec`), a pure parser (`parse_sort_terms`), and an applier (`apply_sort`) that annotates-then-orders and reports unknown keys. Each list view replaces its hardcoded `order_by(...)` with `apply_sort(...)`, eager-loads row relations, and turns unknown keys into warning toasts. Backend only — clickable-header UI is #73.
**Tech Stack:** Django 6, Python 3.13, pytest + pytest-django. Components/views per `CLAUDE.md`.
## Global Constraints
- **Never write to `GeneratedField`s** (`duration_calculated`, `duration_total`, `price_per_game`, `days_to_finish`) — they are read/order-only.
- **Complete-word identifiers** (Python + JS): `descending` not `desc_flag`, `queryset` not `qs` in real code.
- **Cross-relation sorts use annotated aggregates** (`Sum`/`Max`/`Min`) — never bare `order_by("relation__field")` for to-many relations (avoids row duplication). To-one relations (`game__sort_name`, `device__name`) may be ordered directly.
- **Name primitive roles** with PEP 695 transparent aliases (`type SortKey = str`), per the new `CLAUDE.md` convention.
- **`sorting.py` stays HTTP-free** — it reports unknown keys; the view emits `messages.warning`.
- Tests run with `uv run --with pytest-django pytest`.
- Spec: `docs/superpowers/specs/2026-06-21-list-view-sort-param-design.md`.
---
## File Structure
- **Create** `games/sorting.py` — all sorting logic (aliases, `SortSpec`, `SortTerm`, `ParsedSort`, `SortResult`, the three `*_SORTS` maps + `*_DEFAULT_SORT`, `parse_sort_terms`, `apply_sort`, `parse_find_filter`).
- **Create** `tests/test_sorting.py` — unit + integration tests.
- **Modify** `games/views/game.py` — wire sort into `list_games` + `select_related`.
- **Modify** `games/views/session.py` — wire sort into `list_sessions` + `select_related`.
- **Modify** `games/views/purchase.py` — wire sort into `list_purchases` + `prefetch_related`.
---
## Task 1: `sorting.py` core types + `parse_sort_terms`
**Files:**
- Create: `games/sorting.py`
- Test: `tests/test_sorting.py`
**Interfaces:**
- Consumes: nothing.
- Produces:
- `type SortKey = str`, `type SortString = str`, `type AnnotationName = str`, `type OrderField = str`, `type Annotations = dict[AnnotationName, Aggregate]`
- `SortSpec(expression: OrderField, annotate: Annotations | None = None)` (frozen dataclass)
- `SortTerm(NamedTuple)`: `key: SortKey`, `descending: bool`
- `type SortMap = dict[SortKey, SortSpec]`
- `ParsedSort(NamedTuple)`: `terms: list[SortTerm]`, `unknown: list[SortKey]`
- `parse_sort_terms(raw: SortString, sort_map: SortMap) -> ParsedSort`
- [ ] **Step 1: Write the failing test**
```python
# tests/test_sorting.py
"""Tests for the list-view sorting system (games/sorting.py)."""
from games.sorting import SortSpec, SortTerm, parse_sort_terms
# A minimal map; parse_sort_terms only checks key membership, not spec internals.
SAMPLE_MAP = {"name": SortSpec("name"), "date": SortSpec("created_at")}
class TestParseSortTerms:
def test_bare_key_is_ascending(self):
parsed = parse_sort_terms("name", SAMPLE_MAP)
assert parsed.terms == [SortTerm("name", False)]
assert parsed.unknown == []
def test_dash_prefix_is_descending(self):
parsed = parse_sort_terms("-date", SAMPLE_MAP)
assert parsed.terms == [SortTerm("date", True)]
def test_multi_column_preserves_order(self):
parsed = parse_sort_terms("date,-name", SAMPLE_MAP)
assert parsed.terms == [SortTerm("date", False), SortTerm("name", True)]
def test_unknown_key_is_reported_not_raised(self):
parsed = parse_sort_terms("bogus", SAMPLE_MAP)
assert parsed.terms == []
assert parsed.unknown == ["bogus"]
def test_mixed_valid_and_unknown(self):
parsed = parse_sort_terms("-name,bogus", SAMPLE_MAP)
assert parsed.terms == [SortTerm("name", True)]
assert parsed.unknown == ["bogus"]
def test_whitespace_and_empty_tokens_ignored(self):
parsed = parse_sort_terms(" name , , -date ", SAMPLE_MAP)
assert parsed.terms == [SortTerm("name", False), SortTerm("date", True)]
def test_empty_string_yields_nothing(self):
parsed = parse_sort_terms("", SAMPLE_MAP)
assert parsed.terms == []
assert parsed.unknown == []
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run --with pytest-django pytest tests/test_sorting.py -v`
Expected: FAIL — `ModuleNotFoundError: No module named 'games.sorting'`.
- [ ] **Step 3: Write minimal implementation**
```python
# games/sorting.py
"""Structured sorting for list views (Stash-inspired, paired with games/filters.py).
A list view maps a public sort key to a SortSpec; the URL ?sort= param is a
signed comma-list of those keys (e.g. "-playtime,name"). See
docs/superpowers/specs/2026-06-21-list-view-sort-param-design.md.
"""
from dataclasses import dataclass
from typing import NamedTuple
from django.db.models import Aggregate
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")}
type Annotations = dict[AnnotationName, Aggregate]
@dataclass(frozen=True)
class SortSpec:
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)
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
def parse_sort_terms(raw: SortString, sort_map: SortMap) -> ParsedSort:
terms: list[SortTerm] = []
unknown: list[SortKey] = []
for token in raw.split(","):
token = token.strip()
if not token:
continue
descending = token.startswith("-")
key = token.lstrip("-")
if key in sort_map:
terms.append(SortTerm(key, descending))
else:
unknown.append(key)
return ParsedSort(terms, unknown)
```
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run --with pytest-django pytest tests/test_sorting.py -v`
Expected: PASS (7 tests).
- [ ] **Step 5: Commit**
```bash
git add games/sorting.py tests/test_sorting.py
git commit -m "feat(sorting): SortSpec/SortTerm types + parse_sort_terms (#68)"
```
---
## Task 2: Per-model maps + `apply_sort` + `parse_find_filter`
**Files:**
- Modify: `games/sorting.py`
- Test: `tests/test_sorting.py`
**Interfaces:**
- Consumes: Task 1's `SortSpec`, `SortTerm`, `SortMap`, `parse_sort_terms`; `FindFilter` from `games.filters`.
- Produces:
- `GAME_SORTS: SortMap`, `GAME_DEFAULT_SORT = "-created"`
- `SESSION_SORTS: SortMap`, `SESSION_DEFAULT_SORT = "-date,created"`
- `PURCHASE_SORTS: SortMap`, `PURCHASE_DEFAULT_SORT = "-purchased,-created"`
- `SortResult(NamedTuple)`: `queryset: QuerySet`, `terms: list[SortTerm]`, `unknown: list[SortKey]`
- `apply_sort(queryset, find: FindFilter, sort_map: SortMap, default_sort: SortString) -> SortResult`
- `parse_find_filter(request: HttpRequest) -> FindFilter`
- [ ] **Step 1: Write the failing test**
Append to `tests/test_sorting.py`:
```python
from datetime import datetime, timedelta
from zoneinfo import ZoneInfo
import pytest
from django.conf import settings
from django.test import RequestFactory
from games.filters import FindFilter
from games.models import Game, Platform, Purchase, Session
from games.sorting import (
GAME_DEFAULT_SORT,
GAME_SORTS,
PURCHASE_DEFAULT_SORT,
PURCHASE_SORTS,
SESSION_DEFAULT_SORT,
SESSION_SORTS,
apply_sort,
parse_find_filter,
)
ZONEINFO = ZoneInfo(settings.TIME_ZONE)
def _find(sort=None):
return FindFilter(sort=sort)
@pytest.fixture
def two_games(db):
platform = Platform.objects.create(name="P", icon="p")
alpha = Game.objects.create(name="Alpha", sort_name="Alpha", platform=platform)
beta = Game.objects.create(name="Beta", sort_name="Beta", platform=platform)
return alpha, beta
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)
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)
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)
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)
assert result.unknown == ["bogus"]
assert result.queryset.count() == 2 # still returns rows (default order)
def test_playtime_annotation_no_duplicate_rows(self, two_games):
alpha, _ = two_games
device = None
Session.objects.create(
game=alpha,
timestamp_start=datetime(2022, 1, 1, 10, tzinfo=ZONEINFO),
timestamp_end=datetime(2022, 1, 1, 12, tzinfo=ZONEINFO),
)
Session.objects.create(
game=alpha,
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)
# 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"})
assert parse_find_filter(request).sort == "-playtime,name"
def test_absent_sort_is_none(self):
request = RequestFactory().get("/x")
assert parse_find_filter(request).sort is None
class TestSortMapShapes:
def test_default_sort_keys_exist_in_maps(self):
# every key referenced by a default sort string must be defined in its map
for default, sort_map in [
(GAME_DEFAULT_SORT, GAME_SORTS),
(SESSION_DEFAULT_SORT, SESSION_SORTS),
(PURCHASE_DEFAULT_SORT, PURCHASE_SORTS),
]:
for token in default.split(","):
assert token.lstrip("-") in sort_map
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run --with pytest-django pytest tests/test_sorting.py -v`
Expected: FAIL — `ImportError: cannot import name 'GAME_SORTS'` (and friends).
- [ ] **Step 3: Write minimal implementation**
Add imports at the top of `games/sorting.py` (merge with existing):
```python
from django.db.models import Aggregate, Max, Min, QuerySet, Sum
from django.http import HttpRequest
from games.filters import FindFilter
```
Append to `games/sorting.py`:
```python
# ── Per-model sort maps ─────────────────────────────────────────────────────
# Cross-relation sorts use annotated aggregates (group by PK → no row dup).
# To-one relations (game__sort_name, device__name) are ordered directly.
GAME_SORTS: SortMap = {
"name": SortSpec("name"),
"sort_name": SortSpec("sort_name"),
"year": SortSpec("year_released"),
"status": SortSpec("status"),
"wikidata": SortSpec("wikidata"),
"created": SortSpec("created_at"),
"playtime": SortSpec("total_playtime", {"total_playtime": Sum("sessions__duration_total")}),
"finished": SortSpec("last_finished", {"last_finished": Max("playevents__ended")}),
}
GAME_DEFAULT_SORT: SortString = "-created"
SESSION_SORTS: SortMap = {
"name": SortSpec("game__sort_name"),
"date": SortSpec("timestamp_start"),
"duration": SortSpec("duration_total"),
"device": SortSpec("device__name"),
"created": SortSpec("created_at"),
}
SESSION_DEFAULT_SORT: SortString = "-date,created"
PURCHASE_SORTS: SortMap = {
"name": SortSpec("first_game_name", {"first_game_name": Min("games__name")}),
"type": SortSpec("type"),
"price": SortSpec("converted_price"),
"infinite": SortSpec("infinite"),
"purchased": SortSpec("date_purchased"),
"refunded": SortSpec("date_refunded"),
"created": SortSpec("created_at"),
"finished": SortSpec("last_finished", {"last_finished": Max("games__playevents__ended")}),
}
PURCHASE_DEFAULT_SORT: SortString = "-purchased,-created"
# ── Apply ───────────────────────────────────────────────────────────────────
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
def apply_sort(
queryset: QuerySet, find: FindFilter, sort_map: SortMap, default_sort: SortString
) -> SortResult:
terms, unknown = parse_sort_terms(find.sort or "", sort_map)
if not terms:
# default_sort is trusted developer config — ignore any "unknown" from it
terms, _ = parse_sort_terms(default_sort, sort_map)
annotations: Annotations = {}
order_by: list[OrderField] = []
for term in terms:
spec = sort_map[term.key]
if spec.annotate:
annotations.update(spec.annotate)
order_by.append(("-" if term.descending else "") + spec.expression)
if annotations:
queryset = queryset.annotate(**annotations)
return SortResult(queryset.order_by(*order_by), terms, unknown)
def parse_find_filter(request: HttpRequest) -> FindFilter:
return FindFilter(sort=request.GET.get("sort") or None) # FindFilter.sort holds a SortString
```
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run --with pytest-django pytest tests/test_sorting.py -v`
Expected: PASS (all Task 1 + Task 2 tests).
- [ ] **Step 5: Commit**
```bash
git add games/sorting.py tests/test_sorting.py
git commit -m "feat(sorting): per-model maps, apply_sort, parse_find_filter (#68)"
```
---
## Task 3: Wire `list_games` (sort + N+1 + warnings)
**Files:**
- Modify: `games/views/game.py` (`list_games`, starts line 57; base queryset line 59)
- Test: `tests/test_sorting.py`
**Interfaces:**
- Consumes: `apply_sort`, `parse_find_filter`, `GAME_SORTS`, `GAME_DEFAULT_SORT` from `games.sorting`.
- Produces: `GET /games/?sort=<...>` honored; unknown key → warning message.
- [ ] **Step 1: Write the failing test**
Append to `tests/test_sorting.py`:
```python
from django.contrib.messages import get_messages
from django.urls import reverse
@pytest.fixture
def logged_client(client, django_user_model):
user = django_user_model.objects.create_user(username="u", password="p")
client.force_login(user)
return client
class TestListGamesSort:
def test_sort_param_orders_rows(self, logged_client, two_games):
alpha, beta = two_games
response = logged_client.get(reverse("games:list_games"), {"sort": "-name"})
assert response.status_code == 200
body = response.content.decode()
assert body.index("Beta") < body.index("Alpha")
def test_unknown_sort_emits_warning_message(self, logged_client, two_games):
response = logged_client.get(reverse("games:list_games"), {"sort": "bogus"})
assert response.status_code == 200
warnings = [str(m) for m in get_messages(response.wsgi_request)]
assert any("bogus" in w for w in warnings)
def test_valid_sort_emits_no_warning(self, logged_client, two_games):
response = logged_client.get(reverse("games:list_games"), {"sort": "name"})
warnings = [str(m) for m in get_messages(response.wsgi_request)]
assert warnings == []
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestListGamesSort -v`
Expected: FAIL — `test_sort_param_orders_rows` fails (rows still ordered by `-created_at`, so `-name` ignored); `test_unknown_sort_emits_warning_message` fails (no message).
- [ ] **Step 3: Write minimal implementation**
In `games/views/game.py`, add to the imports from `django.contrib`:
```python
from django.contrib import messages
```
Add to the `games.sorting` import (new import line near the other `games.*` imports, e.g. after `from games.filters import parse_game_filter`):
```python
from games.sorting import GAME_DEFAULT_SORT, GAME_SORTS, apply_sort, parse_find_filter
```
Change the base queryset (line 59) from:
```python
games = Game.objects.order_by("-created_at")
```
to:
```python
games = Game.objects.select_related("platform")
```
Then, immediately before `games, page_obj, elided_page_range = paginate(request, games)` (line 89), insert:
```python
sort = apply_sort(games, parse_find_filter(request), GAME_SORTS, GAME_DEFAULT_SORT)
games = sort.queryset
for key in sort.unknown:
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
```
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestListGamesSort -v`
Expected: PASS (3 tests).
- [ ] **Step 5: Commit**
```bash
git add games/views/game.py tests/test_sorting.py
git commit -m "feat(games): honor ?sort= on list_games + eager-load platform (#68)"
```
---
## Task 4: Wire `list_sessions` (sort + N+1 + warnings)
**Files:**
- Modify: `games/views/session.py` (`list_sessions`, starts line 120; base queryset line 122)
- Test: `tests/test_sorting.py`
**Interfaces:**
- Consumes: `apply_sort`, `parse_find_filter`, `SESSION_SORTS`, `SESSION_DEFAULT_SORT`.
- Produces: `GET /sessions/?sort=<...>` honored; unknown key → warning.
- [ ] **Step 1: Write the failing test**
Append to `tests/test_sorting.py`:
```python
class TestListSessionsSort:
def test_sort_by_duration_descending(self, logged_client, two_games):
alpha, beta = two_games
Session.objects.create(
game=alpha,
timestamp_start=datetime(2022, 1, 1, 10, tzinfo=ZONEINFO),
timestamp_end=datetime(2022, 1, 1, 10, 30, tzinfo=ZONEINFO), # 30 min
)
Session.objects.create(
game=beta,
timestamp_start=datetime(2022, 1, 2, 10, tzinfo=ZONEINFO),
timestamp_end=datetime(2022, 1, 2, 13, tzinfo=ZONEINFO), # 3 h
)
response = logged_client.get(reverse("games:list_sessions"), {"sort": "-duration"})
assert response.status_code == 200
body = response.content.decode()
assert body.index("Beta") < body.index("Alpha") # longer session first
def test_unknown_sort_emits_warning(self, logged_client, two_games):
response = logged_client.get(reverse("games:list_sessions"), {"sort": "nope"})
warnings = [str(m) for m in get_messages(response.wsgi_request)]
assert any("nope" in w for w in warnings)
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestListSessionsSort -v`
Expected: FAIL — sort ignored / no warning.
- [ ] **Step 3: Write minimal implementation**
In `games/views/session.py`, add the imports:
```python
from django.contrib import messages
from games.sorting import (
SESSION_DEFAULT_SORT,
SESSION_SORTS,
apply_sort,
parse_find_filter,
)
```
Change the base queryset (line 122) from:
```python
sessions = Session.objects.order_by("-timestamp_start", "created_at")
```
to:
```python
sessions = Session.objects.select_related("game", "game__platform", "device")
```
Then, immediately before `sessions, page_obj, elided_page_range = paginate(request, sessions)` (line 148), insert:
```python
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.")
```
> Note: `last_session = sessions.latest()` (line 145) runs before pagination and is unaffected by `order_by` (`.latest()` uses the model's `get_latest_by`); leave it as-is. The `apply_sort` call goes after it, before `paginate`.
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestListSessionsSort -v`
Expected: PASS (2 tests).
- [ ] **Step 5: Commit**
```bash
git add games/views/session.py tests/test_sorting.py
git commit -m "feat(sessions): honor ?sort= on list_sessions + eager-load relations (#68)"
```
---
## Task 5: Wire `list_purchases` (sort + N+1 + warnings)
**Files:**
- Modify: `games/views/purchase.py` (`list_purchases`, starts line 121; base queryset line 122)
- Test: `tests/test_sorting.py`
**Interfaces:**
- Consumes: `apply_sort`, `parse_find_filter`, `PURCHASE_SORTS`, `PURCHASE_DEFAULT_SORT`.
- Produces: `GET /purchases/?sort=<...>` honored; unknown key → warning; `name`/`finished` aggregate sorts do not duplicate rows.
- [ ] **Step 1: Write the failing test**
Append to `tests/test_sorting.py`:
```python
class TestListPurchasesSort:
@pytest.fixture
def two_purchases(self, db, two_games):
alpha, beta = two_games
cheap = Purchase.objects.create(
date_purchased=datetime(2022, 1, 1, tzinfo=ZONEINFO),
price=10,
converted_price=10,
platform=alpha.platform,
)
cheap.games.add(alpha)
dear = Purchase.objects.create(
date_purchased=datetime(2022, 1, 2, tzinfo=ZONEINFO),
price=90,
converted_price=90,
platform=beta.platform,
)
dear.games.add(beta)
return cheap, dear
def test_sort_by_price_descending(self, logged_client, two_purchases):
response = logged_client.get(reverse("games:list_purchases"), {"sort": "-price"})
assert response.status_code == 200
body = response.content.decode()
assert body.index("Beta") < body.index("Alpha") # 90 before 10
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
from games.models import Game
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()
assert body.count("purchase-row-") == 2 # exactly two purchase rows
def test_unknown_sort_emits_warning(self, logged_client, two_purchases):
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)
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestListPurchasesSort -v`
Expected: FAIL — sort ignored / no warning.
- [ ] **Step 3: Write minimal implementation**
In `games/views/purchase.py`, add the imports:
```python
from django.contrib import messages
from games.sorting import (
PURCHASE_DEFAULT_SORT,
PURCHASE_SORTS,
apply_sort,
parse_find_filter,
)
```
Change the base queryset (line 122) from:
```python
purchases = Purchase.objects.order_by("-date_purchased", "-created_at")
```
to:
```python
purchases = Purchase.objects.prefetch_related("games", "games__platform")
```
Then, immediately before `purchases, page_obj, elided_page_range = paginate(request, purchases)` (line 132), insert:
```python
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.")
```
> If `apply_sort` ever yields duplicate purchase rows for an aggregate sort (it should not — `Min`/`Max` group by PK), add `.distinct()` after `order_by` in the view; the `test_name_aggregate_sort_no_duplicate_rows` test guards this.
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestListPurchasesSort -v`
Expected: PASS (3 tests).
- [ ] **Step 5: Run the full suite + lint**
Run: `uv run --with pytest-django pytest tests/test_sorting.py -v && make lint`
Expected: all sorting tests PASS; ruff clean.
- [ ] **Step 6: Commit**
```bash
git add games/views/purchase.py tests/test_sorting.py
git commit -m "feat(purchases): honor ?sort= on list_purchases + eager-load games (#68)"
```
---
## Task 6: Regression smoke + full suite
**Files:**
- Test: `tests/test_sorting.py`
**Interfaces:**
- Consumes: all prior tasks.
- Produces: confidence that default order is unchanged and every map key returns 200.
- [ ] **Step 1: Write the failing test**
Append to `tests/test_sorting.py`:
```python
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()
assert body.index("Beta") < body.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
```
- [ ] **Step 2: Run to verify it passes (these assert already-correct behavior)**
Run: `uv run --with pytest-django pytest tests/test_sorting.py::TestDefaultOrderUnchanged tests/test_sorting.py::TestEverySortKeyReturns200 -v`
Expected: PASS. If any `?sort=<key>` returns 500, that key's expression/annotation is wrong — fix the offending `SortSpec` in `games/sorting.py` and re-run.
- [ ] **Step 3: Run the entire project test suite**
Run: `make test`
Expected: PASS (no regressions; note `make test` also collects `e2e/` — a browser must be available, or run `uv run --with pytest-django pytest tests/` to scope to unit/integration tests).
- [ ] **Step 4: Lint + format check**
Run: `make check`
Expected: ruff + format + tests clean.
- [ ] **Step 5: Commit**
```bash
git add tests/test_sorting.py
git commit -m "test(sorting): default-order regression + all-keys smoke (#68)"
```
---
## Post-implementation (not tasks)
- **PR description** must call out the #65 coordination (per spec): each stats "view all" link's `sort=` must use a key in the target model's `*_SORTS` map. Verify when #65 lands.
- Follow-ups already filed: #73 (header UI), #74 (FindFilter unify + dead `direction`/`page`/`per_page`), #75 (purchase free-text search), #76 (shared `list_view` helper), #77 (presets persist/restore sort). Do not address them here.
@@ -0,0 +1,310 @@
# Honor `sort`/`direction` query params on list views (#68)
## Problem
The stats page (#65) adds "View all (N) →" links from its sorted lists to the
filtered list views. The list views (`list_games`, `list_sessions`,
`list_purchases`) hardcode `order_by(...)` and ignore any sort parameter, so a
"view all" link lands on the right *set* of rows but not in the same *order* the
stats page showed.
`FindFilter` (in `games/filters.py`) already models `sort` / `direction` but is
never parsed or applied anywhere.
Hardcoded orders today:
- `games/views/game.py:59``Game.objects.order_by("-created_at")`
- `games/views/session.py:122``Session.objects.order_by("-timestamp_start", "created_at")`
- `games/views/purchase.py:122``Purchase.objects.order_by("-date_purchased", "-created_at")`
## Goal
Make all three list views honor a `sort` query param (a signed comma-list of
column keys), with every visible column sortable plus the aggregate sorts the
stats page needs for "view all" parity (games-by-playtime, finish-date). Backend
only — no clickable header UI in this issue (tracked in #73).
## Scope decisions (from brainstorming)
- **Sortable set:** all visible list columns, plus stats-parity aggregates
(playtime, finish date) even where they are not visible columns.
- **UI:** backend only. Honor the query param; do not add clickable column
headers. File a follow-up issue for the header UI.
- **URL contract:** a single signed comma-list of **public keys**
`?sort=-playtime,name`. The `-` attaches to the public key and sets that
key's direction; the whitelist still maps the key to its internal
expression+annotation. Bare key = **ascending** (Django semantics, no hidden
per-key defaults); leading `-` = descending. Rationale: natural multi-column,
one param, each term self-describes direction, mirrors Django `order_by`, and
is **forward-compatible to multi-column with zero contract change**.
- **Multi-column:** the backend parses and applies the **full** term list now
(server-side multi-column works immediately). Only the UI that *generates*
multi-term URLs (clickable headers, shift-click to add a column) is deferred
to the follow-up issue.
- **Default ordering:** since bare keys are ascending and signs are explicit,
each view's default order is itself just a default signed sort string parsed
by the same machinery — no separate `default_direction` field, no per-key
tiebreak concept.
- **Module:** new `games/sorting.py` (keeps the already-large `filters.py`
focused; imports `FindFilter` from it).
- **Purchase name sort:** annotated `Min("games__name")` — one row per purchase,
no M2M duplication.
## Prerequisite: fix N+1 queries on list rows
The list querysets currently have **no** `select_related`/`prefetch_related`, so
each rendered row lazy-loads its relations. The sort work touches these exact
querysets (and adds aggregate annotations), so the eager-loading fix lands here:
- `list_games``Game.objects.select_related("platform")` (icon via
`NameWithIcon`).
- `list_sessions``Session.objects.select_related("game", "game__platform", "device")`.
- `list_purchases``Purchase.objects.prefetch_related("games", "games__platform")`
(M2M rendered by `LinkedPurchase`; confirm `games__platform` need during impl).
These are added to the **base** queryset (before filtering/annotating), so they
compose with `apply_sort`'s annotations.
## Design
### URL contract
`?sort=<signed-key>[,<signed-key>...]` on `list_games`, `list_sessions`,
`list_purchases`. Examples: `?sort=-playtime`, `?sort=status,name`,
`?sort=-date,created`.
- Each term is a public key, optionally prefixed `-`. Bare = ascending; `-` =
descending (Django `order_by` semantics).
- Terms whose key is not in the model's map are **ignored, and a user-facing
warning toast is shown** ("Unknown sort field '<key>' was ignored.") — never a
400. Any remaining valid terms still apply; the page renders normally.
- If `sort` is absent, empty, or has no valid terms → the view's default sort
string is used (parsed by the same machinery).
- Invalid values never raise.
The warning surfaces drift (e.g. a #65 "view all" link with a stale/typo'd key)
without breaking a user-facing, hand-editable URL. It rides the existing
messages→toast path: `render_page()` serializes `get_messages(request)` into the
`#django-messages` JSON block (`common/layout.py`) and `toast.js` renders it —
works on a full-page GET, which is what #65 links are. No new plumbing.
`sorting.py` itself stays HTTP-free: it *reports* unknown keys; the view emits
the `messages.warning`.
Pagination already preserves the param: `_page_url` (in
`common/components/primitives.py`) copies `request.GET` and only replaces `page`.
### `games/sorting.py`
Named string roles (PEP 695 transparent aliases — `requires-python >=3.13`).
They read like TS `type X = string`: no runtime cost, no wrapping ceremony, but
each `str` in a signature now says *which* string it is:
```python
from django.db.models import Aggregate
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")}
type Annotations = dict[AnnotationName, Aggregate]
@dataclass(frozen=True)
class SortSpec:
expression: OrderField # unsigned; a real column path or an AnnotationName
annotate: Annotations | None = None
```
All current sorts use `Aggregate` subclasses (`Sum`/`Max`/`Min`); if a
non-aggregate annotation (`F`, `ExpressionWrapper`) is ever needed, widen
`Annotations`' value to `Combinable` then.
Direction is never stored on the spec — it comes from the sign in the URL term.
Cross-relation sorts use **annotated aggregates** (`Sum`/`Max`/`Min`), which
group by the model PK and therefore produce no duplicate rows. Bare
`order_by("relation__field")` is never used for to-many relations.
#### Per-model maps + default sort strings
All three maps are typed `SortMap` (`dict[SortKey, SortSpec]`); each
`*_DEFAULT_SORT` is a `SortString`.
**`GAME_SORTS`** — `GAME_DEFAULT_SORT = "-created"`:
| key | expression | annotate |
|---|---|---|
| name | `name` | — |
| sort_name | `sort_name` | — |
| year | `year_released` | — |
| status | `status` | — |
| wikidata | `wikidata` | — |
| created | `created_at` | — |
| playtime | `total_playtime` | `Sum("sessions__duration_total")` |
| finished | `last_finished` | `Max("playevents__ended")` |
**`SESSION_SORTS`** — `SESSION_DEFAULT_SORT = "-date,created"`
(reproduces today's `order_by("-timestamp_start", "created_at")`):
| key | expression | annotate |
|---|---|---|
| name | `game__sort_name` | — (to-one; safe) |
| date | `timestamp_start` | — |
| duration | `duration_total` | — |
| device | `device__name` | — (to-one; safe) |
| created | `created_at` | — |
**`PURCHASE_SORTS`** — `PURCHASE_DEFAULT_SORT = "-purchased,-created"`
(reproduces today's `order_by("-date_purchased", "-created_at")`):
| key | expression | annotate |
|---|---|---|
| name | `first_game_name` | `Min("games__name")` |
| type | `type` | — |
| price | `converted_price` | — |
| infinite | `infinite` | — |
| purchased | `date_purchased` | — |
| refunded | `date_refunded` | — |
| created | `created_at` | — |
| finished | `last_finished` | `Max("games__playevents__ended")` |
> `game__sort_name` / `device__name` are to-one relations — no duplication, so
> no annotation needed.
> The `finished` purchase key gives the stats "view all" finish-date parity.
> Model field names confirmed against `games/models.py`: `infinite`,
> `converted_price`, `date_purchased`, `date_refunded`, `type`; session
> `duration_total` is a `GeneratedField` (orderable).
#### Term parsing
```python
class SortTerm(NamedTuple):
key: SortKey
descending: bool # True = "-key" (desc), False = bare key (asc)
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
def parse_sort_terms(raw: SortString, sort_map: SortMap) -> ParsedSort:
terms, unknown = [], []
for token in raw.split(","):
token = token.strip()
if not token:
continue
descending = token.startswith("-")
key = token.lstrip("-")
if key in sort_map:
terms.append(SortTerm(key, descending))
else:
unknown.append(key)
return ParsedSort(terms, unknown)
```
### Apply helper
```python
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
def apply_sort(
queryset: QuerySet, find: FindFilter, sort_map: SortMap, default_sort: SortString
) -> SortResult:
terms, unknown = parse_sort_terms(find.sort or "", sort_map)
if not terms:
# default_sort is trusted developer config — ignore any "unknown" from it
terms, _ = parse_sort_terms(default_sort, sort_map)
annotations: Annotations = {}
order_by: list[OrderField] = []
for term in terms:
spec = sort_map[term.key]
if spec.annotate:
annotations.update(spec.annotate)
order_by.append(("-" if term.descending else "") + spec.expression)
if annotations:
queryset = queryset.annotate(**annotations)
return SortResult(queryset.order_by(*order_by), terms, unknown)
```
The full term list is applied (server-side multi-column). `SortResult.terms` is
what the future header UI (#73) consumes; `SortResult.unknown` is what the view
turns into warning toasts.
### FindFilter parsing
```python
def parse_find_filter(request: HttpRequest) -> FindFilter:
return FindFilter(sort=request.GET.get("sort") or None) # FindFilter.sort holds a SortString
```
`FindFilter.direction` is **not used** by #68 — direction lives in the sign of
each `sort` term, not a separate field. Nothing serializes it today (the
`FilterPreset.find_filter` `JSONField` is currently unpopulated; `FindFilter`
has no JSON round-trip), so #68 leaves the field untouched rather than churn it.
Whether to remove it or formally wire it is decided in **#74**. Page / per_page
likewise stay with `paginate()`; not wired into `FindFilter` now (YAGNI, #74).
### View wiring (×3)
In each list view, remove the hardcoded `.order_by(...)` from the base queryset
and, after filtering and before `paginate()`:
```python
find = parse_find_filter(request)
sort = apply_sort(games, find, GAME_SORTS, GAME_DEFAULT_SORT) # sort: SortResult
games = sort.queryset
for key in sort.unknown:
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
```
(`session.py` / `purchase.py` analogous with their maps + default sort strings.)
## Testing
`tests/test_sorting.py`:
- Default order unchanged for each model when no `sort` param is present
(regression guard against the removed hardcoded `order_by` — the default sort
string must reproduce the old order exactly).
- Each key in each map sorts ascending (bare) and descending (`-` prefix).
- Multi-column term list applies in order (e.g. `?sort=status,name`).
- Unknown keys are reported in `SortResult.unknown`; the view emits a
`messages.warning` per unknown key (assert on `get_messages`). A valid key
emits none.
- An all-invalid/empty `sort` falls back to the default sort string.
- A mixed `?sort=-playtime,bogus` both sorts by `playtime` *and* warns on `bogus`.
- Annotated sorts (game playtime, game/purchase finish date, purchase name)
return no duplicate rows (`count` equals unsorted `count`).
- `parse_sort_terms` unit tests: signs, whitespace, empty tokens, unknown keys.
- Smoke: `?sort=<key>` and `?sort=-key,key` URLs return 200 for each list view.
## Coordination with #65
#65 (stats "view all" links, still unmerged) generates URLs that pass `sort=`.
Those links **must use the same public sort keys** defined in this spec's maps
(`playtime`, `finished`, etc.). When #65 and #68 both land, verify each "view
all" link's `sort=` matches a key in the target model's `*_SORTS` map and
reproduces the stats list's order. Call this out in the #68 PR description.
## Related follow-ups (filed during design review)
- **#73** — clickable sortable column headers + multi-column UI (consumes
`SortResult.terms`).
- **#74** — make `FindFilter` the single request parser (sort + pagination +
free-text); fixes the `paginate()` 10-vs-25 / `limit`-vs-`per_page` mismatch.
- **#75** — purchase list free-text search parity.
- **#76** — extract a shared `list_view` helper across the three list views.
- **#77** — saved filter presets should persist/restore sort (`FilterPreset.find_filter`).
## Out of scope (follow-up issue)
Clickable sortable column headers (asc/desc indicator, toggle on click) and the
multi-column UI affordance (shift-click to add a column → multi-term `?sort=`
URL). The backend `apply_sort` already returns `SortResult.terms` and applies
the full term list, so this is UI-only work. Tracked in **#73**.
+140
View File
@@ -0,0 +1,140 @@
"""Structured sorting for list views (Stash-inspired, paired with games/filters.py).
A list view maps a public sort key to a SortSpec; the URL ?sort= param is a
signed comma-list of those keys (e.g. "-playtime,name"). See
docs/superpowers/specs/2026-06-21-list-view-sort-param-design.md.
"""
from dataclasses import dataclass
from typing import NamedTuple
from django.db.models import Aggregate, Max, Min, QuerySet, Sum
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
)
# alias name -> the ORM aggregate that computes it, applied via queryset.annotate()
# e.g. {"total_playtime": Sum("sessions__duration_total")}
type Annotations = dict[AnnotationName, Aggregate]
@dataclass(frozen=True)
class SortSpec:
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)
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
def parse_sort_terms(raw: SortString, sort_map: SortMap) -> ParsedSort:
terms: list[SortTerm] = []
unknown: list[SortKey] = []
for token in raw.split(","):
token = token.strip()
if not token:
continue
descending = token.startswith("-")
key = token.lstrip("-")
if key in sort_map:
terms.append(SortTerm(key, descending))
else:
unknown.append(key)
return ParsedSort(terms, unknown)
# ── Per-model sort maps ─────────────────────────────────────────────────────
# Cross-relation sorts use annotated aggregates (group by PK → no row dup).
# To-one relations (game__sort_name, device__name) are ordered directly.
GAME_SORTS: SortMap = {
"name": SortSpec("name"),
"sort_name": SortSpec("sort_name"),
"year": SortSpec("year_released"),
"status": SortSpec("status"),
"wikidata": SortSpec("wikidata"),
"created": SortSpec("created_at"),
"playtime": SortSpec(
"total_playtime", {"total_playtime": Sum("sessions__duration_total")}
),
"finished": SortSpec("last_finished", {"last_finished": Max("playevents__ended")}),
}
GAME_DEFAULT_SORT: SortString = "-created"
SESSION_SORTS: SortMap = {
"name": SortSpec("game__sort_name"),
"date": SortSpec("timestamp_start"),
"duration": SortSpec("duration_total"),
"device": SortSpec("device__name"),
"created": SortSpec("created_at"),
}
SESSION_DEFAULT_SORT: SortString = "-date,created"
PURCHASE_SORTS: SortMap = {
"name": SortSpec("first_game_name", {"first_game_name": Min("games__name")}),
"type": SortSpec("type"),
"price": SortSpec("converted_price"),
"infinite": SortSpec("infinite"),
"purchased": SortSpec("date_purchased"),
"refunded": SortSpec("date_refunded"),
"created": SortSpec("created_at"),
"finished": SortSpec(
"last_finished", {"last_finished": Max("games__playevents__ended")}
),
}
PURCHASE_DEFAULT_SORT: SortString = "-purchased,-created"
# ── Apply ───────────────────────────────────────────────────────────────────
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
def apply_sort(
queryset: QuerySet, find: FindFilter, sort_map: SortMap, default_sort: SortString
) -> SortResult:
terms, unknown = parse_sort_terms(find.sort or "", sort_map)
if not terms:
# default_sort is trusted developer config — ignore any "unknown" from it
terms, _ = parse_sort_terms(default_sort, sort_map)
annotations: Annotations = {}
order_by: list[OrderField] = []
for term in terms:
spec = sort_map[term.key]
if spec.annotate:
annotations.update(spec.annotate)
order_by.append(("-" if term.descending else "") + spec.expression)
if annotations:
queryset = queryset.annotate(**annotations)
return SortResult(queryset.order_by(*order_by), terms, unknown)
def parse_find_filter(request: HttpRequest) -> FindFilter:
return FindFilter(
sort=request.GET.get("sort") or None
) # FindFilter.sort holds a SortString
+8 -1
View File
@@ -1,5 +1,6 @@
from typing import Any
from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.core.paginator import Paginator
from django.db.models import Q
@@ -48,6 +49,7 @@ from common.time import (
)
from common.utils import build_dynamic_filter, paginate, safe_division, truncate
from games.filters import parse_game_filter
from games.sorting import GAME_DEFAULT_SORT, GAME_SORTS, apply_sort, parse_find_filter
from games.forms import GameForm
from games.models import Game
from games.views.general import use_custom_redirect
@@ -56,7 +58,7 @@ from games.views.playevent import create_playevent_tabledata
@login_required
def list_games(request: HttpRequest, search_string: str = "") -> HttpResponse:
games = Game.objects.order_by("-created_at")
games = Game.objects.select_related("platform")
# ── Structured filter (Stash-style JSON) ──
filter_json = request.GET.get("filter", "")
@@ -86,6 +88,11 @@ def list_games(request: HttpRequest, search_string: str = "") -> HttpResponse:
filters.append(Q(status=search_status))
games = games.filter(build_dynamic_filter(filters, "|"))
sort = apply_sort(games, parse_find_filter(request), GAME_SORTS, GAME_DEFAULT_SORT)
games = sort.queryset
for key in sort.unknown:
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
games, page_obj, elided_page_range = paginate(request, games)
data = {
+16 -1
View File
@@ -44,6 +44,12 @@ from common.time import dateformat
from common.utils import paginate
from games.forms import PurchaseForm
from games.models import Game, Purchase
from games.sorting import (
PURCHASE_DEFAULT_SORT,
PURCHASE_SORTS,
apply_sort,
parse_find_filter,
)
from games.views.general import use_custom_redirect
@@ -119,7 +125,9 @@ def _render_purchase_row(purchase):
@login_required
def list_purchases(request: HttpRequest) -> HttpResponse:
purchases = Purchase.objects.order_by("-date_purchased", "-created_at")
purchases = Purchase.objects.select_related("platform").prefetch_related(
"games", "games__platform"
)
filter_json = request.GET.get("filter", "")
if filter_json:
@@ -129,6 +137,13 @@ 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
)
purchases = sort.queryset
for key in sort.unknown:
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
purchases, page_obj, elided_page_range = paginate(request, purchases)
data = {
+14 -1
View File
@@ -1,5 +1,6 @@
from typing import Any, TypedDict
from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.db.models import Q
from django.http import HttpRequest, HttpResponse
@@ -38,6 +39,12 @@ from common.time import (
from common.utils import paginate, truncate
from games.forms import SessionForm
from games.models import Device, Game, Session
from games.sorting import (
SESSION_DEFAULT_SORT,
SESSION_SORTS,
apply_sort,
parse_find_filter,
)
class SessionRowData(TypedDict):
@@ -119,7 +126,7 @@ def session_row(session: Session, device_list, csrf_token: str) -> Node:
@login_required
def list_sessions(request: HttpRequest, search_string: str = "") -> HttpResponse:
sessions = Session.objects.order_by("-timestamp_start", "created_at")
sessions = Session.objects.select_related("game", "game__platform", "device")
device_list = Device.objects.order_by("name")
# ── Structured filter (JSON) ──
@@ -145,6 +152,12 @@ 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
)
sessions = sort.queryset
for key in sort.unknown:
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
sessions, page_obj, elided_page_range = paginate(request, sessions)
csrf_token = get_token(request)
+301
View File
@@ -0,0 +1,301 @@
"""Tests for the list-view sorting system (games/sorting.py)."""
import re
from datetime import datetime
from zoneinfo import ZoneInfo
import pytest
from django.conf import settings
from django.contrib.messages import get_messages
from django.test import RequestFactory
from django.urls import reverse
from games.filters import FindFilter
from games.models import Game, Platform, Purchase, Session
from games.sorting import (
GAME_DEFAULT_SORT,
GAME_SORTS,
PURCHASE_DEFAULT_SORT,
PURCHASE_SORTS,
SESSION_DEFAULT_SORT,
SESSION_SORTS,
SortSpec,
SortTerm,
apply_sort,
parse_find_filter,
parse_sort_terms,
)
ZONEINFO = ZoneInfo(settings.TIME_ZONE)
# A minimal map; parse_sort_terms only checks key membership, not spec internals.
SAMPLE_MAP = {"name": SortSpec("name"), "date": SortSpec("created_at")}
class TestParseSortTerms:
def test_bare_key_is_ascending(self):
parsed = parse_sort_terms("name", SAMPLE_MAP)
assert parsed.terms == [SortTerm("name", False)]
assert parsed.unknown == []
def test_dash_prefix_is_descending(self):
parsed = parse_sort_terms("-date", SAMPLE_MAP)
assert parsed.terms == [SortTerm("date", True)]
def test_multi_column_preserves_order(self):
parsed = parse_sort_terms("date,-name", SAMPLE_MAP)
assert parsed.terms == [SortTerm("date", False), SortTerm("name", True)]
def test_unknown_key_is_reported_not_raised(self):
parsed = parse_sort_terms("bogus", SAMPLE_MAP)
assert parsed.terms == []
assert parsed.unknown == ["bogus"]
def test_mixed_valid_and_unknown(self):
parsed = parse_sort_terms("-name,bogus", SAMPLE_MAP)
assert parsed.terms == [SortTerm("name", True)]
assert parsed.unknown == ["bogus"]
def test_whitespace_and_empty_tokens_ignored(self):
parsed = parse_sort_terms(" name , , -date ", SAMPLE_MAP)
assert parsed.terms == [SortTerm("name", False), SortTerm("date", True)]
def test_empty_string_yields_nothing(self):
parsed = parse_sort_terms("", SAMPLE_MAP)
assert parsed.terms == []
assert parsed.unknown == []
def _find(sort=None):
return FindFilter(sort=sort)
@pytest.fixture
def two_games(db):
platform = Platform.objects.create(name="P", icon="p")
alpha = Game.objects.create(name="Alpha", sort_name="Alpha", platform=platform)
beta = Game.objects.create(name="Beta", sort_name="Beta", platform=platform)
return alpha, beta
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
)
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
)
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
)
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
)
assert result.unknown == ["bogus"]
assert result.queryset.count() == 2 # still returns rows (default order)
def test_playtime_annotation_no_duplicate_rows(self, two_games):
alpha, _ = two_games
Session.objects.create(
game=alpha,
timestamp_start=datetime(2022, 1, 1, 10, tzinfo=ZONEINFO),
timestamp_end=datetime(2022, 1, 1, 12, tzinfo=ZONEINFO),
)
Session.objects.create(
game=alpha,
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
)
# 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"})
assert parse_find_filter(request).sort == "-playtime,name"
def test_absent_sort_is_none(self):
request = RequestFactory().get("/x")
assert parse_find_filter(request).sort is None
class TestSortMapShapes:
def test_default_sort_keys_exist_in_maps(self):
# every key referenced by a default sort string must be defined in its map
for default, sort_map in [
(GAME_DEFAULT_SORT, GAME_SORTS),
(SESSION_DEFAULT_SORT, SESSION_SORTS),
(PURCHASE_DEFAULT_SORT, PURCHASE_SORTS),
]:
for token in default.split(","):
assert token.lstrip("-") in sort_map
@pytest.fixture
def logged_client(client, django_user_model):
user = django_user_model.objects.create_user(username="u", password="p")
client.force_login(user)
return client
class TestListGamesSort:
def test_sort_param_orders_rows(self, logged_client, two_games):
alpha, beta = two_games
response = logged_client.get(reverse("games:list_games"), {"sort": "-name"})
assert response.status_code == 200
body = response.content.decode()
assert body.index("Beta") < body.index("Alpha")
# ascending name must flip order vs the default (-created → Beta first)
ascending = logged_client.get(reverse("games:list_games"), {"sort": "name"})
ascending_body = ascending.content.decode()
assert ascending_body.index("Alpha") < ascending_body.index("Beta")
def test_unknown_sort_emits_warning_message(self, logged_client, two_games):
response = logged_client.get(reverse("games:list_games"), {"sort": "bogus"})
assert response.status_code == 200
warnings = [str(m) for m in get_messages(response.wsgi_request)]
assert any("bogus" in w for w in warnings)
def test_valid_sort_emits_no_warning(self, logged_client, two_games):
response = logged_client.get(reverse("games:list_games"), {"sort": "name"})
warnings = [str(m) for m in get_messages(response.wsgi_request)]
assert warnings == []
class TestListSessionsSort:
def test_sort_by_duration_descending(self, logged_client, two_games):
alpha, beta = two_games
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
)
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
)
# default order is -date (beta first); -duration must override it (alpha's 3h first)
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"<tbody[^>]*>(.*?)</tbody>", body, re.DOTALL)
assert tbody_match
tbody = tbody_match.group(1)
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"})
warnings = [str(m) for m in get_messages(response.wsgi_request)]
assert any("nope" in w for w in warnings)
class TestListPurchasesSort:
@pytest.fixture
def two_purchases(self, db, two_games):
alpha, beta = two_games
# cheap (Alpha, price=10) purchased LATER so default -purchased order would show Alpha first
# dear (Beta, price=90) purchased EARLIER — so -price must override default order to pass
dear = Purchase.objects.create(
date_purchased=datetime(2022, 1, 1, tzinfo=ZONEINFO),
price=90,
converted_price=90,
platform=beta.platform,
)
dear.games.add(beta)
cheap = Purchase.objects.create(
date_purchased=datetime(2022, 1, 2, tzinfo=ZONEINFO),
price=10,
converted_price=10,
platform=alpha.platform,
)
cheap.games.add(alpha)
return cheap, dear
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"}
)
assert response.status_code == 200
body = response.content.decode()
tbody_match = re.search(r"<tbody[^>]*>(.*?)</tbody>", body, re.DOTALL)
assert tbody_match
tbody = tbody_match.group(1)
assert tbody.index("Beta") < tbody.index("Alpha") # 90 before 10
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
)
cheap.games.add(extra)
response = logged_client.get(reverse("games:list_purchases"), {"sort": "name"})
body = response.content.decode()
assert body.count("purchase-row-") == 2 # exactly two purchase rows
def test_unknown_sort_emits_warning(self, logged_client, two_purchases):
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"<tbody[^>]*>(.*?)</tbody>", 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