From 265b8c0a187c910d9a42826410233af5e9962b8a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:22:30 +0000 Subject: [PATCH 1/7] fix(browser): send settings in the save request body updateSettings passed the settings as a `params` wrapper, which xior sends as the request body `{"params": {...}}`. The endpoint validates the body's top level keys, so every one of them was unknown, the update validated as empty and the request saved nothing. Nothing was actually lost: the browse page request that follows a settings change carries the same settings as query params and persists them. The save request itself has just been doing nothing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- frontend/src/api/v4/browser.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/api/v4/browser.js b/frontend/src/api/v4/browser.js index fe47f3f1e..200fad001 100644 --- a/frontend/src/api/v4/browser.js +++ b/frontend/src/api/v4/browser.js @@ -149,7 +149,10 @@ export const getSettings = (data) => { export const updateSettings = (settings) => { const params = serializeParams(settings, undefined, false); - return HTTP.patch(_collectionSettingsBase(settings?.collection), { params }); + // The settings go in the request body, not a `params` wrapper: the + // endpoint validates the body's top-level keys, so a wrapped object + // validates as empty and saves nothing. + return HTTP.patch(_collectionSettingsBase(settings?.collection), params); }; export const resetSettings = (settings) => From 5d7684a1982604db250ebbfb3af9fafba249d702 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:22:40 +0000 Subject: [PATCH 2/7] feat(browser): store a remembered sort per top collection Sorting is one global setting today, so sorting issues by added time leaves the publisher list in added-time order too. Issue #415 asks each top collection to keep the sort it was last browsed with. Add SettingsBrowser.collection_order_memory, a map of top collection to the sort it was left in. It follows table_columns, the other per top collection map: a JSON column in DIRECT_KEYS, so loading, saving, resetting and saved-view cloning all carry it without further wiring. The map round-trips out of stored settings, so the validator cleans rather than rejects: unknown collections and sorts that no longer exist are dropped with a warning instead of failing the whole browse page. search_score is never stored, since relevance ordering only means anything while its search is running. Empty for every existing row, which reads as "nothing customized yet" and keeps today's carry-the-sort-over behavior. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- codex/choices/browser.py | 1 + ...settingsbrowser_collection_order_memory.py | 29 +++++++ codex/models/settings.py | 10 +++ codex/serializers/browser/settings.py | 77 ++++++++++++++++++- 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 codex/migrations/0054_settingsbrowser_collection_order_memory.py diff --git a/codex/choices/browser.py b/codex/choices/browser.py index 19317bb8b..81fe70c29 100644 --- a/codex/choices/browser.py +++ b/codex/choices/browser.py @@ -708,6 +708,7 @@ def admin_default_route_for(top_collection: str) -> dict: "order_by": "sort_name", "order_reverse": False, "order_extra_keys": (), + "collection_order_memory": MappingProxyType({}), "search": "", "show": _DEFAULT_SHOW, "top_collection": "publishers", diff --git a/codex/migrations/0054_settingsbrowser_collection_order_memory.py b/codex/migrations/0054_settingsbrowser_collection_order_memory.py new file mode 100644 index 000000000..ffe35e496 --- /dev/null +++ b/codex/migrations/0054_settingsbrowser_collection_order_memory.py @@ -0,0 +1,29 @@ +"""Generated by Django 6.0.7 on 2026-08-30 12:00.""" + +from django.db import migrations, models + + +class Migration(migrations.Migration): + """ + Remember the sort each top collection was last browsed with. + + Switching top collections used to drag one global sort along with it, so + sorting issues by added time and then looking at publishers left the + publisher list in added-time order too. + + The new map is empty for every existing row, which reads as "no collection + has been customized yet" and keeps the current carry-the-sort-over + behavior until someone changes a sort. Nothing to backfill. + """ + + dependencies = [ + ("codex", "0053_reprint_issue_number"), + ] + + operations = [ + migrations.AddField( + model_name="settingsbrowser", + name="collection_order_memory", + field=models.JSONField(default=dict), + ), + ] diff --git a/codex/models/settings.py b/codex/models/settings.py index 2b39a4c1b..268a7c69a 100644 --- a/codex/models/settings.py +++ b/codex/models/settings.py @@ -350,6 +350,15 @@ class SettingsBrowser(SettingsBase): # Empty list means single-column sort (today's behavior). The # frontend table view adds entries via shift-click on a header. order_extra_keys = JSONField(default=list) + # The sort each top collection was last browsed with, so switching + # between them restores the sort that collection was left in instead + # of dragging one global sort everywhere. Keyed by + # ``BROWSER_TOP_COLLECTION_CHOICES`` key; each value is + # ``{"order_by": , "order_reverse": , "order_extra_keys": [...]}``. + # A missing key means "never customized" and the current sort carries + # over. ``search_score`` is never stored: it only exists while a + # search is active. + collection_order_memory = JSONField(default=dict) search = CharField(max_length=4095, default="", blank=True) # Display preferences @@ -387,6 +396,7 @@ class SettingsBrowser(SettingsBase): "order_by", "order_reverse", "order_extra_keys", + "collection_order_memory", "search", "custom_covers", "dynamic_covers", diff --git a/codex/serializers/browser/settings.py b/codex/serializers/browser/settings.py index 2848f661d..0e0132891 100644 --- a/codex/serializers/browser/settings.py +++ b/codex/serializers/browser/settings.py @@ -15,6 +15,7 @@ ) from codex.choices.browser import ( + BROWSER_EXTRA_SORT_UNSUPPORTED_KEYS, BROWSER_ORDER_BY_CHOICES, BROWSER_TABLE_COLUMNS, BROWSER_TABLE_COVER_SIZE_CHOICES, @@ -31,6 +32,45 @@ from codex.serializers.route import SimpleRouteSerializer from codex.serializers.settings import SettingsInputSerializer +# Sorts worth remembering per top collection. ``search_score`` only means +# anything while a search is running, so it never enters the memory. +_MEMORABLE_ORDER_BY_KEYS = frozenset(BROWSER_ORDER_BY_CHOICES.keys()) - {"search_score"} +# Extra (secondary) sort keys the order pipeline can resolve anywhere. +_MEMORABLE_EXTRA_KEYS = ( + frozenset(BROWSER_ORDER_BY_CHOICES.keys()) - BROWSER_EXTRA_SORT_UNSUPPORTED_KEYS +) + + +def _clean_remembered_extra_keys(value) -> list[dict]: + """Keep the well formed, sortable, non duplicate extra sort entries.""" + cleaned: list[dict] = [] + if not isinstance(value, list | tuple): + return cleaned + seen: set[str] = set() + for entry in value: + if not isinstance(entry, dict): + continue + key = entry.get("key") + if key not in _MEMORABLE_EXTRA_KEYS or key in seen: + continue + seen.add(key) # pyright: ignore[reportArgumentType] + cleaned.append({"key": key, "reverse": bool(entry.get("reverse", False))}) + return cleaned + + +def _clean_remembered_order(value) -> dict | None: + """Coerce one remembered sort, or None when it can't be salvaged.""" + if not isinstance(value, dict): + return None + order_by = value.get("order_by") + if order_by not in _MEMORABLE_ORDER_BY_KEYS: + return None + return { + "order_by": order_by, + "order_reverse": bool(value.get("order_reverse", False)), + "order_extra_keys": _clean_remembered_extra_keys(value.get("order_extra_keys")), + } + class BrowserSettingsShowCollectionFlagsSerializer(Serializer): """Show Collection Flags (collection vocabulary).""" @@ -127,7 +167,7 @@ class BrowserSettingsSerializer(BrowserSettingsSerializerBase): JSON_FIELDS = frozenset( BrowserSettingsSerializerBase.JSON_FIELDS - | {"table_columns", "order_extra_keys"} + | {"table_columns", "order_extra_keys", "collection_order_memory"} ) mtime = TimestampField(read_only=True) @@ -154,6 +194,14 @@ class BrowserSettingsSerializer(BrowserSettingsSerializerBase): required=False, allow_empty=True, ) + # The sort each top collection was last browsed with, keyed by + # top_collection. Cleaned in ``validate_collection_order_memory``; + # see the model field for the stored shape. + collection_order_memory = DictField( + child=DictField(), + required=False, + allow_empty=True, + ) def validate_table_columns(self, value): """ @@ -183,6 +231,33 @@ def validate_table_columns(self, value): cleaned[top_collection] = [c for c in columns if c in valid_columns] return cleaned + def validate_collection_order_memory(self, value): + """ + Drop unknown top collections and unusable remembered sorts. + + Like ``table_columns`` this round-trips out of stored settings, so a + stale client must not hard-400 the whole browse page. An entry naming + a sort that no longer exists is dropped with a warning; the collection + then just keeps whatever sort it is browsed with next. + """ + cleaned: dict[str, dict] = {} + for top_collection, order in value.items(): + if top_collection not in BROWSER_TOP_COLLECTION_CHOICES: + logger.warning( + "Dropping unknown collection_order_memory top_collection " + f"{top_collection!r}" + ) + continue + remembered_order = _clean_remembered_order(order) + if remembered_order is None: + logger.warning( + "Dropping unusable collection_order_memory order for " + f"{top_collection!r}" + ) + continue + cleaned[top_collection] = remembered_order + return cleaned + def validate_order_extra_keys(self, value): """ Reject malformed entries; coerce to the canonical shape. From d519489cf488d3d2fd06b33e0175170cbe70b02b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:22:49 +0000 Subject: [PATCH 3/7] feat(browser): swap the remembered sort when the collection changes Switching top collections now files the sort the old one is left in and hands back the sort the new one was last browsed with. A collection with nothing filed keeps whatever sort arrives with it, so nothing changes until you sort something. Searching does the same pair: entering a search files the sort it displaces and clearing the search puts it back, instead of always landing on sort_name. Whether a payload is the user changing collections is decided before the validators run, since they inject an orderBy of their own that would otherwise look like a payload that already carries a sort. Payloads that really do carry one -- settings loads, saved views, redirects -- keep it. The memory rides the settings the browse page request already echoes, so there is no new request to persist it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- frontend/src/stores/browser.js | 111 +++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 5 deletions(-) diff --git a/frontend/src/stores/browser.js b/frontend/src/stores/browser.js index d3f0cfae8..eb4f83e0f 100644 --- a/frontend/src/stores/browser.js +++ b/frontend/src/stores/browser.js @@ -231,6 +231,7 @@ export const useBrowserStore = defineStore("browser", { orderBy: BROWSER_DEFAULTS.orderBy, orderReverse: BROWSER_DEFAULTS.orderReverse, orderExtraKeys: BROWSER_DEFAULTS.orderExtraKeys ?? [], + collectionOrderMemory: BROWSER_DEFAULTS.collectionOrderMemory ?? {}, search: BROWSER_DEFAULTS.search, show: BROWSER_DEFAULTS.show, topCollection: BROWSER_DEFAULTS.topCollection, @@ -496,14 +497,94 @@ export const useBrowserStore = defineStore("browser", { return this.settings.show[topCollection]; } }, + /* + * COLLECTION ORDER MEMORY + * + * Each top collection remembers the sort it was last browsed with, so + * switching between them (or leaving and returning from a search) + * restores that sort instead of dragging one global sort everywhere. + * A collection with nothing filed keeps whatever sort arrives with it. + */ + _stashCollectionOrder(data, topCollection) { + /* + * File the sort ``topCollection`` is being left in. ``search_score`` + * is never filed: it only means anything while that search runs. + */ + const { orderBy, orderReverse, orderExtraKeys } = this.settings; + if (!topCollection || !orderBy || orderBy === "search_score") { + return; + } + data.collectionOrderMemory = { + ...(data.collectionOrderMemory ?? this.settings.collectionOrderMemory), + [topCollection]: { + orderBy, + orderReverse, + orderExtraKeys: [...(orderExtraKeys ?? [])], + }, + }; + }, + _applyOrder(data, order) { + /* + * Write a sort into a settings payload without overruling one the + * payload already asks for: a sort the user just picked outranks + * anything filed away earlier. + */ + if (!Object.hasOwn(data, "orderBy")) { + data.orderBy = order.orderBy; + } + if (!Object.hasOwn(data, "orderReverse")) { + data.orderReverse = order.orderReverse; + } + if (!Object.hasOwn(data, "orderExtraKeys")) { + data.orderExtraKeys = [...(order.orderExtraKeys ?? [])]; + } + }, + _restoreCollectionOrder(data, topCollection) { + // Hand back the sort topCollection was last browsed with, if any. + const memory = + data.collectionOrderMemory ?? this.settings.collectionOrderMemory; + const order = memory?.[topCollection]; + if (!order) { + return false; + } + this._applyOrder(data, order); + return true; + }, + _restoreSearchOrder(data) { + /* + * Hand the sort back when a search ends: the one this collection was + * last browsed with, or its plain default when nothing was filed. + */ + if (this._restoreCollectionOrder(data, this.settings.topCollection)) { + return; + } + this._applyOrder(data, { + orderBy: + this.settings.topCollection === "folders" ? "filename" : "sort_name", + orderReverse: false, + orderExtraKeys: [], + }); + }, + _applyCollectionOrderMemory(data, isCollectionSwitch) { + /* + * Swap the remembered sorts over a user's top collection change. + * Server-sent payloads (settings load, saved view, redirect) carry + * their own sort and are left alone; the backend files those. + */ + if (!isCollectionSwitch) { + return; + } + this._stashCollectionOrder(data, this.settings.topCollection); + if (!this.settings.search && !data.search) { + // A search owns the sort until it's cleared. + this._restoreCollectionOrder(data, data.topCollection); + } + }, _validateSearch(data) { if (!this.settings.search && !data.search) { // if cleared search check for bad order_by if (this.settings.orderBy === "search_score") { - data.orderBy = - this.settings.topCollection === "folders" - ? "filename" - : "sort_name"; + this._restoreSearchOrder(data); } return; } else if (this.settings.search) { @@ -514,6 +595,7 @@ export const useBrowserStore = defineStore("browser", { // Otherwise we'd strand the user at e.g. the series root with no // parent breadcrumbs / up-arrows. if (Object.hasOwn(data, "search") && !data.search) { + this._restoreSearchOrder(data); const { collection, pks } = liveBrowseParams(); if (!pks && collection === this.lowestShownCollection) { return { params: { collection: "root", pks: "", page: "1" } }; @@ -522,7 +604,9 @@ export const useBrowserStore = defineStore("browser", { // Still searching (or nothing to undo): don't redirect. return; } - // If first search redirect to lowest collection and change order + // If first search redirect to lowest collection and change order. + // File the sort first so clearing the search can hand it back. + this._stashCollectionOrder(data, this.settings.topCollection); data.orderBy = "search_score"; data.orderReverse = true; const collection = liveBrowseParams().collection; @@ -672,7 +756,17 @@ export const useBrowserStore = defineStore("browser", { this.startSearchHideTimeout(); }, _validateAndSaveSettings(data) { + /* + * Decide before the validators run: they inject an ``orderBy`` of + * their own, which would otherwise read as a payload that already + * carries a sort. + */ + const isCollectionSwitch = + Boolean(data?.topCollection) && + data.topCollection !== this.settings.topCollection && + !Object.hasOwn(data, "orderBy"); let redirect = this._validateSearch(data); + this._applyCollectionOrderMemory(data, isCollectionSwitch); redirect = this._validateTopCollection(data, redirect); if (dequal(redirect?.params, liveBrowseParams())) { // not triggered if page is numeric, which is intended. @@ -716,6 +810,10 @@ export const useBrowserStore = defineStore("browser", { state.settings.search = data.search; state.settings.orderBy = data.orderBy; state.settings.orderReverse = data.orderReverse; + // Assigned, not merged: a reset empties the memory, and + // ``_addSettings``' merge could never remove an entry. + state.settings.collectionOrderMemory = + data.collectionOrderMemory ?? {}; } state.browserPageLoaded = true; }); @@ -983,6 +1081,9 @@ export const useBrowserStore = defineStore("browser", { return; } this._validateAndSaveSettings(settings); + // Claim the page so the route watcher reloads with these settings + // instead of re-fetching the stored ones over them. + this.browserPageLoaded = true; // ignore redirect router.push(toBrowseRoute(route)).catch(console.error); }, From acbe0bca98079be0d775a4710dcad60d8a1541c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:22:59 +0000 Subject: [PATCH 4/7] feat(browser): carry the sort when the server moves the top collection The browser store handles collection switches the user makes, but the server makes some of its own: a url whose collection forces a different top, or a redirect after folder view is turned off. Those bypass the store, so the old collection's sort leaked into the new one and would later be filed under the wrong collection, overwriting a real memory. Apply the same file-and-restore in the settings GET when validation rewrites the top collection, and in raise_redirect, which is the one place every settings-mask redirect passes through. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- codex/views/browser/settings.py | 35 +++++++++++++++++++++++++++++++++ codex/views/browser/validate.py | 6 ++++++ 2 files changed, 41 insertions(+) diff --git a/codex/views/browser/settings.py b/codex/views/browser/settings.py index 2417b449d..0be7236d5 100644 --- a/codex/views/browser/settings.py +++ b/codex/views/browser/settings.py @@ -27,6 +27,40 @@ # Collections whose nav owns a dedicated route (folders / story arcs); for these # the URL collection becomes the top_collection directly during validation. _OWN_ROUTE_COLLECTIONS = frozenset({FOLDER_COLLECTION, STORY_ARC_COLLECTION}) +_SEARCH_ORDER_BY = "search_score" + + +def apply_collection_order_memory( + params: MutableMapping, old_top_collection: str, new_top_collection: str +) -> None: + """ + Carry the remembered sort across a top collection change. + + Files the sort the old collection is leaving with, then restores the sort + the new collection was last browsed with. The browser store does this for + collection switches the user makes; this covers the ones the server makes + on its own when a url or a settings change forces a different top + collection, which would otherwise leak one collection's sort into another. + """ + if old_top_collection == new_top_collection: + return + memory = dict(params.get("collection_order_memory") or {}) + order_by = params.get("order_by") + if old_top_collection and order_by and order_by != _SEARCH_ORDER_BY: + memory[old_top_collection] = { + "order_by": order_by, + "order_reverse": bool(params.get("order_reverse")), + "order_extra_keys": list(params.get("order_extra_keys") or ()), + } + params["collection_order_memory"] = memory + + remembered = memory.get(new_top_collection) + if not remembered or params.get("search"): + # An active search owns the sort until it's cleared. + return + params["order_by"] = remembered["order_by"] + params["order_reverse"] = remembered["order_reverse"] + params["order_extra_keys"] = list(remembered.get("order_extra_keys") or ()) class BrowserSettingsBaseView(SettingsBaseView): @@ -145,6 +179,7 @@ def _validate_settings_get(self, validated_data, params: dict) -> dict: else Collection.ROOT ) self._validate_top_collection(params, collection, top_collection) + apply_collection_order_memory(params, top_collection, params["top_collection"]) self.set_order_by_default(params) return params diff --git a/codex/views/browser/validate.py b/codex/views/browser/validate.py index 5050379dc..138d76c07 100644 --- a/codex/views/browser/validate.py +++ b/codex/views/browser/validate.py @@ -12,6 +12,7 @@ from codex.models.collections import BrowserCollectionModel from codex.util import mapping_to_dict from codex.views.browser.filters.search.parse import SearchFilterView +from codex.views.browser.settings import apply_collection_order_memory from codex.views.const import ( COLLECTION_MODEL_MAP, COMIC_COLLECTION, @@ -77,7 +78,12 @@ def raise_redirect( route["params"].update(route_mask) settings = cast("dict[str, Any]", deepcopy(mapping_to_dict(self.params))) if settings_mask: + old_top_collection = settings.get("top_collection", "") settings.update(settings_mask) + # A redirect that moves the top collection moves its sort too. + apply_collection_order_memory( + settings, old_top_collection, settings.get("top_collection", "") + ) detail = {"route": route, "settings": settings, "reason": reason} raise SeeOtherRedirectError(detail=detail) From 89796beb0940cbc33102918004d219d927deb420 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:22:59 +0000 Subject: [PATCH 5/7] feat(user_data): back up and restore the collection sort memory The sidecar adds columns it is missing when it opens, so a backup written before this column restores an empty memory rather than failing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- codex/user_data/restore.py | 12 ++++++++++++ codex/user_data/schema.sql | 1 + codex/user_data/serializers.py | 3 +++ 3 files changed, 16 insertions(+) diff --git a/codex/user_data/restore.py b/codex/user_data/restore.py index 6a806ab3c..9ca05c9c6 100644 --- a/codex/user_data/restore.py +++ b/codex/user_data/restore.py @@ -568,6 +568,15 @@ def _resolve_filter_column(row_keys, column: str) -> str | None: return None +def _row_column(row, column: str): + """Read a column a sidecar written by an older codex may not carry.""" + try: + return row[column] + except (IndexError, KeyError): + # sqlite3.Row raises IndexError, a plain mapping raises KeyError. + return None + + def _build_browser_defaults(row, show) -> dict[str, Any]: """Map a sidecar settings_browser row to ``update_or_create`` defaults.""" order_by = row["order_by"] or "" @@ -582,6 +591,9 @@ def _build_browser_defaults(row, show) -> dict[str, Any]: "order_by": order_by, "order_reverse": bool(row["order_reverse"]), "order_extra_keys": json.loads(row["order_extra_keys"] or "[]"), + "collection_order_memory": json.loads( + _row_column(row, "collection_order_memory") or "{}" + ), "search": row["search"] or "", "custom_covers": bool(row["custom_covers"]), "dynamic_covers": bool(row["dynamic_covers"]), diff --git a/codex/user_data/schema.sql b/codex/user_data/schema.sql index 5156c0729..72ec50aaa 100644 --- a/codex/user_data/schema.sql +++ b/codex/user_data/schema.sql @@ -86,6 +86,7 @@ CREATE TABLE IF NOT EXISTS settings_browser ( order_by TEXT NOT NULL DEFAULT '', order_reverse INTEGER NOT NULL DEFAULT 0, order_extra_keys TEXT NOT NULL DEFAULT '[]', + collection_order_memory TEXT NOT NULL DEFAULT '{}', search TEXT NOT NULL DEFAULT '', custom_covers INTEGER NOT NULL DEFAULT 1, dynamic_covers INTEGER NOT NULL DEFAULT 1, diff --git a/codex/user_data/serializers.py b/codex/user_data/serializers.py index 714d9b68d..68b2a4814 100644 --- a/codex/user_data/serializers.py +++ b/codex/user_data/serializers.py @@ -205,6 +205,9 @@ def serialize_settings_browser( "order_extra_keys": json.dumps( browser.order_extra_keys, separators=(",", ":") ), + "collection_order_memory": json.dumps( + browser.collection_order_memory, separators=(",", ":") + ), "search": browser.search, "custom_covers": int(browser.custom_covers), "dynamic_covers": int(browser.dynamic_covers), From ad7e35866b3b23056206d6615340ca94c778f790 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:23:07 +0000 Subject: [PATCH 6/7] test(browser): cover the collection sort memory Backend: the field's wiring, the lenient validator, the file-and-restore helper, and the HTTP round trips -- PATCH, the browse page query params that actually persist the map, reset, saved views, and a url that forces a different top collection. Frontend: switching collections, coming back to one, a collection with nothing filed, payloads that carry their own sort, and the sort a search displaces and gives back. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- .../unit/browser-store-order-memory.test.js | 202 +++++++++++ tests/test_collection_order_memory.py | 326 ++++++++++++++++++ tests/test_user_data_restore.py | 23 ++ 3 files changed, 551 insertions(+) create mode 100644 frontend/tests/unit/browser-store-order-memory.test.js create mode 100644 tests/test_collection_order_memory.py diff --git a/frontend/tests/unit/browser-store-order-memory.test.js b/frontend/tests/unit/browser-store-order-memory.test.js new file mode 100644 index 000000000..d4993bf3a --- /dev/null +++ b/frontend/tests/unit/browser-store-order-memory.test.js @@ -0,0 +1,202 @@ +/* + * Tests for the per-top-collection sort memory in ``stores/browser.js`` + * (issue #415). + * + * Each top collection remembers the sort it was last browsed with, so + * switching between them — or leaving and returning from a search — + * hands that sort back instead of dragging one global sort everywhere. + */ +import { createPinia, setActivePinia } from "pinia"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// ``liveBrowseParams()`` (module scope in browser.js) reads +// ``router.currentRoute.value``; mock the router so the store can be +// driven without a real one. +vi.mock("@/plugins/router", () => ({ + default: { currentRoute: { value: { params: {}, query: {} } } }, +})); + +import { useBrowserStore } from "@/stores/browser"; + +const ADDED_TIME = { + orderBy: "created_at", + orderReverse: true, + orderExtraKeys: [], +}; + +const makeStore = (settings = {}) => { + const store = useBrowserStore(); + store.settings.topCollection = "publishers"; + store.settings.orderBy = "sort_name"; + store.settings.orderReverse = false; + store.settings.orderExtraKeys = []; + store.settings.search = ""; + store.settings.collectionOrderMemory = {}; + store.settings.show = { + publishers: true, + imprints: true, + series: true, + volumes: false, + }; + Object.assign(store.settings, settings); + return store; +}; + +beforeEach(() => { + setActivePinia(createPinia()); +}); + +describe("collection order memory — switching top collections", () => { + it("files the sort the old collection is left in", () => { + const store = makeStore({ orderBy: "created_at", orderReverse: true }); + const data = { topCollection: "comics" }; + + store._validateAndSaveSettings(data); + + expect(data.collectionOrderMemory.publishers).toStrictEqual(ADDED_TIME); + }); + + it("hands back the sort the new collection was last browsed with", () => { + const store = makeStore({ + collectionOrderMemory: { comics: ADDED_TIME }, + }); + const data = { topCollection: "comics" }; + + store._validateAndSaveSettings(data); + + expect(data.orderBy).toBe("created_at"); + expect(data.orderReverse).toBe(true); + expect(data.orderExtraKeys).toStrictEqual([]); + }); + + it("keeps the current sort for a collection with nothing filed", () => { + const store = makeStore({ orderBy: "created_at", orderReverse: true }); + const data = { topCollection: "comics" }; + + store._validateAndSaveSettings(data); + + expect(data.orderBy).toBeUndefined(); + expect(store.settings.orderBy).toBe("created_at"); + }); + + it("round-trips a sort back to the collection it came from", () => { + const store = makeStore({ orderBy: "created_at", orderReverse: true }); + + // Leave issues for publishers... + store.settings.topCollection = "comics"; + const toPublishers = { topCollection: "publishers" }; + store._validateAndSaveSettings(toPublishers); + store.settings.orderBy = "sort_name"; + store.settings.orderReverse = false; + + // ...then come back. + const toComics = { topCollection: "comics" }; + store._validateAndSaveSettings(toComics); + + expect(toComics.orderBy).toBe("created_at"); + expect(toComics.orderReverse).toBe(true); + }); + + it("leaves a payload that carries its own sort alone", () => { + // Settings loads, saved views and redirects all arrive this way. + const store = makeStore({ + collectionOrderMemory: { comics: ADDED_TIME }, + }); + const data = { + topCollection: "comics", + orderBy: "sort_name", + orderReverse: false, + }; + + store._validateAndSaveSettings(data); + + expect(data.orderBy).toBe("sort_name"); + expect(data.collectionOrderMemory).toBeUndefined(); + }); + + it("ignores a settings change that isn't a collection switch", () => { + const store = makeStore(); + const data = { orderBy: "created_at", orderReverse: true }; + + store._validateAndSaveSettings(data); + + expect(data.collectionOrderMemory).toBeUndefined(); + }); + + it("does not disturb an active search", () => { + const store = makeStore({ + search: "batman", + orderBy: "search_score", + orderReverse: true, + collectionOrderMemory: { comics: ADDED_TIME }, + }); + const data = { topCollection: "comics" }; + + store._validateAndSaveSettings(data); + + expect(data.orderBy).toBeUndefined(); + // Relevance ordering is never filed against the collection either. + expect(data.collectionOrderMemory?.publishers).toBeUndefined(); + }); +}); + +describe("collection order memory — around a search", () => { + it("files the sort a search displaces and hands it back on clear", () => { + const store = makeStore({ orderBy: "created_at", orderReverse: true }); + + const searching = { search: "batman" }; + store._validateSearch(searching); + expect(searching.collectionOrderMemory.publishers).toStrictEqual( + ADDED_TIME, + ); + expect(searching.orderBy).toBe("search_score"); + + // The store now looks like it does mid-search. + store.settings.search = "batman"; + store.settings.orderBy = "search_score"; + store.settings.orderReverse = true; + store.settings.collectionOrderMemory = searching.collectionOrderMemory; + + const clearing = { search: "" }; + store._validateSearch(clearing); + + expect(clearing.orderBy).toBe("created_at"); + expect(clearing.orderReverse).toBe(true); + }); + + it("falls back to the collection default when nothing was filed", () => { + const store = makeStore({ search: "batman", orderBy: "search_score" }); + + const clearing = { search: "" }; + store._validateSearch(clearing); + + expect(clearing.orderBy).toBe("sort_name"); + expect(clearing.orderReverse).toBe(false); + }); + + it("falls back to filename in folders", () => { + const store = makeStore({ + topCollection: "folders", + search: "batman", + orderBy: "search_score", + }); + + const clearing = { search: "" }; + store._validateSearch(clearing); + + expect(clearing.orderBy).toBe("filename"); + }); + + it("cleans up a stale relevance sort left behind by a cleared search", () => { + const store = makeStore({ + search: "", + orderBy: "search_score", + collectionOrderMemory: { publishers: ADDED_TIME }, + }); + + const data = {}; + store._validateSearch(data); + + expect(data.orderBy).toBe("created_at"); + }); +}); diff --git a/tests/test_collection_order_memory.py b/tests/test_collection_order_memory.py new file mode 100644 index 000000000..bed215df8 --- /dev/null +++ b/tests/test_collection_order_memory.py @@ -0,0 +1,326 @@ +"""Tests for the per-top-collection sort memory (issue #415).""" + +import json +from typing import Final, override + +from django.contrib.auth.models import User +from django.test import Client, TestCase + +from codex.models.settings import SettingsBrowser +from codex.serializers.browser.settings import BrowserSettingsSerializer +from codex.startup import init_admin_flags +from codex.views.browser.settings import apply_collection_order_memory + +_TEST_PASSWORD: Final = "test-pw-hush-S106" # noqa: S105 +_HTTP_OK: Final = 200 +_HTTP_CREATED: Final = 201 +_HTTP_BAD_REQUEST: Final = 400 +_SETTINGS_URL: Final = "/api/v4/browse/publishers/settings" +_FOLDERS_SETTINGS_URL: Final = "/api/v4/browse/folders/settings" +_SAVED_URL: Final = "/api/v4/browse/publishers/saved-settings" + +_ADDED_TIME_DESC: Final = { + "orderBy": "created_at", + "orderReverse": True, + "orderExtraKeys": [], +} + + +def _v4(response): + """Unwrap the v4 ``{data, meta, errors}`` envelope and return ``data``.""" + body = response.json() + if isinstance(body, dict) and "data" in body and "meta" in body: + return body["data"] + return body + + +class CollectionOrderMemoryModelTestCase(TestCase): + """The field is wired into the generic settings machinery.""" + + def test_default_is_empty_dict(self): + field = SettingsBrowser._meta.get_field("collection_order_memory") + assert field.default is dict + assert field.default() == {} + + def test_direct_keys_includes_field(self): + # DIRECT_KEYS membership is what carries the field through load, + # save, reset and saved-view cloning. + assert "collection_order_memory" in SettingsBrowser.DIRECT_KEYS + + +class CollectionOrderMemorySerializerTestCase(TestCase): + """The validator cleans leniently instead of rejecting.""" + + @staticmethod + def _validated(memory): + serializer = BrowserSettingsSerializer(data={"collectionOrderMemory": memory}) + assert serializer.is_valid(), serializer.errors + return serializer.validated_data["collection_order_memory"] + + def test_valid_memory_round_trips_as_snake_case(self): + cleaned = self._validated( + {"comics": {**_ADDED_TIME_DESC, "orderExtraKeys": [{"key": "year"}]}} + ) + assert cleaned == { + "comics": { + "order_by": "created_at", + "order_reverse": True, + "order_extra_keys": [{"key": "year", "reverse": False}], + } + } + + def test_unknown_top_collection_dropped(self): + cleaned = self._validated( + {"nope": _ADDED_TIME_DESC, "comics": _ADDED_TIME_DESC} + ) + assert set(cleaned) == {"comics"} + + def test_unknown_order_by_dropped(self): + assert self._validated({"comics": {"orderBy": "not_a_sort"}}) == {} + + def test_search_score_never_remembered(self): + # Relevance ordering only means anything while its search runs. + assert self._validated({"comics": {"orderBy": "search_score"}}) == {} + + def test_extra_keys_cleaned_without_raising(self): + cleaned = self._validated( + { + "comics": { + "orderBy": "sort_name", + "orderExtraKeys": [ + "garbage", + {"key": "story_arc_number"}, + {"key": "year", "reverse": True}, + {"key": "year"}, + ], + } + } + ) + # Unsortable-as-an-extra and duplicate entries go; the first + # ``year`` survives with its own reverse flag. + assert cleaned["comics"]["order_extra_keys"] == [ + {"key": "year", "reverse": True} + ] + + def test_non_dict_order_is_rejected(self): + # Type garbage still 400s at the field layer, like table_columns. + serializer = BrowserSettingsSerializer( + data={"collectionOrderMemory": {"comics": 5}} + ) + assert not serializer.is_valid() + + +class CollectionOrderMemoryHelperTestCase(TestCase): + """The server-side stash/restore used when the server moves the top.""" + + @staticmethod + def _params(**overrides): + params = { + "top_collection": "publishers", + "order_by": "sort_name", + "order_reverse": False, + "order_extra_keys": [], + "collection_order_memory": {}, + "search": "", + } + params.update(overrides) + return params + + def test_stashes_the_departing_sort(self): + params = self._params(order_by="created_at", order_reverse=True) + apply_collection_order_memory(params, "publishers", "folders") + assert params["collection_order_memory"]["publishers"] == { + "order_by": "created_at", + "order_reverse": True, + "order_extra_keys": [], + } + + def test_restores_the_arriving_sort(self): + params = self._params( + collection_order_memory={ + "folders": { + "order_by": "filename", + "order_reverse": True, + "order_extra_keys": [], + } + } + ) + apply_collection_order_memory(params, "publishers", "folders") + assert params["order_by"] == "filename" + assert params["order_reverse"] is True + + def test_unremembered_collection_keeps_the_current_sort(self): + params = self._params(order_by="created_at") + apply_collection_order_memory(params, "publishers", "comics") + assert params["order_by"] == "created_at" + + def test_no_change_is_a_noop(self): + params = self._params() + apply_collection_order_memory(params, "publishers", "publishers") + assert params["collection_order_memory"] == {} + + def test_active_search_keeps_its_sort(self): + params = self._params( + order_by="search_score", + search="batman", + collection_order_memory={ + "folders": { + "order_by": "filename", + "order_reverse": False, + "order_extra_keys": [], + } + }, + ) + apply_collection_order_memory(params, "publishers", "folders") + assert params["order_by"] == "search_score" + # ...and relevance ordering is not filed against publishers. + assert "publishers" not in params["collection_order_memory"] + + def test_unset_sort_is_not_remembered(self): + params = self._params(order_by="") + apply_collection_order_memory(params, "publishers", "folders") + assert params["collection_order_memory"] == {} + + +class CollectionOrderMemoryRoundTripTestCase(TestCase): + """End-to-end through the settings HTTP endpoint.""" + + @override + def setUp(self) -> None: + init_admin_flags() + self.user = User.objects.create_user( # pyright: ignore[reportUninitializedInstanceVariable] + username="order_memory_test", password=_TEST_PASSWORD + ) + self.client = Client() + self.client.force_login(self.user) + + def _patch(self, payload: dict): + return self.client.patch( + _SETTINGS_URL, + data=json.dumps(payload), + content_type="application/json", + ) + + def _get(self, url: str = _SETTINGS_URL) -> dict: + response = self.client.get(url) + assert response.status_code == _HTTP_OK, response.content + return _v4(response) + + def test_default_get_is_empty(self): + assert self._get()["collectionOrderMemory"] == {} + + def test_patch_persists_to_the_row(self): + memory = {"comics": _ADDED_TIME_DESC} + response = self._patch({"collectionOrderMemory": memory}) + assert response.status_code == _HTTP_OK, response.content + assert self._get()["collectionOrderMemory"] == memory + row = SettingsBrowser.objects.get(user=self.user, name="") + assert row.collection_order_memory == { + "comics": { + "order_by": "created_at", + "order_reverse": True, + "order_extra_keys": [], + } + } + + def test_patch_drops_unknown_top_collection(self): + response = self._patch( + {"collectionOrderMemory": {"nope": _ADDED_TIME_DESC}}, + ) + assert response.status_code == _HTTP_OK, response.content + assert self._get()["collectionOrderMemory"] == {} + + def test_settings_get_accepts_a_memory_query_string(self): + """A URL-encoded JSON map in the query string must not 400.""" + memory = {"comics": _ADDED_TIME_DESC} + url = f"{_SETTINGS_URL}?collectionOrderMemory={json.dumps(memory)}" + response = self.client.get(url) + assert response.status_code == _HTTP_OK, response.content + + def test_browse_page_query_string_persists_the_memory(self): + """ + The browse page is what actually stores an echoed memory map. + + The browser sends its whole settings object as query params on + every page fetch, and that request persists them; the store + never PATCHes the map on its own. + """ + memory = {"comics": _ADDED_TIME_DESC} + url = f"/api/v4/browse/publishers?page=1&collectionOrderMemory={json.dumps(memory)}" + response = self.client.get(url) + assert response.status_code == _HTTP_OK, response.content + row = SettingsBrowser.objects.get(user=self.user, name="") + assert row.collection_order_memory == { + "comics": { + "order_by": "created_at", + "order_reverse": True, + "order_extra_keys": [], + } + } + + def test_delete_resets_the_memory(self): + self._patch({"collectionOrderMemory": {"comics": _ADDED_TIME_DESC}}) + response = self.client.delete(_SETTINGS_URL) + assert response.status_code == _HTTP_OK, response.content + assert self._get()["collectionOrderMemory"] == {} + + def test_saved_view_round_trips_the_memory(self): + memory = {"comics": _ADDED_TIME_DESC} + assert self._patch({"collectionOrderMemory": memory}).status_code == _HTTP_OK + + save_resp = self.client.post( + _SAVED_URL, + data=json.dumps({"name": "AddedTime"}), + content_type="application/json", + ) + assert save_resp.status_code == _HTTP_CREATED, save_resp.content + listed = next( + entry + for entry in _v4(self.client.get(_SAVED_URL))["savedSettings"] + if entry["name"] == "AddedTime" + ) + + # Reset so the loaded values can only come from the saved row. + self.client.delete(_SETTINGS_URL) + load_resp = self.client.get(f"{_SAVED_URL}/{listed['pk']}") + assert load_resp.status_code == _HTTP_OK, load_resp.content + assert _v4(load_resp)["settings"]["collectionOrderMemory"] == memory + + def test_url_forced_top_collection_swaps_the_sort(self): + """ + A url that forces a different top collection swaps sorts too. + + Entering the folders url while the stored top collection is + publishers is a collection switch the browser store never sees, so + the settings GET has to file the publisher sort and hand back the + folder one itself. + """ + folder_order = { + "orderBy": "filename", + "orderReverse": True, + "orderExtraKeys": [], + } + assert ( + self._patch( + { + "orderBy": "created_at", + "orderReverse": True, + "collectionOrderMemory": {"folders": folder_order}, + } + ).status_code + == _HTTP_OK + ) + + # The client names the collection in the path and the query, and + # the settings GET reads the query one when it validates the top. + body = self._get(f"{_FOLDERS_SETTINGS_URL}?collection=folders") + + assert body["topCollection"] == "folders" + assert body["orderBy"] == "filename" + assert body["orderReverse"] is True + # The sort publishers was left in is filed on the way past. + assert body["collectionOrderMemory"]["publishers"] == { + "orderBy": "created_at", + "orderReverse": True, + "orderExtraKeys": [], + } diff --git a/tests/test_user_data_restore.py b/tests/test_user_data_restore.py index c46633133..069cdf795 100644 --- a/tests/test_user_data_restore.py +++ b/tests/test_user_data_restore.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import shutil import sqlite3 from pathlib import Path @@ -338,6 +339,28 @@ def test_browser_defaults_pass_current_keys_through(self) -> None: assert defaults["order_by"] == "community_rating" assert defaults["table_columns"] == {"community_rating": True} + def test_browser_defaults_restore_collection_order_memory(self) -> None: + """The per-collection sort memory round-trips out of the sidecar.""" + from codex.user_data.restore import _build_browser_defaults + + memory = { + "comics": { + "order_by": "created_at", + "order_reverse": True, + "order_extra_keys": [], + } + } + row = self._browser_row(collection_order_memory=json.dumps(memory)) + defaults = _build_browser_defaults(row, show=None) + assert defaults["collection_order_memory"] == memory + + def test_browser_defaults_tolerate_missing_order_memory(self) -> None: + """A sidecar written before the column existed restores empty.""" + from codex.user_data.restore import _build_browser_defaults + + defaults = _build_browser_defaults(self._browser_row(), show=None) + assert defaults["collection_order_memory"] == {} + def test_filter_restore_reads_legacy_column(self) -> None: """A legacy critical_rating filter column lands on community_rating.""" from codex.models.settings import ( From 5922653e8cf32b6ec47bac0e60a80e86632163a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:23:07 +0000 Subject: [PATCH 7/7] chore: drop an unused order-defaults helper, add news _get_browser_order_defaults has no callers; set_order_by_default is the copy that runs. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PwhwRtnMZ2pLcnhF96AyWA --- NEWS.md | 14 ++++++++++++++ codex/views/settings.py | 16 ---------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/NEWS.md b/NEWS.md index 157185616..d4800b054 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,20 @@ width: 128px; border-radius: 128px; " /> +## v2.4.0 + +- Features + - Each top collection remembers the sort it was last browsed with. Sort + Issues by added time and Publishers alphabetically, and switching between + them keeps each one's sort instead of carrying one sort everywhere. A + collection you haven't sorted yet keeps whatever sort you arrive with. + - Clearing a search puts back the sort the search replaced. + +- Fixes + - Saving browser settings sent an empty request that stored nothing. The + settings were persisted by the page request that followed, so nothing was + lost, but the save request itself did nothing. + ## v2.3.0 - Features diff --git a/codex/views/settings.py b/codex/views/settings.py index 5abc65c37..888e64ea0 100644 --- a/codex/views/settings.py +++ b/codex/views/settings.py @@ -26,7 +26,6 @@ SettingsReader, ) from codex.views.auth import AuthFilterGenericAPIView -from codex.views.const import FOLDER_COLLECTION, STORY_ARC_COLLECTION # Fallback top-collection when the BG flag row is missing, off, or holds # an invalid value. Mirrors ``SettingsBrowser.top_collection``'s model @@ -405,21 +404,6 @@ def load_params_from_settings(self, only: Sequence[str] | None = None) -> dict: # ── Save (write) ──────────────────────────────────────────────── - def _get_browser_order_defaults(self) -> dict: - if collection := self.kwargs.get("collection"): - # order_by has a dynamic collection based default - order_by = ( - "filename" - if collection == FOLDER_COLLECTION - else "story_arc_number" - if collection == STORY_ARC_COLLECTION - else "sort_name" - ) - order_defaults = {"order_by": order_by} - else: - order_defaults = {} - return order_defaults - @staticmethod def _save_browser_show(instance: SettingsBrowser, show_data: dict) -> bool: """