From 84d8918c581c365ade376188783186a23e366423 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 06:45:54 +0000 Subject: [PATCH] Sort and read by alternate series (#360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ComicInfo AlternateSeries / AlternateNumber already imported into the Reprint model, but there was no way to order by the alternate numbering and no way to read an alternate series through in order. Alternate series tags make durable reading lists — they survive a re-import, rename or re-tag — which is what issue #360 asks for. Add an "Alternate Number" sort. Reprint.issue is a string, so "#10" sorted before "#2"; split it into issue_number / issue_suffix columns mirroring Comic's, derived in presave and backfilled by migration. The sort scopes to the alternate series picked by the reprints filter via a FilteredRelation, the way story_arc_number scopes to the browsed arc. Comics carrying no alternate number fall back to their own issue number so a mixed listing stays readable rather than collapsing to NULL. Surface the existing "Alternate Series" sort in cover view too, and sort it through a Coalesce onto the real series sort_name so untagged comics interleave instead of clumping under an empty aggregate. Let the reader follow an alternate series as a reading order. An alternate series is identified by (series_name, volume_number, language) — the Reprint unique key minus the issue — and the arc handle is the whole group's pks, since each issue is its own Reprint row and a per-comic handle would change from book to book. Reprint rows join the TimestampUpdater re-stamp flow and the mtime probe accepts them, so an open reader still notices a re-import. Also fixes a latent error: an M2M primary sort outside table view (cover cards, OPDS) emitted an ORDER BY alias that was only annotated in table mode. Unreachable through the UI before, but surfacing the Alternate Series sort in cover view makes it a normal path. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CrB4hZ7HtGE9YjV5EkXeQ3 --- NEWS.md | 13 ++ codex/choices/browser.py | 10 +- codex/collection.py | 9 + codex/librarian/scribe/importer/const.py | 7 + .../scribe/importer/delete/comics.py | 16 +- codex/librarian/scribe/importer/init.py | 5 +- .../scribe/importer/query/links_m2m.py | 14 +- codex/librarian/scribe/timestamp_update.py | 20 +- codex/migrations/0053_reprint_issue_number.py | 121 ++++++++++++ codex/models/named.py | 45 ++++- codex/models/util.py | 29 +++ codex/serializers/browser/mtime.py | 9 +- codex/serializers/fields/collection.py | 17 ++ codex/serializers/fields/reader.py | 7 +- codex/views/browser/annotate/order.py | 44 ++++- codex/views/browser/browser.py | 42 +++- codex/views/browser/columns.py | 49 ++++- .../filters/search/field/expression.py | 14 +- codex/views/browser/mtime.py | 45 ++++- codex/views/browser/order_by.py | 26 ++- codex/views/const.py | 6 +- codex/views/reader/arcs.py | 88 ++++++++- codex/views/reader/books.py | 15 +- .../browser/card/order-by-caption.vue | 23 +++ .../reader/toolbars/top/reader-arc-select.vue | 20 +- frontend/tests/unit/order-by-caption.test.js | 60 ++++++ frontend/tests/unit/reader-arc-select.test.js | 53 +++++ tests/test_browser_ordering.py | 1 + tests/test_browser_reprints_column.py | 175 +++++++++++++++++ tests/test_reader.py | 185 +++++++++++++++++- 30 files changed, 1111 insertions(+), 57 deletions(-) create mode 100644 codex/migrations/0053_reprint_issue_number.py create mode 100644 frontend/tests/unit/order-by-caption.test.js create mode 100644 frontend/tests/unit/reader-arc-select.test.js 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