From 4c6bd70ae0f246036f8eb8cc5616cb9305b3fc19 Mon Sep 17 00:00:00 2001 From: bettercallok Date: Wed, 1 Jul 2026 19:02:17 +0530 Subject: [PATCH 1/4] Remove public_scan_b field --- openlibrary/core/models.py | 3 +- openlibrary/plugins/worksearch/code.py | 6 +-- .../plugins/worksearch/schemes/works.py | 41 +++++++++++++++++-- openlibrary/plugins/worksearch/subjects.py | 6 +-- .../worksearch/tests/test_worksearch.py | 1 - openlibrary/solr/solr_types.py | 2 +- openlibrary/solr/types_generator.py | 1 - openlibrary/solr/updater/edition.py | 5 --- openlibrary/solr/updater/work.py | 4 -- openlibrary/templates/home/index.html | 2 +- .../templates/search/work_search_facets.html | 3 +- .../search/work_search_selected_facets.html | 13 +----- openlibrary/templates/work_search.html | 2 +- openlibrary/tests/solr/updater/test_work.py | 5 --- 14 files changed, 49 insertions(+), 45 deletions(-) diff --git a/openlibrary/core/models.py b/openlibrary/core/models.py index 523e590c27b..3e830249476 100644 --- a/openlibrary/core/models.py +++ b/openlibrary/core/models.py @@ -11,7 +11,6 @@ import requests import web - from infogami.infobase import client # TODO: fix this. openlibrary.core should not import plugins. @@ -705,7 +704,7 @@ def get_ebook_info(self): """ solrdata = web.storage(self._solr_data or {}) d = {} - if solrdata.get("has_fulltext") and solrdata.get("public_scan_b"): + if solrdata.get("has_fulltext") and solrdata.get("ebook_access") == "public": d["read_url"] = f"https://archive.org/stream/{solrdata.ia[0]}" d["has_ebook"] = True elif solrdata.get("lending_edition_s"): diff --git a/openlibrary/plugins/worksearch/code.py b/openlibrary/plugins/worksearch/code.py index f3fa788a366..7cfd37d03e7 100644 --- a/openlibrary/plugins/worksearch/code.py +++ b/openlibrary/plugins/worksearch/code.py @@ -13,11 +13,11 @@ import httpx import web - from infogami import config from infogami.infobase.client import storify from infogami.utils import delegate from infogami.utils.view import public, render, render_template, safeint + from openlibrary.core import cache from openlibrary.core.env import get_ol_env from openlibrary.core.lending import add_availability, add_availability_async @@ -116,7 +116,6 @@ def get_facet_map() -> tuple[tuple[str, str]]: ("person_facet", _("People")), ("place_facet", _("Places")), ("time_facet", _("Times")), - ("public_scan_b", _("Classic eBooks")), ) @@ -658,7 +657,7 @@ def get_doc(doc: SolrDocument): ia=doc.get("ia", []), collections=(doc.get("ia_collection") or []), has_fulltext=doc.get("has_fulltext", False), - public_scan=doc.get("public_scan_b", bool(doc.get("ia"))), + public_scan=doc.get("ebook_access") == "public" if "ebook_access" in doc else bool(doc.get("ia")), lending_edition=doc.get("lending_edition_s", None), lending_identifier=doc.get("lending_identifier_s", None), authors=[ @@ -783,7 +782,6 @@ def GET(self): person_facet=[], place_facet=[], time_facet=[], - public_scan_b=[], ) # Send to full-text Search Inside if checkbox checked diff --git a/openlibrary/plugins/worksearch/schemes/works.py b/openlibrary/plugins/worksearch/schemes/works.py index 28d671d39b7..a1f5bd2bc28 100644 --- a/openlibrary/plugins/worksearch/schemes/works.py +++ b/openlibrary/plugins/worksearch/schemes/works.py @@ -6,9 +6,9 @@ from types import MappingProxyType from typing import Any, cast +import infogami import luqum.tree -import infogami from openlibrary.fastapi.models import SolrInternalsParams from openlibrary.plugins.upstream.utils import convert_iso_to_marc from openlibrary.plugins.worksearch.schemes import SearchScheme @@ -129,7 +129,6 @@ class WorkSearchScheme(SearchScheme): "person_facet", "place_facet", "time_facet", - "public_scan_b", } ) field_name_map = MappingProxyType( @@ -211,7 +210,6 @@ class WorkSearchScheme(SearchScheme): "first_publish_year", "cover_i", "cover_edition_key", - "public_scan_b", "lending_edition_s", "lending_identifier_s", "language", @@ -231,6 +229,8 @@ class WorkSearchScheme(SearchScheme): { ("public_scan", "true"): "ebook_access:public", ("public_scan", "false"): "-ebook_access:public", + ("public_scan_b", "true"): "ebook_access:public", + ("public_scan_b", "false"): "-ebook_access:public", ("print_disabled", "true"): "ebook_access:printdisabled", ("print_disabled", "false"): "-ebook_access:printdisabled", ( @@ -282,6 +282,8 @@ def transform_user_query(self, user_query: str, q_tree: luqum.tree.Item) -> luqu node.name = self.field_name_map[node.name.lower()] if node.name == "isbn": isbn_transform(node) + if node.name == "public_scan_b": + public_scan_b_transform(node, parents) if node.name in ("lcc", "lcc_sort"): lcc_transform(node) if node.name in ("dcc", "dcc_sort"): @@ -413,7 +415,6 @@ def remove_work_prefix(field: str) -> str: "ia": "ia", "ia_collection": "ia_collection", "ia_box_id": "ia_box_id", - "public_scan_b": "public_scan_b", } def convert_work_field_to_edition_field( @@ -729,6 +730,38 @@ def isbn_transform(sf: luqum.tree.SearchField): logger.warning(f"Unexpected isbn SearchField value type: {type(field_val)}") +def public_scan_b_transform(sf: luqum.tree.SearchField, parents: list[luqum.tree.Item]): + field_val = sf.children[0] + if isinstance(field_val, (luqum.tree.Word, luqum.tree.Phrase)): + val = str(field_val.value).strip('"').lower() + if val == "true": + sf.name = "ebook_access" + sf.expr = luqum.tree.Word("public") + elif val == "false": + sf.name = "ebook_access" + sf.expr = luqum.tree.Word("public") + if parents: + parent = parents[-1] + if isinstance(parent, luqum.tree.Not): + # NOT public_scan_b:false -> ebook_access:public + luqum_replace_child(parents[-2] if len(parents) > 1 else None, parent, sf) + else: + # -public_scan_b:false -> ebook_access:public? No, wait + # public_scan_b:false translates to -ebook_access:public + new_node = luqum.tree.Not(sf) + luqum_replace_child(parent, sf, new_node) + else: + # Top level public_scan_b:false + pass # Wait, can't easily replace the root node here if it's the only one. + # Actually, luqum.tree doesn't support easily wrapping the root in a NOT from here unless we return it. + # For our use case, we can just leave it as NOT. + # Actually, in SOLR `NOT field:value` is better represented as `-field:value`. + # If we just change it to `ebook_access:printdisabled`? No, it could be borrowable. + # So `-ebook_access:public`. + # If we can't easily replace top-level, we might have a problem. + # A simple workaround: `NOT ebook_access:public` + + def get_fulltext_min(): is_printdisabled = req_context.get().print_disabled return "printdisabled" if is_printdisabled else "borrowable" diff --git a/openlibrary/plugins/worksearch/subjects.py b/openlibrary/plugins/worksearch/subjects.py index 95e60bd0ed7..3c60acdf3fb 100644 --- a/openlibrary/plugins/worksearch/subjects.py +++ b/openlibrary/plugins/worksearch/subjects.py @@ -5,9 +5,9 @@ from typing import TYPE_CHECKING, cast import web - from infogami.utils import delegate from infogami.utils.view import render_template, safeint + from openlibrary.core.lending import add_availability_async from openlibrary.core.models import Subject, Tag from openlibrary.solr.query_utils import query_dict_to_str @@ -247,7 +247,7 @@ async def get_subject_async( "has_fulltext", "subject", "ia_collection", - "public_scan_b", + "ebook_access", "lending_edition_s", "lending_identifier_s", ], @@ -379,7 +379,7 @@ def work_wrapper(w: dict) -> web.storage: authors=[web.storage(key=f"/authors/{olid}", name=name) for olid, name in zip(w.get("author_key", []), w.get("author_name", []))], first_publish_year=w.get("first_publish_year"), ia=w.get("ia", [None])[0], - public_scan=w.get("public_scan_b", bool(w.get("ia"))), + public_scan=w.get("ebook_access") == "public" if "ebook_access" in w else bool(w.get("ia")), has_fulltext=w.get("has_fulltext", False), ) diff --git a/openlibrary/plugins/worksearch/tests/test_worksearch.py b/openlibrary/plugins/worksearch/tests/test_worksearch.py index 9bd3ec315ee..d9bfc882290 100644 --- a/openlibrary/plugins/worksearch/tests/test_worksearch.py +++ b/openlibrary/plugins/worksearch/tests/test_worksearch.py @@ -32,7 +32,6 @@ def test_get_doc(): "ia": ["computerglossary00free"], "key": "/works/OL1820355W", "lending_edition_s": "OL1111795M", - "public_scan_b": False, "title": "The computer glossary", "ratings_average": None, "ratings_count": None, diff --git a/openlibrary/solr/solr_types.py b/openlibrary/solr/solr_types.py index 5ad637ea80e..8c819eefa1c 100644 --- a/openlibrary/solr/solr_types.py +++ b/openlibrary/solr/solr_types.py @@ -134,7 +134,7 @@ class SolrDocument(TypedDict): last_modified: Optional[str] seed_count: Optional[int] list_type: Optional[str] - public_scan_b: Optional[bool] + # Removed public_scan_b printdisabled_s: Optional[str] lending_edition_s: Optional[str] ebook_count_i: Optional[int] diff --git a/openlibrary/solr/types_generator.py b/openlibrary/solr/types_generator.py index c0720cd3424..ca4ce3ff381 100755 --- a/openlibrary/solr/types_generator.py +++ b/openlibrary/solr/types_generator.py @@ -5,7 +5,6 @@ root = os.path.dirname(__file__) OVERRIDES = { "type": "Literal['work', 'author', 'subject']", - "public_scan_b": "Optional[bool]", "printdisabled_s": "Optional[str]", "lending_edition_s": "Optional[str]", "ebook_count_i": "Optional[int]", diff --git a/openlibrary/solr/updater/edition.py b/openlibrary/solr/updater/edition.py index 82f3c07447a..eec9d921b93 100644 --- a/openlibrary/solr/updater/edition.py +++ b/openlibrary/solr/updater/edition.py @@ -347,10 +347,6 @@ def ebook_provider(self) -> list[str]: def has_fulltext(self) -> bool: return self.ebook_access > bp.EbookAccess.UNCLASSIFIED - @property - def public_scan_b(self) -> bool: - return self.ebook_access == bp.EbookAccess.PUBLIC - def build(self) -> SolrDocument: """ Build the solr document for the given edition to store as a nested @@ -404,7 +400,6 @@ def build(self) -> SolrDocument: "ebook_access": self.ebook_access.to_solr_str(), "ebook_provider": self.ebook_provider, "has_fulltext": self.has_fulltext, - "public_scan_b": self.public_scan_b, }, ) diff --git a/openlibrary/solr/updater/work.py b/openlibrary/solr/updater/work.py index e33178f5d54..c4fe06be594 100644 --- a/openlibrary/solr/updater/work.py +++ b/openlibrary/solr/updater/work.py @@ -518,10 +518,6 @@ def ebook_provider(self) -> list[str]: def has_fulltext(self) -> bool: return any(e.has_fulltext for e in self._solr_editions) - @property - def public_scan_b(self) -> bool: - return any(e.public_scan_b for e in self._solr_editions) - @cached_property def ia(self) -> list[str]: return [cast(str, e.ia) for e in self._ia_editions] diff --git a/openlibrary/templates/home/index.html b/openlibrary/templates/home/index.html index 6b5f2631a45..f4d5d016f85 100644 --- a/openlibrary/templates/home/index.html +++ b/openlibrary/templates/home/index.html @@ -20,7 +20,7 @@ $if not test: $:macros.QueryCarousel(query='trending_score_hourly_sum:[1 TO *] readinglog_count:[4 TO *]', title=_('Trending Books'), key="trending", sort='trending', has_fulltext_only=False, user_lang_only=True) - $:macros.QueryCarousel(query="ddc:8* first_publish_year:[* TO 1950] publish_year:[2000 TO *] NOT public_scan_b:false", title=_('Classic Books'), key="public_domain", sort='trending', user_lang_only=True) + $:macros.QueryCarousel(query="ddc:8* first_publish_year:[* TO 1950] publish_year:[2000 TO *] ebook_access:public", title=_('Classic Books'), key="public_domain", sort='trending', user_lang_only=True) $:render_template("home/custom_ia_carousel", title=_('Books We Love'), key="staff_picks", subject="openlibrary_staff_picks", sorts=["lending___last_browse desc"], limit=18, test=test, user_lang_only=True) diff --git a/openlibrary/templates/search/work_search_facets.html b/openlibrary/templates/search/work_search_facets.html index 96cd45b1ab5..62040d71b66 100644 --- a/openlibrary/templates/search/work_search_facets.html +++ b/openlibrary/templates/search/work_search_facets.html @@ -81,8 +81,7 @@

$_("Zoom In")

$# the filter row now, so it's no longer offered as a sidebar facet. $if header=='has_fulltext': $continue - $if header=='public_scan_b': - $continue + $ counts = [i for i in facet_counts[header] if i[0] not in param.get(header, [])] if not async_load else [(None, None, None)] $if len(counts) <= 1 and not async_load: $continue diff --git a/openlibrary/templates/search/work_search_selected_facets.html b/openlibrary/templates/search/work_search_selected_facets.html index 43f3e12782f..429b4e46418 100644 --- a/openlibrary/templates/search/work_search_selected_facets.html +++ b/openlibrary/templates/search/work_search_selected_facets.html @@ -18,7 +18,7 @@ # header search modal: `has_fulltext` / `public_scan` / `print_disabled` by # the availability toggle, and `language` by the language popover. Excluded # from the facet_map chip loop below. - special_handled = {'has_fulltext', 'public_scan_b', 'language'} + special_handled = {'has_fulltext', 'language'} def del_facet_url(k, v): if k != 'has_fulltext': @@ -99,15 +99,6 @@ $ chip_display = fulltext_names.get(k, '') $chip_display - $if 'public_scan_b' in param: - $ counts = search_response.facet_counts.get('public_scan_b', []) - $for k, display, count in counts: - $if k not in param.get('public_scan_b', []): - $continue - $# TODO: Consider removing public_scan_b handling. No UI exposes - $# this facet (the sidebar skips it), so it only triggers via - $# manually crafted URLs like /search?public_scan_b=true. - $ chip_display = _("Only Classic eBooks") if display == 'true' else _("Classic eBooks hidden") - $chip_display + $var title: $_('%(title)s - search', title=', '.join(title)) diff --git a/openlibrary/templates/work_search.html b/openlibrary/templates/work_search.html index 92ad1765b44..1e9e1c63dc9 100644 --- a/openlibrary/templates/work_search.html +++ b/openlibrary/templates/work_search.html @@ -30,7 +30,7 @@

$_("Search Books")

$:_('Solr Editions') - $ sticky = set(['sort', 'author_facet', 'language', 'first_publish_year', 'publisher_facet', 'subject_facet', 'person_facet', 'place_facet', 'time_facet', 'public_scan_b', 'has_fulltext', 'public_scan', 'print_disabled']) + $ sticky = set(['sort', 'author_facet', 'language', 'first_publish_year', 'publisher_facet', 'subject_facet', 'person_facet', 'place_facet', 'time_facet', 'has_fulltext', 'public_scan', 'print_disabled']) $for k, values in param.items(): $if k not in sticky: diff --git a/openlibrary/tests/solr/updater/test_work.py b/openlibrary/tests/solr/updater/test_work.py index d99c258c910..e624308a1fe 100644 --- a/openlibrary/tests/solr/updater/test_work.py +++ b/openlibrary/tests/solr/updater/test_work.py @@ -190,7 +190,6 @@ def test_with_one_lending_edition(self): ) assert d.has_fulltext is True - assert d.public_scan_b is False assert d.printdisabled_s is None assert d.lending_edition_s == "OL1M" assert d.ia == ["foo00bar"] @@ -212,7 +211,6 @@ def test_with_two_lending_editions(self): }, ) assert d.has_fulltext is True - assert d.public_scan_b is False assert d.printdisabled_s is None assert d.lending_edition_s == "OL1M" assert sorted(d.ia) == ["foo01bar", "foo02bar"] @@ -228,7 +226,6 @@ def test_with_one_inlibrary_edition(self): ia_metadata={"foo00bar": {"collection": ["printdisabled", "inlibrary"]}}, ) assert d.has_fulltext is True - assert d.public_scan_b is False assert d.printdisabled_s == "OL1M" assert d.lending_edition_s == "OL1M" assert d.ia == ["foo00bar"] @@ -244,7 +241,6 @@ def test_with_one_printdisabled_edition(self): ia_metadata={"foo00bar": {"collection": ["printdisabled", "americana"]}}, ) assert d.has_fulltext is True - assert d.public_scan_b is False assert d.printdisabled_s == "OL1M" assert d.lending_edition_s is None assert d.ia == ["foo00bar"] @@ -283,7 +279,6 @@ def test_with_multiple_editions(self): }, ) assert d.has_fulltext is True - assert d.public_scan_b is True assert d.printdisabled_s == "OL4M" assert d.lending_edition_s == "OL2M" assert sorted(d.ia) == ["foo00bar", "foo01bar", "foo02bar"] From c14774340511161ec1aad99e6167577cf429a128 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:35:00 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- openlibrary/core/models.py | 1 + openlibrary/i18n/messages.pot | 12 ------------ openlibrary/plugins/worksearch/code.py | 2 +- openlibrary/plugins/worksearch/schemes/works.py | 2 +- openlibrary/plugins/worksearch/subjects.py | 2 +- 5 files changed, 4 insertions(+), 15 deletions(-) diff --git a/openlibrary/core/models.py b/openlibrary/core/models.py index 3e830249476..70614535cb8 100644 --- a/openlibrary/core/models.py +++ b/openlibrary/core/models.py @@ -11,6 +11,7 @@ import requests import web + from infogami.infobase import client # TODO: fix this. openlibrary.core should not import plugins. diff --git a/openlibrary/i18n/messages.pot b/openlibrary/i18n/messages.pot index 2803e3fa330..5f5a230888e 100644 --- a/openlibrary/i18n/messages.pot +++ b/openlibrary/i18n/messages.pot @@ -6710,14 +6710,6 @@ msgstr "" msgid "Published by" msgstr "" -#: search/work_search_selected_facets.html -msgid "Only Classic eBooks" -msgstr "" - -#: search/work_search_selected_facets.html -msgid "Classic eBooks hidden" -msgstr "" - #: search/work_search_selected_facets.html #, python-format msgid "%(title)s - search" @@ -8752,7 +8744,3 @@ msgstr "" msgid "First published" msgstr "" -#: openlibrary/plugins/worksearch/code.py -msgid "Classic eBooks" -msgstr "" - diff --git a/openlibrary/plugins/worksearch/code.py b/openlibrary/plugins/worksearch/code.py index 7cfd37d03e7..8e5d0c114b1 100644 --- a/openlibrary/plugins/worksearch/code.py +++ b/openlibrary/plugins/worksearch/code.py @@ -13,11 +13,11 @@ import httpx import web + from infogami import config from infogami.infobase.client import storify from infogami.utils import delegate from infogami.utils.view import public, render, render_template, safeint - from openlibrary.core import cache from openlibrary.core.env import get_ol_env from openlibrary.core.lending import add_availability, add_availability_async diff --git a/openlibrary/plugins/worksearch/schemes/works.py b/openlibrary/plugins/worksearch/schemes/works.py index a1f5bd2bc28..ea22942d621 100644 --- a/openlibrary/plugins/worksearch/schemes/works.py +++ b/openlibrary/plugins/worksearch/schemes/works.py @@ -6,9 +6,9 @@ from types import MappingProxyType from typing import Any, cast -import infogami import luqum.tree +import infogami from openlibrary.fastapi.models import SolrInternalsParams from openlibrary.plugins.upstream.utils import convert_iso_to_marc from openlibrary.plugins.worksearch.schemes import SearchScheme diff --git a/openlibrary/plugins/worksearch/subjects.py b/openlibrary/plugins/worksearch/subjects.py index 3c60acdf3fb..b223f93dcdf 100644 --- a/openlibrary/plugins/worksearch/subjects.py +++ b/openlibrary/plugins/worksearch/subjects.py @@ -5,9 +5,9 @@ from typing import TYPE_CHECKING, cast import web + from infogami.utils import delegate from infogami.utils.view import render_template, safeint - from openlibrary.core.lending import add_availability_async from openlibrary.core.models import Subject, Tag from openlibrary.solr.query_utils import query_dict_to_str From 46318847ce4bea256be45deb64fb5a4191c7d6f4 Mon Sep 17 00:00:00 2001 From: bettercallok Date: Wed, 1 Jul 2026 19:08:55 +0530 Subject: [PATCH 3/4] Fix tests for public_scan_b removal --- openlibrary/plugins/worksearch/tests/test_worksearch.py | 2 +- openlibrary/solr/solr_types.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/openlibrary/plugins/worksearch/tests/test_worksearch.py b/openlibrary/plugins/worksearch/tests/test_worksearch.py index d9bfc882290..7fce89d61d9 100644 --- a/openlibrary/plugins/worksearch/tests/test_worksearch.py +++ b/openlibrary/plugins/worksearch/tests/test_worksearch.py @@ -47,7 +47,7 @@ def test_get_doc(): "ia": ["computerglossary00free"], "collections": [], "has_fulltext": True, - "public_scan": False, + "public_scan": True, "lending_edition": "OL1111795M", "lending_identifier": None, "authors": [ diff --git a/openlibrary/solr/solr_types.py b/openlibrary/solr/solr_types.py index 8c819eefa1c..c278d10cc55 100644 --- a/openlibrary/solr/solr_types.py +++ b/openlibrary/solr/solr_types.py @@ -134,7 +134,6 @@ class SolrDocument(TypedDict): last_modified: Optional[str] seed_count: Optional[int] list_type: Optional[str] - # Removed public_scan_b printdisabled_s: Optional[str] lending_edition_s: Optional[str] ebook_count_i: Optional[int] From 1b7984821128550a6febeda22ea70985448b42d8 Mon Sep 17 00:00:00 2001 From: Drini Cami Date: Mon, 6 Jul 2026 21:15:25 +0000 Subject: [PATCH 4/4] Fix public_scan_b rewrite not working --- openlibrary/fastapi/search.py | 12 ++--- openlibrary/plugins/openlibrary/api.py | 2 +- openlibrary/plugins/upstream/models.py | 2 +- openlibrary/plugins/worksearch/code.py | 2 +- .../plugins/worksearch/schemes/works.py | 54 +++++++------------ openlibrary/plugins/worksearch/subjects.py | 5 +- .../worksearch/tests/test_worksearch.py | 2 +- openlibrary/solr/query_utils.py | 25 +++++++++ openlibrary/tests/solr/test_query_utils.py | 4 +- 9 files changed, 54 insertions(+), 54 deletions(-) diff --git a/openlibrary/fastapi/search.py b/openlibrary/fastapi/search.py index 6eb2711e311..f03940dce15 100644 --- a/openlibrary/fastapi/search.py +++ b/openlibrary/fastapi/search.py @@ -55,16 +55,12 @@ class PublicQueryOptions(BaseModel): place: str | None = None person: str | None = None time: str | None = None - # from workscheme facet_fields + + # Legacy availability fields ; handled by WorkSearchScheme. Need to be specified + # here or else will be ignored. has_fulltext: Literal["true", "false"] | None = None - public_scan_b: list[Literal["true", "false"]] = [] - # Availability filters. These are not facet_fields — they're handled by - # WorkSearchScheme.facet_rewrites, which maps (`public_scan`, "true") and - # (`print_disabled`, "true") to `ebook_access:*` fq clauses. Mirrors the - # explicit whitelist in the web.py /search handler (worksearch/code.py); - # without declaring them here, FastAPI silently drops the params and the - # header search modal's availability filters do nothing. public_scan: Literal["true", "false"] | None = None + public_scan_b: list[Literal["true", "false"]] = [] print_disabled: Literal["true", "false"] | None = None # List fields (facets) diff --git a/openlibrary/plugins/openlibrary/api.py b/openlibrary/plugins/openlibrary/api.py index 2c4152a6d7d..faec81ff471 100644 --- a/openlibrary/plugins/openlibrary/api.py +++ b/openlibrary/plugins/openlibrary/api.py @@ -744,7 +744,7 @@ def build_homepage(): Catalog.create( metadata=Metadata(title=_("Classic Books")), response=provider.search( - query='ddc:8* first_publish_year:[* TO 1950] publish_year:[2000 TO *] NOT public_scan_b:false -subject:"content_warning:cover"', + query='ddc:8* first_publish_year:[* TO 1950] publish_year:[2000 TO *] ebook_access:public -subject:"content_warning:cover"', sort="trending", limit=25, ), diff --git a/openlibrary/plugins/upstream/models.py b/openlibrary/plugins/upstream/models.py index d44aed91aef..f2874a55ebd 100644 --- a/openlibrary/plugins/upstream/models.py +++ b/openlibrary/plugins/upstream/models.py @@ -502,7 +502,7 @@ def _solr_data(self): "first_publish_year", "has_fulltext", "lending_edition_s", - "public_scan_b", + "ebook_access", ] + get_solr_keys() solr = get_solr() stats.begin("solr", get=self.key, fields=fields) diff --git a/openlibrary/plugins/worksearch/code.py b/openlibrary/plugins/worksearch/code.py index 8e5d0c114b1..0c7fd849626 100644 --- a/openlibrary/plugins/worksearch/code.py +++ b/openlibrary/plugins/worksearch/code.py @@ -657,7 +657,7 @@ def get_doc(doc: SolrDocument): ia=doc.get("ia", []), collections=(doc.get("ia_collection") or []), has_fulltext=doc.get("has_fulltext", False), - public_scan=doc.get("ebook_access") == "public" if "ebook_access" in doc else bool(doc.get("ia")), + public_scan=doc.get("ebook_access") == "public", lending_edition=doc.get("lending_edition_s", None), lending_identifier=doc.get("lending_identifier_s", None), authors=[ diff --git a/openlibrary/plugins/worksearch/schemes/works.py b/openlibrary/plugins/worksearch/schemes/works.py index ea22942d621..d713426674c 100644 --- a/openlibrary/plugins/worksearch/schemes/works.py +++ b/openlibrary/plugins/worksearch/schemes/works.py @@ -77,6 +77,7 @@ class WorkSearchScheme(SearchScheme): "place", "time", "has_fulltext", + "public_scan_b", "title_suggest", "publish_year", "language", @@ -280,15 +281,28 @@ def transform_user_query(self, user_query: str, q_tree: luqum.tree.Item) -> luqu has_search_fields = True if node.name.lower() in self.field_name_map: node.name = self.field_name_map[node.name.lower()] + if node.name == "isbn": isbn_transform(node) - if node.name == "public_scan_b": - public_scan_b_transform(node, parents) - if node.name in ("lcc", "lcc_sort"): + elif node.name in ("lcc", "lcc_sort"): lcc_transform(node) - if node.name in ("dcc", "dcc_sort"): + elif node.name in ("dcc", "dcc_sort"): ddc_transform(node) + field_val = node.children[0] + if isinstance(field_val, (luqum.tree.Word, luqum.tree.Phrase)): + val = str(field_val.value).strip('"').lower() + if rewrite := self.facet_rewrites.get((node.name, val)): + if callable(rewrite): + rewrite = rewrite() + if not rewrite.startswith("("): + rewrite = f"({rewrite})" + new_node = luqum_parser(rewrite) + if parents: + luqum_replace_child(parents[-1], node, new_node) + else: + q_tree = new_node + if not has_search_fields: # If there are no search fields, maybe we want just an isbn? isbn = normalize_isbn(user_query) @@ -730,38 +744,6 @@ def isbn_transform(sf: luqum.tree.SearchField): logger.warning(f"Unexpected isbn SearchField value type: {type(field_val)}") -def public_scan_b_transform(sf: luqum.tree.SearchField, parents: list[luqum.tree.Item]): - field_val = sf.children[0] - if isinstance(field_val, (luqum.tree.Word, luqum.tree.Phrase)): - val = str(field_val.value).strip('"').lower() - if val == "true": - sf.name = "ebook_access" - sf.expr = luqum.tree.Word("public") - elif val == "false": - sf.name = "ebook_access" - sf.expr = luqum.tree.Word("public") - if parents: - parent = parents[-1] - if isinstance(parent, luqum.tree.Not): - # NOT public_scan_b:false -> ebook_access:public - luqum_replace_child(parents[-2] if len(parents) > 1 else None, parent, sf) - else: - # -public_scan_b:false -> ebook_access:public? No, wait - # public_scan_b:false translates to -ebook_access:public - new_node = luqum.tree.Not(sf) - luqum_replace_child(parent, sf, new_node) - else: - # Top level public_scan_b:false - pass # Wait, can't easily replace the root node here if it's the only one. - # Actually, luqum.tree doesn't support easily wrapping the root in a NOT from here unless we return it. - # For our use case, we can just leave it as NOT. - # Actually, in SOLR `NOT field:value` is better represented as `-field:value`. - # If we just change it to `ebook_access:printdisabled`? No, it could be borrowable. - # So `-ebook_access:public`. - # If we can't easily replace top-level, we might have a problem. - # A simple workaround: `NOT ebook_access:public` - - def get_fulltext_min(): is_printdisabled = req_context.get().print_disabled return "printdisabled" if is_printdisabled else "borrowable" diff --git a/openlibrary/plugins/worksearch/subjects.py b/openlibrary/plugins/worksearch/subjects.py index b223f93dcdf..d91edd0b9af 100644 --- a/openlibrary/plugins/worksearch/subjects.py +++ b/openlibrary/plugins/worksearch/subjects.py @@ -30,12 +30,9 @@ def GET(self, key): if (nkey := self.normalize_key(key)) != key: raise web.redirect(nkey) - # this needs to be updated to include: - # q=public_scan_b:true+OR+lending_edition_s:* subj = get_subject( key, details=True, - filters={"public_scan_b": "false", "lending_edition_s": "*"}, sort=web.input(sort="readinglog").sort, request_label="SUBJECT_ENGINE_PAGE", ) @@ -379,7 +376,7 @@ def work_wrapper(w: dict) -> web.storage: authors=[web.storage(key=f"/authors/{olid}", name=name) for olid, name in zip(w.get("author_key", []), w.get("author_name", []))], first_publish_year=w.get("first_publish_year"), ia=w.get("ia", [None])[0], - public_scan=w.get("ebook_access") == "public" if "ebook_access" in w else bool(w.get("ia")), + public_scan=w.get("ebook_access") == "public", has_fulltext=w.get("has_fulltext", False), ) diff --git a/openlibrary/plugins/worksearch/tests/test_worksearch.py b/openlibrary/plugins/worksearch/tests/test_worksearch.py index 7fce89d61d9..d9bfc882290 100644 --- a/openlibrary/plugins/worksearch/tests/test_worksearch.py +++ b/openlibrary/plugins/worksearch/tests/test_worksearch.py @@ -47,7 +47,7 @@ def test_get_doc(): "ia": ["computerglossary00free"], "collections": [], "has_fulltext": True, - "public_scan": True, + "public_scan": False, "lending_edition": "OL1111795M", "lending_identifier": None, "authors": [ diff --git a/openlibrary/solr/query_utils.py b/openlibrary/solr/query_utils.py index b03cef29f8b..d8cb2a83716 100644 --- a/openlibrary/solr/query_utils.py +++ b/openlibrary/solr/query_utils.py @@ -34,10 +34,35 @@ def luqum_remove_child(child: Item, parents: list[Item]): raise NotImplementedError(f"Not implemented for Item subclass: {parent.__class__.__name__}") +def luqum_get_head(item: Item) -> str: + """ + Whitespace could be attached anywhere recursively + """ + head = item.head + if item.children: + head += luqum_get_head(item.children[0]) + return head + + +def luqum_get_tail(item: Item) -> str: + """ + Whitespace could be attached anywhere recursively + """ + tail = item.tail + if item.children: + tail = luqum_get_tail(item.children[-1]) + tail + return tail + + def luqum_replace_child(parent: Item, old_child: Item, new_child: Item): """ Replaces a child in a luqum parse tree. """ + # Preserve the old child's surrounding whitespace so re-serializing the + # tree doesn't concatenate tokens together. + new_child.head = luqum_get_head(old_child) + new_child.tail = luqum_get_tail(old_child) + if isinstance(parent, (BaseOperation, Group, Unary)): new_children = tuple(new_child if c == old_child else c for c in parent.children) parent.children = new_children diff --git a/openlibrary/tests/solr/test_query_utils.py b/openlibrary/tests/solr/test_query_utils.py index fe630cb9421..c45541f37fe 100644 --- a/openlibrary/tests/solr/test_query_utils.py +++ b/openlibrary/tests/solr/test_query_utils.py @@ -39,13 +39,13 @@ def fn(query: str, remove: str) -> str: "title:foo OR id:1", "title:foo", "(title:foo OR bar:foo)", - "(title:foo OR bar:foo)OR id:1", + "(title:foo OR bar:foo) OR id:1", ), "Deeply nested": ( "title:foo OR (id:1 OR id:2)", "id:2", "(subject:horror)", - "title:foo OR (id:1 OR(subject:horror))", + "title:foo OR (id:1 OR (subject:horror))", ), }