diff --git a/api/views.py b/api/views.py index a5a42a24..80cb616b 100644 --- a/api/views.py +++ b/api/views.py @@ -8,11 +8,14 @@ DecimalField, F, IntegerField, + OuterRef, Prefetch, Q, + Subquery, Sum, When, ) +from django.db.models.functions import Coalesce from django.http import Http404 from django.shortcuts import get_object_or_404 from django.utils import timezone @@ -116,6 +119,20 @@ from users.models import CustomUser from wish_list.models import WishList, WishListItem +# Correlated per-series issue count. Unlike Count("issues") -- which joins and +# GROUPs BY every issue row before sorting and paginating -- Postgres only +# evaluates this for the rows actually returned, and the pagination COUNT(*) +# drops it entirely. Coalesce because a series with no issues yields no row. +_series_issue_count = Coalesce( + Subquery( + Issue.objects.filter(series=OuterRef("pk")) + .values("series") + .annotate(count=Count("pk")) + .values("count") + ), + 0, +) + class ReadingListItemsPagination(PageNumberPagination): """Custom pagination for reading list items with 50 items per page.""" @@ -761,10 +778,8 @@ def series_list(self, request, pk=None): if cached is not None: return _mark_cache_status(Response(cached), hit=True) - queryset = ( - publisher.series.select_related("series_type", "publisher") - .annotate(num_issues=Count("issues", distinct=True)) - .order_by("sort_name", "year_began") + queryset = publisher.series.select_related("series_type", "publisher").annotate( + num_issues=_series_issue_count ) page = self.paginate_queryset(queryset) if page is None: @@ -819,24 +834,21 @@ class SeriesViewSet( cache_detail_dependent_labels = (ModelLabel.PUBLISHER_NAME, ModelLabel.IMPRINT_NAME) def get_modified_queryset(self): - # get_queryset() annotates num_issues for list/retrieve -- an - # aggregate that survives into (pk, modified) values_list() as an - # extra JOIN + GROUP BY even though the annotated field isn't - # selected. This lookup only needs the row's own pk/modified. + # get_queryset() adds select_related joins and a num_issues + # subquery for list/retrieve. This lookup only needs the row's own + # pk/modified. return Series.objects.all() def get_queryset(self): queryset = super().get_queryset() match self.action: case "list": - queryset = queryset.annotate(num_issues=Count("issues", distinct=True)).order_by( - "sort_name", "year_began" - ) + queryset = queryset.annotate(num_issues=_series_issue_count) case "retrieve": queryset = ( queryset.select_related("imprint") .prefetch_related("genres", "associated") - .annotate(num_issues=Count("issues", distinct=True)) + .annotate(num_issues=_series_issue_count) ) return queryset diff --git a/comicsdb/migrations/0061_ordering_tiebreak_by_id.py b/comicsdb/migrations/0061_ordering_tiebreak_by_id.py new file mode 100644 index 00000000..969eac36 --- /dev/null +++ b/comicsdb/migrations/0061_ordering_tiebreak_by_id.py @@ -0,0 +1,51 @@ +# Generated by Django 6.1.2 on 2026-10-19 12:24 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("comicsdb", "0060_attribution_ordering"), + ] + + operations = [ + migrations.AlterModelOptions( + name="arc", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="character", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="creator", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="imprint", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="issue", + options={"ordering": ["series__sort_name", "cover_date", "store_date", "number", "id"]}, + ), + migrations.AlterModelOptions( + name="publisher", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="series", + options={ + "ordering": ["sort_name", "year_began", "id"], + "verbose_name_plural": "Series", + }, + ), + migrations.AlterModelOptions( + name="team", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="universe", + options={"ordering": ["name", "designation", "id"]}, + ), + ] diff --git a/comicsdb/models/arc.py b/comicsdb/models/arc.py index c17c8641..fb00f63c 100644 --- a/comicsdb/models/arc.py +++ b/comicsdb/models/arc.py @@ -57,7 +57,7 @@ class Meta: models.Index(fields=["cv_id"], name="arc_cv_id_idx"), models.Index(fields=["gcd_id"], name="arc_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Arc, dispatch_uid="pre_save_arc") diff --git a/comicsdb/models/character.py b/comicsdb/models/character.py index 1e1983f9..b9292311 100644 --- a/comicsdb/models/character.py +++ b/comicsdb/models/character.py @@ -81,7 +81,7 @@ class Meta: models.Index(fields=["cv_id"], name="character_cv_id_idx"), models.Index(fields=["gcd_id"], name="character_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Character, dispatch_uid="pre_save_character") diff --git a/comicsdb/models/creator.py b/comicsdb/models/creator.py index 81b47bb4..3b28379f 100644 --- a/comicsdb/models/creator.py +++ b/comicsdb/models/creator.py @@ -73,7 +73,7 @@ class Meta: models.Index(fields=["cv_id"], name="creator_cv_id_idx"), models.Index(fields=["gcd_id"], name="creator_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Creator, dispatch_uid="pre_save_creator") diff --git a/comicsdb/models/imprint.py b/comicsdb/models/imprint.py index 59830a17..9d1c4eba 100644 --- a/comicsdb/models/imprint.py +++ b/comicsdb/models/imprint.py @@ -68,7 +68,7 @@ class Meta: models.Index(fields=["cv_id"], name="imprint_cv_id_idx"), models.Index(fields=["gcd_id"], name="imprint_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Imprint, dispatch_uid="pre_save_imprint") diff --git a/comicsdb/models/issue.py b/comicsdb/models/issue.py index 0c544bea..d567bdcb 100644 --- a/comicsdb/models/issue.py +++ b/comicsdb/models/issue.py @@ -187,7 +187,7 @@ class Meta: models.Index(fields=["cv_id"], name="issue_cv_id_idx"), models.Index(fields=["gcd_id"], name="issue_gcd_id_idx"), ] - ordering = ["series__sort_name", "cover_date", "store_date", "number"] + ordering = ["series__sort_name", "cover_date", "store_date", "number", "id"] unique_together = ["series", "number"] diff --git a/comicsdb/models/publisher.py b/comicsdb/models/publisher.py index ed6b39f7..3b99b343 100644 --- a/comicsdb/models/publisher.py +++ b/comicsdb/models/publisher.py @@ -81,7 +81,7 @@ class Meta: models.Index(fields=["cv_id"], name="publisher_cv_id_idx"), models.Index(fields=["gcd_id"], name="publisher_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Publisher, dispatch_uid="pre_save_publisher") diff --git a/comicsdb/models/series.py b/comicsdb/models/series.py index 30d49767..443f5c87 100644 --- a/comicsdb/models/series.py +++ b/comicsdb/models/series.py @@ -123,7 +123,7 @@ class Meta: models.Index(fields=["cv_id"], name="series_cv_id_idx"), models.Index(fields=["gcd_id"], name="series_gcd_id_idx"), ] - ordering = ["sort_name", "year_began"] + ordering = ["sort_name", "year_began", "id"] unique_together = ["publisher", "name", "volume", "series_type"] verbose_name_plural = _("Series") diff --git a/comicsdb/models/team.py b/comicsdb/models/team.py index b188ebf4..db6af443 100644 --- a/comicsdb/models/team.py +++ b/comicsdb/models/team.py @@ -69,7 +69,7 @@ class Meta: models.Index(fields=["cv_id"], name="team_cv_id_idx"), models.Index(fields=["gcd_id"], name="team_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Team, dispatch_uid="pre_save_team") diff --git a/comicsdb/models/universe.py b/comicsdb/models/universe.py index 59d860ea..56e47eeb 100644 --- a/comicsdb/models/universe.py +++ b/comicsdb/models/universe.py @@ -78,7 +78,7 @@ class Meta: models.Index(fields=["cv_id"], name="universe_cv_id_idx"), models.Index(fields=["gcd_id"], name="universe_gcd_id_idx"), ] - ordering = ["name", "designation"] + ordering = ["name", "designation", "id"] unique_together = ["publisher", "name", "designation"] db_table_comment = "Publisher Universes" diff --git a/tests/comicsdb/test_api_series.py b/tests/comicsdb/test_api_series.py index e1e893ee..91d8de38 100644 --- a/tests/comicsdb/test_api_series.py +++ b/tests/comicsdb/test_api_series.py @@ -5,10 +5,11 @@ import pytest from django.db.models import Count, Q from django.urls import reverse +from django.utils import timezone from rest_framework import status from api.v1_0.serializers import SeriesListSerializer -from comicsdb.models import Credits, Series +from comicsdb.models import Credits, Issue, Series from comicsdb.models.character import Character from comicsdb.models.creator import Creator from comicsdb.models.credits import Role @@ -393,3 +394,60 @@ def test_list_series_year_end_set( assert resp.status_code == status.HTTP_200_OK completed = next(r for r in resp.data["results"] if r["year_end"] is not None) assert completed["year_end"] == 2005 + + +def test_issue_count( + api_client_with_credentials, create_user, dc_comics, fc_series: Series, bat_sups_series +): + user = create_user() + for number in ("1", "2"): + Issue.objects.create( + series=fc_series, + number=number, + cover_date=timezone.now().date(), + edited_by=user, + created_by=user, + ) + expected = {fc_series.id: 2, bat_sups_series.id: 0} + + for url in ( + reverse("api:series-list"), + reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk}), + ): + resp = api_client_with_credentials.get(url) + assert resp.status_code == status.HTTP_200_OK + assert {r["id"]: r["issue_count"] for r in resp.data["results"]} == expected + + for pk, count in expected.items(): + resp = api_client_with_credentials.get(reverse("api:series-detail", kwargs={"pk": pk})) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["issue_count"] == count + + +def test_list_orders_ties_by_id( + api_client_with_credentials, create_user, dc_comics, single_issue_type +): + user = create_user() + tied = [ + Series.objects.create( + name="Miles Morales: Spider-Man", + sort_name="Miles Morales: Spider-Man", + publisher=dc_comics, + volume=volume, + year_began=2019, + series_type=single_issue_type, + status=Series.Status.ONGOING, + edited_by=user, + created_by=user, + ) + for volume in (1, 2) + ] + expected = sorted(s.id for s in tied) + + for url in ( + reverse("api:series-list"), + reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk}), + ): + resp = api_client_with_credentials.get(url) + assert resp.status_code == status.HTTP_200_OK + assert [r["id"] for r in resp.data["results"]] == expected