diff --git a/src/palace/manager/api/lanes.py b/src/palace/manager/api/lanes.py index f532d4dbe..9c014104b 100644 --- a/src/palace/manager/api/lanes.py +++ b/src/palace/manager/api/lanes.py @@ -1087,7 +1087,6 @@ def overview_facets(self, _db, facets): # the best recommendations are in the front. return Facets.default( self.get_library(_db), - collection=facets.COLLECTION_FULL, availability=facets.AVAILABLE_ALL, entrypoint=facets.entrypoint, ) @@ -1157,7 +1156,6 @@ def overview_facets(self, _db, facets): """ return SeriesFacets.default( self.get_library(_db), - collection=facets.COLLECTION_FULL, availability=facets.AVAILABLE_ALL, entrypoint=facets.entrypoint, ) @@ -1225,7 +1223,6 @@ def overview_facets(self, _db, facets): """ return ContributorFacets.default( self.get_library(_db), - collection=facets.COLLECTION_FULL, availability=facets.AVAILABLE_ALL, entrypoint=facets.entrypoint, ) @@ -1354,7 +1351,6 @@ class CrawlableFacets(Facets): SETTINGS = { Facets.ORDER_FACET_GROUP_NAME: Facets.ORDER_LAST_UPDATE, Facets.AVAILABILITY_FACET_GROUP_NAME: Facets.AVAILABLE_ALL, - Facets.COLLECTION_FACET_GROUP_NAME: Facets.COLLECTION_FULL, Facets.DISTRIBUTOR_FACETS_GROUP_NAME: Facets.DISTRIBUTOR_ALL, Facets.COLLECTION_NAME_FACETS_GROUP_NAME: Facets.COLLECTION_NAME_ALL, } diff --git a/src/palace/manager/core/facets.py b/src/palace/manager/core/facets.py index 692967fca..b61b8f2b6 100644 --- a/src/palace/manager/core/facets.py +++ b/src/palace/manager/core/facets.py @@ -8,15 +8,6 @@ class FacetConstants: ENTRY_POINT_REL = "http://librarysimplified.org/terms/rel/entrypoint" ENTRY_POINT_FACET_GROUP_NAME = "entrypoint" - # Subset the collection, roughly, by quality. - COLLECTION_FACET_GROUP_NAME = "collection" - COLLECTION_FULL = "full" - COLLECTION_FEATURED = "featured" - COLLECTION_FACETS = [ - COLLECTION_FULL, - COLLECTION_FEATURED, - ] - # Subset the collection by availability. AVAILABILITY_FACET_GROUP_NAME = "available" AVAILABLE_NOW = "now" @@ -63,7 +54,6 @@ class FacetConstants: COLLECTION_NAME_ALL = "All" FACETS_BY_GROUP = { - COLLECTION_FACET_GROUP_NAME: COLLECTION_FACETS, AVAILABILITY_FACET_GROUP_NAME: AVAILABILITY_FACETS, ORDER_FACET_GROUP_NAME: ORDER_FACETS, } @@ -71,7 +61,6 @@ class FacetConstants: GROUP_DISPLAY_TITLES = { ORDER_FACET_GROUP_NAME: _("Sort by"), AVAILABILITY_FACET_GROUP_NAME: _("Availability"), - COLLECTION_FACET_GROUP_NAME: _("Collection"), DISTRIBUTOR_FACETS_GROUP_NAME: _("Distributor"), COLLECTION_NAME_FACETS_GROUP_NAME: _("Collection Name"), } @@ -79,7 +68,6 @@ class FacetConstants: GROUP_DESCRIPTIONS = { ORDER_FACET_GROUP_NAME: _("Allow patrons to sort by"), AVAILABILITY_FACET_GROUP_NAME: _("Allow patrons to filter availability to"), - COLLECTION_FACET_GROUP_NAME: _("Allow patrons to filter collection to"), DISTRIBUTOR_FACETS_GROUP_NAME: _("Allow patrons to filter by distributor"), COLLECTION_NAME_FACETS_GROUP_NAME: _( "Allow patrons to filter by collection name" @@ -96,8 +84,6 @@ class FacetConstants: AVAILABLE_NOW: _("Available now"), AVAILABLE_ALL: _("All"), AVAILABLE_OPEN_ACCESS: _("Yours to keep"), - COLLECTION_FULL: _("Everything"), - COLLECTION_FEATURED: _("Popular Books"), } # For titles generated based on some runtime value @@ -115,7 +101,6 @@ class FacetConstants: AVAILABLE_NOW, AVAILABLE_OPEN_ACCESS, ], - COLLECTION_FACET_GROUP_NAME: [COLLECTION_FULL, COLLECTION_FEATURED], DISTRIBUTOR_FACETS_GROUP_NAME: [DISTRIBUTOR_ALL], COLLECTION_NAME_FACETS_GROUP_NAME: [COLLECTION_NAME_ALL], } @@ -125,7 +110,6 @@ class FacetConstants: DEFAULT_FACET = { ORDER_FACET_GROUP_NAME: ORDER_AUTHOR, AVAILABILITY_FACET_GROUP_NAME: AVAILABLE_ALL, - COLLECTION_FACET_GROUP_NAME: COLLECTION_FULL, DISTRIBUTOR_FACETS_GROUP_NAME: DISTRIBUTOR_ALL, COLLECTION_NAME_FACETS_GROUP_NAME: COLLECTION_NAME_ALL, } diff --git a/src/palace/manager/integration/configuration/library.py b/src/palace/manager/integration/configuration/library.py index 0e6bf3866..ef8434631 100644 --- a/src/palace/manager/integration/configuration/library.py +++ b/src/palace/manager/integration/configuration/library.py @@ -234,35 +234,7 @@ class LibrarySettings(BaseSettings): skip=True, ), ) - facets_enabled_collection: list[str] = FormField( - FacetConstants.DEFAULT_ENABLED_FACETS[ - FacetConstants.COLLECTION_FACET_GROUP_NAME - ], - form=LibraryConfFormItem( - label="Allow patrons to filter collection to", - type=ConfigurationFormItemType.MENU, - options={ - facet: FacetConstants.FACET_DISPLAY_TITLES[facet] - for facet in FacetConstants.COLLECTION_FACETS - }, - category="Lanes & Filters", - paired="facets_default_collection", - level=Level.SYS_ADMIN_OR_MANAGER, - ), - ) - facets_default_collection: str = FormField( - FacetConstants.COLLECTION_FULL, - form=LibraryConfFormItem( - label="Default Collection", - type=ConfigurationFormItemType.SELECT, - options={ - facet: FacetConstants.FACET_DISPLAY_TITLES[facet] - for facet in FacetConstants.COLLECTION_FACETS - }, - category="Lanes & Filters", - skip=True, - ), - ) + library_description: str | None = FormField( None, form=LibraryConfFormItem( diff --git a/src/palace/manager/search/external_search.py b/src/palace/manager/search/external_search.py index 44191b8ae..e1ae016e6 100644 --- a/src/palace/manager/search/external_search.py +++ b/src/palace/manager/search/external_search.py @@ -1832,14 +1832,6 @@ def build(self, _chain_filters=None): Bool(must=[not_open_access, licensed, not_available]) ) - if self.subcollection == FacetConstants.COLLECTION_FEATURED: - # Exclude books with a quality of less than the library's - # minimum featured quality. - range_query = self._match_range( - "quality", "gte", self.minimum_featured_quality - ) - f = chain(f, Bool(must=range_query)) - if self.identifiers: # Check every identifier for a match. clauses = [] diff --git a/src/palace/manager/sqlalchemy/model/lane.py b/src/palace/manager/sqlalchemy/model/lane.py index 0b3046048..8bced8683 100644 --- a/src/palace/manager/sqlalchemy/model/lane.py +++ b/src/palace/manager/sqlalchemy/model/lane.py @@ -396,18 +396,6 @@ def _values_from_request( 400, ) - g = Facets.COLLECTION_FACET_GROUP_NAME - collection = get_argument(g, cls.default_facet(config, g)) - collection_facets = cls.available_facets(config, g) - if collection and not collection in collection_facets: - return INVALID_INPUT.detailed( - _( - "I don't understand what '%(collection)s' refers to.", - collection=collection, - ), - 400, - ) - g = Facets.DISTRIBUTOR_FACETS_GROUP_NAME distributor = get_argument(g, cls.default_facet(config, g)) distributor_facets = cls.available_facets(config, g) @@ -443,7 +431,6 @@ def _values_from_request( enabled = { Facets.ORDER_FACET_GROUP_NAME: order_facets, Facets.AVAILABILITY_FACET_GROUP_NAME: availability_facets, - Facets.COLLECTION_FACET_GROUP_NAME: collection_facets, Facets.DISTRIBUTOR_FACETS_GROUP_NAME: distributor_facets, Facets.COLLECTION_NAME_FACETS_GROUP_NAME: collection_name_facets, } @@ -451,7 +438,6 @@ def _values_from_request( return dict( order=order, availability=availability, - collection=collection, distributor=distributor, collection_name=collection_name, enabled_facets=enabled, @@ -483,7 +469,6 @@ def from_request( def __init__( self, library, - collection, availability, order, distributor, @@ -496,17 +481,11 @@ def __init__( ): """Constructor. - :param collection: This is not a Collection object; it's a value for - the 'collection' facet, e.g. 'main' or 'featured'. - :param entrypoint: An EntryPoint class. The 'entry point' facet group is configured on a per-WorkList basis rather than a per-library basis. """ super().__init__(entrypoint, entrypoint_is_default, **constructor_kwargs) - collection = collection or self.default_facet( - library, self.COLLECTION_FACET_GROUP_NAME - ) availability = availability or self.default_facet( library, self.AVAILABILITY_FACET_GROUP_NAME ) @@ -531,7 +510,6 @@ def __init__( availability = self.AVAILABLE_NOW self.library = library - self.collection = collection self.availability = availability self.order = order self.distributor = distributor or self.default_facet( @@ -549,7 +527,6 @@ def __init__( def navigate( self, - collection=None, availability=None, order=None, entrypoint=None, @@ -559,7 +536,6 @@ def navigate( """Create a slightly different Facets object from this one.""" return self.__class__( library=self.library, - collection=collection or self.collection, availability=availability or self.availability, order=order or self.order, distributor=distributor or self.distributor, @@ -575,8 +551,6 @@ def items(self): yield (self.ORDER_FACET_GROUP_NAME, self.order) if self.availability: yield (self.AVAILABILITY_FACET_GROUP_NAME, self.availability) - if self.collection: - yield (self.COLLECTION_FACET_GROUP_NAME, self.collection) if self.distributor: yield (self.DISTRIBUTOR_FACETS_GROUP_NAME, self.distributor) if self.collection_name: @@ -596,7 +570,6 @@ def enabled_facets(self): facet_types = [ self.ORDER_FACET_GROUP_NAME, self.AVAILABILITY_FACET_GROUP_NAME, - self.COLLECTION_FACET_GROUP_NAME, self.DISTRIBUTOR_FACETS_GROUP_NAME, self.COLLECTION_NAME_FACETS_GROUP_NAME, ] @@ -607,7 +580,6 @@ def enabled_facets(self): for group_name in ( Facets.ORDER_FACET_GROUP_NAME, Facets.AVAILABILITY_FACET_GROUP_NAME, - Facets.COLLECTION_FACET_GROUP_NAME, Facets.DISTRIBUTOR_FACETS_GROUP_NAME, Facets.COLLECTION_NAME_FACETS_GROUP_NAME, ): @@ -626,7 +598,6 @@ def facet_groups(self): ( order_facets, availability_facets, - collection_facets, distributor_facets, collection_name_facets, ) = self.enabled_facets @@ -674,23 +645,6 @@ def dy(new_value): for facet in availability_facets: yield dy(facet) - # Next, the collection facets. - def dy(new_value): - group = self.COLLECTION_FACET_GROUP_NAME - current_value = self.collection - facets = self.navigate(collection=new_value) - return ( - group, - new_value, - facets, - new_value == current_value, - is_default_facet(facets, new_value, group), - ) - - if len(collection_facets) > 1: - for facet in collection_facets: - yield dy(facet) - if len(distributor_facets) > 1: for facet in distributor_facets: group = self.DISTRIBUTOR_FACETS_GROUP_NAME @@ -733,7 +687,6 @@ def modify_search_filter(self, filter): ) filter.availability = self.availability - filter.subcollection = self.collection # We can only have distributor and collection name facets if we have a library if self.library: @@ -797,16 +750,6 @@ def modify_database_query(self, _db, qu): qu = qu.filter(availability_clause) - if self.collection == self.COLLECTION_FULL: - # Include everything. - pass - elif self.collection == self.COLLECTION_FEATURED: - # Exclude books with a quality of less than the library's - # minimum featured quality. - qu = qu.filter( - Work.quality >= self.library.settings.minimum_featured_quality - ) - return qu @@ -1011,7 +954,6 @@ def __init__(self, **kwargs): # usage, a Library will be provided via # SearchFacets.from_request. kwargs.setdefault("library", None) - kwargs.setdefault("collection", None) kwargs.setdefault("availability", None) kwargs.setdefault("distributor", None) kwargs.setdefault("collection_name", None) @@ -1043,13 +985,10 @@ def __init__(self, **kwargs): def default_facet(cls, ignore, group_name): """The default facet settings for SearchFacets are hard-coded. - By default, we will search the full collection and all + By default, we will search all availabilities, and order by match quality rather than any bibliographic field. """ - if group_name == cls.COLLECTION_FACET_GROUP_NAME: - return cls.COLLECTION_FULL - if group_name == cls.AVAILABILITY_FACET_GROUP_NAME: return cls.AVAILABLE_ALL @@ -2971,7 +2910,6 @@ def update_size(self, _db, search_engine: ExternalSearchIndex): for entrypoint in EntryPoint.ENTRY_POINTS: facets = DatabaseBackedFacets( library, - FacetConstants.COLLECTION_FULL, FacetConstants.AVAILABLE_ALL, order=FacetConstants.ORDER_WORK_ID, distributor=FacetConstants.DISTRIBUTOR_ALL, diff --git a/tests/manager/api/controller/test_work.py b/tests/manager/api/controller/test_work.py index 2b77ed4d0..2e493cb48 100644 --- a/tests/manager/api/controller/test_work.py +++ b/tests/manager/api/controller/test_work.py @@ -138,7 +138,7 @@ def test_contributor(self, work_fixture: WorkFixture): facet_links = [ link for link in links if link["rel"] == "http://opds-spec.org/facet" ] - assert 10 == len(facet_links) + assert 8 == len(facet_links) # At this point we don't want to generate real feeds anymore. # We can't do a real end-to-end test without setting up a real @@ -868,7 +868,7 @@ def test_series(self, work_fixture: WorkFixture): facet_links = [ link for link in links if link["rel"] == "http://opds-spec.org/facet" ] - assert 11 == len(facet_links) + assert 9 == len(facet_links) # The facet link we care most about is the default sort order, # put into place by SeriesFacets. diff --git a/tests/manager/api/test_lanes.py b/tests/manager/api/test_lanes.py index 521154748..c95af17d7 100644 --- a/tests/manager/api/test_lanes.py +++ b/tests/manager/api/test_lanes.py @@ -736,7 +736,6 @@ def test_overview_facets(self, lane_fixture: LaneFixture): ) overview = lane.overview_facets(lane_fixture.db.session, featured) assert isinstance(overview, Facets) - assert Facets.COLLECTION_FULL == overview.collection assert Facets.AVAILABLE_ALL == overview.availability assert Facets.ORDER_AUTHOR == overview.order @@ -814,7 +813,6 @@ def test_overview_facets(self, lane_fixture: LaneFixture): lane = SeriesLane(lane_fixture.db.default_library(), "Alrighty Then") overview = lane.overview_facets(lane_fixture.db.session, featured) assert isinstance(overview, SeriesFacets) - assert Facets.COLLECTION_FULL == overview.collection assert Facets.AVAILABLE_ALL == overview.availability assert Facets.ORDER_SERIES_POSITION == overview.order @@ -902,7 +900,6 @@ def test_overview_facets(self, lane_fixture: LaneFixture): ) overview = lane.overview_facets(lane_fixture.db.session, featured) assert isinstance(overview, ContributorFacets) - assert Facets.COLLECTION_FULL == overview.collection assert Facets.AVAILABLE_ALL == overview.availability assert Facets.ORDER_TITLE == overview.order @@ -913,7 +910,6 @@ def test_overview_facets(self, lane_fixture: LaneFixture): class TestCrawlableFacets: def test_default(self, db: DatabaseTransactionFixture): facets = CrawlableFacets.default(db.default_library()) - assert facets.collection == CrawlableFacets.COLLECTION_FULL assert facets.availability == CrawlableFacets.AVAILABLE_ALL assert facets.order == CrawlableFacets.ORDER_LAST_UPDATE assert facets.order_ascending is False @@ -921,13 +917,12 @@ def test_default(self, db: DatabaseTransactionFixture): [ order, availability, - collection, distributor, collectionName, ] = facets.enabled_facets # The default facets are the only ones enabled. - for facet in [order, availability, collection]: + for facet in [order, availability]: assert len(facet) == 1 # Except for distributor and collectionName, which have the default @@ -940,7 +935,6 @@ def test_default(self, db: DatabaseTransactionFixture): [ (Facets.ORDER_FACET_GROUP_NAME, Facets.ORDER_LAST_UPDATE), (Facets.AVAILABILITY_FACET_GROUP_NAME, Facets.AVAILABLE_ALL), - (Facets.COLLECTION_FACET_GROUP_NAME, Facets.COLLECTION_FULL), (Facets.DISTRIBUTOR_FACETS_GROUP_NAME, Facets.DISTRIBUTOR_ALL), (Facets.COLLECTION_NAME_FACETS_GROUP_NAME, Facets.COLLECTION_NAME_ALL), ], @@ -953,7 +947,6 @@ def test_available_none(self, group_name: str, expected: list[str]) -> None: [ (Facets.ORDER_FACET_GROUP_NAME, [Facets.ORDER_LAST_UPDATE]), (Facets.AVAILABILITY_FACET_GROUP_NAME, [Facets.AVAILABLE_ALL]), - (Facets.COLLECTION_FACET_GROUP_NAME, [Facets.COLLECTION_FULL]), (Facets.DISTRIBUTOR_FACETS_GROUP_NAME, [Facets.DISTRIBUTOR_ALL, "foo"]), ( Facets.COLLECTION_NAME_FACETS_GROUP_NAME, @@ -1124,13 +1117,9 @@ def test_default_facet(self, db: DatabaseTransactionFixture): # For other facet groups, the class defers to the Facets # superclass. (But this doesn't matter because it's not relevant # to the creation of jackpot feeds.) - for group in ( - Facets.COLLECTION_FACET_GROUP_NAME, - Facets.ORDER_FACET_GROUP_NAME, - ): - assert m(db.default_library(), group) == Facets.default_facet( - db.default_library(), group - ) + assert m( + db.default_library(), Facets.ORDER_FACET_GROUP_NAME + ) == Facets.default_facet(db.default_library(), Facets.ORDER_FACET_GROUP_NAME) def test_available_facets(self, db: DatabaseTransactionFixture): # A JackpotFacets object always has the same availability @@ -1148,10 +1137,7 @@ def test_available_facets(self, db: DatabaseTransactionFixture): # For other facet groups, the class defers to the Facets # superclass. (But this doesn't matter because it's not relevant # to the creation of jackpot feeds.) - for group in ( - Facets.COLLECTION_FACET_GROUP_NAME, - Facets.ORDER_FACET_GROUP_NAME, - ): + for group in (Facets.ORDER_FACET_GROUP_NAME,): assert m(db.default_library(), group) == Facets.available_facets( db.default_library(), group ) diff --git a/tests/manager/feed/test_library_annotator.py b/tests/manager/feed/test_library_annotator.py index c0276960c..18f21e9cf 100644 --- a/tests/manager/feed/test_library_annotator.py +++ b/tests/manager/feed/test_library_annotator.py @@ -464,17 +464,15 @@ def test_search_url(self, annotator_fixture: LibraryAnnotatorFixture): def test_facet_url(self, annotator_fixture: LibraryAnnotatorFixture): # A regular Lane. facets = Facets.default( - annotator_fixture.db.default_library(), collection="main" + annotator_fixture.db.default_library(), ) facet_url = annotator_fixture.annotator.facet_url(facets) - assert "collection=main" in facet_url assert str(annotator_fixture.lane.id) in facet_url # A QueryGeneratedLane. annotator_fixture.annotator.lane = annotator_fixture.contributor_lane facet_url_contributor = annotator_fixture.annotator.facet_url(facets) - assert "collection=main" in facet_url_contributor assert annotator_fixture.contributor_lane.ROUTE in facet_url_contributor assert ( annotator_fixture.contributor_lane.contributor_key in facet_url_contributor diff --git a/tests/manager/feed/test_opds_acquisition_feed.py b/tests/manager/feed/test_opds_acquisition_feed.py index b49136759..2e1d3895f 100644 --- a/tests/manager/feed/test_opds_acquisition_feed.py +++ b/tests/manager/feed/test_opds_acquisition_feed.py @@ -866,33 +866,6 @@ def facet_groups(self): bunch that don't (which will be ignored). """ - # Real facet group, real facet - yield ( - Facets.COLLECTION_FACET_GROUP_NAME, - Facets.COLLECTION_FULL, - "try the featured collection instead", - True, - False, - ) - - # Real facet group, nonexistent facet - yield ( - Facets.COLLECTION_FACET_GROUP_NAME, - "no such facet", - "this facet does not exist", - True, - False, - ) - - # Nonexistent facet group, real facet - yield ( - "no such group", - Facets.COLLECTION_FULL, - "this facet exists but it's in a nonexistent group", - True, - False, - ) - # Nonexistent facet group, nonexistent facet yield ( "no such group", @@ -913,20 +886,11 @@ def facet_link(cls, url, facet_title, group_title, selected, is_default): annotator = MockAnnotator() facets = MockFacets() - # The only 5-tuple yielded by facet_groups was passed on to us. - # The link was run through MockAnnotator.facet_url(), - # and the human-readable titles were found using lookups. - # - # The other three 5-tuples were ignored since we don't know + # The 5-tuples were ignored since we don't know # how to generate human-readable titles for them. - [[url, facet, group, selected, is_default]] = MockFeed.facet_links( - annotator, facets - ) - assert "url: try the featured collection instead" == url - assert Facets.FACET_DISPLAY_TITLES[Facets.COLLECTION_FULL] == facet - assert Facets.GROUP_DISPLAY_TITLES[Facets.COLLECTION_FACET_GROUP_NAME] == group - assert selected - assert not is_default + links = MockFeed.facet_links(annotator, facets) + + assert len([x for x in links]) == 0 def test_active_loans_for_with_holds( self, db: DatabaseTransactionFixture, patch_url_for: PatchedUrlFor @@ -1166,7 +1130,7 @@ def run(wl=None, facets=None, pagination=None): # The make_link function that was passed in calls # TestAnnotator.feed_url() when passed an EntryPoint. The # Facets object's other facet groups are propagated in this URL. - first_page_url = "http://wl/?available=all&collection=full&collectionName=All&distributor=All&entrypoint=Book&order=author" + first_page_url = "http://wl/?available=all&collectionName=All&distributor=All&entrypoint=Book&order=author" assert first_page_url == make_link(EbooksEntryPoint) # Pagination information is not propagated through entry point links @@ -1238,7 +1202,7 @@ def mock_search(self, *args, **kwargs): # The make_link function that was passed in calls # TestAnnotator.search_url() when passed an EntryPoint. - first_page_url = "http://localhost/default/search/?entrypoint=Book&order=relevance&available=all&collection=full&search_type=default" + first_page_url = "http://localhost/default/search/?entrypoint=Book&order=relevance&available=all&search_type=default" with app.test_request_context("/"): assert first_page_url == make_link(EbooksEntryPoint) diff --git a/tests/manager/search/test_external_search.py b/tests/manager/search/test_external_search.py index 0be96664d..adc3382ea 100644 --- a/tests/manager/search/test_external_search.py +++ b/tests/manager/search/test_external_search.py @@ -940,7 +940,6 @@ def pages(worklist): facets = Facets( transaction.default_library(), None, - None, order=Facets.ORDER_TITLE, distributor=None, collection_name=None, @@ -988,7 +987,6 @@ def pages(worklist): facets = SearchFacets by_author_facet = facets( library=transaction.default_library(), - collection=facets.COLLECTION_FULL, availability=facets.AVAILABLE_ALL, order=facets.ORDER_AUTHOR, ) @@ -996,7 +994,6 @@ def pages(worklist): by_title_facet = facets( library=transaction.default_library(), - collection=facets.COLLECTION_FULL, availability=facets.AVAILABLE_ALL, order=facets.ORDER_TITLE, ) @@ -1124,11 +1121,10 @@ def test_facet_filtering(self, end_to_end_search_fixture: EndToEndSearchFixture) data = self._populate_works(fixture) fixture.populate_search_index() - def expect(availability, collection, works): + def expect(availability, works): facets = Facets( transaction.default_library(), availability, - collection, order=Facets.ORDER_TITLE, distributor=None, collection_name=None, @@ -1137,7 +1133,6 @@ def expect(availability, collection, works): # Get all the books in alphabetical order by title. expect( - Facets.COLLECTION_FULL, Facets.AVAILABLE_ALL, [ data.becoming, @@ -1149,28 +1144,19 @@ def expect(availability, collection, works): # Show only works that can be borrowed right now. expect( - Facets.COLLECTION_FULL, Facets.AVAILABLE_NOW, [data.horse, data.moby, data.duck], ) # Show only works that can *not* be borrowed right now. - expect(Facets.COLLECTION_FULL, Facets.AVAILABLE_NOT_NOW, [data.becoming]) + expect(Facets.AVAILABLE_NOT_NOW, [data.becoming]) # Show only open-access works. expect( - Facets.COLLECTION_FULL, Facets.AVAILABLE_OPEN_ACCESS, [data.horse, data.moby], ) - # Show only featured-quality works. - expect( - Facets.COLLECTION_FEATURED, - Facets.AVAILABLE_ALL, - [data.becoming, data.moby], - ) - class TestSearchOrderData: a1: LicensePool @@ -1401,7 +1387,6 @@ def assert_order(sort_field, order, **filter_kwargs): expect = fixture.expect_results facets = Facets( transaction.default_library(), - Facets.COLLECTION_FULL, Facets.AVAILABLE_ALL, order=sort_field, distributor=None, @@ -2524,25 +2509,8 @@ def from_facets(*args, **kwargs): # Return the rest to be verified in a test-specific way. return built - # When using the 'featured' collection... - built = from_facets(Facets.COLLECTION_FEATURED, None, None, None, None) - - # There is no nested filter. - assert [] == built.nested_filter_calls - - # A non-nested filter is applied on the 'quality' field. - [quality_filter] = built._query.filter - quality_range = Filter._match_range( - "quality", - "gte", - db.default_library().settings.minimum_featured_quality, - ) - assert Q("bool", must=[quality_range], must_not=[RESEARCH]) == quality_filter - # When using the AVAILABLE_OPEN_ACCESS availability restriction... - built = from_facets( - Facets.COLLECTION_FULL, Facets.AVAILABLE_OPEN_ACCESS, None, None, None - ) + built = from_facets(Facets.AVAILABLE_OPEN_ACCESS, None, None, None) # An additional nested filter is applied. [available_now] = built.nested_filter_calls @@ -2555,9 +2523,7 @@ def from_facets(*args, **kwargs): assert nested_filter.to_dict() == {"bool": {"filter": [open_access]}} # When using the AVAILABLE_NOW restriction... - built = from_facets( - Facets.COLLECTION_FULL, Facets.AVAILABLE_NOW, None, None, None - ) + built = from_facets(Facets.AVAILABLE_NOW, None, None, None) # An additional nested filter is applied. [available_now] = built.nested_filter_calls @@ -2582,9 +2548,7 @@ def from_facets(*args, **kwargs): } # When using the AVAILABLE_NOT_NOW restriction... - built = from_facets( - Facets.COLLECTION_FULL, Facets.AVAILABLE_NOT_NOW, None, None, None - ) + built = from_facets(Facets.AVAILABLE_NOT_NOW, None, None, None) # An additional nested filter is applied. [not_available_now] = built.nested_filter_calls @@ -2608,7 +2572,6 @@ def from_facets(*args, **kwargs): # Distributor builds _id = DataSource.lookup(db.session, DataSource.OVERDRIVE).id built = from_facets( - Facets.COLLECTION_FULL, Facets.AVAILABLE_ALL, None, DataSource.OVERDRIVE, @@ -2622,9 +2585,7 @@ def from_facets(*args, **kwargs): # Collection Name builds collection = db.default_collection() - built = from_facets( - Facets.COLLECTION_FULL, Facets.AVAILABLE_ALL, None, None, collection.name - ) + built = from_facets(Facets.AVAILABLE_ALL, None, None, collection.name) [collection_only] = built.nested_filter_calls nested_filter = collection_only["query"] assert nested_filter.to_dict() == { @@ -2645,7 +2606,6 @@ def from_facets(*args, **kwargs): # used to convert it to appropriate Opensearch syntax, and # the MockSearch object is modified appropriately. built = from_facets( - None, None, order=Facets.ORDER_AUTHOR, distributor=None, diff --git a/tests/manager/sqlalchemy/model/test_lane.py b/tests/manager/sqlalchemy/model/test_lane.py index 415c09d45..ad6fd88ca 100644 --- a/tests/manager/sqlalchemy/model/test_lane.py +++ b/tests/manager/sqlalchemy/model/test_lane.py @@ -309,7 +309,6 @@ def test_facet_groups(self, db: DatabaseTransactionFixture): db.default_collection().data_source = DataSource.AMAZON # type: ignore[assignment] facets = Facets( db.default_library(), - Facets.COLLECTION_FULL, Facets.AVAILABLE_ALL, Facets.ORDER_TITLE, Facets.DISTRIBUTOR_ALL, @@ -318,15 +317,14 @@ def test_facet_groups(self, db: DatabaseTransactionFixture): all_groups = list(facets.facet_groups) # By default, there are 10 facet transitions: two groups of three - # and one group of two and 2 datasource groups and 2 for collection names - assert 12 == len(all_groups) + # and 2 datasource groups and 2 for collection names + assert 10 == len(all_groups) - # available=all, collection=full, and order=title are the selected + # available=all and order=title are the selected # facets. selected = sorted(x[:2] for x in all_groups if x[-2] == True) assert [ ("available", "all"), - ("collection", "full"), ("collectionName", "All"), ("distributor", "All"), ("order", "title"), @@ -335,20 +333,16 @@ def test_facet_groups(self, db: DatabaseTransactionFixture): # Distributor and CollectionName facets are generated at runtime, they are not a setting value test_enabled_facets = { Facets.ORDER_FACET_GROUP_NAME: [Facets.ORDER_WORK_ID, Facets.ORDER_TITLE], - Facets.COLLECTION_FACET_GROUP_NAME: [Facets.COLLECTION_FEATURED], Facets.AVAILABILITY_FACET_GROUP_NAME: [Facets.AVAILABLE_ALL], } test_default_facets = { Facets.ORDER_FACET_GROUP_NAME: Facets.ORDER_TITLE, - Facets.COLLECTION_FACET_GROUP_NAME: Facets.COLLECTION_FEATURED, Facets.AVAILABILITY_FACET_GROUP_NAME: Facets.AVAILABLE_ALL, } library = db.default_library() self._configure_facets(library, test_enabled_facets, test_default_facets) - facets = Facets( - db.default_library(), None, None, Facets.ORDER_TITLE, None, None - ) + facets = Facets(db.default_library(), None, Facets.ORDER_TITLE, None, None) all_groups = list(facets.facet_groups) # We have disabled almost all the facets, so the list of # facet transitions includes only two items. @@ -435,7 +429,6 @@ def test_default_availability( # facet. test_enabled_facets = { Facets.ORDER_FACET_GROUP_NAME: [Facets.ORDER_WORK_ID], - Facets.COLLECTION_FACET_GROUP_NAME: [Facets.COLLECTION_FULL], Facets.AVAILABILITY_FACET_GROUP_NAME: [ Facets.AVAILABLE_ALL, Facets.AVAILABLE_NOW, @@ -443,7 +436,6 @@ def test_default_availability( } test_default_facets = { Facets.ORDER_FACET_GROUP_NAME: Facets.ORDER_TITLE, - Facets.COLLECTION_FACET_GROUP_NAME: Facets.COLLECTION_FULL, Facets.AVAILABILITY_FACET_GROUP_NAME: Facets.AVAILABLE_ALL, } library = db.default_library() @@ -475,7 +467,6 @@ def test_facets_can_be_enabled_at_initialization( Facets.ORDER_TITLE, Facets.ORDER_AUTHOR, ], - Facets.COLLECTION_FACET_GROUP_NAME: [Facets.COLLECTION_FULL], Facets.AVAILABILITY_FACET_GROUP_NAME: [Facets.AVAILABLE_OPEN_ACCESS], } library = db.default_library() @@ -485,7 +476,6 @@ def test_facets_can_be_enabled_at_initialization( # no matter the Configuration. facets = Facets( db.default_library(), - Facets.COLLECTION_FULL, Facets.AVAILABLE_OPEN_ACCESS, Facets.ORDER_TITLE, Facets.DISTRIBUTOR_ALL, @@ -502,13 +492,11 @@ def test_facets_dont_need_a_library(self): Facets.ORDER_TITLE, Facets.ORDER_AUTHOR, ], - Facets.COLLECTION_FACET_GROUP_NAME: [Facets.COLLECTION_FULL], Facets.AVAILABILITY_FACET_GROUP_NAME: [Facets.AVAILABLE_OPEN_ACCESS], } facets = Facets( None, - Facets.COLLECTION_FULL, Facets.AVAILABLE_OPEN_ACCESS, Facets.ORDER_TITLE, Facets.DISTRIBUTOR_ALL, @@ -525,7 +513,6 @@ def test_items(self, db: DatabaseTransactionFixture): """ facets = Facets( db.default_library(), - Facets.COLLECTION_FULL, Facets.AVAILABLE_ALL, Facets.ORDER_TITLE, Facets.DISTRIBUTOR_ALL, @@ -534,7 +521,6 @@ def test_items(self, db: DatabaseTransactionFixture): ) assert [ ("available", Facets.AVAILABLE_ALL), - ("collection", Facets.COLLECTION_FULL), ("collectionName", Facets.COLLECTION_NAME_ALL), ("distributor", Facets.DISTRIBUTOR_ALL), ("entrypoint", AudiobooksEntryPoint.INTERNAL_NAME), @@ -546,7 +532,6 @@ def test_default_order_ascending(self, db: DatabaseTransactionFixture): for order in (Facets.ORDER_TITLE, Facets.ORDER_AUTHOR): f = Facets( db.default_library(), - collection=Facets.COLLECTION_FULL, availability=Facets.AVAILABLE_ALL, order=order, distributor=Facets.DISTRIBUTOR_ALL, @@ -562,7 +547,6 @@ def test_default_order_ascending(self, db: DatabaseTransactionFixture): for order in Facets.ORDER_DESCENDING_BY_DEFAULT: f = Facets( db.default_library(), - collection=Facets.COLLECTION_FULL, availability=Facets.AVAILABLE_ALL, order=order, distributor=Facets.DISTRIBUTOR_ALL, @@ -579,7 +563,6 @@ def test_navigate(self, db: DatabaseTransactionFixture): ebooks = EbooksEntryPoint f = Facets( db.default_library(), - F.COLLECTION_FULL, F.AVAILABLE_ALL, F.ORDER_TITLE, Facets.DISTRIBUTOR_ALL, @@ -587,16 +570,7 @@ def test_navigate(self, db: DatabaseTransactionFixture): entrypoint=ebooks, ) - different_collection = f.navigate(collection=F.COLLECTION_FEATURED) - assert F.COLLECTION_FEATURED == different_collection.collection - assert F.AVAILABLE_ALL == different_collection.availability - assert F.ORDER_TITLE == different_collection.order - assert F.DISTRIBUTOR_ALL == different_collection.distributor - assert F.COLLECTION_NAME_ALL == different_collection.collection_name - assert ebooks == different_collection.entrypoint - different_availability = f.navigate(availability=F.AVAILABLE_NOW) - assert F.COLLECTION_FULL == different_availability.collection assert F.AVAILABLE_NOW == different_availability.availability assert F.ORDER_TITLE == different_availability.order assert F.DISTRIBUTOR_ALL == different_availability.distributor @@ -604,7 +578,6 @@ def test_navigate(self, db: DatabaseTransactionFixture): assert ebooks == different_availability.entrypoint different_order = f.navigate(order=F.ORDER_AUTHOR) - assert F.COLLECTION_FULL == different_order.collection assert F.AVAILABLE_ALL == different_order.availability assert F.ORDER_AUTHOR == different_order.order assert F.DISTRIBUTOR_ALL == different_order.distributor @@ -613,7 +586,6 @@ def test_navigate(self, db: DatabaseTransactionFixture): audiobooks = AudiobooksEntryPoint different_entrypoint = f.navigate(entrypoint=audiobooks) - assert F.COLLECTION_FULL == different_entrypoint.collection assert F.AVAILABLE_ALL == different_entrypoint.availability assert F.ORDER_TITLE == different_entrypoint.order assert F.DISTRIBUTOR_ALL == different_entrypoint.distributor @@ -621,14 +593,12 @@ def test_navigate(self, db: DatabaseTransactionFixture): assert audiobooks == different_entrypoint.entrypoint different_distributor = f.navigate(distributor=DataSource.AMAZON) - assert F.COLLECTION_FULL == different_distributor.collection assert F.AVAILABLE_ALL == different_distributor.availability assert F.ORDER_TITLE == different_distributor.order assert F.COLLECTION_NAME_ALL == different_distributor.collection_name assert DataSource.AMAZON == different_distributor.distributor different_collection_name = f.navigate(collection_name="Collection Name") - assert F.COLLECTION_FULL == different_collection_name.collection assert F.AVAILABLE_ALL == different_collection_name.availability assert F.ORDER_TITLE == different_collection_name.order assert F.DISTRIBUTOR_ALL == different_collection_name.distributor @@ -652,7 +622,6 @@ def test_from_request( # Valid object using the default settings. default_order = config.default_facet(Facets.ORDER_FACET_GROUP_NAME) - default_collection = config.default_facet(Facets.COLLECTION_FACET_GROUP_NAME) default_availability = config.default_facet( Facets.AVAILABILITY_FACET_GROUP_NAME ) @@ -660,7 +629,6 @@ def test_from_request( headers: dict = {} facets = m(library, library, args.get, headers.get, worklist) assert default_order == facets.order - assert default_collection == facets.collection assert default_availability == facets.availability assert library == facets.library @@ -671,13 +639,11 @@ def test_from_request( # Valid object using non-default settings. args = dict( order=Facets.ORDER_TITLE, - collection=Facets.COLLECTION_FULL, available=Facets.AVAILABLE_OPEN_ACCESS, entrypoint=EbooksEntryPoint.INTERNAL_NAME, ) facets = m(library, library, args.get, headers.get, worklist) assert Facets.ORDER_TITLE == facets.order - assert Facets.COLLECTION_FULL == facets.collection assert Facets.AVAILABLE_OPEN_ACCESS == facets.availability assert library == facets.library assert EbooksEntryPoint == facets.entrypoint @@ -700,15 +666,6 @@ def test_from_request( == invalid_availability.detail ) - # Invalid collection - args = dict(collection="no such collection") - invalid_collection = m(library, library, args.get, headers.get, None) - assert INVALID_INPUT.uri == invalid_collection.uri - assert ( - "I don't understand what 'no such collection' refers to." - == invalid_collection.detail - ) - def test_from_request_gets_available_facets_through_hook_methods( self, db: DatabaseTransactionFixture ): @@ -724,7 +681,6 @@ class Mock(Facets): mock_enabled = dict( order=[Facets.ORDER_TITLE], available=[Facets.AVAILABLE_OPEN_ACCESS], - collection=[Facets.COLLECTION_FULL], distributor=[Facets.DISTRIBUTOR_ALL], collectionName=[Facets.COLLECTION_NAME_ALL], ) @@ -745,15 +701,13 @@ def default_facet(cls, config, facet_group_name): ( order, available, - collection, distributor, collection_name, ) = Mock.available_facets_calls # available_facets was called three times, to ask the Mock class what it thinks - # the options for order, availability, and collection should be. + # the options for order and availability should be. assert (library, "order") == order assert (library, "available") == available - assert (library, "collection") == collection assert (library, "distributor") == distributor assert (library, "collectionName") == collection_name @@ -762,13 +716,11 @@ def default_facet(cls, config, facet_group_name): ( order_d, available_d, - collection_d, distributor_d, collection_name_d, ) = Mock.default_facet_calls assert (library, "order") == order_d assert (library, "available") == available_d - assert (library, "collection") == collection_d assert (library, "distributor") == distributor_d assert (library, "collectionName") == collection_name_d @@ -781,7 +733,6 @@ def default_facet(cls, config, facet_group_name): # The current values came from the defaults provided by default_facet(). assert Facets.ORDER_TITLE == result.order assert Facets.AVAILABLE_OPEN_ACCESS == result.availability - assert Facets.COLLECTION_FULL == result.collection assert Facets.DISTRIBUTOR_ALL == result.distributor assert Facets.COLLECTION_NAME_ALL == result.collection_name @@ -793,7 +744,6 @@ def test_modify_search_filter(self, db: DatabaseTransactionFixture): None, None, None, - None, entrypoint=AudiobooksEntryPoint, ) filter = Filter() @@ -803,7 +753,6 @@ def test_modify_search_filter(self, db: DatabaseTransactionFixture): # Now test the subclass behavior. facets = Facets( db.default_library(), - "some collection", "some availability", order=Facets.ORDER_ADDED_TO_COLLECTION, distributor=DataSource.OVERDRIVE, @@ -821,7 +770,6 @@ def test_modify_search_filter(self, db: DatabaseTransactionFixture): # Availability and collection and distributor are propagated with no # validation. assert "some availability" == filter.availability - assert "some collection" == filter.subcollection assert [ DataSource.lookup(db.session, DataSource.OVERDRIVE).id ] == filter.license_datasources @@ -836,13 +784,13 @@ def test_modify_search_filter(self, db: DatabaseTransactionFixture): # Specifying an invalid sort order doesn't cause a crash, but you # don't get a sort order. - facets = Facets(db.default_library(), None, None, "invalid order", None, None) + facets = Facets(db.default_library(), None, "invalid order", None, None) filter = Filter() facets.modify_search_filter(filter) assert None == filter.order facets = Facets( - db.default_library(), None, None, None, None, db.default_collection().name + db.default_library(), None, None, None, db.default_collection().name ) filter = Filter() facets.modify_search_filter(filter) @@ -888,46 +836,25 @@ def test_modify_database_query(self, db: DatabaseTransactionFixture): ), (Facets.AVAILABLE_NOT_NOW, [not_available]), ]: - facets = Facets(db.default_library(), None, availability, None, None, None) + facets = Facets(db.default_library(), availability, None, None, None) modified = facets.modify_database_query(db.session, qu) assert (availability, sorted(x.title for x in modified)) == ( availability, sorted(x.title for x in expect), ) - # Setting the 'featured' collection includes only known - # high-quality works. - for collection, expect in [ - ( - Facets.COLLECTION_FULL, - [open_access, available, unlimited_access], - ), - (Facets.COLLECTION_FEATURED, [open_access]), - ]: - facets = Facets( - db.default_library(), collection, Facets.AVAILABLE_NOW, None, None, None - ) - modified = facets.modify_database_query(db.session, qu) - assert (collection, sorted(x.title for x in modified)) == ( - collection, - sorted(x.title for x in expect), - ) - class TestDefaultSortOrderFacets: def _check_other_groups_not_changed(self, cls, config: Library): - # Verify that nothing has changed for the collection or - # availability facet groups. - for group_name in ( - Facets.COLLECTION_FACET_GROUP_NAME, - Facets.AVAILABILITY_FACET_GROUP_NAME, - ): - assert Facets.available_facets(config, group_name) == cls.available_facets( - config, group_name - ) - assert Facets.default_facet(config, group_name) == cls.default_facet( - config, group_name - ) + # Verify that nothing has changed for the + # availability facet group. + group_name = Facets.AVAILABILITY_FACET_GROUP_NAME + assert Facets.available_facets(config, group_name) == cls.available_facets( + config, group_name + ) + assert Facets.default_facet(config, group_name) == cls.default_facet( + config, group_name + ) def test_sort_order_rearrangement(self, db: DatabaseTransactionFixture): config = db.default_library() @@ -1003,29 +930,16 @@ def test_available_facets(self, db: DatabaseTransactionFixture): for order in f2_orders: assert order in f1_orders and order in f2.ORDER_FACET_TO_DATABASE_FIELD - # The rules for collection and availability are the same. - for group in ( - FacetConstants.COLLECTION_FACET_GROUP_NAME, - FacetConstants.AVAILABILITY_FACET_GROUP_NAME, - ): - assert f1.available_facets( - db.default_library(), group - ) == f2.available_facets(db.default_library(), group) - def test_default_facets(self, db: DatabaseTransactionFixture): # If the configured default sort order is not available, # DatabaseBackedFacets chooses the first enabled sort order. f1 = Facets f2 = DatabaseBackedFacets - # The rules for collection and availability are the same. - for group in ( - FacetConstants.COLLECTION_FACET_GROUP_NAME, - FacetConstants.AVAILABILITY_FACET_GROUP_NAME, - ): - assert f1.default_facet(db.default_library(), group) == f2.default_facet( - db.default_library(), group - ) + group = FacetConstants.AVAILABILITY_FACET_GROUP_NAME + assert f1.default_facet(db.default_library(), group) == f2.default_facet( + db.default_library(), group + ) # In this bizarre library, the default sort order is 'time # added to collection' -- an order not supported by @@ -1066,7 +980,6 @@ def test_order_by(self, db: DatabaseTransactionFixture): def order(facet, ascending=None): f = DatabaseBackedFacets( db.default_library(), - collection=Facets.COLLECTION_FULL, availability=Facets.AVAILABLE_ALL, order=facet, distributor=None, @@ -1148,13 +1061,10 @@ def test_modify_database_query( qu = DatabaseBackedWorkList.base_query(db.session) def facetify( - collection=Facets.COLLECTION_FULL, available=Facets.AVAILABLE_ALL, order=Facets.ORDER_TITLE, ): - f = DatabaseBackedFacets( - db.default_library(), collection, available, order, None, None - ) + f = DatabaseBackedFacets(db.default_library(), available, order, None, None) return f.modify_database_query(db.session, qu) # When holds are allowed, we can find all works by asking @@ -1187,13 +1097,6 @@ def facetify( assert licensed_low not in open_access assert unlimited_access_high not in open_access - # If we restrict to the featured collection we lose the two - # low-quality books. - featured_collection = facetify(collection=Facets.COLLECTION_FEATURED) - assert 3 == featured_collection.count() - assert open_access_low not in featured_collection - assert licensed_low not in featured_collection - # Try some different orderings to verify that order_by() # is called and used properly. title_order = facetify(order=Facets.ORDER_TITLE) @@ -1519,7 +1422,6 @@ def test_items(self): ("entrypoint", EverythingEntryPoint.INTERNAL_NAME), (Facets.ORDER_FACET_GROUP_NAME, SearchFacets.ORDER_BY_RELEVANCE), (Facets.AVAILABILITY_FACET_GROUP_NAME, Facets.AVAILABLE_ALL), - (Facets.COLLECTION_FACET_GROUP_NAME, Facets.COLLECTION_FULL), ("media", Edition.BOOK_MEDIUM), ("min_score", "123"), ("search_type", "default"), @@ -2332,7 +2234,6 @@ def works_for_hits(self, _db, work_ids, facets=None): facets = Facets( db.default_library(), None, - None, order=Facets.ORDER_TITLE, distributor=None, collection_name=None, @@ -2978,7 +2879,6 @@ def test_works_from_database_end_to_end(self, db: DatabaseTransactionFixture): # are returned. facets = DatabaseBackedFacets( db.default_library(), - collection=Facets.COLLECTION_FULL, availability=Facets.AVAILABLE_ALL, order=Facets.ORDER_TITLE, distributor=None, @@ -4406,7 +4306,6 @@ def database_facets(self, library: Library | None = None) -> DatabaseBackedFacet library = library or self.db.default_library() return DatabaseBackedFacets( library, - collection=Facets.COLLECTION_FULL, availability=Facets.AVAILABLE_ALL, order=Facets.ORDER_TITLE, distributor=None, diff --git a/tests/manager/sqlalchemy/model/test_library.py b/tests/manager/sqlalchemy/model/test_library.py index e4a8bca76..452f53df2 100644 --- a/tests/manager/sqlalchemy/model/test_library.py +++ b/tests/manager/sqlalchemy/model/test_library.py @@ -165,8 +165,6 @@ def test_explain(self, db: DatabaseTransactionFixture): facets_default_order='author' facets_enabled_available='['all', 'now', 'always']' facets_default_available='all' -facets_enabled_collection='['full', 'featured']' -facets_default_collection='full' help_web='http://library.com/support' default_notification_email_address='noreply@thepalaceproject.org' color_scheme='blue'