Skip to content

Commit

Permalink
[PP-1914] Remove collection facet from feed. (#2182)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbernstein authored Dec 2, 2024
1 parent b6090d2 commit c2af3a4
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 357 deletions.
4 changes: 0 additions & 4 deletions src/palace/manager/api/lanes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
}
Expand Down
16 changes: 0 additions & 16 deletions src/palace/manager/core/facets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -63,23 +54,20 @@ 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,
}

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"),
}

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"
Expand All @@ -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
Expand All @@ -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],
}
Expand 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,
}
Expand Down
30 changes: 1 addition & 29 deletions src/palace/manager/integration/configuration/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 0 additions & 8 deletions src/palace/manager/search/external_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
64 changes: 1 addition & 63 deletions src/palace/manager/sqlalchemy/model/lane.py
Original file line number Diff line number Diff line change
Expand Up @@ -395,18 +395,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)
Expand Down Expand Up @@ -442,15 +430,13 @@ 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,
}

return dict(
order=order,
availability=availability,
collection=collection,
distributor=distributor,
collection_name=collection_name,
enabled_facets=enabled,
Expand Down Expand Up @@ -482,7 +468,6 @@ def from_request(
def __init__(
self,
library,
collection,
availability,
order,
distributor,
Expand All @@ -495,17 +480,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
)
Expand All @@ -530,7 +509,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(
Expand All @@ -548,7 +526,6 @@ def __init__(

def navigate(
self,
collection=None,
availability=None,
order=None,
entrypoint=None,
Expand All @@ -558,7 +535,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,
Expand All @@ -574,8 +550,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:
Expand All @@ -595,7 +569,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,
]
Expand All @@ -606,7 +579,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,
):
Expand All @@ -625,7 +597,6 @@ def facet_groups(self):
(
order_facets,
availability_facets,
collection_facets,
distributor_facets,
collection_name_facets,
) = self.enabled_facets
Expand Down Expand Up @@ -673,23 +644,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
Expand Down Expand Up @@ -732,7 +686,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:
Expand Down Expand Up @@ -796,16 +749,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


Expand Down Expand Up @@ -1010,7 +953,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)
Expand Down Expand Up @@ -1042,13 +984,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

Expand Down Expand Up @@ -2977,7 +2916,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,
Expand Down
4 changes: 2 additions & 2 deletions tests/manager/api/controller/test_work.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -870,7 +870,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.
Expand Down
Loading

0 comments on commit c2af3a4

Please sign in to comment.