From 46c5e387c80f7eacb10ed6869458f3e699250c53 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 27 Oct 2026 17:39:43 -0700 Subject: [PATCH] feat(saved-views): keep a collection in a saved view Images saved views gain a collection key (#331 D140): empty or a collection id of 1 to 9 digits. Writes refuse favorites together with a collection; older rows read an empty collection, and a stored row with both opens the collection, as the listing does for a URL with both. Part of #331 --- database/saved_views.py | 14 +++++ tests/browser/workspace-saved-views.spec.mjs | 2 +- tests/browser/workspace-system.spec.mjs | 2 +- tests/js/workspace-saved-views.test.js | 25 +++++++-- tests/test_saved_views.py | 55 ++++++++++++++++++++ web/js/workspace/saved-views.mjs | 12 +++-- 6 files changed, 100 insertions(+), 10 deletions(-) diff --git a/database/saved_views.py b/database/saved_views.py index 7652c59a..3a4d7afa 100644 --- a/database/saved_views.py +++ b/database/saved_views.py @@ -35,6 +35,9 @@ _LIBRARY_SORTS = ("newest", "oldest", "name", "rating") _MEDIA = ("all", "image", "video") _CONTROL = re.compile(r"[\x00-\x1f\x7f]") _SOURCE = re.compile(r"all|0|[1-9][0-9]{0,8}") +# A collection id as the listing's ``collection`` query takes it (#331): none +# ("") or 1 to 9 digits without a leading zero. +_COLLECTION = re.compile(r"|[1-9][0-9]{0,8}") FALLBACK_NAME = "Saved view" @@ -55,6 +58,9 @@ _STATE_FIELDS = { "tags": ("tags", None, ()), # The Favorites page's filter (#249 D51): a view saved there reopens it. "favorites": ("bool", None, False), + # A view saved on a collection page (#331 D140) reopens that + # collection; never together with favorites. + "collection": ("collection", None, ""), }, "library": { "q": ("text", 500, ""), @@ -125,6 +131,8 @@ def _validate_value(key: str, kind: str, limit, value: object) -> None: raise SavedViewInvalid(f"State '{key}' is not a supported value") if kind == "source" and not _SOURCE.fullmatch(value): raise SavedViewInvalid(f"State '{key}' must be 'all' or a source index") + if kind == "collection" and not _COLLECTION.fullmatch(value): + raise SavedViewInvalid(f"State '{key}' must be empty or a collection id") if kind == "text" and (len(value) > limit or _CONTROL.search(value)): raise SavedViewInvalid(f"State '{key}' is too long or has control characters") @@ -142,6 +150,8 @@ def encode_state(page: object, state: object) -> str: ) for key, (kind, limit, _default) in fields.items(): _validate_value(key, kind, limit, state[key]) + if state.get("favorites") is True and state.get("collection"): + raise SavedViewInvalid("A view cannot filter by favorites and a collection") encoded = json.dumps( {key: state[key] for key in fields}, ensure_ascii=False, separators=(",", ":") ) @@ -181,6 +191,10 @@ def normalize_state(page: str, raw: object) -> dict: except SavedViewInvalid: value = default state[key] = list(value) if kind == "tags" else value + # Never written by this build; a stored row with both opens the collection, + # as the listing does for a URL with both (#331 D140). + if state.get("favorites") is True and state.get("collection"): + state["favorites"] = False return state diff --git a/tests/browser/workspace-saved-views.spec.mjs b/tests/browser/workspace-saved-views.spec.mjs index 85a6ecab..d7cdf8e5 100644 --- a/tests/browser/workspace-saved-views.spec.mjs +++ b/tests/browser/workspace-saved-views.spec.mjs @@ -23,7 +23,7 @@ const heading = page => main(page).getByRole('heading', { level: 1 }); const saveDialog = page => page.getByRole('dialog', { name: 'Save this view', exact: true }); const refineDialog = page => page.getByRole('dialog', { name: 'Refine this view', exact: true }); const toast = page => page.locator('#workspace-toast'); -const IMAGE_DEFAULTS = { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false }; +const IMAGE_DEFAULTS = { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false, collection: '' }; async function refine(page, values) { await filtersButton(page).click(); diff --git a/tests/browser/workspace-system.spec.mjs b/tests/browser/workspace-system.spec.mjs index 7dab8663..db75c85a 100644 --- a/tests/browser/workspace-system.spec.mjs +++ b/tests/browser/workspace-system.spec.mjs @@ -472,7 +472,7 @@ async function openImages(page) { test('defaults apply to a new visit but not to the current view or a saved view', async ({ page, request }) => { const created = await request.post('/prompt_manager/saved-views', { data: { page: 'images', name: 'Everything', - state: { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false } } }); + state: { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false, collection: '' } } }); expect(created.status()).toBe(201); const view = (await created.json()).view; try { diff --git a/tests/js/workspace-saved-views.test.js b/tests/js/workspace-saved-views.test.js index 79f530aa..46d2fe06 100644 --- a/tests/js/workspace-saved-views.test.js +++ b/tests/js/workspace-saved-views.test.js @@ -4,7 +4,7 @@ const assert = require('node:assert/strict'); const source = import('../../web/js/workspace/saved-views.mjs'); -const IMAGE_DEFAULTS = { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false }; +const IMAGE_DEFAULTS = { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false, collection: '' }; const LIBRARY_DEFAULTS = { q: '', category: '', sort: 'newest', tags: [] }; test('a saved state always holds every page key and keeps only values its page accepts', async () => { @@ -12,8 +12,17 @@ test('a saved state always holds every page key and keeps only values its page a assert.deepEqual(SAVED_VIEW_PAGES, ['images', 'library']); assert.deepEqual(normalizeViewState('images', {}), IMAGE_DEFAULTS); assert.deepEqual(normalizeViewState('library', null), LIBRARY_DEFAULTS); - const full = { q: 'castle', category: 'Portraits', sort: 'name-desc', source: '2', folder: 'a/b', media: 'video', tags: ['moody', 'fog'], favorites: true }; + const full = { q: 'castle', category: 'Portraits', sort: 'name-desc', source: '2', folder: 'a/b', media: 'video', tags: ['moody', 'fog'], favorites: true, collection: '' }; assert.deepEqual(normalizeViewState('images', full), full); + // #331 D140: a collection id is '' or 1 to 9 digits without a leading zero; an old view without the key reads ''. + for (const collection of ['1', '12', '999999999']) { + assert.equal(normalizeViewState('images', { collection }).collection, collection); + } + for (const collection of ['0', 'x', 12, '012', '1234567890', null]) { + assert.equal(normalizeViewState('images', { collection }).collection, ''); + } + // Favorites and a collection are never both a view's filter: the collection wins, as the listing's URL does. + assert.deepEqual(normalizeViewState('images', { ...full, collection: '4' }), { ...full, favorites: false, collection: '4' }); // D51: favorites is a boolean; anything else is the whole listing. for (const favorites of ['true', 1, null]) assert.equal(normalizeViewState('images', { ...full, favorites }).favorites, false); // Each rejected value falls back on its own; the rest of the view still applies. @@ -63,11 +72,17 @@ test('the listing keeps usable views in order and normalizes their state', async test('page filters and saved states convert both ways', async () => { const { imagesFilters, imagesViewState, libraryFilters, libraryViewState } = await source; - const filters = { q: 'castle', category: 'Portraits', sort: 'rating', root: '1', subfolder: 'x', media: 'image', tags: ['fog'], favorites: true }; + const filters = { q: 'castle', category: 'Portraits', sort: 'rating', root: '1', subfolder: 'x', media: 'image', tags: ['fog'], favorites: true, collection: '' }; const state = imagesViewState(filters); - assert.deepEqual(state, { q: 'castle', category: 'Portraits', sort: 'rating', source: '1', folder: 'x', media: 'image', tags: ['fog'], favorites: true }); + assert.deepEqual(state, { q: 'castle', category: 'Portraits', sort: 'rating', source: '1', folder: 'x', media: 'image', tags: ['fog'], favorites: true, collection: '' }); assert.deepEqual(imagesFilters(state), filters); - assert.deepEqual(imagesFilters({}), { q: '', category: '', sort: 'newest', root: 'all', subfolder: '', media: 'all', tags: [], favorites: false }); + assert.deepEqual(imagesFilters({}), { q: '', category: '', sort: 'newest', root: 'all', subfolder: '', media: 'all', tags: [], favorites: false, collection: '' }); + // #331 D140: the open collection round-trips; a missing one is ''. + const inCollection = { ...filters, favorites: false, collection: '7' }; + assert.equal(imagesViewState(inCollection).collection, '7'); + assert.deepEqual(imagesFilters(imagesViewState(inCollection)), inCollection); + assert.equal(imagesViewState({ ...filters, collection: undefined }).collection, ''); + assert.equal(imagesViewState({ ...filters, collection: 7 }).collection, '7'); assert.equal(imagesViewState({ ...filters, favorites: undefined }).favorites, false); const shelf = { text: 'neon', category: 'Cities', sort: 'oldest', tags: ['night'] }; assert.deepEqual(libraryViewState(shelf), { q: 'neon', category: 'Cities', sort: 'oldest', tags: ['night'] }); diff --git a/tests/test_saved_views.py b/tests/test_saved_views.py index a92d09a2..662e7573 100644 --- a/tests/test_saved_views.py +++ b/tests/test_saved_views.py @@ -40,6 +40,7 @@ IMAGES_STATE = { "media": "all", "tags": ["fluffy"], "favorites": False, + "collection": "", } LIBRARY_STATE = {"q": "", "category": "", "sort": "rating", "tags": []} IMAGES_DEFAULTS = { @@ -51,6 +52,7 @@ IMAGES_DEFAULTS = { "media": "all", "tags": [], "favorites": False, + "collection": "", } LIBRARY_DEFAULTS = {"q": "", "category": "", "sort": "newest", "tags": []} # A row saved by an older build: no favorites key, a retired key, a sort @@ -230,6 +232,48 @@ class TestSavedViewsApi(ImageAPITestCase): _, listed = await self.send("GET") self.assertEqual(len(listed["views"]), 20) + async def test_write_requires_the_collection_key(self): + # #331 D140: a view saved on a collection page keeps the collection id. + without = {k: v for k, v in IMAGES_STATE.items() if k != "collection"} + good = {"page": "images", "name": "Grid", "state": IMAGES_STATE} + cases = [ + {**good, "state": without}, + {**good, "state": {**IMAGES_STATE, "collection": "0"}}, + {**good, "state": {**IMAGES_STATE, "collection": "x"}}, + {**good, "state": {**IMAGES_STATE, "collection": 12}}, + {**good, "state": {**IMAGES_STATE, "collection": "012"}}, + {**good, "state": {**IMAGES_STATE, "collection": "1" * 10}}, + {**good, "state": {**IMAGES_STATE, "collection": None}}, + # Favorites and a collection are never both a view's filter. + { + **good, + "state": {**IMAGES_STATE, "favorites": True, "collection": "3"}, + }, + ] + for body in cases: + with self.subTest(body=body): + response, data = await self.send("POST", body=body) + self.assertEqual(response.status, 400, data) + self.assertFalse(data["success"]) + for collection in ("1", "12", "9" * 9): + view = await self.create(state={"collection": collection}) + self.assertEqual(view["state"]["collection"], collection) + self.assertIs(view["state"]["favorites"], False) + + async def test_an_old_row_without_a_collection_reads_empty(self): + with self.api.db.model.get_connection() as conn: + conn.execute( + f"INSERT INTO {TABLE} (page, name, state, position, created_at, " + "updated_at) VALUES ('images', 'Old', ?, 0, 'x', 'x')", + (json.dumps({"q": "cat", "favorites": True}),), + ) + _, data = await self.send("GET") + (view,) = data["views"] + self.assertEqual( + view["state"], {**IMAGES_DEFAULTS, "q": "cat", "favorites": True} + ) + self.assertEqual(view["state"]["collection"], "") + async def test_rename_and_delete_validate_id_and_body(self): view = await self.create() for path in ("/0", "/-1", "/abc", "/1.5", f"/{2**63}"): @@ -618,6 +662,17 @@ class TestSavedViewsSchema(unittest.TestCase): ), {**IMAGES_DEFAULTS, "folder": "kept"}, ) + # A stored row with both filters (never written by this build) opens the + # collection, as a listing URL with both does. + self.assertEqual( + normalize_state("images", {"favorites": True, "collection": "4"}), + {**IMAGES_DEFAULTS, "collection": "4"}, + ) + for bad in ("0", "x", 12, "012"): + with self.subTest(collection=bad): + self.assertEqual( + normalize_state("images", {"collection": bad}), IMAGES_DEFAULTS + ) self.assertEqual(normalize_state("history", {}), {}) def test_validation_errors_are_saved_view_invalid(self): diff --git a/web/js/workspace/saved-views.mjs b/web/js/workspace/saved-views.mjs index a179f523..a14f6d91 100644 --- a/web/js/workspace/saved-views.mjs +++ b/web/js/workspace/saved-views.mjs @@ -41,6 +41,8 @@ const FIELDS = Object.freeze({ folder: text(1024), media: choice(['all', 'image', 'video'], 'all'), tags, // A view saved on Favorites (#249 D51) reopens Images filtered to favorites. favorites: { fallback: false, accepts: value => typeof value === 'boolean' }, + // #331 D140: a view saved on a collection page reopens that collection ('' or 1 to 9 digits, no leading zero). + collection: { fallback: '', accepts: value => typeof value === 'string' && /^(|[1-9][0-9]{0,8})$/.test(value) }, }), library: Object.freeze({ q: text(500), category: text(100), sort: choice(['newest', 'oldest', 'name', 'rating'], 'newest'), tags, @@ -54,10 +56,13 @@ export function normalizeViewState(page, state) { const fields = FIELDS[page]; if (!fields) throw new TypeError(`Unknown saved-view page: ${page}`); const source = state && typeof state === 'object' && !Array.isArray(state) ? state : {}; - return Object.fromEntries(Object.entries(fields).map(([key, field]) => { + const normalized = Object.fromEntries(Object.entries(fields).map(([key, field]) => { const value = field.accepts(source[key]) ? source[key] : field.fallback; return [key, Array.isArray(value) ? [...value] : value]; })); + // Favorites and a collection are never both a view's filter: the collection wins, as the listing does (D140). + if (normalized.favorites === true && normalized.collection) normalized.favorites = false; + return normalized; } /** True when the state's compact JSON fits the API's 4 KB cap (the server's encoding: no spaces, UTF-8). */ @@ -96,11 +101,12 @@ export const savedViewHash = id => `#/saved/${id}`; export function imagesFilters(state) { const view = normalizeViewState('images', state); return { q: view.q, category: view.category, sort: view.sort, root: view.source, subfolder: view.folder, media: view.media, - tags: view.tags, favorites: view.favorites }; + tags: view.tags, favorites: view.favorites, collection: view.collection }; } export function imagesViewState(filters) { return normalizeViewState('images', { q: filters.q, category: filters.category, sort: filters.sort, source: filters.root, - folder: filters.subfolder, media: filters.media, tags: filters.tags, favorites: filters.favorites === true }); + folder: filters.subfolder, media: filters.media, tags: filters.tags, favorites: filters.favorites === true, + collection: filters.collection == null ? '' : String(filters.collection) }); } /** Library shelf filters (library.mjs `filters`) from a saved state, and the state of the current filters. */