diff --git a/NEWS.md b/NEWS.md index 42be969f5..48130d68a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,8 +8,21 @@ border-radius: 128px; ## v2.2.12 +- Features + - Sort by Alternate Number, the issue number a comic carries inside its + alternate series (ComicInfo AlternateSeries / AlternateNumber). Filter by + an alternate series first to pick which one to sort by. Comics with no + alternate number fall back to their own issue number. + - The Alternate Series sort is available in cover view, not just the table. + Comics with no alternate series sort by their real series name. + - Read an alternate series as a reading order: pick it in the reader's + reading-order menu and next/prev follow the alternate numbering. Handy for + using alternate series tags as durable reading lists. + - Fixes - The Admin Tagging Status table shows what is being looked up right now. + - Sorting by a tag column outside the table view (cover cards, OPDS feeds) + no longer errors. ## v2.2.11 diff --git a/codex/choices/browser.py b/codex/choices/browser.py index d570642c8..19317bb8b 100644 --- a/codex/choices/browser.py +++ b/codex/choices/browser.py @@ -18,6 +18,7 @@ { "created_at": "Added Time", "age_rating": "Age Rating", + "alternate_number": "Alternate Number", "reprints": "Alternate Series", "characters": "Characters", "child_count": "Child Count", @@ -75,6 +76,8 @@ { "created_at", "age_rating", + "alternate_number", + "reprints", "child_count", "community_rating", "filename", @@ -94,12 +97,15 @@ # They sort fine as the primary, but the per-extra annotation # pipeline can't safely produce a value for them on every model # / context: ``story_arc_number`` requires StoryArc-context ``pks`` -# to resolve which arc's number to pick, and ``search_score``'s -# ``ComicFTSRank`` only resolves when an FTS subquery is joined. +# to resolve which arc's number to pick, ``alternate_number`` likewise +# needs the ``reprints`` filter to resolve which alternate series' +# number to pick, and ``search_score``'s ``ComicFTSRank`` only +# resolves when an FTS subquery is joined. # Mirrored on the frontend so the table headers can grey out the # affected columns and refuse the shift-click. BROWSER_EXTRA_SORT_UNSUPPORTED_KEYS = frozenset( { + "alternate_number", "story_arc_number", "search_score", } diff --git a/codex/collection.py b/codex/collection.py index ab25a05db..974879fac 100644 --- a/codex/collection.py +++ b/codex/collection.py @@ -93,3 +93,12 @@ def collection(self) -> str: Collection.ARC: "Story Arcs", } ) + +# A reader-only pseudo-collection: the reader can follow an alternate +# series (ComicInfo ``AlternateSeries``) as a reading order, ordered by +# ``AlternateNumber``. Deliberately *not* a :class:`Collection` member — +# alternate series aren't browsable, and every map above is exhaustive +# over the enum, so a member without entries would break lookups that +# assume a browse route and a cover exists. +READER_REPRINT_COLLECTION: Final[str] = "reprints" +READER_REPRINT_LABEL: Final[str] = "Alternate Series" diff --git a/codex/librarian/scribe/importer/const.py b/codex/librarian/scribe/importer/const.py index 56497aaec..d5e798c38 100644 --- a/codex/librarian/scribe/importer/const.py +++ b/codex/librarian/scribe/importer/const.py @@ -501,7 +501,14 @@ def get_through_model(field: ManyToManyField) -> type[BaseModel]: *COLLECTION_FIELD_NAMES, "story_arc_numbers", "folders", + # Not a browse collection, but the reader reads alternate series as a + # reading order off ``Reprint.updated_at``, so a comic leaving one must + # re-stamp it the same way a story arc does. + REPRINTS_FIELD_NAME, ) +# Comic m2m fields whose target model *is* the collection row (no +# intermediate like ``StoryArcNumber`` to walk through). +DIRECT_M2M_COLLECTION_FIELD_NAMES = frozenset({"folders", REPRINTS_FIELD_NAME}) ########## # Failed # diff --git a/codex/librarian/scribe/importer/delete/comics.py b/codex/librarian/scribe/importer/delete/comics.py index 1087794a6..55fc99466 100644 --- a/codex/librarian/scribe/importer/delete/comics.py +++ b/codex/librarian/scribe/importer/delete/comics.py @@ -1,10 +1,13 @@ """Delete comics methods.""" -from codex.librarian.scribe.importer.const import ALL_COMIC_COLLECTION_FIELD_NAMES +from codex.librarian.scribe.importer.const import ( + ALL_COMIC_COLLECTION_FIELD_NAMES, + DIRECT_M2M_COLLECTION_FIELD_NAMES, +) from codex.librarian.scribe.importer.delete.covers import DeletedCoversImporter from codex.librarian.scribe.importer.delete.existence import confirm_deleted from codex.librarian.scribe.importer.statii.delete import ImporterRemoveComicsStatus -from codex.models import Comic, Folder, StoryArc +from codex.models import Comic, StoryArc from codex.settings import ( IMPORTER_DELETE_MAX_CHUNK_SIZE, IMPORTER_LINK_FK_BATCH_SIZE, @@ -40,9 +43,10 @@ def _populate_deleted_comic_collection(deleted_comic_collections, comic) -> None "story_arc" ): deleted_comic_collections[StoryArc].add(san.story_arc.pk) - elif field_name == "folders": - for folder in comic.folders.only("pk"): - deleted_comic_collections[Folder].add(folder.pk) + elif field_name in DIRECT_M2M_COLLECTION_FIELD_NAMES: + related_model = comic._meta.get_field(field_name).related_model + for obj in getattr(comic, field_name).only("pk"): + deleted_comic_collections[related_model].add(obj.pk) else: related_model = comic._meta.get_field(field_name).related_model related_id = getattr(comic, field_name).pk @@ -55,7 +59,7 @@ def _populate_deleted_comic_collections( """Populate changed collections for cover timestamp updater.""" comics_deleted_qs = delete_qs.only( *ALL_COMIC_COLLECTION_FIELD_NAMES - ).prefetch_related("story_arc_numbers__story_arc") + ).prefetch_related("story_arc_numbers__story_arc", *DIRECT_M2M_COLLECTION_FIELD_NAMES) for comic in comics_deleted_qs.iterator( chunk_size=IMPORTER_DELETE_MAX_CHUNK_SIZE ): diff --git a/codex/librarian/scribe/importer/init.py b/codex/librarian/scribe/importer/init.py index 6cdf11065..56c2287fd 100644 --- a/codex/librarian/scribe/importer/init.py +++ b/codex/librarian/scribe/importer/init.py @@ -57,6 +57,7 @@ if TYPE_CHECKING: from codex.models.base import BaseModel from codex.models.collections import BrowserCollectionModel, Folder + from codex.models.named import Reprint _WRITE_WAIT_EXPIRY = 60 @@ -129,7 +130,9 @@ def __init__( # move only re-stamps the destination (the one collection a current # comic still points into) and the browser's ``library.changed`` refresh # gate never sees the source view change. Keyed by collection model. - self.moved_source_collections: dict[type[BrowserCollectionModel], set[int]] = {} + self.moved_source_collections: dict[ + type[BrowserCollectionModel | Reprint], set[int] + ] = {} # Full set of paths this import touched, captured before the chunking # loop and extract phase zero out ``task.files_*``. Consumed at finish # to stamp ``Comic.metadata_imported_at`` on every comic a forced/lazy diff --git a/codex/librarian/scribe/importer/query/links_m2m.py b/codex/librarian/scribe/importer/query/links_m2m.py index 1d47e37bc..0df6e3b1c 100644 --- a/codex/librarian/scribe/importer/query/links_m2m.py +++ b/codex/librarian/scribe/importer/query/links_m2m.py @@ -9,6 +9,7 @@ FIELD_NAME_KEY_ATTRS_MAP, FOLDERS_FIELD_NAME, LINK_M2MS, + REPRINTS_FIELD_NAME, STORY_ARC_FIELD_NAME, STORY_ARC_NUMBERS_FIELD_NAME, get_through_model, @@ -17,7 +18,7 @@ from codex.models.base import NamedModel from codex.models.collections import BrowserCollectionModel, Folder from codex.models.comic import Comic -from codex.models.named import StoryArc +from codex.models.named import Reprint, StoryArc from codex.settings import ( IMPORTER_LINK_FK_BATCH_SIZE, IMPORTER_LINK_M2M_BATCH_SIZE, @@ -71,15 +72,20 @@ def _record_removed_m2m_source_collection( leaving one must re-stamp the SOURCE arc/folder — ``TimestampUpdater`` only re-stamps collections a *current* comic still links into. Mirrors the FK move capture in ``CreateComicsImporter``; the delete phase folds - these into the force-update map. Tag-style m2ms (genres, characters, …) - are not collections and are ignored here. + these into the force-update map. ``Reprint`` rides along because the + reader reads alternate series as a reading order off its timestamp. + Tag-style m2ms (genres, characters, …) are not collections and are + ignored here. """ if field_name == STORY_ARC_NUMBERS_FIELD_NAME: - model: type[BrowserCollectionModel] = StoryArc + model: type[BrowserCollectionModel | Reprint] = StoryArc source_pk = story_arc_pk elif field_name == FOLDERS_FIELD_NAME: model = Folder source_pk = target_pk + elif field_name == REPRINTS_FIELD_NAME: + model = Reprint + source_pk = target_pk else: return if source_pk is not None: diff --git a/codex/librarian/scribe/timestamp_update.py b/codex/librarian/scribe/timestamp_update.py index 0626b59d5..7434d2659 100644 --- a/codex/librarian/scribe/timestamp_update.py +++ b/codex/librarian/scribe/timestamp_update.py @@ -10,19 +10,29 @@ from codex.librarian.notifier.tasks import LIBRARY_CHANGED_TASK from codex.librarian.scribe.status import UpdateCollectionTimestampsStatus from codex.librarian.worker import WorkerStatusBase -from codex.models import StoryArc, Volume +from codex.models import Reprint, StoryArc, Volume from codex.models.collections import BrowserCollectionModel from codex.models.library import Library from codex.settings import IMPORTER_LINK_FK_BATCH_SIZE from codex.views.const import COLLECTION_MODELS +# Rows whose ``updated_at`` gates a client-side reload. Browse collections +# bust cover caches; ``Reprint`` is not browsable but the reader offers +# alternate series as a reading order, and its arc mtime is read from these +# rows — without a re-stamp an open reader never notices a re-import. +_TIMESTAMP_MODELS = (*COLLECTION_MODELS, Reprint) + +# Volumes never carry their own custom cover, and ``Reprint`` has no +# ``custom_cover`` column at all. +_NO_CUSTOM_COVER_MODELS = frozenset({Volume, Reprint}) + class TimestampUpdater(WorkerStatusBase): """Update Collections timestamp for cover cache busting.""" @staticmethod def _get_update_filter( - model: type[BrowserCollectionModel], + model: type[BrowserCollectionModel | Reprint], start_time: datetime, force_update_collection_map: Mapping, library: Library, @@ -55,7 +65,7 @@ def _get_update_filter( # its own has-children join test. This used to be a # Count-aggregate filter over the whole OR — a per-row # GROUP BY join explosion costing ~1.5s per import. - if model != Volume: + if model not in _NO_CUSTOM_COVER_MODELS: update_filter |= Q(custom_cover__updated_at__gt=start_floor) & Q( **{rel + "comic__isnull": False} ) @@ -75,7 +85,7 @@ def _get_update_filter( def _update_collection_model( cls, force_update_collection_map: Mapping, - model: type[BrowserCollectionModel], + model: type[BrowserCollectionModel | Reprint], start_time: datetime, library: Library, log_list, @@ -113,7 +123,7 @@ def update_library_collections( self.status_controller.start(status) try: log_list = [] - for model in COLLECTION_MODELS: + for model in _TIMESTAMP_MODELS: count = self._update_collection_model( force_update_collection_map, model, start_time, library, log_list ) diff --git a/codex/migrations/0053_reprint_issue_number.py b/codex/migrations/0053_reprint_issue_number.py new file mode 100644 index 000000000..fc397463b --- /dev/null +++ b/codex/migrations/0053_reprint_issue_number.py @@ -0,0 +1,121 @@ +""" +Split Reprint.issue into sortable number & suffix columns. + +The ``alternate_number`` order_by key sorts comics by their +ComicInfo ``AlternateNumber`` within an alternate series. ``issue`` +is a string, so sorting it directly puts "#10" before "#2"; these +derived columns mirror ``Comic.issue_number`` / ``issue_suffix``. + +Generated by Django 6.1 on 2026-08-30 06:03. +""" + +from django.db import migrations, models + +import codex.models.fields +from codex.models.util import parse_issue_parts + +_BATCH_SIZE = 1000 + + +def _backfill_reprint_issue_parts(apps, _schema_editor) -> None: + """Derive issue_number & issue_suffix for existing reprints.""" + reprint_model = apps.get_model("codex", "Reprint") + # Historical models don't carry ``presave``, so parse here. + updates = [] + for reprint in reprint_model.objects.exclude(issue="").iterator(): + issue_number, issue_suffix = parse_issue_parts(reprint.issue) + if issue_number is None and not issue_suffix: + continue + reprint.issue_number = issue_number + reprint.issue_suffix = issue_suffix + updates.append(reprint) + + for start in range(0, len(updates), _BATCH_SIZE): + reprint_model.objects.bulk_update( + updates[start : start + _BATCH_SIZE], ["issue_number", "issue_suffix"] + ) + + +class Migration(migrations.Migration): + """Add Reprint issue sort columns and backfill them.""" + + dependencies = [ + ("codex", "0052_comicboxtaggingdefaults_comicvine_url"), + ] + + operations = [ + migrations.AddField( + model_name="reprint", + name="issue_number", + field=codex.models.fields.CoercingDecimalField( + decimal_places=2, max_digits=10, null=True + ), + ), + migrations.AddField( + model_name="reprint", + name="issue_suffix", + field=codex.models.fields.CleaningCharField( + db_collation="nocase", default="", max_length=16 + ), + ), + migrations.AlterField( + model_name="settingsbrowser", + name="order_by", + field=models.CharField( + choices=[ + ("created_at", "Added Time"), + ("age_rating", "Age Rating"), + ("alternate_number", "Alternate Number"), + ("reprints", "Alternate Series"), + ("characters", "Characters"), + ("child_count", "Child Count"), + ("community_rating", "Community Rating"), + ("country", "Country"), + ("credits", "Credits"), + ("day", "Day"), + ("favorite", "Favorite"), + ("filename", "Filename"), + ("size", "File Size"), + ("file_type", "File Type"), + ("original_format", "Format"), + ("genres", "Genres"), + ("identifiers", "Identifiers"), + ("imprint_name", "Imprint"), + ("issue", "Issue"), + ("language", "Language"), + ("bookmark_updated_at", "Last Read"), + ("locations", "Locations"), + ("main_character", "Main Character"), + ("main_team", "Main Team"), + ("metadata_mtime", "Tags Updated"), + ("month", "Month"), + ("monochrome", "Monochrome"), + ("sort_name", "Name"), + ("page_count", "Page Count"), + ("publisher_name", "Publisher"), + ("date", "Publish Date"), + ("reading_direction", "Reading Direction"), + ("scan_info", "Scan Info"), + ("search_score", "Search Score"), + ("series_name", "Series"), + ("series_groups", "Series Groups"), + ("stories", "Stories"), + ("story_arc_number", "Story Arc Number"), + ("story_arcs", "Story Arcs"), + ("tags", "Tags"), + ("tagger", "Tagger"), + ("teams", "Teams"), + ("universes", "Universes"), + ("updated_at", "Updated Time"), + ("volume_name", "Volume"), + ("year", "Year"), + ], + default="", + max_length=32, + ), + ), + migrations.RunPython( + _backfill_reprint_issue_parts, + migrations.RunPython.noop, + ), + ] diff --git a/codex/models/named.py b/codex/models/named.py index fc7c860c9..12bf58cf2 100644 --- a/codex/models/named.py +++ b/codex/models/named.py @@ -8,10 +8,21 @@ ForeignKey, ) -from codex.models.base import MAX_FIELD_LEN, MAX_NAME_LEN, BaseModel, NamedModel +from codex.models.base import ( + MAX_FIELD_LEN, + MAX_ISSUE_SUFFIX_LEN, + MAX_NAME_LEN, + BaseModel, + NamedModel, +) from codex.models.collections import BrowserCollectionModel, Volume -from codex.models.fields import CleaningCharField, CoercingPositiveSmallIntegerField +from codex.models.fields import ( + CleaningCharField, + CoercingDecimalField, + CoercingPositiveSmallIntegerField, +) from codex.models.identifier import Identifier +from codex.models.util import parse_issue_parts __all__ = ( "Character", @@ -129,6 +140,17 @@ class Reprint(BaseModel): issue = CleaningCharField(max_length=MAX_FIELD_LEN, default="") language = CleaningCharField(max_length=MAX_FIELD_LEN, default="") identifier = ForeignKey(Identifier, on_delete=SET_NULL, null=True) + # ``issue`` split into its sortable parts, mirroring + # ``Comic.issue_number`` / ``issue_suffix``. Without them the + # ``alternate_number`` sort would order "#10" before "#2". Derived + # in ``presave``, never imported directly; unindexed because they're + # only read after an indexed join on pk or series_name. + issue_number = CoercingDecimalField(decimal_places=2, max_digits=10, null=True) + issue_suffix = CleaningCharField( + max_length=MAX_ISSUE_SUFFIX_LEN, + default="", + db_collation="nocase", + ) class Meta(BaseModel.Meta): """Declare constraints and indexes.""" @@ -165,6 +187,25 @@ def name(self) -> str: self.series_name, self.volume_number, self.issue, self.language ) + @override + def presave(self) -> None: + """Split ``issue`` into its sortable number and suffix.""" + super().presave() + self.issue_number, self.issue_suffix = parse_issue_parts(self.issue) + + @override + def save(self, *args, **kwargs) -> None: + """ + Save computed fields. + + The importer's bulk create / update paths call ``presave`` + themselves, but direct ``save()`` callers (the tag editor, tests) + would otherwise persist a row whose sort columns don't match its + ``issue``. + """ + self.presave() + super().save(*args, **kwargs) + class ScanInfo(NamedModel): """Whomever scanned the comic.""" diff --git a/codex/models/util.py b/codex/models/util.py index 58aa4b8a7..2785b3c3a 100644 --- a/codex/models/util.py +++ b/codex/models/util.py @@ -1,5 +1,17 @@ """Utilities for models.""" +import re +from decimal import Decimal, InvalidOperation + +from comicbox.formats.base.fields.fields import IssueField + +# Splits a normalized issue string into its numeric head and the +# remaining suffix ("10a" -> "10" + "a"). Shared by every compound +# issue column (``Comic.issue_number``/``issue_suffix``, +# ``Reprint.issue_number``/``issue_suffix``) and by the search +# field parser so a typed query and a stored column agree. +_PARSE_ISSUE_MATCHER = re.compile(r"(?P\d*\.?\d*)(?P.*)") + # Multi-language leading-article set used by ``get_sort_name`` to # move a leading "the"/"el"/"der"/etc. to the end so titles sort by # the first significant word. Comments mark which language each @@ -30,6 +42,23 @@ ) # fmt: skip +def parse_issue_parts(value) -> tuple[Decimal | None, str]: + """Split a compound issue string into its number and suffix parts.""" + value = IssueField.parse_issue(value) + if not value: + return None, "" + matches = _PARSE_ISSUE_MATCHER.match(value) + if not matches: + return None, "" + try: + number = Decimal(matches.group("issue_number")) + except InvalidOperation: + # A suffix-only issue ("annual", "½") leaves the numeric group + # empty, which Decimal rejects. Keep the suffix; sort it as null. + number = None + return number, matches.group("issue_suffix") + + def get_sort_name(name: str) -> str: """Create sort_name from name.""" lower_name = name.lower() diff --git a/codex/serializers/browser/mtime.py b/codex/serializers/browser/mtime.py index 97dd3d946..e54147484 100644 --- a/codex/serializers/browser/mtime.py +++ b/codex/serializers/browser/mtime.py @@ -4,9 +4,16 @@ from codex.serializers.browser.settings import BrowserFilterChoicesInputSerializer from codex.serializers.fields import TimestampField +from codex.serializers.fields.collection import MtimeCollectionField from codex.serializers.route import SimpleRouteSerializer +class MtimeRouteSerializer(SimpleRouteSerializer): + """A route the mtime probe accepts, including reader-only arcs.""" + + collection = MtimeCollectionField() + + class CollectionsMtimeSerializer(BrowserFilterChoicesInputSerializer): """Collections Mtimes.""" @@ -14,7 +21,7 @@ class CollectionsMtimeSerializer(BrowserFilterChoicesInputSerializer): BrowserFilterChoicesInputSerializer.JSON_FIELDS | {"collections"} ) - collections = SimpleRouteSerializer(many=True, required=True) + collections = MtimeRouteSerializer(many=True, required=True) class MtimeSerializer(Serializer): diff --git a/codex/serializers/fields/collection.py b/codex/serializers/fields/collection.py index 8e7ddaccf..c66fbbc15 100644 --- a/codex/serializers/fields/collection.py +++ b/codex/serializers/fields/collection.py @@ -4,6 +4,7 @@ BROWSER_ROUTE_COLLECTION_CHOICES, BROWSER_TOP_COLLECTION_CHOICES, ) +from codex.collection import READER_REPRINT_COLLECTION from codex.serializers.fields.base import CodexChoiceField @@ -17,3 +18,19 @@ class BrowserRouteCollectionField(CodexChoiceField): """Valid Top Collections Only (+ root) — collection vocabulary.""" class_choices = tuple(BROWSER_ROUTE_COLLECTION_CHOICES.keys()) + + +class MtimeCollectionField(BrowserRouteCollectionField): + """ + Browse routes plus the reader's alternate-series pseudo-collection. + + The reader probes the mtime of every arc it offers, and one of those + is an alternate series, which has no browse route of its own. Kept + separate from :class:`BrowserRouteCollectionField` so a reader-only + value can't leak into an actual browse route. + """ + + class_choices = ( + *BROWSER_ROUTE_COLLECTION_CHOICES.keys(), + READER_REPRINT_COLLECTION, + ) diff --git a/codex/serializers/fields/reader.py b/codex/serializers/fields/reader.py index cfcb1d468..9b4ee07e8 100644 --- a/codex/serializers/fields/reader.py +++ b/codex/serializers/fields/reader.py @@ -1,17 +1,20 @@ """Reader Fields.""" -from codex.collection import Collection +from codex.collection import READER_REPRINT_COLLECTION, Collection from codex.models.choices import ReadingDirectionChoices from codex.models.settings import FitToChoices from codex.serializers.fields.base import CodexChoiceField -# Browse collections a comic can be read "within". All collection-valued now; +# Collections a comic can be read "within". Mostly browse collections; # p/i/root have no arc of their own (params collapses them to series). +# ``reprints`` is the one reader-only entry — an alternate series is a +# reading order without a browse route. VALID_ARC_COLLECTIONS = ( Collection.SERIES, Collection.VOLUME, Collection.FOLDER, Collection.ARC, + READER_REPRINT_COLLECTION, ) diff --git a/codex/views/browser/annotate/order.py b/codex/views/browser/annotate/order.py index a7076d40e..063ba16c4 100644 --- a/codex/views/browser/annotate/order.py +++ b/codex/views/browser/annotate/order.py @@ -12,7 +12,7 @@ ) from django.db.models.aggregates import Avg, Count, Max, Min, Sum from django.db.models.fields import CharField -from django.db.models.functions import Reverse, Right, StrIndex +from django.db.models.functions import Coalesce, Reverse, Right, StrIndex from codex.choices.browser import BROWSER_EXTRA_SORT_UNSUPPORTED_KEYS from codex.models import ( @@ -84,6 +84,7 @@ _ANNOTATED_ORDER_FIELDS = frozenset( # These are annotated with their own functions { + "alternate_number", "bookmark_updated_at", "child_count", "favorite", @@ -226,6 +227,46 @@ def _alias_story_arc_number(self, qs): return qs.alias(story_arc_number=story_arc_number) + def _alias_alternate_number(self, qs): + """Alias the alternate series issue number & suffix for ordering.""" + if self.order_key != "alternate_number": + return qs + + # Unlike ``story_arc_number`` there's no alternate series browse + # collection, so the ``reprints`` filter is the only thing that can + # say *which* alternate series' number to sort by. + reprint_pks = self.params.get("filters", {}).get("reprints", ()) + # ``self.rel_prefix`` is memoized off the *view's* model, but this + # runs for the book queryset too — take the prefix from the + # queryset being annotated, as ``_alias_story_arc_number`` does. + rel_prefix = self.get_rel_prefix(qs.model) + own_number = rel_prefix + "issue_number" + own_suffix = rel_prefix + "issue_suffix" + + if reprint_pks: + rel = rel_prefix + "reprints" + condition = Q(**{f"{rel}__pk__in": reprint_pks}) + qs = qs.alias(selected_reprint=FilteredRelation(rel, condition=condition)) + # Comics carrying no alternate number fall back to their own + # issue number so a mixed listing stays readable instead of + # collapsing every untagged comic to NULL. Coalescing *inside* + # the aggregate lets each comic contribute its own effective + # value; coalescing outside would compare one series' minimum + # alternate number against another's minimum issue number. + number = self.order_agg_func( + Coalesce("selected_reprint__issue_number", own_number) + ) + suffix = self.order_agg_func( + Coalesce("selected_reprint__issue_suffix", own_suffix) + ) + else: + # No alternate series selected: degrade to the plain issue sort + # rather than ordering everything by NULL. + number = self.order_agg_func(own_number) + suffix = self.order_agg_func(own_suffix) + + return qs.alias(alternate_number=number, alternate_number_suffix=suffix) + def _annotate_page_count(self, qs): """Hoist up total page_count of children.""" # Used for sorting and progress @@ -563,6 +604,7 @@ def annotate_order_aggregates(self, qs: QuerySet, *, for_cover: bool = False): qs = self._alias_sort_names(qs) qs = self._alias_filename(qs) qs = self._alias_story_arc_number(qs) + qs = self._alias_alternate_number(qs) if not for_cover: qs = self._annotate_page_count(qs) qs = self._annotate_bookmark_updated_at(qs) diff --git a/codex/views/browser/browser.py b/codex/views/browser/browser.py index 5f8fbdc8a..3a0d5f57c 100644 --- a/codex/views/browser/browser.py +++ b/codex/views/browser/browser.py @@ -30,6 +30,8 @@ favorite_annotation_for, fk_name_annotations_for, m2m_annotations_for, + m2m_columns, + m2m_sort_annotations_for, ) from codex.views.browser.intersections import compute_collection_intersections from codex.views.browser.title import BrowserTitleView @@ -158,28 +160,48 @@ def _add_table_view_favorite_annotation(self, qs): qs = qs.annotate(**annotations) return qs + def _sort_annotation_keys(self) -> tuple[str, ...]: + """ + Return the keys whose ORDER BY aliases this queryset must carry. + + Table view annotates the primary ``order_by`` key *and* every + entry in ``order_extra_keys`` so multi-column sort can reference + the same M2M / FK-name aliases. Every other view mode (cover + cards, OPDS feeds) can't add extras, but its *primary* key may + still be an M2M column — ``_comic_order_fields_head`` emits the + M2M alias for those, so without this the ORDER BY would name a + column that was never annotated. + """ + if self.params.get("view_mode") == "table": + return self._table_view_sort_keys() + order_key = self.order_key + return (order_key,) if order_key in m2m_columns() else () + def _add_table_view_sort_annotations(self, qs): """ - Add only the table-view annotations needed for ORDER BY. - - Covers the primary ``order_by`` key *and* every entry in - ``order_extra_keys`` so multi-column sort can reference the - same M2M / FK-name aliases. Display annotations are added - post-pagination so the M2M aggregate runs only over the - visible page; ORDER BY runs over the full queryset and - needs every alias upstream. + Add only the annotations needed for ORDER BY. + + Display annotations are added post-pagination so the M2M + aggregate runs only over the visible page; ORDER BY runs over + the full queryset and needs every alias upstream. """ - if self.params.get("view_mode") != "table" or qs.model is not Comic: + if qs.model is not Comic: return qs - sort_keys = self._table_view_sort_keys() + sort_keys = self._sort_annotation_keys() if not sort_keys: return qs fk_anns = fk_name_annotations_for(sort_keys) m2m_anns = m2m_annotations_for(sort_keys) + # Every key that sorts through a fallback alias needs it annotated, + # extras included — ``_comic_extra_fields`` resolves an extra to the + # same alias the primary uses. + m2m_sort_anns = m2m_sort_annotations_for(sort_keys) if fk_anns: qs = qs.annotate(**fk_anns) if m2m_anns: qs = qs.annotate(**m2m_anns) + if m2m_sort_anns: + qs = qs.annotate(**m2m_sort_anns) return qs def _add_table_view_display_annotations(self, qs): diff --git a/codex/views/browser/columns.py b/codex/views/browser/columns.py index a37a705ec..d5a9a80de 100644 --- a/codex/views/browser/columns.py +++ b/codex/views/browser/columns.py @@ -18,8 +18,9 @@ Value, When, ) +from django.db.models.aggregates import Min from django.db.models.fields import CharField -from django.db.models.functions import Cast, Concat +from django.db.models.functions import Cast, Coalesce, Concat from codex.choices.browser import ( BROWSER_TABLE_COLUMNS, @@ -278,6 +279,52 @@ def m2m_columns() -> frozenset[str]: return frozenset(_M2M_COLUMN_PATHS.keys()) +# M2M sort keys that have a meaningful scalar counterpart on Comic to +# fall back to. Sorting by the JSON aggregate alone parks every comic +# lacking the relation in one undifferentiated clump; ``reprints`` is +# an alternate *series* name, so a comic without one sorts by its real +# series instead and the listing stays readable. Other M2M columns +# (genres, tags, …) have no such counterpart and keep the plain +# aggregate sort. +_M2M_SORT_FALLBACK_PATHS = MappingProxyType({"reprints": "series__sort_name"}) +_M2M_SORT_ANNOTATION_PREFIX = "_table_m2m_sort_" + + +def m2m_sort_alias_for(column_key: str) -> str: + """Return the ORDER BY alias for an M2M column with a scalar fallback.""" + return _M2M_SORT_ANNOTATION_PREFIX + column_key + + +def m2m_sort_columns() -> frozenset[str]: + """Return M2M column keys that sort through a fallback alias.""" + return frozenset(_M2M_SORT_FALLBACK_PATHS.keys()) + + +def m2m_sort_annotations_for(columns: tuple[str, ...]) -> dict[str, Coalesce]: + """ + Build ``alias -> Coalesce(Min(expr), fallback)`` sort annotations. + + Separate from the display aggregate: the cell still renders the + full JSON list, while ORDER BY uses the first label so a comic + with no relation can fall through to its scalar counterpart. + """ + annotations: dict[str, Coalesce] = {} + for col in columns: + fallback = _M2M_SORT_FALLBACK_PATHS.get(col) + path = _M2M_COLUMN_PATHS.get(col) + if fallback is None or path is None: + continue + agg_filter = _M2M_AGGREGATE_FILTERS.get(col) + kwargs: dict = {"filter": agg_filter} if agg_filter is not None else {} + # The composed label and the fallback column are both text but + # different field classes (``CharField`` vs ``CleaningCharField``), + # which Django refuses to unify on its own. + annotations[m2m_sort_alias_for(col)] = Coalesce( + Min(path, **kwargs), F(fallback), output_field=CharField() + ) + return annotations + + # FK-name annotations live in their own alias namespace so they don't # collide with the matching Comic FK field attributes (``country`` / # ``language`` / etc.). diff --git a/codex/views/browser/filters/search/field/expression.py b/codex/views/browser/filters/search/field/expression.py index bbbf6cf23..8320a37eb 100644 --- a/codex/views/browser/filters/search/field/expression.py +++ b/codex/views/browser/filters/search/field/expression.py @@ -6,7 +6,6 @@ from types import MappingProxyType from typing import Any -from comicbox.formats.base.fields.fields import IssueField from dateparser import parse from django.db.backends.base.operations import BaseDatabaseOperations from django.db.models import ( @@ -18,12 +17,12 @@ ) from django.db.models.fields import DecimalField, PositiveSmallIntegerField +from codex.models.util import parse_issue_parts from codex.settings import FALSY _QUOTES_RE = re.compile(r"[\"']") _OP_MAP = MappingProxyType({">": "gt", ">=": "gte", "<": "lt", "<=": "lte"}) _RANGE_RE = re.compile(r"\.{2,}") -_PARSE_ISSUE_MATCHER = re.compile(r"(?P\d*\.?\d*)(?P.*)") _LIKE_QUERY_VALUE = re.compile(r"\S\*+\S") _ICONTAINS_QUERY_VALUE = re.compile(r"^(\*.*\*|[^*].*[^*]|^\**$)$") _IENDSWITH_QEURY_VALUE = re.compile(r"^\*") @@ -43,14 +42,11 @@ def parse_size(s: str) -> int: def _parse_issue_value(value) -> tuple | tuple[None, None]: """Parse a compound issue value into number & suffix.""" - value = IssueField.parse_issue(value) - if not value: + numeric_value, suffix_value = parse_issue_parts(value) + if numeric_value is None and not suffix_value: + # The filter distinguishes "no suffix term" (None, skip the + # lookup) from the empty-suffix column value the models store. return None, None - matches = _PARSE_ISSUE_MATCHER.match(value) - if not matches: - return None, None - numeric_value = Decimal(matches.group("issue_number")) - suffix_value = matches.group("issue_suffix") return numeric_value, suffix_value diff --git a/codex/views/browser/mtime.py b/codex/views/browser/mtime.py index facc0fb18..053ee0cb2 100644 --- a/codex/views/browser/mtime.py +++ b/codex/views/browser/mtime.py @@ -1,14 +1,24 @@ """Get the mtimes for the submitted collections.""" +from cachalot.api import cachalot_disabled +from django.db.models.aggregates import Max +from django.db.models.functions import Greatest +from django.db.utils import OperationalError from drf_spectacular.utils import extend_schema from rest_framework.response import Response from rest_framework.serializers import BaseSerializer +from codex.collection import READER_REPRINT_COLLECTION from codex.models.collections import Publisher +from codex.models.named import Reprint from codex.serializers.browser.mtime import CollectionsMtimeSerializer, MtimeSerializer from codex.util import max_none from codex.views.browser.collection_mtime import BrowserCollectionMtimeView -from codex.views.const import COLLECTION_MODEL_MAP +from codex.views.const import ( + COLLECTION_MODEL_MAP, + EPOCH_START, + EPOCH_START_DATETIMEFIELD, +) class MtimeView(BrowserCollectionMtimeView): @@ -21,11 +31,44 @@ class MtimeView(BrowserCollectionMtimeView): TARGET: str = "mtime" + def _get_reprint_mtime(self, pks): + """ + Get the mtime of an alternate series the reader is reading. + + ``Reprint`` isn't a browse collection, so it has no filtered + queryset to aggregate — read its rows directly. + ``TimestampUpdater`` keeps ``updated_at`` current when a member + comic changes, which is what makes this probe meaningful. + """ + if not pks: + return None + qs = Reprint.objects.filter(pk__in=pks) + agg_terms = [ + Max("updated_at", default=EPOCH_START_DATETIMEFIELD), + self.get_max_bookmark_updated_at_aggregate( + Reprint, default=EPOCH_START_DATETIMEFIELD + ), + ] + try: + with cachalot_disabled(): + mtime = qs.aggregate(max=Greatest(*agg_terms))["max"] + if mtime == NotImplemented: + mtime = None + elif not mtime: + mtime = EPOCH_START + except OperationalError as exc: + self._handle_operational_error(exc) + mtime = None + return mtime + def _get_collection_mtime(self, item): """Get one collection's mtimes.""" collection = item["collection"] pks = item["pks"] + if collection == READER_REPRINT_COLLECTION: + return self._get_reprint_mtime(pks) + model = COLLECTION_MODEL_MAP[collection] if not model: model = Publisher diff --git a/codex/views/browser/order_by.py b/codex/views/browser/order_by.py index 7ad3c124b..ea3259ef8 100644 --- a/codex/views/browser/order_by.py +++ b/codex/views/browser/order_by.py @@ -6,7 +6,12 @@ from codex.models import Comic from codex.models.collections import Volume from codex.views.browser.collection_mtime import BrowserCollectionMtimeView -from codex.views.browser.columns import m2m_alias_for, m2m_columns +from codex.views.browser.columns import ( + m2m_alias_for, + m2m_columns, + m2m_sort_alias_for, + m2m_sort_columns, +) # Order keys that don't map directly to a Comic field name need an # explicit ORM path. The map is consumed both by ``_add_comic_order_by`` @@ -124,6 +129,19 @@ def _comic_order_fields_head(self, order_key: str, comic_sort_names) -> list: # natural multi-field sort that matches how the compound # ``Issue`` table column is rendered. return ["issue_number", "issue_suffix"] + if order_key == "alternate_number": + # The same compound expansion over the alternate series' + # number, whose parts are annotated aliases rather than + # columns (see ``_alias_alternate_number``). ``date`` breaks + # ties between comics sharing an alternate number, matching + # the ``story_arc_number`` tail. + return ["alternate_number", "alternate_number_suffix", "date"] + if order_key in m2m_sort_columns(): + # M2M sort with a scalar counterpart (``reprints`` → + # ``series__sort_name``): order on the fallback alias so + # comics carrying no alternate series interleave by their + # real series instead of clumping under an empty list. + return [m2m_sort_alias_for(order_key)] if order_key in m2m_columns(): # M2M sort: ``ORDER BY `` where the alias is the # JsonGroupArray annotation added by the table-view path. @@ -211,6 +229,12 @@ def add_order_by( # ``_age_rating_sort_value`` is the metron index (sort). # See ``BrowserAnnotateOrderView.annotate_order_value``. order_fields_head = ["_age_rating_sort_value"] + elif self.order_key == "alternate_number": + # ``order_value`` carries the aggregated alternate number + # (and the card caption renders it); the parallel suffix + # alias is the secondary, mirroring the Comic-row compound + # expansion in ``_comic_order_fields_head``. + order_fields_head = ["order_value", "alternate_number_suffix"] else: order_fields_head = ["order_value"] diff --git a/codex/views/const.py b/codex/views/const.py index 55bfbe901..093a90ff3 100644 --- a/codex/views/const.py +++ b/codex/views/const.py @@ -9,7 +9,7 @@ from django.db.models.expressions import Value from django.db.models.fields import DateTimeField, PositiveSmallIntegerField -from codex.collection import Collection +from codex.collection import READER_REPRINT_COLLECTION, Collection from codex.models import ( AgeRating, Bookmark, @@ -82,6 +82,10 @@ COMIC_COLLECTION: "pk", FOLDER_COLLECTION: "parent_folder", STORY_ARC_COLLECTION: "story_arc_numbers__story_arc", + # Reader-only: alternate series are a reading order, not a browse + # collection. Browse callers only ever index this map with a + # URL-regex-validated collection, so the extra key is inert there. + READER_REPRINT_COLLECTION: "reprints", } ) FILTER_ONLY_COLLECTION_RELATION: MappingProxyType[str, str] = MappingProxyType( diff --git a/codex/views/reader/arcs.py b/codex/views/reader/arcs.py index c80f707a0..6b9c616c2 100644 --- a/codex/views/reader/arcs.py +++ b/codex/views/reader/arcs.py @@ -4,14 +4,14 @@ from types import MappingProxyType from typing import TYPE_CHECKING -from django.db.models import Max +from django.db.models import Max, Q from codex.choices.admin import AdminFlagChoices -from codex.collection import Collection +from codex.collection import READER_REPRINT_COLLECTION, Collection from codex.models import AdminFlag from codex.models.comic import Comic from codex.models.functions import JsonGroupArray -from codex.models.named import StoryArc +from codex.models.named import Reprint, StoryArc from codex.util import max_none from codex.views.const import ( STORY_ARC_COLLECTION, @@ -32,6 +32,24 @@ ) _COMIC_ARC_FIELD_NAMES = tuple(_COMIC_ARC_FIELD_COLLECTIONS) +# Arc collections whose rows are groups rather than a single row, so the +# requested ids may be a stale subset of the current group. Both story +# arcs (grouped by sort_name) and alternate series (grouped by identity) +# accept an intersecting id set as the same arc. +_MULTI_ROW_ARC_COLLECTIONS = frozenset( + {STORY_ARC_COLLECTION, READER_REPRINT_COLLECTION} +) + +# Preference order when the requested arc collection has no arc for this +# comic. Series first: it's the reader's default reading order. +_ARC_COLLECTION_FALLBACK_ORDER = ( + Collection.SERIES, + Collection.VOLUME, + Collection.FOLDER, + STORY_ARC_COLLECTION, + READER_REPRINT_COLLECTION, +) + class ReaderArcsView(ReaderParamsView): """Reader get Arcs methods.""" @@ -116,16 +134,75 @@ def _get_story_arcs(self, comic: Comic, arcs, max_mtime: int | None): max_mtime = max_none(max_mtime, mtime) return max_mtime + def _get_reprint_arcs(self, comic: Comic, arcs, max_mtime: int | None): + """Append the alternate series (ComicInfo AlternateSeries) arcs.""" + # An alternate series is identified by everything but the issue — + # that's ``Reprint``'s unique key minus ``issue``. Splitting on + # volume and language keeps a v1 and a v2, or an English and a + # Spanish edition, from merging into one reading order. + identities = tuple( + Reprint.objects.filter(comic__pk=comic.pk) + .values_list("series_name", "volume_number", "language") + .distinct() + ) + if not identities: + return max_mtime + + identity_filter = Q() + for series_name, volume_number, language in identities: + identity_filter |= Q( + series_name=series_name, + volume_number=volume_number, + language=language, + ) + + # Every issue of an alternate series is its own ``Reprint`` row, so + # the arc handle has to be the whole group's pks, not just this + # comic's. Keying on one comic's row would make the *next* book + # report a different id set and silently drop the reading order. + qs = Reprint.objects.filter(identity_filter) + qs = qs.group_by("series_name", "volume_number", "language") # pyright: ignore[reportAttributeAccessIssue] + qs = qs.annotate( + ids=JsonGroupArray("id", distinct=True, order_by="id"), + mtime=Max("updated_at"), + ) + qs = qs.order_by("series_name", "volume_number", "language") + + arcs[READER_REPRINT_COLLECTION] = {} + for reprint in qs: + ids = tuple(sorted(set(reprint.ids))) + mtime = reprint.mtime + name = Reprint.compose_name( + reprint.series_name, reprint.volume_number, "", reprint.language + ) + arcs[READER_REPRINT_COLLECTION][ids] = {"name": name, "mtime": mtime} + max_mtime = max_none(max_mtime, mtime) + return max_mtime + + @staticmethod + def _fallback_arc_collection(arcs) -> str: + """Pick a collection this comic actually has an arc for.""" + # The requested collection can be valid yet absent for this comic + # (an alternate series the comic isn't in, a story arc it lost on + # re-tag). Reading must still work, so fall back to the most + # series-like arc available instead of raising. + for collection in _ARC_COLLECTION_FALLBACK_ORDER: + if arcs.get(collection): + return collection + return next(iter(arcs), "") + def _set_selected_arc(self, arcs) -> None: arc = self.params["arc"] arc_collection = arc["collection"] requested_arc_ids = arc.get("ids", ()) + if not arcs.get(arc_collection): + arc_collection = self._fallback_arc_collection(arcs) arc_id_infos = arcs.get(arc_collection) all_arc_ids: frozenset[tuple[int, ...]] = ( frozenset(arc_id_infos.keys()) if arc_id_infos else frozenset() ) arc_ids = () - if arc_collection == STORY_ARC_COLLECTION: + if arc_collection in _MULTI_ROW_ARC_COLLECTIONS: if requested_arc_ids in all_arc_ids: arc_ids = requested_arc_ids else: @@ -136,7 +213,7 @@ def _set_selected_arc(self, arcs) -> None: if requested_set.intersection(candidate): arc_ids = candidate break - if not arc_ids: + if not arc_ids and all_arc_ids: arc_ids = next(iter(all_arc_ids)) self._selected_arc_collection = arc_collection self._selected_arc_ids = arc_ids @@ -156,5 +233,6 @@ def get_arcs(self) -> tuple[dict, int | None]: for field_name in field_names: max_mtime = self._get_collection_arc(comic, field_name, arcs, max_mtime) max_mtime = self._get_story_arcs(comic, arcs, max_mtime) + max_mtime = self._get_reprint_arcs(comic, arcs, max_mtime) self._set_selected_arc(arcs) return arcs, max_mtime diff --git a/codex/views/reader/books.py b/codex/views/reader/books.py index 2c544f913..bd9dc9128 100644 --- a/codex/views/reader/books.py +++ b/codex/views/reader/books.py @@ -7,7 +7,7 @@ from django.urls import reverse from rest_framework.exceptions import NotFound -from codex.collection import Collection +from codex.collection import READER_REPRINT_COLLECTION, Collection from codex.models import Comic from codex.models.bookmark import Bookmark from codex.models.settings import SettingsReader @@ -127,6 +127,7 @@ def _get_comics_list(self) -> QuerySet: fields = _COMIC_FIELDS arc_pk_rel = rel + "__pk" arc_index = NONE_INTEGERFIELD + arc_index_suffix = None select_related = () prefetch_related = () ordering = () @@ -135,6 +136,14 @@ def _get_comics_list(self) -> QuerySet: arc_index = F("story_arc_numbers__number") prefetch_related = (*prefetch_related, rel) ordering = ("arc_index", "date", "pk") + elif self._selected_arc_collection == READER_REPRINT_COLLECTION: + # ComicInfo AlternateNumber, split into its sortable parts by + # ``Reprint.presave`` so "#2" reads before "#10". The nav + # filter already joins ``reprints``, as it does for story arcs. + arc_index = F("reprints__issue_number") + arc_index_suffix = F("reprints__issue_suffix") + prefetch_related = (*prefetch_related, rel) + ordering = ("arc_index", "arc_index_suffix", "date", "pk") elif self._selected_arc_collection == FOLDER_COLLECTION: fields = (*_COMIC_FIELDS, rel) select_related = (rel,) @@ -163,6 +172,10 @@ def _get_comics_list(self) -> QuerySet: output_field=BooleanField(), ), ) + if arc_index_suffix is not None: + # Alias, not annotate: only ORDER BY reads it, so it never + # needs to ride along in the serialized row. + qs = qs.alias(arc_index_suffix=arc_index_suffix) sort_names_alias, ordering = self._get_comics_annotation_and_ordering( qs.model, ordering ) diff --git a/frontend/src/components/browser/card/order-by-caption.vue b/frontend/src/components/browser/card/order-by-caption.vue index f1fcbad77..ed32fcdfa 100644 --- a/frontend/src/components/browser/card/order-by-caption.vue +++ b/frontend/src/components/browser/card/order-by-caption.vue @@ -74,6 +74,10 @@ export default { return prettyBytes(Number.parseInt(ov, 10)); } else if (STAR_SORT_BY.has(this.orderBy)) { return `★ ${this.formatStarRating(ov)}`; + } else if (this.orderBy === "alternate_number") { + return this.formatAlternateNumber(ov); + } else if (this.orderBy === "reprints") { + return this.formatReprints(ov); } } catch (error) { // Often orderBy gets updated before orderValue gets returned. @@ -102,6 +106,25 @@ export default { if (!Number.isFinite(n)) return ov; return n.toFixed(2).replace(/\.?0+$/, ""); }, + /* + * The alternate issue number is a DecimalField aggregate, so it + * arrives as "2.00". Show "#2" the way the issue column does. + */ + formatAlternateNumber(ov) { + const n = Number.parseFloat(ov); + if (!Number.isFinite(n)) return ov; + return `#${n.toFixed(2).replace(/\.?0+$/, "")}`; + }, + /* + * The alternate series order_value is the JSON array the table cell + * renders. Collection rows sort by a fallback the caption can't + * show, so only comics get a caption. + */ + formatReprints(ov) { + if (this.item.collection !== "comics") return ""; + const labels = JSON.parse(ov); + return Array.isArray(labels) ? labels.join(", ") : ov; + }, }, }; diff --git a/frontend/src/components/reader/toolbars/top/reader-arc-select.vue b/frontend/src/components/reader/toolbars/top/reader-arc-select.vue index f4276e486..1088d6bf5 100644 --- a/frontend/src/components/reader/toolbars/top/reader-arc-select.vue +++ b/frontend/src/components/reader/toolbars/top/reader-arc-select.vue @@ -42,6 +42,7 @@ import { mdiBookshelf, mdiCheck, mdiChessRook, + mdiContentDuplicate, mdiFeather, mdiFilterOutline, mdiFolderOutline, @@ -58,9 +59,19 @@ const ARC_ICONS = { folders: mdiFolderOutline, publishers: mdiChessRook, imprints: mdiFeather, + reprints: mdiContentDuplicate, series: mdiBookshelf, volumes: mdiBookMultiple, }; +/* + * Subtitles otherwise come from the browse TOP_COLLECTION labels, + * singularized. ``reprints`` is a reader-only reading order with no + * browse collection, so it has no label there to singularize — and + * slicing ``undefined`` used to throw. + */ +const ARC_SUBTITLES = { + reprints: "Alternate Series", +}; export default { name: "ReaderArcSelect", @@ -84,9 +95,12 @@ export default { } for (const [collection, arcIdsInfo] of Object.entries(this.arcs)) { for (const [ids, arcInfo] of Object.entries(arcIdsInfo)) { - let subtitle = Reflect.get(TOP_COLLECTION, collection); - if (collection !== "series") { - subtitle = subtitle.slice(0, -1); + let subtitle = Reflect.get(ARC_SUBTITLES, collection); + if (!subtitle) { + subtitle = Reflect.get(TOP_COLLECTION, collection); + if (collection !== "series") { + subtitle = subtitle.slice(0, -1); + } } const prependIcon = Reflect.get(ARC_ICONS, collection); const appendIcon = diff --git a/frontend/tests/unit/order-by-caption.test.js b/frontend/tests/unit/order-by-caption.test.js new file mode 100644 index 000000000..a46d47c7e --- /dev/null +++ b/frontend/tests/unit/order-by-caption.test.js @@ -0,0 +1,60 @@ +/* + * Tests for the browser card's order-by caption. + * + * Behavior locked in here: + * - "alternate_number" is a DecimalField aggregate ("2.00") and renders + * as "#2", not "#2.00". + * - "reprints" order_value is the JSON array the table cell renders; + * comic cards join the labels, collection cards show nothing because + * they sort by a fallback the caption can't represent. + */ +import { createTestingPinia } from "@pinia/testing"; +import { mount } from "@vue/test-utils"; +import { describe, expect, test } from "vitest"; + +import OrderByCaption from "@/components/browser/card/order-by-caption.vue"; +import vuetify from "@/plugins/vuetify"; + +function mountCaption(orderBy, item) { + const pinia = createTestingPinia({ + initialState: { browser: { settings: { orderBy } } }, + }); + return mount(OrderByCaption, { + props: { item }, + global: { plugins: [pinia, vuetify] }, + }); +} + +describe("order by caption", () => { + test("alternate number trims the decimal aggregate", () => { + const wrapper = mountCaption("alternate_number", { + orderValue: "2.00", + collection: "comics", + }); + expect(wrapper.text()).toBe("#2"); + }); + + test("alternate number keeps a real fraction", () => { + const wrapper = mountCaption("alternate_number", { + orderValue: "1.50", + collection: "comics", + }); + expect(wrapper.text()).toBe("#1.5"); + }); + + test("alternate series joins the label list on a comic card", () => { + const wrapper = mountCaption("reprints", { + orderValue: JSON.stringify(["Crossover v2", "Otra Serie (es)"]), + collection: "comics", + }); + expect(wrapper.text()).toBe("Crossover v2, Otra Serie (es)"); + }); + + test("alternate series shows nothing on a collection card", () => { + const wrapper = mountCaption("reprints", { + orderValue: JSON.stringify(["Crossover"]), + collection: "series", + }); + expect(wrapper.text()).toBe(""); + }); +}); diff --git a/frontend/tests/unit/reader-arc-select.test.js b/frontend/tests/unit/reader-arc-select.test.js new file mode 100644 index 000000000..634792cb9 --- /dev/null +++ b/frontend/tests/unit/reader-arc-select.test.js @@ -0,0 +1,53 @@ +/* + * Tests for the reader's reading-order picker. + * + * Behavior locked in here: + * - An alternate-series arc ("reprints") renders with its own subtitle. + * Subtitles otherwise come from the browse TOP_COLLECTION labels, which + * have no "reprints" key — singularizing that undefined used to throw. + * - Browse-collection arcs keep their singularized subtitles. + */ +import { createTestingPinia } from "@pinia/testing"; +import { mount } from "@vue/test-utils"; +import { describe, expect, test } from "vitest"; + +import ReaderArcSelect from "@/components/reader/toolbars/top/reader-arc-select.vue"; +import vuetify from "@/plugins/vuetify"; + +function mountArcSelect(arcs, arc = { collection: "series", ids: "1" }) { + const pinia = createTestingPinia({ + initialState: { reader: { arcs, arc } }, + }); + return mount(ReaderArcSelect, { + global: { plugins: [pinia, vuetify] }, + }); +} + +describe("reader arc select", () => { + test("an alternate series arc gets its own subtitle and icon", () => { + const wrapper = mountArcSelect({ + series: { 1: { name: "Ser" } }, + reprints: { "2,3": { name: "Crossover" } }, + }); + const items = wrapper.vm.items; + const reprint = items.find((item) => item.collection === "reprints"); + expect(reprint).toBeTruthy(); + expect(reprint.subtitle).toBe("Alternate Series"); + expect(reprint.title).toBe("Crossover"); + expect(reprint.prependIcon).toBeTruthy(); + }); + + test("browse collections keep their singularized subtitles", () => { + const wrapper = mountArcSelect({ + series: { 1: { name: "Ser" } }, + arcs: { 5: { name: "The Big One" } }, + reprints: { "2,3": { name: "Crossover" } }, + }); + const byCollection = Object.fromEntries( + wrapper.vm.items.map((item) => [item.collection, item.subtitle]), + ); + expect(byCollection.series).toBe("Series"); + expect(byCollection.arcs).toBe("Story Arc"); + expect(byCollection.reprints).toBe("Alternate Series"); + }); +}); diff --git a/tests/test_browser_ordering.py b/tests/test_browser_ordering.py index cadc6094e..cd29bd16f 100644 --- a/tests/test_browser_ordering.py +++ b/tests/test_browser_ordering.py @@ -38,6 +38,7 @@ ) _NEW_ORDER_BY_KEYS: Final = ( + "alternate_number", "country", "day", "file_type", diff --git a/tests/test_browser_reprints_column.py b/tests/test_browser_reprints_column.py index 62875e296..e2d3b014f 100644 --- a/tests/test_browser_reprints_column.py +++ b/tests/test_browser_reprints_column.py @@ -228,3 +228,178 @@ def test_filter_narrows_to_tagged_comics(self) -> None: body = self._browse(f"/api/v4/browse/series/{self.series.pk}?page=1") names = [book["name"] for book in body["books"]] assert names == ["C1"], body + + +class BrowserAlternateNumberSortTestCase(_ReprintsFixtureTestCase): + """Sorting by the alternate series' issue number (ComicInfo AlternateNumber).""" + + def _tag(self, comic: Comic, issue: str, series_name: str = "Crossover") -> Reprint: + """Put ``comic`` in an alternate series at ``issue``.""" + reprint = Reprint.objects.create(series_name=series_name, issue=issue) + comic.reprints.add(reprint) + return reprint + + def _set_settings(self, **settings) -> None: + response = self.client.patch( + _SETTINGS_URL, + data=json.dumps(settings), + content_type="application/json", + ) + assert response.status_code == _HTTP_OK, response.content + cache.clear() + + def _book_names(self) -> list[str]: + body = self._browse(f"/api/v4/browse/series/{self.series.pk}?page=1") + return [book["name"] for book in body["books"]] + + def test_sorts_numerically_not_lexically(self) -> None: + """#2 sorts before #10 — the whole point of the derived columns.""" + # ``self.comic`` is C1. Issue numbers are deliberately the + # reverse of the alternate numbers so a fallback to the regular + # issue sort can't accidentally produce the expected order. + self._tag(self.comic, "2") + reprints = [self._tag(self._create_comic("C2", 2), "10")] + reprints.append(self._tag(self._create_comic("C3", 3), "3")) + reprints.append(Reprint.objects.get(issue="2")) + + self._set_settings( + orderBy="alternate_number", + orderReverse=False, + filters={"reprints": [reprint.pk for reprint in reprints]}, + ) + assert self._book_names() == ["C1", "C3", "C2"] + + def test_reverse_sort(self) -> None: + """Reversing the alternate number sort reverses the books.""" + first = self._tag(self.comic, "2") + second = self._tag(self._create_comic("C2", 2), "10") + + self._set_settings( + orderBy="alternate_number", + orderReverse=True, + filters={"reprints": [first.pk, second.pk]}, + ) + assert self._book_names() == ["C2", "C1"] + + def test_suffix_breaks_ties(self) -> None: + """Alternate numbers sharing a number order by their suffix.""" + plain = self._tag(self.comic, "2") + suffixed = self._tag(self._create_comic("C2", 2), "2a") + + self._set_settings( + orderBy="alternate_number", + orderReverse=False, + filters={"reprints": [plain.pk, suffixed.pk]}, + ) + assert self._book_names() == ["C1", "C2"] + + def test_untagged_comic_falls_back_to_its_issue_number(self) -> None: + """A comic with no alternate number sorts by its own issue number.""" + # C1 carries alternate number 2; C2 has no alternate series and + # issue #50. The fallback sorts C2 by 50, i.e. last. Without it + # C2's key would be NULL, which SQLite sorts *first* ascending — + # so the expected order only holds if the fallback is applied. + tagged = self._tag(self.comic, "2") + self._create_comic("C2", 50) + + self._set_settings( + orderBy="alternate_number", + orderReverse=False, + filters={"reprints": [tagged.pk, VUETIFY_NULL_CODE]}, + ) + assert self._book_names() == ["C1", "C2"] + + def test_without_filter_degrades_to_issue_sort(self) -> None: + """With no alternate series selected the sort is the plain issue sort.""" + self._tag(self.comic, "10") + self._create_comic("C2", 2) + self._create_comic("C3", 3) + + self._set_settings(orderBy="alternate_number", orderReverse=False) + assert self._book_names() == ["C1", "C2", "C3"] + + def test_collection_rows_sort_by_child_alternate_number(self) -> None: + """Series rows aggregate their children's alternate numbers.""" + self._tag(self.comic, "10") + other_series = self._create_series("Aaa") + early = self._tag(self._create_comic("C2", 2, series=other_series), "3") + + self._set_settings( + orderBy="alternate_number", + orderReverse=False, + filters={"reprints": [early.pk, Reprint.objects.get(issue="10").pk]}, + ) + body = self._browse(f"/api/v4/browse/publishers/{self.publisher.pk}?page=1") + names = [collection["name"] for collection in body["collections"]] + assert names == ["Aaa", "Ser"], body + + +class BrowserReprintsCoverSortTestCase(_ReprintsFixtureTestCase): + """The Alternate Series sort outside table view (cover cards, OPDS).""" + + def test_cover_view_sorts_comics_by_label(self) -> None: + """Cover view can sort by the M2M label without a missing-alias error.""" + # The ORDER BY alias for an M2M primary sort used to be annotated + # only in table view, so this request raised a FieldError. + self.comic.reprints.add(Reprint.objects.create(series_name="Zulu")) + sibling = self._create_comic("C2", 2) + sibling.reprints.add(Reprint.objects.create(series_name="Alpha")) + + response = self.client.patch( + _SETTINGS_URL, + data=json.dumps( + {"orderBy": "reprints", "orderReverse": False, "viewMode": "cover"} + ), + content_type="application/json", + ) + assert response.status_code == _HTTP_OK, response.content + cache.clear() + body = self._browse(f"/api/v4/browse/series/{self.series.pk}?page=1") + assert [book["name"] for book in body["books"]] == ["C2", "C1"], body + + def test_untagged_comic_falls_back_to_series_name(self) -> None: + """A comic with no alternate series sorts by its real series name.""" + # All three live in series "Ser" (sort_name "ser"). C1 and C3 + # carry alternate series that bracket it alphabetically, so the + # untagged C2 must land *between* them. Without the fallback its + # key would be the empty aggregate and it would clump at one end. + self.comic.reprints.add(Reprint.objects.create(series_name="zzz")) + self._create_comic("C2", 2) + self._create_comic("C3", 3).reprints.add( + Reprint.objects.create(series_name="aaa") + ) + + response = self.client.patch( + _SETTINGS_URL, + data=json.dumps( + {"orderBy": "reprints", "orderReverse": False, "viewMode": "cover"} + ), + content_type="application/json", + ) + assert response.status_code == _HTTP_OK, response.content + cache.clear() + body = self._browse(f"/api/v4/browse/series/{self.series.pk}?page=1") + assert [book["name"] for book in body["books"]] == ["C3", "C2", "C1"], body + + def test_alternate_series_works_as_a_multi_sort_extra(self) -> None: + """A secondary sort on the column resolves its ORDER BY alias.""" + # ``reprints`` sorts through a fallback alias, which has to be + # annotated for extras too, not just the primary key. + self.comic.reprints.add(Reprint.objects.create(series_name="zzz")) + self._create_comic("C2", 2) + + self._set_view_mode_table() + response = self.client.patch( + _SETTINGS_URL, + data=json.dumps( + { + "orderBy": "sort_name", + "orderExtraKeys": [{"key": "reprints", "reverse": False}], + } + ), + content_type="application/json", + ) + assert response.status_code == _HTTP_OK, response.content + cache.clear() + rows = self._browse_comics()["rows"] + assert {row["name"] for row in rows} == {"C1", "C2"}, rows diff --git a/tests/test_reader.py b/tests/test_reader.py index e770fc1c4..a252ba7af 100644 --- a/tests/test_reader.py +++ b/tests/test_reader.py @@ -13,21 +13,28 @@ import json import shutil +from datetime import timedelta from pathlib import Path from typing import Final, override +from unittest.mock import Mock from django.contrib.auth.models import User from django.core.cache import cache +from django.db.models.functions.datetime import Now from django.test import Client, TestCase +from codex.librarian.scribe.timestamp_update import TimestampUpdater from codex.models import Comic, Folder, Imprint, Library, Publisher, Series, Volume -from codex.models.named import StoryArc, StoryArcNumber +from codex.models.named import Reprint, StoryArc, StoryArcNumber from codex.models.settings import SettingsReader from codex.startup import init_admin_flags _TEST_PASSWORD: Final = "test-pw-hush-S106" # noqa: S105 _HTTP_OK: Final = 200 _TMP_DIR: Final = Path("/tmp/codex.tests.reader") # noqa: S108 +_ALT_TMP_DIR: Final = Path("/tmp/codex.tests.reader_alt_series") # noqa: S108 +# The alternate series fixture holds three comics. +_ALT_SERIES_LEN: Final = 3 def _v4(response): @@ -146,3 +153,179 @@ def test_scoped_patch_persists_on_series_row(self) -> None: row = SettingsReader.objects.filter(series_id=self.series.pk).first() assert row is not None assert row.fit_to == "H" + + +class ReaderAlternateSeriesArcTestCase(TestCase): + """Reading an alternate series (ComicInfo AlternateSeries) as a reading order.""" + + @override + def setUp(self) -> None: + cache.clear() + init_admin_flags() + _ALT_TMP_DIR.mkdir(parents=True, exist_ok=True) + self.library = Library.objects.create(path=str(_ALT_TMP_DIR)) # pyright: ignore[reportUninitializedInstanceVariable] + self.publisher = Publisher.objects.create(name="Pub") # pyright: ignore[reportUninitializedInstanceVariable] + self.imprint = Imprint.objects.create( # pyright: ignore[reportUninitializedInstanceVariable] + name="Imp", publisher=self.publisher + ) + self.series = Series.objects.create( # pyright: ignore[reportUninitializedInstanceVariable] + name="Ser", imprint=self.imprint, publisher=self.publisher + ) + self.volume = Volume.objects.create( # pyright: ignore[reportUninitializedInstanceVariable] + name="2024", + series=self.series, + imprint=self.imprint, + publisher=self.publisher, + ) + folder_path = _ALT_TMP_DIR / "f" + folder_path.mkdir(exist_ok=True) + self.folder = Folder.objects.create( # pyright: ignore[reportUninitializedInstanceVariable] + library=self.library, path=str(folder_path) + ) + # Regular issue order is deliberately the reverse of the alternate + # order, so a reading order that silently fell back to the series + # would produce the opposite sequence. + self.c_first = self._create_comic("C1", 30, "2") # pyright: ignore[reportUninitializedInstanceVariable] + self.c_middle = self._create_comic("C2", 20, "3") # pyright: ignore[reportUninitializedInstanceVariable] + self.c_last = self._create_comic("C3", 10, "10") # pyright: ignore[reportUninitializedInstanceVariable] + user = User.objects.create_user( + username="reader_alt_series_test", password=_TEST_PASSWORD + ) + self.client = Client() + self.client.force_login(user) + + @override + def tearDown(self) -> None: + shutil.rmtree(_ALT_TMP_DIR, ignore_errors=True) + + def _create_comic(self, name: str, issue_number: int, alt_issue: str) -> Comic: + path = _ALT_TMP_DIR / f"{name.lower()}.cbz" + path.touch() + comic = Comic.objects.create( + library=self.library, + path=path, + issue_number=issue_number, + name=name, + publisher=self.publisher, + imprint=self.imprint, + series=self.series, + volume=self.volume, + parent_folder=self.folder, + size=42 + issue_number, + year=2024, + page_count=20, + ) + comic.reprints.add( + Reprint.objects.create(series_name="Crossover", issue=alt_issue) + ) + return comic + + def _reader(self, comic: Comic, arc: dict | None = None) -> dict: + url = f"/api/v4/reader/comics/{comic.pk}" + if arc: + url += f"?arc={json.dumps(arc)}" + response = self.client.get(url) + assert response.status_code == _HTTP_OK, response.content + return _v4(response) + + def _alt_arc(self, data: dict) -> tuple[str, dict]: + """Return the single alternate-series arc's (ids, info).""" + arcs = data["arcs"]["reprints"] + assert len(arcs) == 1, arcs + ids, info = next(iter(arcs.items())) + return ids, info + + def test_alternate_series_is_offered_as_an_arc(self) -> None: + """The reader lists the alternate series among its reading orders.""" + data = self._reader(self.c_first) + assert "reprints" in data["arcs"], data["arcs"] + _, info = self._alt_arc(data) + assert info["name"] == "Crossover", info + assert info["mtime"], info + + def test_arc_ids_cover_the_whole_group(self) -> None: + """The arc handle is every Reprint row in the series, not just this comic's.""" + # Each issue of an alternate series is its own Reprint row, so a + # per-comic handle would change from book to book. + ids, _ = self._alt_arc(self._reader(self.c_first)) + expected = sorted( + Reprint.objects.filter(series_name="Crossover").values_list("pk", flat=True) + ) + assert sorted(int(pk) for pk in str(ids).split(",")) == expected + + def test_books_follow_the_alternate_number(self) -> None: + """prev/next walk 2 -> 3 -> 10, not the regular issue order.""" + ids, _ = self._alt_arc(self._reader(self.c_first)) + arc = {"collection": "reprints", "ids": [int(pk) for pk in str(ids).split(",")]} + + data = self._reader(self.c_middle, arc) + assert data["arc"]["collection"] == "reprints", data["arc"] + assert data["books"]["prev"]["pk"] == self.c_first.pk, data["books"] + assert data["books"]["next"]["pk"] == self.c_last.pk, data["books"] + + def test_arc_selection_survives_the_next_book(self) -> None: + """The same ids keep selecting the alternate series on another comic.""" + ids, _ = self._alt_arc(self._reader(self.c_first)) + arc = {"collection": "reprints", "ids": [int(pk) for pk in str(ids).split(",")]} + + data = self._reader(self.c_last, arc) + assert data["arc"]["collection"] == "reprints", data["arc"] + # C3 carries alternate number 10, the last of the three. + assert data["arc"]["index"] == _ALT_SERIES_LEN, data["arc"] + assert data["arc"]["count"] == _ALT_SERIES_LEN, data["arc"] + + def test_absent_alternate_series_falls_back(self) -> None: + """Requesting an alternate series a comic isn't in still reads.""" + path = _ALT_TMP_DIR / "lonely.cbz" + path.touch() + lonely = Comic.objects.create( + library=self.library, + path=path, + issue_number=99, + name="Lonely", + publisher=self.publisher, + imprint=self.imprint, + series=self.series, + volume=self.volume, + parent_folder=self.folder, + size=1, + year=2024, + page_count=1, + ) + data = self._reader(lonely, {"collection": "reprints", "ids": [1, 2, 3]}) + assert data["arc"]["collection"] != "reprints", data["arc"] + + def test_reader_settings_ignore_the_reprint_scope(self) -> None: + """``reprints`` has no settings scope of its own and doesn't error.""" + url = ( + f"/api/v4/comics/{self.c_first.pk}/reader-settings" + "?scopes=global,reprints,comics" + ) + response = self.client.get(url) + assert response.status_code == _HTTP_OK, response.content + scopes = _v4(response)["scopes"] + assert "reprints" not in scopes, scopes + assert "global" in scopes, scopes + + def test_mtime_probe_accepts_the_alternate_series_arc(self) -> None: + """The reader probes every arc it offers, alternate series included.""" + ids, _ = self._alt_arc(self._reader(self.c_first)) + collections = json.dumps([{"collection": "reprints", "pks": str(ids)}]) + response = self.client.get(f"/api/v4/mtime?collections={collections}") + assert response.status_code == _HTTP_OK, response.content + assert _v4(response)["maxMtime"], response.content + + def test_timestamp_updater_restamps_reprints(self) -> None: + """A changed comic advances its alternate series' mtime.""" + # Without this the reader's mtime probe never notices a re-import + # and an open reader keeps showing stale books. + reprint = Reprint.objects.get(comic=self.c_first) + before = reprint.updated_at + start_time = before - timedelta(seconds=5) + Comic.objects.filter(pk=self.c_first.pk).update(updated_at=Now()) + + updater = TimestampUpdater(Mock(), Mock(), Mock()) + updater.update_library_collections(self.library, start_time, {}) + + reprint.refresh_from_db() + assert reprint.updated_at > before