test(sorting): default-order regression + all-keys smoke (#68)
This commit is contained in:
+24
-12
@@ -13,10 +13,16 @@ from django.http import HttpRequest
|
|||||||
|
|
||||||
from games.filters import FindFilter
|
from games.filters import FindFilter
|
||||||
|
|
||||||
type SortKey = str # public column key in a *_SORTS map and in a URL term ("playtime", "name")
|
type SortKey = (
|
||||||
type SortString = str # comma-list of signed SortKeys: the URL ?sort= value and *_DEFAULT_SORT ("-date,created")
|
str # public column key in a *_SORTS map and in a URL term ("playtime", "name")
|
||||||
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 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()
|
# alias name -> the ORM aggregate that computes it, applied via queryset.annotate()
|
||||||
# e.g. {"total_playtime": Sum("sessions__duration_total")}
|
# e.g. {"total_playtime": Sum("sessions__duration_total")}
|
||||||
@@ -25,13 +31,13 @@ type Annotations = dict[AnnotationName, Aggregate]
|
|||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class SortSpec:
|
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
|
annotate: Annotations | None = None
|
||||||
|
|
||||||
|
|
||||||
class SortTerm(NamedTuple):
|
class SortTerm(NamedTuple):
|
||||||
key: SortKey
|
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]
|
type SortMap = dict[SortKey, SortSpec]
|
||||||
@@ -39,7 +45,7 @@ type SortMap = dict[SortKey, SortSpec]
|
|||||||
|
|
||||||
class ParsedSort(NamedTuple):
|
class ParsedSort(NamedTuple):
|
||||||
terms: list[SortTerm]
|
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:
|
def parse_sort_terms(raw: SortString, sort_map: SortMap) -> ParsedSort:
|
||||||
@@ -69,7 +75,9 @@ GAME_SORTS: SortMap = {
|
|||||||
"status": SortSpec("status"),
|
"status": SortSpec("status"),
|
||||||
"wikidata": SortSpec("wikidata"),
|
"wikidata": SortSpec("wikidata"),
|
||||||
"created": SortSpec("created_at"),
|
"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")}),
|
"finished": SortSpec("last_finished", {"last_finished": Max("playevents__ended")}),
|
||||||
}
|
}
|
||||||
GAME_DEFAULT_SORT: SortString = "-created"
|
GAME_DEFAULT_SORT: SortString = "-created"
|
||||||
@@ -91,7 +99,9 @@ PURCHASE_SORTS: SortMap = {
|
|||||||
"purchased": SortSpec("date_purchased"),
|
"purchased": SortSpec("date_purchased"),
|
||||||
"refunded": SortSpec("date_refunded"),
|
"refunded": SortSpec("date_refunded"),
|
||||||
"created": SortSpec("created_at"),
|
"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"
|
PURCHASE_DEFAULT_SORT: SortString = "-purchased,-created"
|
||||||
|
|
||||||
@@ -101,8 +111,8 @@ PURCHASE_DEFAULT_SORT: SortString = "-purchased,-created"
|
|||||||
|
|
||||||
class SortResult(NamedTuple):
|
class SortResult(NamedTuple):
|
||||||
queryset: QuerySet
|
queryset: QuerySet
|
||||||
terms: list[SortTerm] # the order actually applied — #73's header UI consumes this
|
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
|
unknown: list[SortKey] # rejected keys — the view turns these into warning toasts
|
||||||
|
|
||||||
|
|
||||||
def apply_sort(
|
def apply_sort(
|
||||||
@@ -125,4 +135,6 @@ def apply_sort(
|
|||||||
|
|
||||||
|
|
||||||
def parse_find_filter(request: HttpRequest) -> FindFilter:
|
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
|
||||||
|
|||||||
@@ -135,7 +135,9 @@ def list_purchases(request: HttpRequest) -> HttpResponse:
|
|||||||
if pf is not None:
|
if pf is not None:
|
||||||
purchases = purchases.filter(pf.to_q())
|
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
|
purchases = sort.queryset
|
||||||
for key in sort.unknown:
|
for key in sort.unknown:
|
||||||
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
|
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
|
||||||
|
|||||||
@@ -152,7 +152,9 @@ def list_sessions(request: HttpRequest, search_string: str = "") -> HttpResponse
|
|||||||
last_session = sessions.latest()
|
last_session = sessions.latest()
|
||||||
except Session.DoesNotExist:
|
except Session.DoesNotExist:
|
||||||
last_session = None
|
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
|
sessions = sort.queryset
|
||||||
for key in sort.unknown:
|
for key in sort.unknown:
|
||||||
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
|
messages.warning(request, f"Unknown sort field '{key}' was ignored.")
|
||||||
|
|||||||
+64
-12
@@ -82,23 +82,31 @@ def two_games(db):
|
|||||||
class TestApplySortGames:
|
class TestApplySortGames:
|
||||||
def test_name_ascending(self, two_games):
|
def test_name_ascending(self, two_games):
|
||||||
alpha, beta = 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 list(result.queryset) == [alpha, beta]
|
||||||
assert result.terms[0].key == "name"
|
assert result.terms[0].key == "name"
|
||||||
assert result.unknown == []
|
assert result.unknown == []
|
||||||
|
|
||||||
def test_name_descending(self, two_games):
|
def test_name_descending(self, two_games):
|
||||||
alpha, beta = 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]
|
assert list(result.queryset) == [beta, alpha]
|
||||||
|
|
||||||
def test_default_sort_when_absent_is_created_desc(self, two_games):
|
def test_default_sort_when_absent_is_created_desc(self, two_games):
|
||||||
alpha, beta = two_games # beta created after alpha
|
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]
|
assert list(result.queryset) == [beta, alpha]
|
||||||
|
|
||||||
def test_unknown_key_reported_and_falls_back(self, two_games):
|
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.unknown == ["bogus"]
|
||||||
assert result.queryset.count() == 2 # still returns rows (default order)
|
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_start=datetime(2022, 1, 2, 10, tzinfo=ZONEINFO),
|
||||||
timestamp_end=datetime(2022, 1, 2, 11, 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
|
# two sessions on alpha must not duplicate the alpha row
|
||||||
assert result.queryset.count() == 2
|
assert result.queryset.count() == 2
|
||||||
assert list(result.queryset)[0] == alpha # most playtime first
|
assert list(result.queryset)[0] == alpha # most playtime first
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
class TestParseFindFilter:
|
class TestParseFindFilter:
|
||||||
def test_reads_sort_param(self):
|
def test_reads_sort_param(self):
|
||||||
request = RequestFactory().get("/x", {"sort": "-playtime,name"})
|
request = RequestFactory().get("/x", {"sort": "-playtime,name"})
|
||||||
@@ -182,22 +191,30 @@ class TestListSessionsSort:
|
|||||||
Session.objects.create(
|
Session.objects.create(
|
||||||
game=alpha,
|
game=alpha,
|
||||||
timestamp_start=datetime(2022, 1, 1, 10, tzinfo=ZONEINFO),
|
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(
|
Session.objects.create(
|
||||||
game=beta,
|
game=beta,
|
||||||
timestamp_start=datetime(2022, 1, 2, 10, tzinfo=ZONEINFO),
|
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)
|
# 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
|
assert response.status_code == 200
|
||||||
body = response.content.decode()
|
body = response.content.decode()
|
||||||
# Extract only the table body to avoid finding "Beta" in header's last-session button
|
# 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)
|
tbody_match = re.search(r"<tbody[^>]*>(.*?)</tbody>", body, re.DOTALL)
|
||||||
assert tbody_match
|
assert tbody_match
|
||||||
tbody = tbody_match.group(1)
|
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):
|
def test_unknown_sort_emits_warning(self, logged_client, two_games):
|
||||||
response = logged_client.get(reverse("games:list_sessions"), {"sort": "nope"})
|
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):
|
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
|
# 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
|
assert response.status_code == 200
|
||||||
body = response.content.decode()
|
body = response.content.decode()
|
||||||
tbody = re.search(r"<tbody[^>]*>(.*?)</tbody>", body, re.DOTALL).group(1)
|
tbody = re.search(r"<tbody[^>]*>(.*?)</tbody>", body, re.DOTALL).group(1)
|
||||||
@@ -238,7 +257,9 @@ class TestListPurchasesSort:
|
|||||||
def test_name_aggregate_sort_no_duplicate_rows(self, logged_client, two_purchases):
|
def test_name_aggregate_sort_no_duplicate_rows(self, logged_client, two_purchases):
|
||||||
# a multi-game purchase must still render exactly one row
|
# a multi-game purchase must still render exactly one row
|
||||||
cheap, _ = two_purchases
|
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)
|
cheap.games.add(extra)
|
||||||
response = logged_client.get(reverse("games:list_purchases"), {"sort": "name"})
|
response = logged_client.get(reverse("games:list_purchases"), {"sort": "name"})
|
||||||
body = response.content.decode()
|
body = response.content.decode()
|
||||||
@@ -248,3 +269,34 @@ class TestListPurchasesSort:
|
|||||||
response = logged_client.get(reverse("games:list_purchases"), {"sort": "nope"})
|
response = logged_client.get(reverse("games:list_purchases"), {"sort": "nope"})
|
||||||
warnings = [str(m) for m in get_messages(response.wsgi_request)]
|
warnings = [str(m) for m in get_messages(response.wsgi_request)]
|
||||||
assert any("nope" in w for w in warnings)
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user