From 836a2f89ac542cdf52176d9725511221e6afe72a Mon Sep 17 00:00:00 2001 From: Lokesh Dhakar Date: Tue, 16 Jun 2026 23:03:09 -0700 Subject: [PATCH 1/6] Phase 0: make availability+language a durable global reading preference Foundations for unifying filter/sort across listing surfaces. Storage: availability and language move from per-session sessionStorage to cross-session localStorage, re-scoped from surface-specific keys (ol-header-search-*) to a global reading-preference namespace (ol-reading-pref-*). The read/write helpers are centralized in constants.js (readStoredAvailability/writeStoredAvailability/writeStoredLanguages) and shared by the header modal and the /search filter row, so every surface persists the preference identically. This makes availability + language durable across visits and ready to be inherited by author and other listing pages (Phase 1+). De-dup guard: AVAILABILITY_TO_PARAMS is mirrored in code.py and constants.js; test_availability_sync.py now parses both from source and fails CI on any drift, replacing the bare 'keep in sync' comment. --- .../plugins/openlibrary/js/SearchFilterBar.js | 52 ++++----- .../js/search-modal/SearchModal.js | 17 ++- .../openlibrary/js/search-modal/constants.js | 68 ++++++++--- openlibrary/plugins/worksearch/code.py | 4 +- .../tests/test_availability_sync.py | 106 ++++++++++++++++++ tests/unit/js/searchModalConstants.test.js | 70 ++++++++++-- 6 files changed, 256 insertions(+), 61 deletions(-) create mode 100644 openlibrary/plugins/worksearch/tests/test_availability_sync.py diff --git a/openlibrary/plugins/openlibrary/js/SearchFilterBar.js b/openlibrary/plugins/openlibrary/js/SearchFilterBar.js index d9f29ee9642..6c4ae8944d1 100644 --- a/openlibrary/plugins/openlibrary/js/SearchFilterBar.js +++ b/openlibrary/plugins/openlibrary/js/SearchFilterBar.js @@ -2,23 +2,23 @@ * Wires the availability toggle + language filter popover on the search results * page (openlibrary/templates/work_search.html). They render empty from the * template; this module seeds them with the current selection, navigates to an - * updated /search URL when a filter changes, and keeps the cross-page - * sticky-filter state in sessionStorage so the header search modal and the - * search-page filters stay in sync. + * updated /search URL when a filter changes, and keeps the durable reading + * preference (availability + language) in localStorage so the header search + * modal, the search-page filters, and other listing surfaces stay in sync. * * Persistence model — URL is the source of truth on /search: * * - On init, if the URL carries any filter param (availability params or - * `language`), sessionStorage is mirrored from the URL. This way the modal + * `language`), the stored preference is mirrored from the URL. This way the modal * will reflect a filter change made via the toggle, the language popover, or * the sidebar language facet (which navigates the page with a new `language=` * param) the next time it opens. * - * - On init, if the URL carries NO filter params and sessionStorage has a - * non-default value, we replace-navigate to /search with those sticky + * - On init, if the URL carries NO filter params and the stored preference has + * a non-default value, we replace-navigate to /search with those sticky * filters applied. This handles arriving at /search from a search box * submit on another page or from `?q=foo` typed straight into the address - * bar — the user gets the filters they last set in this session. + * bar — the user gets the filters they last set, including on a prior visit. * * The full language catalogue is fetched lazily on first popover open. */ @@ -27,12 +27,11 @@ import { AVAILABILITY_TO_PARAMS, DEFAULT_AVAILABILITY, DEFAULT_LANGUAGE_OPTIONS, - SS_AVAILABILITY_KEY, - SS_LANGUAGES_KEY, - ssGet, - ssSet, availabilityFromParams, + readStoredAvailability, readStoredLanguages, + writeStoredAvailability, + writeStoredLanguages, } from './search-modal/constants.js'; import { fetchLanguageOptions } from './search-modal/languages.js'; @@ -43,14 +42,6 @@ const AVAILABILITY_PARAM_KEYS = [ ...new Set(Object.values(AVAILABILITY_TO_PARAMS).flatMap(Object.keys)), ]; -function writeStoredAvailability(value) { - ssSet(SS_AVAILABILITY_KEY, value || DEFAULT_AVAILABILITY); -} - -function writeStoredLanguages(values) { - ssSet(SS_LANGUAGES_KEY, JSON.stringify(values || [])); -} - // ── URL / sticky-filter helpers ──────────────────────────────────────────── function urlHasAnyFilterParam(params) { @@ -60,18 +51,19 @@ function urlHasAnyFilterParam(params) { } /** - * Mirror the current URL's filter state to sessionStorage so the modal opens - * with the same selection next time. We always write both keys so removing a - * filter via the popovers/sidebar clears the stored value too. + * Mirror the current URL's filter state to the stored preference so the modal + * (and other surfaces) open with the same selection next time. We always write + * both keys so removing a filter via the popovers/sidebar clears the stored + * value too. */ -function syncSessionStorageFromUrl(params) { +function syncStoredPrefFromUrl(params) { writeStoredAvailability(availabilityFromParams(name => params.get(name))); writeStoredLanguages(params.getAll('language')); } /** - * If the URL has no filter params at all and sessionStorage has a non-default - * value, replace-navigate to /search with the sticky filters applied. Returns + * If the URL has no filter params at all and the stored preference has a + * non-default value, replace-navigate to /search with the sticky filters applied. Returns * true when a navigation was kicked off (caller should stop further init). * * `replace` is used so the unfiltered URL doesn't end up in the back-stack. @@ -79,7 +71,7 @@ function syncSessionStorageFromUrl(params) { function maybeApplyStickyFilters(params) { if (urlHasAnyFilterParam(params)) return false; - const storedAvail = ssGet(SS_AVAILABILITY_KEY) || DEFAULT_AVAILABILITY; + const storedAvail = readStoredAvailability(); const storedLangs = readStoredLanguages(); if (storedAvail === DEFAULT_AVAILABILITY && storedLangs.length === 0) { return false; @@ -118,10 +110,10 @@ export function initSearchFilterBar(container) { // about to unload. if (maybeApplyStickyFilters(currentParams)) return; - // URL is now the source of truth — mirror it into sessionStorage so the - // modal sees the same filters next time it opens. Has to run *before* the - // popovers are seeded so a stale sessionStorage doesn't leak into them. - syncSessionStorageFromUrl(currentParams); + // URL is now the source of truth — mirror it into the stored preference so + // the modal sees the same filters next time it opens. Has to run *before* the + // popovers are seeded so a stale stored value doesn't leak into them. + syncStoredPrefFromUrl(currentParams); const availabilityEl = container.querySelector('ol-toggle'); const languageEl = container.querySelector('ol-select-popover'); diff --git a/openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js b/openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js index 62011b06200..9fc66d07cf3 100644 --- a/openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js +++ b/openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js @@ -14,14 +14,13 @@ import { DEFAULT_AVAILABILITY, DEFAULT_LANGUAGE_OPTIONS, DEFAULT_SEARCH_MODAL_STRINGS, - SS_AVAILABILITY_KEY, - SS_LANGUAGES_KEY, - ssGet, - ssSet, availabilityOptionsFromElement, readableLanguageMismatch, readableEditionLanguages, + readStoredAvailability, readStoredLanguages, + writeStoredAvailability, + writeStoredLanguages, searchModalStringsFromElement, siteLanguageToMarc, readRecentSearches, @@ -899,7 +898,7 @@ export class SearchModal extends LitElement { // explicit stored 'readable'; everything else — no preference, or a // legacy 'open'/'borrowable' value from before the toggle — collapses to // the default 'all' (toggle off). - const _storedAvailability = ssGet(SS_AVAILABILITY_KEY); + const _storedAvailability = readStoredAvailability(); this._availability = _storedAvailability === 'readable' ? 'readable' : DEFAULT_AVAILABILITY; this._languages = readStoredLanguages(); @@ -1555,21 +1554,21 @@ export class SearchModal extends LitElement { _onLanguagesChange(e) { this._languages = [...e.detail.selected]; - ssSet(SS_LANGUAGES_KEY, JSON.stringify(this._languages)); + writeStoredLanguages(this._languages); this._refetchIfActive(); } _setAvailability(value) { this._availability = value; - ssSet(SS_AVAILABILITY_KEY, value); + writeStoredAvailability(value); this._refetchIfActive(); } _clearAllFilters() { this._availability = DEFAULT_AVAILABILITY; this._languages = []; - ssSet(SS_AVAILABILITY_KEY, DEFAULT_AVAILABILITY); - ssSet(SS_LANGUAGES_KEY, JSON.stringify([])); + writeStoredAvailability(DEFAULT_AVAILABILITY); + writeStoredLanguages([]); this._refetchIfActive(); } diff --git a/openlibrary/plugins/openlibrary/js/search-modal/constants.js b/openlibrary/plugins/openlibrary/js/search-modal/constants.js index 5adcf729549..5d20591c788 100644 --- a/openlibrary/plugins/openlibrary/js/search-modal/constants.js +++ b/openlibrary/plugins/openlibrary/js/search-modal/constants.js @@ -125,6 +125,9 @@ export function searchModalStringsFromElement(el) { * Shared by the header modal and the search-page filter row so both produce * identical filters. The param names match WorkSearchScheme.facet_rewrites * (`public_scan`, `print_disabled`, `has_fulltext`). + * + * Mirrored server-side in openlibrary/plugins/worksearch/code.py; the two copies + * are kept in lockstep by openlibrary/plugins/worksearch/tests/test_availability_sync.py. */ export const AVAILABILITY_TO_PARAMS = { all: {}, @@ -340,30 +343,69 @@ export function readableEditionLanguages({ edition, languages, options }) { } /** - * sessionStorage keys for per-session filter persistence. + * Reading-preference storage keys. + * + * Availability and language are a durable, cross-session *reading preference* — + * "what I can read" and "what language I read in" — not per-search scope. They + * live in localStorage (survive a closed tab) and are shared by every listing + * surface that renders the filter controls: the header search modal, the + * /search filter row, and (Phase 1+) author and other listing pages. Facets + * (subject, publisher, year, …) are deliberately NOT stored here — they're + * scope, kept in the URL only. + * + * The keys are intentionally un-prefixed by surface ("reading-pref", not + * "header-search") because the value is global, owned by no single surface. */ -export const SS_AVAILABILITY_KEY = 'ol-header-search-availability'; -export const SS_LANGUAGES_KEY = 'ol-header-search-languages'; +export const LS_AVAILABILITY_KEY = 'ol-reading-pref-availability'; +export const LS_LANGUAGES_KEY = 'ol-reading-pref-languages'; /** - * sessionStorage read/write that swallow access errors (private browsing, quota, - * disabled storage). `ssGet` returns null on failure; `ssSet` is a no-op. Shared - * by the header modal and the search-page filter row so both persist filter - * state the same way. + * localStorage read/write that swallow access errors (private browsing, quota, + * disabled storage). `lsGet` returns null on failure; `lsSet` is a no-op. * * @param {string} key * @returns {string|null} */ -export function ssGet(key) { - try { return sessionStorage.getItem(key); } catch { return null; } +export function lsGet(key) { + try { return localStorage.getItem(key); } catch { return null; } } /** * @param {string} key * @param {string} value */ -export function ssSet(key, value) { - try { sessionStorage.setItem(key, value); } catch { /* ignore */ } +export function lsSet(key, value) { + try { localStorage.setItem(key, value); } catch { /* ignore */ } +} + +/** + * Read the stored availability preference, falling back to DEFAULT_AVAILABILITY + * when unset or unreadable. The single read path shared by every surface. + * + * @returns {string} + */ +export function readStoredAvailability() { + return lsGet(LS_AVAILABILITY_KEY) || DEFAULT_AVAILABILITY; +} + +/** + * Persist the availability preference. A falsy value is coerced to the default + * so an empty write clears the preference rather than storing "". + * + * @param {string} value + */ +export function writeStoredAvailability(value) { + lsSet(LS_AVAILABILITY_KEY, value || DEFAULT_AVAILABILITY); +} + +/** + * Persist the language preference as a JSON array of MARC codes. The single + * write path shared by every surface; `readStoredLanguages` is its inverse. + * + * @param {string[]} values + */ +export function writeStoredLanguages(values) { + lsSet(LS_LANGUAGES_KEY, JSON.stringify(values || [])); } /** @@ -420,7 +462,7 @@ export function removeRecentSearch(query) { } /** - * Read the language list from sessionStorage. Guards against missing values, + * Read the language preference from localStorage. Guards against missing values, * unparseable JSON, and values that parse to a non-array (e.g. a previously * stored object or string), any of which would otherwise leave callers with a * non-iterable or character-iterable value. Non-string entries are dropped so a @@ -429,7 +471,7 @@ export function removeRecentSearch(query) { * @returns {string[]} */ export function readStoredLanguages() { - const raw = ssGet(SS_LANGUAGES_KEY); + const raw = lsGet(LS_LANGUAGES_KEY); if (!raw) return []; try { const parsed = JSON.parse(raw); diff --git a/openlibrary/plugins/worksearch/code.py b/openlibrary/plugins/worksearch/code.py index f3fa788a366..dd0e7292d2b 100644 --- a/openlibrary/plugins/worksearch/code.py +++ b/openlibrary/plugins/worksearch/code.py @@ -121,7 +121,9 @@ def get_facet_map() -> tuple[tuple[str, str]]: # Server-side mirror of AVAILABILITY_TO_PARAMS in -# openlibrary/plugins/openlibrary/js/search-modal/constants.js. Keep in sync. +# openlibrary/plugins/openlibrary/js/search-modal/constants.js. Keep in sync — +# drift between the two copies is caught by +# openlibrary/plugins/worksearch/tests/test_availability_sync.py. # The keys are the user-facing availability "value" the header modal and the # search-page filter row use; the values are the Solr filter params they # materialize as in the URL. diff --git a/openlibrary/plugins/worksearch/tests/test_availability_sync.py b/openlibrary/plugins/worksearch/tests/test_availability_sync.py new file mode 100644 index 00000000000..92a0b580787 --- /dev/null +++ b/openlibrary/plugins/worksearch/tests/test_availability_sync.py @@ -0,0 +1,106 @@ +"""Guards against drift between the two copies of the availability taxonomy. + +`AVAILABILITY_TO_PARAMS` is defined twice — once in Python +(`openlibrary/plugins/worksearch/code.py`, used to render filtered URLs and +detect the active availability server-side) and once in JavaScript +(`openlibrary/plugins/openlibrary/js/search-modal/constants.js`, used by the +header search modal and the /search filter row to build the same URLs +client-side without a round-trip). The two genuinely run in different runtimes, +so a single shared literal isn't practical; this test makes any divergence fail +CI instead of relying on the "keep in sync" comment. + +Both files are parsed from source (no imports) so the test stays cheap and free +of the infogami/web runtime the worksearch code otherwise needs. +""" + +import ast +import json +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).parents[4] +PY_SOURCE = REPO_ROOT / "openlibrary" / "plugins" / "worksearch" / "code.py" +JS_SOURCE = REPO_ROOT / "openlibrary" / "plugins" / "openlibrary" / "js" / "search-modal" / "constants.js" + + +def _python_availability_to_params() -> dict[str, dict[str, str]]: + """Extract the AVAILABILITY_TO_PARAMS literal from code.py via the AST, so we + read the real value without importing the module (which pulls infogami).""" + tree = ast.parse(PY_SOURCE.read_text()) + for node in ast.walk(tree): + # The Python copy carries a type annotation, so it's an AnnAssign + # (`NAME: type = {...}`) rather than a plain Assign — handle both. + targets: list[ast.expr] + if isinstance(node, ast.AnnAssign): + targets = [node.target] + elif isinstance(node, ast.Assign): + targets = node.targets + else: + continue + if node.value is not None and any(isinstance(t, ast.Name) and t.id == "AVAILABILITY_TO_PARAMS" for t in targets): + return ast.literal_eval(node.value) + raise AssertionError("AVAILABILITY_TO_PARAMS not found in code.py") + + +def _extract_js_literal(source: str, name: str) -> str: + """Return the bracket-balanced literal (`{...}` object or `[...]` array) + assigned to `export const `.""" + start = source.index(f"export const {name}") + opens = {"{": "}", "[": "]"} + open_idx = min( + (source.index(ch, start) for ch in opens if ch in source[start:]), + default=-1, + ) + if open_idx < 0: + raise AssertionError(f"No literal found for {name} in constants.js") + open_ch = source[open_idx] + close_ch = opens[open_ch] + depth = 0 + for i in range(open_idx, len(source)): + if source[i] == open_ch: + depth += 1 + elif source[i] == close_ch: + depth -= 1 + if depth == 0: + return source[open_idx : i + 1] + raise AssertionError(f"Unbalanced brackets extracting {name} from constants.js") + + +def _js_literal_to_python(literal: str): + """Convert a small JS object/array literal (the availability config) to a + Python value: strip line comments, quote bare keys, swap quote style, drop + trailing commas, then json.loads. Deliberately narrow — it only needs to + handle the stable, comment-annotated availability literals.""" + no_comments = re.sub(r"//[^\n]*", "", literal) + quoted_keys = re.sub(r"([{,]\s*)([A-Za-z_]\w*)(\s*):", r'\1"\2"\3:', no_comments) + double_quoted = quoted_keys.replace("'", '"') + no_trailing = re.sub(r",(\s*[}\]])", r"\1", double_quoted) + return json.loads(no_trailing) + + +def _js_availability_to_params() -> dict[str, dict[str, str]]: + source = JS_SOURCE.read_text() + return _js_literal_to_python(_extract_js_literal(source, "AVAILABILITY_TO_PARAMS")) + + +def _js_availability_option_values() -> list[str]: + source = JS_SOURCE.read_text() + array = _js_literal_to_python(_extract_js_literal(source, "AVAILABILITY_OPTIONS")) + return [opt["value"] for opt in array] + + +def test_availability_to_params_python_matches_js(): + """The param mappings must be identical across the two runtimes.""" + assert _python_availability_to_params() == _js_availability_to_params() + + +def test_availability_value_set_is_consistent(): + """The availability values the UI offers (AVAILABILITY_OPTIONS) must be + exactly the keys the param mapping knows how to materialize, on both sides — + no orphaned option that maps to nothing, no param key with no UI value.""" + py_params = _python_availability_to_params() + js_params = _js_availability_to_params() + option_values = _js_availability_option_values() + + assert set(option_values) == set(py_params) + assert set(option_values) == set(js_params) diff --git a/tests/unit/js/searchModalConstants.test.js b/tests/unit/js/searchModalConstants.test.js index de2cd514443..872827fbb51 100644 --- a/tests/unit/js/searchModalConstants.test.js +++ b/tests/unit/js/searchModalConstants.test.js @@ -1,14 +1,17 @@ import { AVAILABILITY_OPTIONS, + DEFAULT_AVAILABILITY, DEFAULT_SEARCH_MODAL_STRINGS, + LS_AVAILABILITY_KEY, + LS_LANGUAGES_KEY, LS_RECENT_SEARCHES_KEY, RECENT_SEARCHES_MAX, - SS_LANGUAGES_KEY, availabilityFromParams, availabilityOptionsFromElement, languageNameFromOptions, localizeAvailabilityOptions, readRecentSearches, + readStoredAvailability, readStoredLanguages, readableEditionLanguages, readableLanguageMismatch, @@ -16,6 +19,8 @@ import { saveRecentSearch, searchModalStringsFromElement, siteLanguageToMarc, + writeStoredAvailability, + writeStoredLanguages, } from '../../../openlibrary/plugins/openlibrary/js/search-modal/constants'; describe('localizeAvailabilityOptions', () => { @@ -297,9 +302,9 @@ describe('recent searches (localStorage)', () => { }); }); -describe('readStoredLanguages (sessionStorage)', () => { +describe('readStoredLanguages (localStorage)', () => { beforeEach(() => { - sessionStorage.clear(); + localStorage.clear(); jest.restoreAllMocks(); }); @@ -308,31 +313,80 @@ describe('readStoredLanguages (sessionStorage)', () => { }); test('returns the stored array of codes', () => { - sessionStorage.setItem(SS_LANGUAGES_KEY, JSON.stringify(['eng', 'fre'])); + localStorage.setItem(LS_LANGUAGES_KEY, JSON.stringify(['eng', 'fre'])); expect(readStoredLanguages()).toEqual(['eng', 'fre']); }); test('returns [] when the parsed value is not an array', () => { - sessionStorage.setItem(SS_LANGUAGES_KEY, JSON.stringify('eng')); + localStorage.setItem(LS_LANGUAGES_KEY, JSON.stringify('eng')); expect(readStoredLanguages()).toEqual([]); }); test('drops non-string entries so a corrupt value cannot leak a bogus filter', () => { - sessionStorage.setItem(SS_LANGUAGES_KEY, JSON.stringify(['eng', 1, null, 'spa'])); + localStorage.setItem(LS_LANGUAGES_KEY, JSON.stringify(['eng', 1, null, 'spa'])); expect(readStoredLanguages()).toEqual(['eng', 'spa']); }); test('returns [] on unparseable JSON', () => { - sessionStorage.setItem(SS_LANGUAGES_KEY, '{nope'); + localStorage.setItem(LS_LANGUAGES_KEY, '{nope'); expect(readStoredLanguages()).toEqual([]); }); - test('returns [] when sessionStorage.getItem throws (private browsing)', () => { + test('returns [] when localStorage.getItem throws (private browsing)', () => { jest.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { throw new Error('denied'); }); expect(readStoredLanguages()).toEqual([]); }); }); +describe('availability/language preference round-trip (localStorage)', () => { + beforeEach(() => { + localStorage.clear(); + jest.restoreAllMocks(); + }); + + test('readStoredAvailability falls back to the default when unset', () => { + expect(readStoredAvailability()).toBe(DEFAULT_AVAILABILITY); + }); + + test('writeStoredAvailability round-trips a value', () => { + writeStoredAvailability('readable'); + expect(localStorage.getItem(LS_AVAILABILITY_KEY)).toBe('readable'); + expect(readStoredAvailability()).toBe('readable'); + }); + + test('writeStoredAvailability coerces a falsy value to the default', () => { + writeStoredAvailability(''); + expect(localStorage.getItem(LS_AVAILABILITY_KEY)).toBe(DEFAULT_AVAILABILITY); + }); + + test('writeStoredLanguages round-trips through readStoredLanguages', () => { + writeStoredLanguages(['eng', 'fre']); + expect(localStorage.getItem(LS_LANGUAGES_KEY)).toBe(JSON.stringify(['eng', 'fre'])); + expect(readStoredLanguages()).toEqual(['eng', 'fre']); + }); + + test('writeStoredLanguages treats a nullish list as empty', () => { + writeStoredLanguages(null); + expect(readStoredLanguages()).toEqual([]); + }); + + test('the preference persists across a simulated session boundary', () => { + // localStorage (not sessionStorage) is what makes availability + language + // durable across visits — the whole point of the Phase 0 migration. + writeStoredAvailability('readable'); + writeStoredLanguages(['spa']); + // A new page load reads the same backing store; nothing is cleared. + expect(readStoredAvailability()).toBe('readable'); + expect(readStoredLanguages()).toEqual(['spa']); + }); + + test('write helpers are no-ops when localStorage.setItem throws', () => { + jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { throw new Error('denied'); }); + expect(() => writeStoredAvailability('readable')).not.toThrow(); + expect(() => writeStoredLanguages(['eng'])).not.toThrow(); + }); +}); + describe('readableEditionLanguages', () => { const opts = [ { value: 'fre', label: 'French' }, From fafea46f7d98cd2a483915a4925b9d7064b8b45d Mon Sep 17 00:00:00 2001 From: Lokesh Dhakar Date: Tue, 16 Jun 2026 23:08:42 -0700 Subject: [PATCH 2/6] Phase 1 (server): let author works filter by availability + language MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit works_by_author now accepts an availability value ('all'/'readable'/'borrowable'/ 'open') and a list of language codes, merging them into the Solr param dict exactly as /search does — availability via the shared AVAILABILITY_TO_PARAMS (rewritten into ebook_access filters by run_solr_query), language as an OR'd fq. 'all' with no languages leaves the author scope unchanged, so existing behavior is preserved. Author.get_books reads these off the request: the legacy ?mode=ebooks link still maps to the readable filter, and an explicit availability filter in the URL (has_fulltext/public_scan) is detected via get_active_availability and wins. This is the server half of promoting the author page's read-only facets to a live filter bar; the template + client wiring follow. Tests cover the param merge and the unchanged default scope. --- openlibrary/plugins/upstream/models.py | 13 +++++-- openlibrary/plugins/worksearch/code.py | 13 ++++++- .../worksearch/tests/test_worksearch.py | 34 +++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/openlibrary/plugins/upstream/models.py b/openlibrary/plugins/upstream/models.py index d44aed91aef..298034e125f 100644 --- a/openlibrary/plugins/upstream/models.py +++ b/openlibrary/plugins/upstream/models.py @@ -17,7 +17,7 @@ from openlibrary.plugins.upstream import borrow from openlibrary.plugins.upstream.table_of_contents import TableOfContents from openlibrary.plugins.upstream.utils import MultiDict, get_identifier_config -from openlibrary.plugins.worksearch.code import works_by_author +from openlibrary.plugins.worksearch.code import get_active_availability, works_by_author from openlibrary.plugins.worksearch.search import get_solr from openlibrary.utils import dateutil # noqa: F401 side effects may be needed from openlibrary.utils.isbn import ( @@ -421,19 +421,28 @@ def get_olid(self): return self.key.split("/")[-1] def get_books(self, q=""): - i = web.input(sort="editions", page=1, rows=20, mode="") + i = web.input(sort="editions", page=1, rows=20, mode="", language=[]) try: # safeguard from passing zero/negative offsets to solr page = max(1, int(i.page)) except ValueError: page = 1 + # web.input gives `language` back as a list (its default is []) but a + # single ?language=eng can arrive as a scalar; normalize and drop blanks. + languages = i.language if isinstance(i.language, list) else [i.language] + languages = [lang for lang in languages if lang] return works_by_author( self.get_olid(), sort=i.sort, page=page, rows=i.rows, + # The legacy ?mode=ebooks link is the readable availability filter; + # an explicit availability filter in the URL (has_fulltext/public_scan) + # is picked up by get_active_availability and takes precedence. has_fulltext=i.mode == "ebooks", query=q, + availability=get_active_availability(i), + languages=languages, facet=True, request_label="AUTHOR_BOOKS_PAGE", ) diff --git a/openlibrary/plugins/worksearch/code.py b/openlibrary/plugins/worksearch/code.py index dd0e7292d2b..f56f83b4337 100644 --- a/openlibrary/plugins/worksearch/code.py +++ b/openlibrary/plugins/worksearch/code.py @@ -895,11 +895,22 @@ def works_by_author( facet=False, has_fulltext=False, query: str | None = None, + availability: str = "all", + languages: list[str] | None = None, request_label: SolrRequestLabel = "UNLABELLED", ): - param = {"q": query or "*:*"} + param: dict = {"q": query or "*:*"} if has_fulltext: param["has_fulltext"] = "true" + # Availability + language let an author page filter its works the same way + # /search does. AVAILABILITY_TO_PARAMS materializes the chosen availability + # ('readable'/'borrowable'/'open') into the has_fulltext/public_scan params + # run_solr_query already rewrites into ebook_access filters; `language` is a + # facet field run_solr_query turns into an (OR'd) fq. 'all' contributes + # nothing, so the default leaves the query unchanged. + param.update(AVAILABILITY_TO_PARAMS.get(availability, {})) + if languages: + param["language"] = languages result = run_solr_query( WorkSearchScheme(), diff --git a/openlibrary/plugins/worksearch/tests/test_worksearch.py b/openlibrary/plugins/worksearch/tests/test_worksearch.py index 9bd3ec315ee..e838ae537ca 100644 --- a/openlibrary/plugins/worksearch/tests/test_worksearch.py +++ b/openlibrary/plugins/worksearch/tests/test_worksearch.py @@ -7,6 +7,7 @@ _prepare_solr_query_params, get_doc, process_facet, + works_by_author, ) from openlibrary.plugins.worksearch.schemes.works import WorkSearchScheme from openlibrary.utils.request_context import RequestContextVars, req_context @@ -230,3 +231,36 @@ def run(): assert readable_param["has_fulltext"] == "true" assert "public_scan" not in readable_param assert mock_query.call_args.kwargs["rows"] == 0 + + +def test_works_by_author_merges_availability_and_languages_into_param(): + """An author page can scope its works by availability + language; the values + are merged into the param dict the same way /search expresses them, so + run_solr_query rewrites them into identical Solr filters.""" + with ( + patch( + "openlibrary.plugins.worksearch.code.run_solr_query", + return_value=web.storage(docs=[]), + ) as mock_query, + patch("openlibrary.plugins.worksearch.code.add_availability"), + ): + works_by_author("OL1A", availability="borrowable", languages=["eng", "fre"]) + param = mock_query.call_args.kwargs["param"] + # 'borrowable' == readable but not public-domain. + assert param["has_fulltext"] == "true" + assert param["public_scan"] == "false" + assert param["language"] == ["eng", "fre"] + + +def test_works_by_author_default_scope_injects_no_filters(): + """The default availability ('all') with no languages leaves a bare author + scope — no availability or language params added to the query.""" + with ( + patch( + "openlibrary.plugins.worksearch.code.run_solr_query", + return_value=web.storage(docs=[]), + ) as mock_query, + patch("openlibrary.plugins.worksearch.code.add_availability"), + ): + works_by_author("OL1A") + assert mock_query.call_args.kwargs["param"] == {"q": "*:*"} From 87ad02e77860ac2aa539f4f3b9d9b44fbbfcb617 Mon Sep 17 00:00:00 2001 From: Lokesh Dhakar Date: Tue, 16 Jun 2026 23:16:08 -0700 Subject: [PATCH 3/6] Phase 1 (UI): live filter bar on author pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promotes the author works list from a single 'Show only ebooks?' link to the shared filter bar (availability toggle + language popover), reusing SearchFilterBar.js — generalized to navigate the current page's path instead of a hardcoded /search, so the same controller drives /search and author pages. The global reading preference is inherited here: a stored 'Readable only' lands pre-applied and visibly reflected in the toggle. Guardrails per the agreed model: - Visibility: the bar shows when there's more than one work to act on, OR whenever a filter is active (so an inherited filter is always clearable). A single-work author with no filter has nothing to filter, so it's hidden. - Escape hatch: when an active/inherited filter matches zero works, an inline 'Show all N works?' clears the filters back to the full bibliography rather than implying the author has none. The search-within-author box now preserves the active availability/language/sort instead of unconditionally forcing has_fulltext. Verified against the running app: filters reach Solr (3->0 works), escape hatch fires with the real total, and the visibility rule holds across 1-work and 3-work authors. --- openlibrary/i18n/messages.pot | 21 ++++--- .../plugins/openlibrary/js/SearchFilterBar.js | 18 ++++-- openlibrary/templates/type/author/view.html | 55 +++++++++++++++---- 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/openlibrary/i18n/messages.pot b/openlibrary/i18n/messages.pot index 290c170b94b..41fac338691 100644 --- a/openlibrary/i18n/messages.pot +++ b/openlibrary/i18n/messages.pot @@ -1023,29 +1023,30 @@ msgid "Solr Editions" msgstr "" #: design/toggle.html openlibrary/plugins/worksearch/code.py -#: search/availability_i18n.html work_search.html +#: search/availability_i18n.html type/author/view.html work_search.html msgid "Readable Only" msgstr "" -#: work_search.html +#: type/author/view.html work_search.html msgid "Filter by availability" msgstr "" #: openlibrary/plugins/worksearch/code.py search/search_modal_i18n.html -#: type/edition/view.html type/work/view.html work_search.html +#: type/author/view.html type/edition/view.html type/work/view.html +#: work_search.html msgid "Language" msgstr "" -#: search/search_modal_i18n.html work_search.html +#: search/search_modal_i18n.html type/author/view.html work_search.html msgid "Search languages…" msgstr "" #: books/edit/edition.html languages/index.html search/search_modal_i18n.html -#: work_search.html +#: type/author/view.html work_search.html msgid "Languages" msgstr "" -#: work_search.html +#: type/author/view.html work_search.html msgid "Filter by language" msgstr "" @@ -1750,7 +1751,7 @@ msgstr "" msgid "Search your reading log" msgstr "" -#: account/reading_log.html type/author/view.html +#: account/reading_log.html #, python-format msgid "— Show only ebooks?" msgstr "" @@ -6907,9 +6908,13 @@ msgstr "" msgid "Search %(author)s books" msgstr "" +#: type/author/view.html +msgid "No matching works by this author with the current filters." +msgstr "" + #: type/author/view.html #, python-format -msgid "— Show everything by this author?" +msgid "Show all %(count)s works?" msgstr "" #: RelatedSubjects.html SubjectTags.html openlibrary/plugins/worksearch/code.py diff --git a/openlibrary/plugins/openlibrary/js/SearchFilterBar.js b/openlibrary/plugins/openlibrary/js/SearchFilterBar.js index 6c4ae8944d1..4e757537ee4 100644 --- a/openlibrary/plugins/openlibrary/js/SearchFilterBar.js +++ b/openlibrary/plugins/openlibrary/js/SearchFilterBar.js @@ -63,8 +63,13 @@ function syncStoredPrefFromUrl(params) { /** * If the URL has no filter params at all and the stored preference has a - * non-default value, replace-navigate to /search with the sticky filters applied. Returns - * true when a navigation was kicked off (caller should stop further init). + * non-default value, replace-navigate to the current page with the sticky + * filters applied. Returns true when a navigation was kicked off (caller should + * stop further init). + * + * The reading preference is global, so this inherits it onto whatever listing + * page hosts the bar (/search, an author page, …) — the bar's own controls then + * render the inherited state visibly, which is what makes the stickiness safe. * * `replace` is used so the unfiltered URL doesn't end up in the back-stack. */ @@ -81,20 +86,21 @@ function maybeApplyStickyFilters(params) { const mapped = AVAILABILITY_TO_PARAMS[storedAvail] || {}; Object.entries(mapped).forEach(([key, value]) => next.set(key, value)); storedLangs.forEach(code => next.append('language', code)); - window.location.replace(`/search?${next.toString()}`); + window.location.replace(`${window.location.pathname}?${next.toString()}`); return true; } /** - * Navigate to /search with the current query string mutated by `mutate`. - * Pagination is reset because the result set changes. + * Navigate to the current page with its query string mutated by `mutate`. + * Pagination is reset because the result set changes. Uses the live pathname so + * the same bar drives /search and other listing surfaces (e.g. author pages). * @param {(params: URLSearchParams) => void} mutate */ function navigateWithParams(mutate) { const params = new URLSearchParams(window.location.search); mutate(params); params.delete('page'); - window.location.assign(`/search?${params.toString()}`); + window.location.assign(`${window.location.pathname}?${params.toString()}`); } /** diff --git a/openlibrary/templates/type/author/view.html b/openlibrary/templates/type/author/view.html index 167a199db53..e9e59c6e918 100644 --- a/openlibrary/templates/type/author/view.html +++ b/openlibrary/templates/type/author/view.html @@ -117,33 +117,66 @@

$_('Add another?')

+ $# Current filter state, derived from the URL the same way /search does. + $# `active_availability` collapses the has_fulltext/public_scan params + $# into one value; languages are the selected `language=` codes. + $ author_req = input(language=[]) + $ active_availability = get_active_availability(author_req) + $ selected_languages = [lang for lang in (author_req.language if isinstance(author_req.language, list) else [author_req.language]) if lang] + $ filter_active = active_availability != 'all' or bool(selected_languages) +
- - $if (query_param('sort')): - - $if (query_param('mode')): - + $# Preserve the active filters + sort across a search-within-author + $# submit so the keyword search doesn't silently drop them. + $for k in ['has_fulltext', 'public_scan', 'print_disabled', 'sort']: + $if query_param(k): + + $for lang in selected_languages: + $:render_template("search/searchbox", q=query_param('q'), placeholder=_('Search %(author)s books', author=title))
+ $# The filter bar is wired client-side by SearchFilterBar.js (shared + $# with /search). Shown when there's more than one work to act on, or + $# whenever a filter is active so the user can always clear it — the + $# single-work, no-filter case has nothing to filter, so it's hidden. + $if filter_active or books_count > 1: +
+ + +
+
$if books_count > 1: $:render_template("search/sort_options.html", books.sort, exclude='relevance', default_sort='editions') $if books_count > 0: $:render_template("search/layout_options.html", selected_layout=layout) - - $if input(mode="everything").mode == "everything": - $if books_count > 0: - $:_('— Show only ebooks?', url=changequery(mode='ebooks')) - $else: - $:_('— Show everything by this author?', url=changequery(mode='everything'))
$:macros.OlPagination(safeint(query_param('page'), default=1), ceil(books_count / 20))
+ $# Escape hatch: an inherited/active filter that hides every work + $# would otherwise look like the author has none. Offer a one-click + $# clear back to the full bibliography, with its real size. + $if filter_active and books_count == 0: + $ clear_url = changequery(has_fulltext=None, public_scan=None, print_disabled=None, language=None, mode=None, page=None) +

+ $_('No matching works by this author with the current filters.') +
+ $:_('Show all %(count)s works?', url=clear_url, count=commify(page.get_work_count())) +

    $for doc in books.docs: $:macros.SearchResultsWork(doc, show_librarian_extras=show_librarian_extras, include_dropper=True, seq_index=loop.index0) From e17a418c38635a02218a2dee89c2a3586a7f4c7c Mon Sep 17 00:00:00 2001 From: Lokesh Dhakar Date: Tue, 16 Jun 2026 23:22:03 -0700 Subject: [PATCH 4/6] Fix author escape hatch: clear the stored preference, not just the URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'Show all N works' link cleared the filter params from the URL, but the durable global reading preference was untouched — so maybeApplyStickyFilters re-inherited it on reload and zeroed the page again (e.g. a stored French language filter). The link now carries js-clear-reading-prefs; SearchFilterBar.js wipes the stored availability + language preference on click so 'show all' sticks. Degrades cleanly without JS (no inheritance runs without JS). --- .../plugins/openlibrary/js/SearchFilterBar.js | 13 +++++++++++++ openlibrary/templates/type/author/view.html | 6 +++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/openlibrary/plugins/openlibrary/js/SearchFilterBar.js b/openlibrary/plugins/openlibrary/js/SearchFilterBar.js index 4e757537ee4..61a9b69279f 100644 --- a/openlibrary/plugins/openlibrary/js/SearchFilterBar.js +++ b/openlibrary/plugins/openlibrary/js/SearchFilterBar.js @@ -171,4 +171,17 @@ export function initSearchFilterBar(container) { }); }); } + + // Empty-state escape hatch ("Show all N works"): the link already clears the + // filter params from the URL, but the stored global preference would be + // re-inherited on reload and zero the page again. Wipe the stored preference + // here so "show all" actually sticks; the link's own href then navigates to + // the unfiltered page. + const clearLink = document.querySelector('.js-clear-reading-prefs'); + if (clearLink) { + clearLink.addEventListener('click', () => { + writeStoredAvailability(DEFAULT_AVAILABILITY); + writeStoredLanguages([]); + }); + } } diff --git a/openlibrary/templates/type/author/view.html b/openlibrary/templates/type/author/view.html index e9e59c6e918..ce52548f4eb 100644 --- a/openlibrary/templates/type/author/view.html +++ b/openlibrary/templates/type/author/view.html @@ -172,10 +172,14 @@

    $# clear back to the full bibliography, with its real size. $if filter_active and books_count == 0: $ clear_url = changequery(has_fulltext=None, public_scan=None, print_disabled=None, language=None, mode=None, page=None) + $# The js-clear-reading-prefs class lets SearchFilterBar.js wipe the + $# stored global preference on click — otherwise it'd be re-inherited + $# on reload and zero the page again. Works without JS too (no + $# inheritance happens without JS).

    $_('No matching works by this author with the current filters.')
    - $:_('Show all %(count)s works?', url=clear_url, count=commify(page.get_work_count())) + $_('Show all %(count)s works', count=commify(page.get_work_count()))?

      $for doc in books.docs: From 4a151f4f3e49e813ea79282e0e22f69ee86d6c73 Mon Sep 17 00:00:00 2001 From: Lokesh Dhakar Date: Tue, 16 Jun 2026 23:24:04 -0700 Subject: [PATCH 5/6] Render Readable Only toggle's on/off state server-side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toggle rendered in its default off state, then SearchFilterBar.js set checked=true after the JS bundle loaded — so a readable-scoped page visibly animated the toggle on after first paint. Both /search and author pages now emit `checked` when availability is active (get_active_availability(param) != 'all'), so the toggle paints in its final position with no transition; the client-side seed becomes a no-op when the state already matches. --- openlibrary/templates/type/author/view.html | 4 ++++ openlibrary/templates/work_search.html | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/openlibrary/templates/type/author/view.html b/openlibrary/templates/type/author/view.html index ce52548f4eb..cd71773410b 100644 --- a/openlibrary/templates/type/author/view.html +++ b/openlibrary/templates/type/author/view.html @@ -143,10 +143,14 @@

      $# single-work, no-filter case has nothing to filter, so it's hidden. $if filter_active or books_count > 1:
      + $# Render the on/off state server-side so the toggle paints in + $# its final position — without `checked` it starts off and the + $# client-side seed animates it on after load. $_("Search Books")

      $# rendered into the sublabel here, so it's present on first paint with no $# async flash-in or layout shift. Empty when there's nothing to count. $ readable_sublabel = commify(readable_count) if readable_count is not None else '' + $# Render the on/off state server-side so the toggle paints in its final + $# position — without `checked` it starts off and the client-side seed + $# animates it on after load.
      Date: Tue, 16 Jun 2026 23:28:53 -0700 Subject: [PATCH 6/6] Sync messages.pot with escape-hatch string change --- openlibrary/i18n/messages.pot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlibrary/i18n/messages.pot b/openlibrary/i18n/messages.pot index 41fac338691..060034726fc 100644 --- a/openlibrary/i18n/messages.pot +++ b/openlibrary/i18n/messages.pot @@ -6914,7 +6914,7 @@ msgstr "" #: type/author/view.html #, python-format -msgid "Show all %(count)s works?" +msgid "Show all %(count)s works" msgstr "" #: RelatedSubjects.html SubjectTags.html openlibrary/plugins/worksearch/code.py