From 5a76a132ac40fbe3d9e49f415f93d784b24a30d6 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:14:31 +0200 Subject: [PATCH 01/35] EKIRJASTO-131 Create Selected Book table Includes the many-tp-many relationship between Patron and Work via the SelectedBook table. --- ...41210_d3aaeb6a9e6b_create_selectedbooks.py | 37 +++++++++++++++++++ core/model/__init__.py | 1 + core/model/patron.py | 30 +++++++++++++++ core/model/work.py | 5 +++ 4 files changed, 73 insertions(+) create mode 100644 alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py diff --git a/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py b/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py new file mode 100644 index 000000000..261a14983 --- /dev/null +++ b/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py @@ -0,0 +1,37 @@ +"""Create SelectedBooks + +Revision ID: d3aaeb6a9e6b +Revises: b28ac9090d40 +Create Date: 2024-12-10 14:16:32.223456+00:00 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'd3aaeb6a9e6b' +down_revision = 'b28ac9090d40' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('selected_books', + sa.Column('id', sa.Integer()), + sa.Column('patron_id', sa.Integer()), + sa.Column('work_id', sa.Integer()), + sa.Column('creation_date', sa.DateTime(timezone=True)), + sa.ForeignKeyConstraint(['patron_id'], ['patrons.id'], ), + sa.ForeignKeyConstraint(['work_id'], ['works.id'], ), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('patron_id', 'work_id') + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('selected_books') + # ### end Alembic commands ### diff --git a/core/model/__init__.py b/core/model/__init__.py index 5a18153e5..4060f1fc5 100644 --- a/core/model/__init__.py +++ b/core/model/__init__.py @@ -564,6 +564,7 @@ def _bulk_operation(self): LoanCheckout, Patron, PatronProfileStorage, + SelectedBook, ) from core.model.resource import ( Hyperlink, diff --git a/core/model/patron.py b/core/model/patron.py index 1e0c7272a..052a06570 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -194,6 +194,13 @@ class Patron(Base): # than this time. MAX_SYNC_TIME = datetime.timedelta(hours=12) + selected_books: Mapped[list[SelectedBook]] = relationship( + "SelectedBook", + backref="patron", + cascade="delete", + order_by="SelectedBook.creation_date", + ) + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.neighborhood: str | None = None @@ -724,6 +731,29 @@ def update(self, start, end, position): __table_args__ = (UniqueConstraint("patron_id", "license_pool_id"),) +class SelectedBook(Base): + __tablename__ = "selected_books" + + id = Column(Integer, primary_key=True) + patron_id = Column(Integer, ForeignKey("patrons.id")) + work_id = Column(Integer, ForeignKey("works.id")) + creation_date = Column(DateTime(timezone=True)) + + __table_args__ = (UniqueConstraint("patron_id", "work_id"),) + + def __init__(self, patron, work): + self.patron_id = patron.id + self.work_id = work.id + self.creation_date = utc_now() + + def __repr__(self): + return "".format( + self.patron.id, + self.work.title, + self.creation_date, + ) + + class Annotation(Base): # The Web Annotation Data Model defines a basic set of motivations. # https://www.w3.org/TR/annotation-model/#motivation-and-purpose diff --git a/core/model/work.py b/core/model/work.py index 36f59c310..b2a799970 100644 --- a/core/model/work.py +++ b/core/model/work.py @@ -48,6 +48,7 @@ from core.model.edition import Edition from core.model.identifier import Identifier, RecursiveEquivalencyCache from core.model.measurement import Measurement +from core.model.patron import Patron from core.util import LanguageCodes from core.util.datetime_helpers import utc_now @@ -215,6 +216,10 @@ class Work(Base): "summary_text", ] + selected_by_patrons: Mapped[list[Patron]] = relationship( + "SelectedBook", backref="work" + ) + @property def title(self): if self.presentation_edition: From 54c57317b5d5c5bc8d409e41fd925717d38bfa05 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:17:20 +0200 Subject: [PATCH 02/35] EKIRJASTO-131 Methods to add/delete/get books --- core/model/patron.py | 51 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/core/model/patron.py b/core/model/patron.py index 052a06570..330d048bf 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -524,6 +524,57 @@ def ensure_tuple(x): log.debug("Both audience and target age match; it's age-appropriate.") return True + def select_book(self, work) -> SelectedBook: + """ + Add a book to patron's selected books. If the book is already selected, the + existing book is returned. + + :param work: Work to select + + :return: SelectedBook object + """ + selected_book = self.load_selected_book(work) + if not selected_book: + selected_book = SelectedBook(patron=self, work=work) + db = Session.object_session(self) + db.add(selected_book) + db.commit() + return selected_book + + def unselect_book(self, work) -> None: + """ + Remove a book from patron's selected books. + + :param work: Work to select + + :return: None + """ + selected_book = self.load_selected_book(work) + if selected_book: + db = Session.object_session(self) + db.delete(selected_book) + db.commit() + return None + + def load_selected_book(self, work) -> SelectedBook | None: + """ + Load the selected book for the given work. + + :param work: Work to load + + :return: SelectedBook object or None + """ + selected_book = [sb for sb in self.selected_books if sb.work_id == work.id] + return selected_book[0] if selected_book else None + + def get_selected_books(self): + """ + Get the selected books for the patron. + + :return: A list of SelectedBook objects + """ + return self.selected_books + Index( "ix_patron_library_id_external_identifier", From 1af495e08468508fa5473304df50db4f764c01b6 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:18:25 +0200 Subject: [PATCH 03/35] EKIRJASTO-131 Routes to add/delete/get books --- api/routes.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/api/routes.py b/api/routes.py index de085b103..35433b83c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -384,6 +384,40 @@ def patron_auth_token(): return app.manager.patron_auth_token.get_token() +@library_dir_route( + "/works///select_book", methods=["POST"] +) +@has_library +@allows_patron_web +@requires_auth +@returns_problem_detail +@compressible +def select_book(identifier_type, identifier): + return app.manager.select_books.select(identifier_type, identifier) + + +@library_dir_route( + "/works///unselect_book", methods=["DELETE"] +) +@has_library +@allows_patron_web +@requires_auth +@returns_problem_detail +@compressible +def unselect_book(identifier_type, identifier): + return app.manager.select_books.unselect(identifier_type, identifier) + + +@library_dir_route("/selected_books", methods=["GET", "HEAD"]) +@has_library +@allows_patron_web +@requires_auth +@returns_problem_detail +@compressible +def selected_books(): + return app.manager.select_books.fetch_books() + + @library_dir_route("/loans", methods=["GET", "HEAD"]) @has_library @allows_patron_web From 12b4ec42bcf824589364ca1e7c2c36c19ea00cca Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:22:25 +0200 Subject: [PATCH 04/35] EKIRJASTO-131 Responses for select/unselect requests --- api/controller/select_books.py | 110 +++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 api/controller/select_books.py diff --git a/api/controller/select_books.py b/api/controller/select_books.py new file mode 100644 index 000000000..3260a8aff --- /dev/null +++ b/api/controller/select_books.py @@ -0,0 +1,110 @@ +from __future__ import annotations + +from typing import Any + +import flask +from flask import Response, redirect +from flask_babel import lazy_gettext as _ +from lxml import etree +from werkzeug import Response as wkResponse + +from api.circulation_exceptions import ( + AuthorizationBlocked, + AuthorizationExpired, + CirculationException, + PatronAuthorizationFailedException, +) +from api.controller.circulation_manager import CirculationManagerController +from core.feed.acquisition import OPDSAcquisitionFeed +from core.model.patron import SelectedBook +from core.util.http import RemoteIntegrationException +from core.util.opds_writer import OPDSFeed +from core.util.problem_detail import ProblemDetail + +class SelectBooksController(CirculationManagerController): + + def fetch_books(self, work_identifier): + patron = flask.request.patron + selected_booklist = patron.get_selected_books() + + for book in selected_booklist: + if book.identifier == work_identifier: + return book + + return None + + def unselect(self, identifier_type, identifier): + """ + Unselect a book from the authenticated patron's selected books list. + + This method returns an OPDS entry with loan or hold-specific information. + + :param identifier_type: The type of identifier for the book + :param identifier: The identifier for the book + + :return: a Response object + """ + library = flask.request.library + work = self.load_work(library, identifier_type, identifier) + patron = flask.request.patron + pools = self.load_licensepools(library, identifier_type, identifier) + + unselected_book = patron.unselect_book(work) + item = self._get_patron_loan_or_hold(patron, pools) + + return OPDSAcquisitionFeed.single_entry_loans_feed( + self.circulation, item, selected_book=unselected_book + ) + + def select(self, identifier_type, identifier): + """ + Add a book to the authenticated patron's selected books list. + + This method returns an OPDS entry with the selected book and + loan or hold-specific information. + + :param identifier_type: The type of the book identifier (e.g., ISBN). + :param identifier: The identifier for the book. + + :return: An OPDSEntryResponse containing the selected book information. + """ + library = flask.request.library + work = self.load_work(library, identifier_type, identifier) + patron = flask.request.patron + pools = self.load_licensepools(library, identifier_type, identifier) + + if isinstance(pools, ProblemDetail): + return pools + + selected_book = patron.select_book(work) + + + item = self._get_patron_loan_or_hold(patron, pools) + + return OPDSAcquisitionFeed.single_entry_loans_feed( + self.circulation, item, selected_book=selected_book + ) + + def _get_patron_loan_or_hold(self, patron, pools): + """ + Retrieve the active loan or hold for a patron from a set of license pools. + + This method checks if the patron has an active loan or hold for any of the + given license pools. If an active loan is found, it is returned alongside + the corresponding license pool. If no loan is found, it checks for an active + hold. If neither a loan nor a hold is found, it returns the first license + pool from the list. + + :param patron: The patron for whom to find an active loan or hold. + :param pools: A list of LicensePool objects associated with the identifier. + :return: An active Loan or Hold object, or a LicensePool if no loan or hold is found. + """ + loan, pool = self.get_patron_loan(patron, pools) + hold = None + + if not loan: + hold, pool = self.get_patron_hold(patron, pools) + + item = loan or hold + pool = pool or pools[0] + return item or pool \ No newline at end of file From e4cba0aa59287bf853a7135892c52e058baf9e4d Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:23:27 +0200 Subject: [PATCH 05/35] EKIRJASTO-131 Add controller to circ manager --- api/circulation_manager.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/circulation_manager.py b/api/circulation_manager.py index 701b8144c..c43afe94c 100644 --- a/api/circulation_manager.py +++ b/api/circulation_manager.py @@ -25,6 +25,7 @@ from api.controller.patron_auth_token import PatronAuthTokenController from api.controller.playtime_entries import PlaytimeEntriesController from api.controller.profile import ProfileController +from api.controller.select_books import SelectBooksController from api.controller.urn_lookup import URNLookupController from api.controller.work import WorkController from api.custom_index import CustomIndexView @@ -91,6 +92,7 @@ class CirculationManager(LoggerMixin): version: ApplicationVersionController odl_notification_controller: ODLNotificationController playtime_entries: PlaytimeEntriesController + select_books: SelectBooksController # Admin controllers admin_sign_in_controller: SignInController @@ -327,6 +329,7 @@ def setup_one_time_controllers(self): self.patron_auth_token = PatronAuthTokenController(self) self.catalog_descriptions = CatalogDescriptionsController(self) self.playtime_entries = PlaytimeEntriesController(self) + self.select_books = SelectBooksController(self) def setup_configuration_dependent_controllers(self): """Set up all the controllers that depend on the From ec42aff786288c34b954ef97ba45400a2564f193 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:25:07 +0200 Subject: [PATCH 06/35] EKIRJASTO-131 Add selected book to library annotator --- core/feed/annotator/circulation.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/feed/annotator/circulation.py b/core/feed/annotator/circulation.py index 8f2f500c7..3b4eefb41 100644 --- a/core/feed/annotator/circulation.py +++ b/core/feed/annotator/circulation.py @@ -53,7 +53,7 @@ LicensePool, LicensePoolDeliveryMechanism, ) -from core.model.patron import Hold, Loan, Patron +from core.model.patron import Hold, Loan, Patron, SelectedBook from core.model.work import Work from core.service.container import Services from core.util.datetime_helpers import from_timestamp @@ -696,6 +696,7 @@ def __init__( top_level_title: str = "All Books", library_identifies_patrons: bool = True, facets: FacetsWithEntryPoint | None = None, + selected_book: SelectedBook | None = None, ) -> None: """Constructor. @@ -727,6 +728,7 @@ def __init__( self._top_level_title = top_level_title self.identifies_patrons = library_identifies_patrons self.facets = facets or None + self.selected_book = selected_book def top_level_title(self) -> str: return self._top_level_title @@ -934,6 +936,9 @@ def annotate_work_entry( ) ) + if self.selected_book: + entry.computed.selected = strftime(self.selected_book.creation_date) + if self.analytics.is_configured(): entry.computed.other_links.append( Link( From 09594eb702f0fdb677127c2efe1615049b720310 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:25:52 +0200 Subject: [PATCH 07/35] EKIRJASTO-131 Add selected book to patron's feed --- core/feed/acquisition.py | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/core/feed/acquisition.py b/core/feed/acquisition.py index 81504e3ed..6598b1f44 100644 --- a/core/feed/acquisition.py +++ b/core/feed/acquisition.py @@ -26,7 +26,7 @@ from core.model.edition import Edition from core.model.identifier import Identifier from core.model.licensing import LicensePool -from core.model.patron import Hold, Loan, Patron +from core.model.patron import Hold, Loan, Patron, SelectedBook from core.model.work import Work from core.problem_details import INVALID_INPUT from core.util.datetime_helpers import utc_now @@ -518,9 +518,36 @@ def single_entry_loans_feed( item: LicensePool | Loan, annotator: LibraryAnnotator | None = None, fulfillment: FulfillmentInfo | None = None, + selected_book: SelectedBook | None = None, **response_kwargs: Any, ) -> OPDSEntryResponse | ProblemDetail | None: - """A single entry as a standalone feed specific to a patron""" + """ + Returns a single entry feed for a patron's loan or hold, including + information about the loan, hold, and selected book. + + Args: + circulation: The circulation object associated with the patron. + item: The loan or hold object to generate the feed for. Can be + a LicensePool, Loan, or Hold. + annotator: An optional LibraryAnnotator object to use for + annotating the feed. If not provided, a default annotator + will be created. + fulfillment: An optional FulfillmentInfo object to include in + the feed. + selected_book: An optional SelectedBook object to include in + the feed. + **response_kwargs: Additional keyword arguments to pass to the + response generation. + + Returns: + An OPDSEntryResponse object containing the feed, a ProblemDetail + object if an error occurs, or None if the feed cannot be + generated. + + Raises: + ValueError: If the 'item' argument is empty or not an instance of + LicensePool, Loan, or Hold. + """ if not item: raise ValueError("Argument 'item' must be non-empty") @@ -578,6 +605,8 @@ def single_entry_loans_feed( annotator.active_fulfillments_by_work = active_fulfillments_by_work identifier = license_pool.identifier + annotator.selected_book = selected_book + entry = cls.single_entry(work, annotator, even_if_no_license_pool=True) if isinstance(entry, WorkEntry) and entry.computed: From b34decdaf1057f6f4dcafd2688f187daba78029a Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:26:47 +0200 Subject: [PATCH 08/35] EKIRJASTO-131 Add selected as WorkEntryData --- core/feed/types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/feed/types.py b/core/feed/types.py index 18cbaaaec..41b06c1e1 100644 --- a/core/feed/types.py +++ b/core/feed/types.py @@ -163,6 +163,7 @@ class WorkEntryData(BaseModel): subtitle: FeedEntryType | None = None series: FeedEntryType | None = None imprint: FeedEntryType | None = None + selected: datetime | date | None = None authors: list[Author] = field(default_factory=list) contributors: list[Author] = field(default_factory=list) From 8a3bb2093e070cf95736286135cdebd86363c562 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:27:39 +0200 Subject: [PATCH 09/35] EKIRJASTO-131 Add selected tag to OPDS feed --- core/feed/serializer/opds.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/feed/serializer/opds.py b/core/feed/serializer/opds.py index 76d78c377..a57b29b0f 100644 --- a/core/feed/serializer/opds.py +++ b/core/feed/serializer/opds.py @@ -29,6 +29,7 @@ "patron": f"{{{OPDSFeed.SIMPLIFIED_NS}}}patron", "series": f"{{{OPDSFeed.SCHEMA_NS}}}series", "hashed_passphrase": f"{{{OPDSFeed.LCP_NS}}}hashed_passphrase", + "selected": f"{{{OPDSFeed.SIMPLIFIED_NS}}}selected", } ATTRIBUTE_MAPPING = { @@ -264,6 +265,9 @@ def serialize_work_entry(self, feed_entry: WorkEntryData) -> etree._Element: for link in feed_entry.other_links: entry.append(OPDSFeed.link(**link.asdict())) + if feed_entry.selected: + entry.append(OPDSFeed.E("selected", feed_entry.selected)) + return entry def serialize_opds_message(self, entry: OPDSMessage) -> etree._Element: From be64d30a853220cd1256a29c6ff525f37870f0df Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 10:30:04 +0200 Subject: [PATCH 10/35] EKIRJASTO-131 Add selected book to work feed --- api/controller/work.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/controller/work.py b/api/controller/work.py index d50928f8a..49f43f273 100644 --- a/api/controller/work.py +++ b/api/controller/work.py @@ -96,8 +96,7 @@ def contributor( def permalink(self, identifier_type, identifier): """Serve an entry for a single book. - This does not include any loan or hold-specific information for - the authenticated patron. + This includes any loan or hold-specific information as well as selected book information for an authenticated patron. This is different from the /works lookup protocol, in that it returns a single entry while the /works lookup protocol returns a @@ -124,8 +123,10 @@ def permalink(self, identifier_type, identifier): item = loan or hold pool = pool or pools[0] + selected_book = patron.load_selected_book(work) + return OPDSAcquisitionFeed.single_entry_loans_feed( - self.circulation, item or pool + self.circulation, item or pool, selected_book=selected_book ) else: annotator = self.manager.annotator(lane=None) From 5922d964c68a8a48ba00ee7baf06c667ace845e1 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 17:21:14 +0200 Subject: [PATCH 11/35] EKIRJASTO-131 Fetch Works in patron.selected_books --- .gitignore | 2 +- core/model/patron.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 5ab1f2460..e4cdf44f1 100644 --- a/.gitignore +++ b/.gitignore @@ -5,7 +5,7 @@ # Editors .vscode/ .idea/ - +.venv_311 # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] diff --git a/core/model/patron.py b/core/model/patron.py index 330d048bf..b50c208db 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -567,13 +567,15 @@ def load_selected_book(self, work) -> SelectedBook | None: selected_book = [sb for sb in self.selected_books if sb.work_id == work.id] return selected_book[0] if selected_book else None - def get_selected_books(self): + def get_selected_works(self) -> list[Work]: """ - Get the selected books for the patron. + Fetch a list of Works that the patron has selected. - :return: A list of SelectedBook objects + :return: A list of Work objects """ - return self.selected_books + selected_book_objects = self.selected_books + selected_works = [sb.work for sb in selected_book_objects] + return selected_works Index( From d316b0c3c281d38589f2102a98d724271384fd4f Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 17:25:08 +0200 Subject: [PATCH 12/35] EKIRJASTO-131 Feed with all patron's selected books --- api/controller/select_books.py | 75 +++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/api/controller/select_books.py b/api/controller/select_books.py index 3260a8aff..931d32c8e 100644 --- a/api/controller/select_books.py +++ b/api/controller/select_books.py @@ -1,38 +1,39 @@ from __future__ import annotations -from typing import Any - import flask -from flask import Response, redirect -from flask_babel import lazy_gettext as _ -from lxml import etree -from werkzeug import Response as wkResponse - -from api.circulation_exceptions import ( - AuthorizationBlocked, - AuthorizationExpired, - CirculationException, - PatronAuthorizationFailedException, -) + from api.controller.circulation_manager import CirculationManagerController from core.feed.acquisition import OPDSAcquisitionFeed -from core.model.patron import SelectedBook -from core.util.http import RemoteIntegrationException -from core.util.opds_writer import OPDSFeed from core.util.problem_detail import ProblemDetail + class SelectBooksController(CirculationManagerController): + def fetch_books(self): + """ + Generate an OPDS feed response containing the selected books for the + authenticated patron. - def fetch_books(self, work_identifier): + This method creates an OPDS acquisition feed with the books currently + selected by the patron and returns it as a response. + + :return: A Response object. + """ patron = flask.request.patron - selected_booklist = patron.get_selected_books() - for book in selected_booklist: - if book.identifier == work_identifier: - return book + feed = OPDSAcquisitionFeed.selected_books_for(self.circulation, patron) + + response = feed.as_response( + max_age=0, + private=True, + mime_types=flask.request.accept_mimetypes, + ) + + # For loans, the patron's last loan activity sync time was set. Not yet + # clear if such is needed for selected books. + # response.last_modified = last_modified + + return response - return None - def unselect(self, identifier_type, identifier): """ Unselect a book from the authenticated patron's selected books list. @@ -50,6 +51,7 @@ def unselect(self, identifier_type, identifier): pools = self.load_licensepools(library, identifier_type, identifier) unselected_book = patron.unselect_book(work) + item = self._get_patron_loan_or_hold(patron, pools) return OPDSAcquisitionFeed.single_entry_loans_feed( @@ -78,27 +80,32 @@ def select(self, identifier_type, identifier): selected_book = patron.select_book(work) - item = self._get_patron_loan_or_hold(patron, pools) return OPDSAcquisitionFeed.single_entry_loans_feed( self.circulation, item, selected_book=selected_book ) - + def _get_patron_loan_or_hold(self, patron, pools): """ - Retrieve the active loan or hold for a patron from a set of license pools. + Retrieve the active loan or hold for a patron from a set of license + pools. - This method checks if the patron has an active loan or hold for any of the - given license pools. If an active loan is found, it is returned alongside - the corresponding license pool. If no loan is found, it checks for an active - hold. If neither a loan nor a hold is found, it returns the first license - pool from the list. + This method checks if the patron has an active loan or hold for any of + the given license pools. If an active loan is found, it is returned + alongside the corresponding license pool. If no loan is found, it + checks for an active hold. If neither a loan nor a hold is found, it + returns the first license pool from the list. :param patron: The patron for whom to find an active loan or hold. - :param pools: A list of LicensePool objects associated with the identifier. - :return: An active Loan or Hold object, or a LicensePool if no loan or hold is found. + :param pools: A list of LicensePool objects associated with the + identifier. + + :return: An active Loan or Hold object, or a LicensePool if no loan + or hold is found. """ + # TODO: move this function to circulation_manager.py becuase it's + # used in multiple controllers loan, pool = self.get_patron_loan(patron, pools) hold = None @@ -107,4 +114,4 @@ def _get_patron_loan_or_hold(self, patron, pools): item = loan or hold pool = pool or pools[0] - return item or pool \ No newline at end of file + return item or pool From 3b809142ce3734578175aae01260beb4b49cb164 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 17:27:20 +0200 Subject: [PATCH 13/35] EKIRJASTO-131 OPDS feed of selected books --- core/feed/acquisition.py | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/core/feed/acquisition.py b/core/feed/acquisition.py index 6598b1f44..fb4f6e8b4 100644 --- a/core/feed/acquisition.py +++ b/core/feed/acquisition.py @@ -511,6 +511,47 @@ def active_loans_for( feed.generate_feed() return feed + @classmethod + def selected_books_for( + cls, + circulation: CirculationAPI | None, + patron: Patron, + annotator: LibraryAnnotator | None = None, + **response_kwargs: Any, + ) -> OPDSAcquisitionFeed: + """ + Generates a patron-specific OPDS acquisition feed containing their selected books. + + Args: + circulation: The circulation API instance (optional). + patron: The authenticated patron. + annotator: The library annotator instance (optional). If not provided, a new instance will be created. + **response_kwargs: Additional keyword arguments to customize the feed generation. + + Returns: + An OPDSAcquisitionFeed object representing the patron's selected books. + """ + selected_books_by_work = {} + for selected_book in patron.selected_books: + work = selected_book.work + if work: + selected_books_by_work[work] = selected_book + + if not annotator: + annotator = LibraryAnnotator(circulation, None, patron.library, patron) + + annotator.selected_books_by_work = selected_books_by_work + url = annotator.url_for( + "selected_books", + library_short_name=patron.library.short_name, + _external=True, + ) + works = patron.get_selected_works() + + feed = OPDSAcquisitionFeed("Selected books", url, works, annotator) + feed.generate_feed() + return feed + @classmethod def single_entry_loans_feed( cls, From e98297a6f0efe39009c79bbae32c1aa6ec4b5b7c Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 17:29:07 +0200 Subject: [PATCH 14/35] EKIRJASTO-131 Change object to dictionary --- core/feed/acquisition.py | 4 +++- core/feed/annotator/circulation.py | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/feed/acquisition.py b/core/feed/acquisition.py index fb4f6e8b4..54155bfcf 100644 --- a/core/feed/acquisition.py +++ b/core/feed/acquisition.py @@ -646,7 +646,9 @@ def single_entry_loans_feed( annotator.active_fulfillments_by_work = active_fulfillments_by_work identifier = license_pool.identifier - annotator.selected_book = selected_book + selected_books_by_work: Any = {} + selected_books_by_work[work] = selected_book + annotator.selected_books_by_work = selected_books_by_work entry = cls.single_entry(work, annotator, even_if_no_license_pool=True) diff --git a/core/feed/annotator/circulation.py b/core/feed/annotator/circulation.py index 3b4eefb41..b5e265695 100644 --- a/core/feed/annotator/circulation.py +++ b/core/feed/annotator/circulation.py @@ -696,7 +696,7 @@ def __init__( top_level_title: str = "All Books", library_identifies_patrons: bool = True, facets: FacetsWithEntryPoint | None = None, - selected_book: SelectedBook | None = None, + selected_books_by_work: dict[Work, SelectedBook] | None = None, ) -> None: """Constructor. @@ -728,7 +728,7 @@ def __init__( self._top_level_title = top_level_title self.identifies_patrons = library_identifies_patrons self.facets = facets or None - self.selected_book = selected_book + self.selected_books_by_work = selected_books_by_work def top_level_title(self) -> str: return self._top_level_title @@ -936,8 +936,10 @@ def annotate_work_entry( ) ) - if self.selected_book: - entry.computed.selected = strftime(self.selected_book.creation_date) + if self.selected_books_by_work[work]: + entry.computed.selected = strftime( + self.selected_books_by_work[work].creation_date + ) if self.analytics.is_configured(): entry.computed.other_links.append( From 74556b1206db929057bb94dac98affefe740b7b1 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 16 Dec 2024 17:31:32 +0200 Subject: [PATCH 15/35] EKIRJASTO-131 Fix quotes --- ...41210_d3aaeb6a9e6b_create_selectedbooks.py | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py b/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py index 261a14983..648b05cc1 100644 --- a/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py +++ b/alembic/versions/20241210_d3aaeb6a9e6b_create_selectedbooks.py @@ -5,33 +5,40 @@ Create Date: 2024-12-10 14:16:32.223456+00:00 """ -from alembic import op import sqlalchemy as sa +from alembic import op # revision identifiers, used by Alembic. -revision = 'd3aaeb6a9e6b' -down_revision = 'b28ac9090d40' +revision = "d3aaeb6a9e6b" +down_revision = "b28ac9090d40" branch_labels = None depends_on = None def upgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - op.create_table('selected_books', - sa.Column('id', sa.Integer()), - sa.Column('patron_id', sa.Integer()), - sa.Column('work_id', sa.Integer()), - sa.Column('creation_date', sa.DateTime(timezone=True)), - sa.ForeignKeyConstraint(['patron_id'], ['patrons.id'], ), - sa.ForeignKeyConstraint(['work_id'], ['works.id'], ), - sa.PrimaryKeyConstraint('id'), - sa.UniqueConstraint('patron_id', 'work_id') + op.create_table( + "selected_books", + sa.Column("id", sa.Integer()), + sa.Column("patron_id", sa.Integer()), + sa.Column("work_id", sa.Integer()), + sa.Column("creation_date", sa.DateTime(timezone=True)), + sa.ForeignKeyConstraint( + ["patron_id"], + ["patrons.id"], + ), + sa.ForeignKeyConstraint( + ["work_id"], + ["works.id"], + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("patron_id", "work_id"), ) # ### end Alembic commands ### def downgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - op.drop_table('selected_books') + op.drop_table("selected_books") # ### end Alembic commands ### From 467e769f124186291f94740f4ccc320d7708fb92 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 10:40:07 +0200 Subject: [PATCH 16/35] EKIRJASTO-131 Include selected books to response --- api/controller/loan.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/api/controller/loan.py b/api/controller/loan.py index fa59fab49..0107fb977 100644 --- a/api/controller/loan.py +++ b/api/controller/loan.py @@ -152,8 +152,13 @@ def borrow(self, identifier_type, identifier, mechanism_id=None): "status": 201 if is_new else 200, "mime_types": flask.request.accept_mimetypes, } + + work = self.load_work(library, identifier_type, identifier) + selected_book = patron.load_selected_book(work) + + return OPDSAcquisitionFeed.single_entry_loans_feed( - self.circulation, loan_or_hold, **response_kwargs + self.circulation, loan_or_hold, selected_book=selected_book, **response_kwargs ) def _borrow(self, patron, credential, pool, mechanism): @@ -555,6 +560,7 @@ def revoke(self, license_pool_id): def detail(self, identifier_type, identifier): if flask.request.method == "DELETE": + # Causes an error becuase the function is not in LoansController but route! return self.revoke_loan_or_hold(identifier_type, identifier) patron = flask.request.patron @@ -578,9 +584,12 @@ def detail(self, identifier_type, identifier): status_code=404, ) + work = self.load_work(library, identifier_type, identifier) + selected_book = patron.load_selected_book(work) + if flask.request.method == "GET": if loan: item = loan else: item = hold - return OPDSAcquisitionFeed.single_entry_loans_feed(self.circulation, item) + return OPDSAcquisitionFeed.single_entry_loans_feed(self.circulation, item, selected_book=selected_book) From 4704f9069567a2c69b9bdd763f202bba50009732 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 10:40:49 +0200 Subject: [PATCH 17/35] EKIRJASTO-131 Check for empty dictionary --- core/feed/annotator/circulation.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/feed/annotator/circulation.py b/core/feed/annotator/circulation.py index b5e265695..2b21e2f1f 100644 --- a/core/feed/annotator/circulation.py +++ b/core/feed/annotator/circulation.py @@ -936,10 +936,12 @@ def annotate_work_entry( ) ) - if self.selected_books_by_work[work]: - entry.computed.selected = strftime( - self.selected_books_by_work[work].creation_date - ) + # Selected books is from LibraryAnnotator + if self.selected_books_by_work and work in self.selected_books_by_work: + if self.selected_books_by_work[work]: + entry.computed.selected = strftime( + self.selected_books_by_work[work].creation_date + ) if self.analytics.is_configured(): entry.computed.other_links.append( From 1b3f36dd56961bd1a499b21434e7f8b00d1fab67 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 10:41:34 +0200 Subject: [PATCH 18/35] EKIRJASTO-131 Route for selected book details --- api/routes.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/api/routes.py b/api/routes.py index 35433b83c..e12d2d2ee 100644 --- a/api/routes.py +++ b/api/routes.py @@ -418,6 +418,16 @@ def selected_books(): return app.manager.select_books.fetch_books() +@library_route("/selected_books//", methods=["GET", "DELETE"]) +@has_library +@allows_patron_web +@requires_auth +@returns_problem_detail +def selected_book_detail(identifier_type, identifier): + return app.manager.select_books.detail(identifier_type, identifier) + + + @library_dir_route("/loans", methods=["GET", "HEAD"]) @has_library @allows_patron_web From cb7a6e6ab9f7bcbc6e0a3ed9b7d13ce2871e4d74 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 10:42:47 +0200 Subject: [PATCH 19/35] EKIRJASTO-131 Response to details get/delete req --- api/controller/select_books.py | 40 +++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/api/controller/select_books.py b/api/controller/select_books.py index 931d32c8e..ebd9e68ea 100644 --- a/api/controller/select_books.py +++ b/api/controller/select_books.py @@ -16,7 +16,7 @@ def fetch_books(self): This method creates an OPDS acquisition feed with the books currently selected by the patron and returns it as a response. - :return: A Response object. + :return: An OPDSEntryResponse. """ patron = flask.request.patron @@ -43,7 +43,7 @@ def unselect(self, identifier_type, identifier): :param identifier_type: The type of identifier for the book :param identifier: The identifier for the book - :return: a Response object + :return: An OPDSEntryResponse """ library = flask.request.library work = self.load_work(library, identifier_type, identifier) @@ -68,7 +68,7 @@ def select(self, identifier_type, identifier): :param identifier_type: The type of the book identifier (e.g., ISBN). :param identifier: The identifier for the book. - :return: An OPDSEntryResponse containing the selected book information. + :return: An OPDSEntryResponse. """ library = flask.request.library work = self.load_work(library, identifier_type, identifier) @@ -115,3 +115,37 @@ def _get_patron_loan_or_hold(self, patron, pools): item = loan or hold pool = pool or pools[0] return item or pool + + def detail(self, identifier_type, identifier): + + """ + Return an OPDS feed entry for a selected book. + + If the request method is DELETE, this method unselects the book. + + Whether the request is GET or DELETE, it returns an OPDS entry with + loan or hold-specific information and the selected book information. + + :param identifier_type: The type of the book identifier (e.g., ISBN). + :param identifier: The identifier for the book. + + :return: An OPDSEntryResponse. + """ + patron = flask.request.patron + library = flask.request.library + + if flask.request.method == "DELETE": + return self.unselect(identifier_type, identifier) + + if flask.request.method == "GET": + + pools = self.load_licensepools(library, identifier_type, identifier) + if isinstance(pools, ProblemDetail): + return pools + + item = self._get_patron_loan_or_hold(patron, pools) + + work = self.load_work(library, identifier_type, identifier) + selected_book = patron.load_selected_book(work) + + return OPDSAcquisitionFeed.single_entry_loans_feed(self.circulation, item, selected_book=selected_book) From 33c7f1f6007fa9f85936bc5bd9dac0d1dfc39a82 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 10:44:33 +0200 Subject: [PATCH 20/35] EKIRJASTO-131 Remove redundant HEAD request --- api/routes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/routes.py b/api/routes.py index e12d2d2ee..71b58dc36 100644 --- a/api/routes.py +++ b/api/routes.py @@ -408,7 +408,7 @@ def unselect_book(identifier_type, identifier): return app.manager.select_books.unselect(identifier_type, identifier) -@library_dir_route("/selected_books", methods=["GET", "HEAD"]) +@library_dir_route("/selected_books", methods=["GET"]) @has_library @allows_patron_web @requires_auth From 9fbf3ba590460c2f02bff972769fa8686af952fa Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 10:46:01 +0200 Subject: [PATCH 21/35] EKIRJASTO-131 Linting fixes --- api/controller/loan.py | 10 +++++++--- api/controller/select_books.py | 6 +++--- api/routes.py | 5 +++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/api/controller/loan.py b/api/controller/loan.py index 0107fb977..2414138f5 100644 --- a/api/controller/loan.py +++ b/api/controller/loan.py @@ -156,9 +156,11 @@ def borrow(self, identifier_type, identifier, mechanism_id=None): work = self.load_work(library, identifier_type, identifier) selected_book = patron.load_selected_book(work) - return OPDSAcquisitionFeed.single_entry_loans_feed( - self.circulation, loan_or_hold, selected_book=selected_book, **response_kwargs + self.circulation, + loan_or_hold, + selected_book=selected_book, + **response_kwargs, ) def _borrow(self, patron, credential, pool, mechanism): @@ -592,4 +594,6 @@ def detail(self, identifier_type, identifier): item = loan else: item = hold - return OPDSAcquisitionFeed.single_entry_loans_feed(self.circulation, item, selected_book=selected_book) + return OPDSAcquisitionFeed.single_entry_loans_feed( + self.circulation, item, selected_book=selected_book + ) diff --git a/api/controller/select_books.py b/api/controller/select_books.py index ebd9e68ea..225837a44 100644 --- a/api/controller/select_books.py +++ b/api/controller/select_books.py @@ -117,7 +117,6 @@ def _get_patron_loan_or_hold(self, patron, pools): return item or pool def detail(self, identifier_type, identifier): - """ Return an OPDS feed entry for a selected book. @@ -138,7 +137,6 @@ def detail(self, identifier_type, identifier): return self.unselect(identifier_type, identifier) if flask.request.method == "GET": - pools = self.load_licensepools(library, identifier_type, identifier) if isinstance(pools, ProblemDetail): return pools @@ -148,4 +146,6 @@ def detail(self, identifier_type, identifier): work = self.load_work(library, identifier_type, identifier) selected_book = patron.load_selected_book(work) - return OPDSAcquisitionFeed.single_entry_loans_feed(self.circulation, item, selected_book=selected_book) + return OPDSAcquisitionFeed.single_entry_loans_feed( + self.circulation, item, selected_book=selected_book + ) diff --git a/api/routes.py b/api/routes.py index 71b58dc36..2dc718b39 100644 --- a/api/routes.py +++ b/api/routes.py @@ -418,7 +418,9 @@ def selected_books(): return app.manager.select_books.fetch_books() -@library_route("/selected_books//", methods=["GET", "DELETE"]) +@library_route( + "/selected_books//", methods=["GET", "DELETE"] +) @has_library @allows_patron_web @requires_auth @@ -427,7 +429,6 @@ def selected_book_detail(identifier_type, identifier): return app.manager.select_books.detail(identifier_type, identifier) - @library_dir_route("/loans", methods=["GET", "HEAD"]) @has_library @allows_patron_web From fe94ba32bc8730f8bec11a9459f1f8e44a0c0a3b Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Wed, 18 Dec 2024 14:13:47 +0200 Subject: [PATCH 22/35] EKIRJASTO-131 Test Patron selected books --- tests/core/models/test_patron.py | 50 +++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/tests/core/models/test_patron.py b/tests/core/models/test_patron.py index d2dcb69f1..fe9a00dd0 100644 --- a/tests/core/models/test_patron.py +++ b/tests/core/models/test_patron.py @@ -9,7 +9,15 @@ from core.model.credential import Credential from core.model.datasource import DataSource from core.model.licensing import PolicyException -from core.model.patron import Annotation, Hold, Loan, Patron, PatronProfileStorage +from core.model.patron import ( + Annotation, + Hold, + Loan, + Patron, + PatronProfileStorage, + SelectedBook, +) +from core.model.work import Work from core.util.datetime_helpers import datetime_utc, utc_now from tests.fixtures.database import DatabaseTransactionFixture from tests.fixtures.library import LibraryFixture @@ -737,6 +745,46 @@ def test_age_appropriate_match(self): work_audience, work_target_age, reader_audience, reader_age ) + def test_selected_books(self, db: DatabaseTransactionFixture): + """Test that a patron can have books added to their selected books list + and removed from it. Also, test that the booklist is cleaned up when + the patron is deleted.""" + + patron = db.patron() + book = db.work() + + # Add the book to the patron's booklist + selected_book = patron.select_book(book) + db.session.commit() + assert selected_book in patron.selected_books + + patron.load_selected_book(book) + db.session.commit() + assert selected_book != None + assert selected_book in patron.selected_books + + # Delete the book from the patron's booklist + patron.unselect_book(book) + db.session.commit() + assert book not in patron.selected_books + + # Delete the patron + db.session.delete(patron) + db.session.commit() + + # Check that the patron's booklist is empty + assert ( + len( + db.session.query(SelectedBook) + .filter(SelectedBook.patron_id == patron.id) + .all() + ) + == 0 + ) + + # Check that the book still exists (just for our sanity) + assert len(db.session.query(Work).all()) == 1 + def mock_url_for(url, **kwargs): item_list = [f"{k}={v}" for k, v in kwargs.items()] From a7cfef8a32ee59c69a9d1e62c6dba4c17a27f313 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 11:55:33 +0200 Subject: [PATCH 23/35] EKIRJASTO-131 Add work relationship Also removed the relationship from work side because there's no use for it --- core/model/patron.py | 5 ++++- core/model/work.py | 3 --- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/model/patron.py b/core/model/patron.py index b50c208db..d6957c07e 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -567,7 +567,7 @@ def load_selected_book(self, work) -> SelectedBook | None: selected_book = [sb for sb in self.selected_books if sb.work_id == work.id] return selected_book[0] if selected_book else None - def get_selected_works(self) -> list[Work]: + def get_selected_works(self): """ Fetch a list of Works that the patron has selected. @@ -792,12 +792,15 @@ class SelectedBook(Base): work_id = Column(Integer, ForeignKey("works.id")) creation_date = Column(DateTime(timezone=True)) + work = relationship("Work", backref="selected_books") + __table_args__ = (UniqueConstraint("patron_id", "work_id"),) def __init__(self, patron, work): self.patron_id = patron.id self.work_id = work.id self.creation_date = utc_now() + self.work = work def __repr__(self): return "".format( diff --git a/core/model/work.py b/core/model/work.py index b2a799970..4b33f3ecc 100644 --- a/core/model/work.py +++ b/core/model/work.py @@ -216,9 +216,6 @@ class Work(Base): "summary_text", ] - selected_by_patrons: Mapped[list[Patron]] = relationship( - "SelectedBook", backref="work" - ) @property def title(self): From d5a89e682b9a5372b1809fab44f47ecfbda9f418 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 11:56:18 +0200 Subject: [PATCH 24/35] EKIRJASTO-131 Remove date type --- core/feed/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/feed/types.py b/core/feed/types.py index 41b06c1e1..a44cca41f 100644 --- a/core/feed/types.py +++ b/core/feed/types.py @@ -163,7 +163,7 @@ class WorkEntryData(BaseModel): subtitle: FeedEntryType | None = None series: FeedEntryType | None = None imprint: FeedEntryType | None = None - selected: datetime | date | None = None + selected: datetime | None = None authors: list[Author] = field(default_factory=list) contributors: list[Author] = field(default_factory=list) From 87490e0625403f0d7177b17355d4cd6c254ab707 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 14:19:09 +0200 Subject: [PATCH 25/35] EKIRJASTO-131 Fix work relation issue --- core/feed/acquisition.py | 2 +- core/model/patron.py | 5 +---- core/model/work.py | 3 +++ 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/feed/acquisition.py b/core/feed/acquisition.py index 54155bfcf..d64ea6cb6 100644 --- a/core/feed/acquisition.py +++ b/core/feed/acquisition.py @@ -533,7 +533,7 @@ def selected_books_for( """ selected_books_by_work = {} for selected_book in patron.selected_books: - work = selected_book.work + work = selected_book.work # type: ignore if work: selected_books_by_work[work] = selected_book diff --git a/core/model/patron.py b/core/model/patron.py index d6957c07e..77cec3bad 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -574,7 +574,7 @@ def get_selected_works(self): :return: A list of Work objects """ selected_book_objects = self.selected_books - selected_works = [sb.work for sb in selected_book_objects] + selected_works = [sb.work for sb in selected_book_objects] # type: ignore return selected_works @@ -792,15 +792,12 @@ class SelectedBook(Base): work_id = Column(Integer, ForeignKey("works.id")) creation_date = Column(DateTime(timezone=True)) - work = relationship("Work", backref="selected_books") - __table_args__ = (UniqueConstraint("patron_id", "work_id"),) def __init__(self, patron, work): self.patron_id = patron.id self.work_id = work.id self.creation_date = utc_now() - self.work = work def __repr__(self): return "".format( diff --git a/core/model/work.py b/core/model/work.py index 4b33f3ecc..0863435b0 100644 --- a/core/model/work.py +++ b/core/model/work.py @@ -216,6 +216,9 @@ class Work(Base): "summary_text", ] + selected_by_patrons: Mapped[list[Patron]] = relationship( + "SelectedBook", backref="related_work" + ) @property def title(self): From 775ccac236a6ac5f04b0d118120170ca77ce2610 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 14:20:16 +0200 Subject: [PATCH 26/35] EKIRJASTO-131 Add link to auth doc --- api/authenticator.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/api/authenticator.py b/api/authenticator.py index 1b2faf7ee..c9d55c00f 100644 --- a/api/authenticator.py +++ b/api/authenticator.py @@ -697,6 +697,9 @@ def create_authentication_document(self) -> str: loans_url = url_for( "active_loans", _external=True, library_short_name=self.library_short_name ) + selected_books_url = url_for( + "selected_books", _external=True, library_short_name=self.library_short_name + ) profile_url = url_for( "patron_profile", _external=True, library_short_name=self.library_short_name ) @@ -711,6 +714,13 @@ def create_authentication_document(self) -> str: type=OPDSFeed.ACQUISITION_FEED_TYPE, ) ) + links.append( + dict( + rel="http://opds-spec.org/shelf", + href=selected_books_url, + type=OPDSFeed.ACQUISITION_FEED_TYPE, + ) + ) links.append( dict( rel=ProfileController.LINK_RELATION, From 04dcf0f125668f41c9ec591dd3427259a0ac317b Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 14:44:44 +0200 Subject: [PATCH 27/35] EKIRJASTO-131 Fix type checks --- core/feed/acquisition.py | 2 +- core/feed/annotator/circulation.py | 4 ++-- core/feed/types.py | 2 +- core/model/patron.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/feed/acquisition.py b/core/feed/acquisition.py index d64ea6cb6..9be25b1f2 100644 --- a/core/feed/acquisition.py +++ b/core/feed/acquisition.py @@ -533,7 +533,7 @@ def selected_books_for( """ selected_books_by_work = {} for selected_book in patron.selected_books: - work = selected_book.work # type: ignore + work = selected_book.work # type: ignore if work: selected_books_by_work[work] = selected_book diff --git a/core/feed/annotator/circulation.py b/core/feed/annotator/circulation.py index 2b21e2f1f..f2934ce79 100644 --- a/core/feed/annotator/circulation.py +++ b/core/feed/annotator/circulation.py @@ -939,8 +939,8 @@ def annotate_work_entry( # Selected books is from LibraryAnnotator if self.selected_books_by_work and work in self.selected_books_by_work: if self.selected_books_by_work[work]: - entry.computed.selected = strftime( - self.selected_books_by_work[work].creation_date + entry.computed.selected = strftime( # type: ignore + self.selected_books_by_work[work].creation_date # type: ignore ) if self.analytics.is_configured(): diff --git a/core/feed/types.py b/core/feed/types.py index a44cca41f..41b06c1e1 100644 --- a/core/feed/types.py +++ b/core/feed/types.py @@ -163,7 +163,7 @@ class WorkEntryData(BaseModel): subtitle: FeedEntryType | None = None series: FeedEntryType | None = None imprint: FeedEntryType | None = None - selected: datetime | None = None + selected: datetime | date | None = None authors: list[Author] = field(default_factory=list) contributors: list[Author] = field(default_factory=list) diff --git a/core/model/patron.py b/core/model/patron.py index 77cec3bad..4f4a0a0ee 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -574,7 +574,7 @@ def get_selected_works(self): :return: A list of Work objects """ selected_book_objects = self.selected_books - selected_works = [sb.work for sb in selected_book_objects] # type: ignore + selected_works = [sb.work for sb in selected_book_objects] return selected_works From 5dea53d895f5b2e0c970dd2dae46893bdef51dfc Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 14:45:14 +0200 Subject: [PATCH 28/35] EKIRJASTO-131 Change rel for selected books --- api/authenticator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/authenticator.py b/api/authenticator.py index c9d55c00f..3c9f40a50 100644 --- a/api/authenticator.py +++ b/api/authenticator.py @@ -716,7 +716,7 @@ def create_authentication_document(self) -> str: ) links.append( dict( - rel="http://opds-spec.org/shelf", + rel="http://opds-spec.org/shelf/selected_books", href=selected_books_url, type=OPDSFeed.ACQUISITION_FEED_TYPE, ) From ddd6a3b1d48d395be6627aa20d624bf1b390dde9 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Thu, 19 Dec 2024 15:59:23 +0200 Subject: [PATCH 29/35] EKIRJASTO-131 Fix auth doc test --- .gitignore | 2 +- tests/api/test_authenticator.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index e4cdf44f1..5ab1f2460 100644 --- a/.gitignore +++ b/.gitignore @@ -5,7 +5,7 @@ # Editors .vscode/ .idea/ -.venv_311 + # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] diff --git a/tests/api/test_authenticator.py b/tests/api/test_authenticator.py index f1553f066..8bfc533a4 100644 --- a/tests/api/test_authenticator.py +++ b/tests/api/test_authenticator.py @@ -1180,6 +1180,7 @@ def annotate_authentication_document(library, doc, url_for): reset_link, profile, loans, + selected_books, license, logo, privacy_policy, @@ -1200,6 +1201,10 @@ def annotate_authentication_document(library, doc, url_for): assert "http://opds-spec.org/shelf" == loans["rel"] assert OPDSFeed.ACQUISITION_FEED_TYPE == loans["type"] + assert "/selected_books" in selected_books["href"] + assert "http://opds-spec.org/shelf/selected_books" == selected_books["rel"] + assert OPDSFeed.ACQUISITION_FEED_TYPE == selected_books["type"] + assert "/patrons/me" in profile["href"] assert ProfileController.LINK_RELATION == profile["rel"] assert ProfileController.MEDIA_TYPE == profile["type"] From 3df242c29538efc0aed567160d35a6ad0877a78b Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Fri, 20 Dec 2024 13:13:18 +0200 Subject: [PATCH 30/35] EKIRJASTO-131 Work fixes --- core/model/patron.py | 7 ++++--- core/model/work.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/model/patron.py b/core/model/patron.py index 4f4a0a0ee..14446d8e0 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -792,6 +792,7 @@ class SelectedBook(Base): work_id = Column(Integer, ForeignKey("works.id")) creation_date = Column(DateTime(timezone=True)) + __table_args__ = (UniqueConstraint("patron_id", "work_id"),) def __init__(self, patron, work): @@ -800,9 +801,9 @@ def __init__(self, patron, work): self.creation_date = utc_now() def __repr__(self): - return "".format( - self.patron.id, - self.work.title, + return "".format( + self.patron_id, + self.work_id, self.creation_date, ) diff --git a/core/model/work.py b/core/model/work.py index 0863435b0..b2a799970 100644 --- a/core/model/work.py +++ b/core/model/work.py @@ -217,7 +217,7 @@ class Work(Base): ] selected_by_patrons: Mapped[list[Patron]] = relationship( - "SelectedBook", backref="related_work" + "SelectedBook", backref="work" ) @property From 47e4eb0bcce39b49a1f40afd6f68c5bcf45a90ed Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 23 Dec 2024 13:24:56 +0200 Subject: [PATCH 31/35] EKIRJASTO-131 remove comment --- api/controller/select_books.py | 4 ---- core/model/patron.py | 1 - 2 files changed, 5 deletions(-) diff --git a/api/controller/select_books.py b/api/controller/select_books.py index 225837a44..0fc1f3065 100644 --- a/api/controller/select_books.py +++ b/api/controller/select_books.py @@ -28,10 +28,6 @@ def fetch_books(self): mime_types=flask.request.accept_mimetypes, ) - # For loans, the patron's last loan activity sync time was set. Not yet - # clear if such is needed for selected books. - # response.last_modified = last_modified - return response def unselect(self, identifier_type, identifier): diff --git a/core/model/patron.py b/core/model/patron.py index 14446d8e0..4aaea211b 100644 --- a/core/model/patron.py +++ b/core/model/patron.py @@ -792,7 +792,6 @@ class SelectedBook(Base): work_id = Column(Integer, ForeignKey("works.id")) creation_date = Column(DateTime(timezone=True)) - __table_args__ = (UniqueConstraint("patron_id", "work_id"),) def __init__(self, patron, work): From e0564b6ac3fdbf36b1e37e8472205a578ec23d41 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 23 Dec 2024 13:26:50 +0200 Subject: [PATCH 32/35] EKIRJASTO-131 Add tests for SelectBooksController --- tests/api/controller/test_select_books.py | 191 ++++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 tests/api/controller/test_select_books.py diff --git a/tests/api/controller/test_select_books.py b/tests/api/controller/test_select_books.py new file mode 100644 index 000000000..8a37e84f7 --- /dev/null +++ b/tests/api/controller/test_select_books.py @@ -0,0 +1,191 @@ +import datetime +import urllib.parse +from collections.abc import Generator +from decimal import Decimal +from unittest.mock import MagicMock, patch + +import flask + +import feedparser +import pytest +from flask import Response as FlaskResponse +from flask import url_for +from werkzeug import Response as wkResponse + +from typing import TYPE_CHECKING + +from api.circulation import ( + BaseCirculationAPI, + CirculationAPI, + FulfillmentInfo, + HoldInfo, + LoanInfo, +) +from core.model import ( + DataSource, + Edition, + Identifier, + LicensePool, + SelectedBook, + Work, + get_one, + get_one_or_create, +) +from core.util.datetime_helpers import datetime_utc, utc_now +from core.util.flask_util import Response +from core.util.opds_writer import OPDSFeed +from core.feed.acquisition import OPDSAcquisitionFeed + +from tests.fixtures.api_controller import CirculationControllerFixture +from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture +from core.feed.annotator.circulation import LibraryAnnotator +from core.feed.types import WorkEntry +from core.feed.acquisition import OPDSAcquisitionFeed + + +if TYPE_CHECKING: + from api.controller.select_books import SelectBooksController + + +class SelectBooksFixture(CirculationControllerFixture): + identifier: Identifier + lp: LicensePool + datasource: DataSource + edition: Edition + + + def __init__(self, db: DatabaseTransactionFixture): + super().__init__(db) + [self.lp] = self.english_1.license_pools + self.identifier = self.lp.identifier + self.edition = self.lp.presentation_edition + self.datasource = self.lp.data_source.name # type: ignore + + +@pytest.fixture(scope="function") +def selected_book_fixture(db: DatabaseTransactionFixture): + fixture = SelectBooksFixture(db) + with fixture.wired_container(): + yield fixture + + +class TestSelectBooksController: + + def test_select_unselect_success(self, selected_book_fixture: SelectBooksFixture): + """ + Test that a book can be successfully selected and unselected by a patron. + A successful selection returns a 200 status code, the work is found in + the database and a feed is returned with a tag. + A successful unselection returns a 200 status code, the work is no longer + in the database and a feed is returned without a tag. + """ + + with selected_book_fixture.request_context_with_library("/", headers=dict(Authorization=selected_book_fixture.valid_auth)): + # We have an authenticated patron + patron = selected_book_fixture.manager.select_books.authenticated_patron_from_request() + identifier_type = Identifier.GUTENBERG_ID + identifier = "1234567890" + edition, _ = selected_book_fixture.db.edition( + title="Test Book", + identifier_type=identifier_type, + identifier_id=identifier, + with_license_pool=True, + ) + # The work has an edition + work = selected_book_fixture.db.work( + "Test Book", presentation_edition=edition, with_license_pool=True + ) + + # 1. The patron selects the work + response = selected_book_fixture.manager.select_books.select( + identifier_type, identifier + ) + + assert 200 == response.status_code + # The book should be in the database + selected_book = get_one( + selected_book_fixture.db.session, SelectedBook, work_id=work.id + ) + assert selected_book != None + # The feed should have a tag + feed = feedparser.parse(response.data) + [entry] = feed.entries + assert entry["selected"] + + # 2. Then the patron unselects the work + response = selected_book_fixture.manager.select_books.unselect( + identifier_type, identifier + ) + assert 200 == response.status_code + # The book should no longer show up in the database + selected_book = get_one( + selected_book_fixture.db.session, SelectedBook, work_id=work.id + ) + assert selected_book == None + # The feed should no longer have a tag + feed = feedparser.parse(response.data) + [entry] = feed.entries + assert "selected" not in entry + + + def test_detail(self, selected_book_fixture: SelectBooksFixture): + """ + Test that a selected book's details are fetched successfully. + """ + + with selected_book_fixture.request_context_with_library("/", headers=dict(Authorization=selected_book_fixture.valid_auth)): + # A patron first selects a work + selected_book_fixture.manager.select_books.authenticated_patron_from_request() + identifier_type = Identifier.GUTENBERG_ID + identifier = "1234567890" + edition, _ = selected_book_fixture.db.edition( + title="Test Book", + identifier_type=identifier_type, + identifier_id=identifier, + with_license_pool=True, + ) + work = selected_book_fixture.db.work( + "Test Book", presentation_edition=edition, with_license_pool=True + ) + selected_book_fixture.manager.select_books.select( + identifier_type, identifier + ) + # And then later on the book's details are requested + response = selected_book_fixture.manager.select_books.detail( + identifier_type, identifier + ) + + assert response.status_code == 200 + feed = feedparser.parse(response.data) + [entry] = feed.entries + assert entry["selected"] + + def test_fetch_selected_books_feed(self, selected_book_fixture: SelectBooksFixture): + """ + Test that the selected books acquisition feed is fetched successfully. + """ + + with selected_book_fixture.request_context_with_library("/", headers=dict(Authorization=selected_book_fixture.valid_auth)): + # A patron first selects a work + selected_book_fixture.manager.select_books.authenticated_patron_from_request() + identifier_type = Identifier.GUTENBERG_ID + identifier = "1234567890" + edition, _ = selected_book_fixture.db.edition( + title="Test Book", + identifier_type=identifier_type, + identifier_id=identifier, + with_license_pool=True, + ) + work = selected_book_fixture.db.work( + "Test Book", presentation_edition=edition, with_license_pool=True + ) + selected_book_fixture.manager.select_books.select( + identifier_type, identifier + ) + # And then later the selected books feed is requested + response = selected_book_fixture.manager.select_books.fetch_books() + # The feed contains the selected book + assert response.status_code == 200 + feed = feedparser.parse(response.get_data()) + assert len(feed.entries) == 1 From c0902aa68a8efa73c195ffaf6b362d04385861f9 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 23 Dec 2024 13:54:59 +0200 Subject: [PATCH 33/35] EKIRJASTO-131 Fix linting --- tests/api/controller/test_select_books.py | 53 ++++++----------------- 1 file changed, 14 insertions(+), 39 deletions(-) diff --git a/tests/api/controller/test_select_books.py b/tests/api/controller/test_select_books.py index 8a37e84f7..60d55539b 100644 --- a/tests/api/controller/test_select_books.py +++ b/tests/api/controller/test_select_books.py @@ -1,51 +1,21 @@ -import datetime -import urllib.parse -from collections.abc import Generator -from decimal import Decimal -from unittest.mock import MagicMock, patch - -import flask +from typing import TYPE_CHECKING import feedparser import pytest -from flask import Response as FlaskResponse -from flask import url_for -from werkzeug import Response as wkResponse - -from typing import TYPE_CHECKING -from api.circulation import ( - BaseCirculationAPI, - CirculationAPI, - FulfillmentInfo, - HoldInfo, - LoanInfo, -) from core.model import ( DataSource, Edition, Identifier, LicensePool, SelectedBook, - Work, get_one, - get_one_or_create, ) -from core.util.datetime_helpers import datetime_utc, utc_now -from core.util.flask_util import Response -from core.util.opds_writer import OPDSFeed -from core.feed.acquisition import OPDSAcquisitionFeed - from tests.fixtures.api_controller import CirculationControllerFixture from tests.fixtures.database import DatabaseTransactionFixture -from tests.fixtures.library import LibraryFixture -from core.feed.annotator.circulation import LibraryAnnotator -from core.feed.types import WorkEntry -from core.feed.acquisition import OPDSAcquisitionFeed - if TYPE_CHECKING: - from api.controller.select_books import SelectBooksController + pass class SelectBooksFixture(CirculationControllerFixture): @@ -54,7 +24,6 @@ class SelectBooksFixture(CirculationControllerFixture): datasource: DataSource edition: Edition - def __init__(self, db: DatabaseTransactionFixture): super().__init__(db) [self.lp] = self.english_1.license_pools @@ -71,7 +40,6 @@ def selected_book_fixture(db: DatabaseTransactionFixture): class TestSelectBooksController: - def test_select_unselect_success(self, selected_book_fixture: SelectBooksFixture): """ Test that a book can be successfully selected and unselected by a patron. @@ -81,9 +49,13 @@ def test_select_unselect_success(self, selected_book_fixture: SelectBooksFixture in the database and a feed is returned without a tag. """ - with selected_book_fixture.request_context_with_library("/", headers=dict(Authorization=selected_book_fixture.valid_auth)): + with selected_book_fixture.request_context_with_library( + "/", headers=dict(Authorization=selected_book_fixture.valid_auth) + ): # We have an authenticated patron - patron = selected_book_fixture.manager.select_books.authenticated_patron_from_request() + patron = ( + selected_book_fixture.manager.select_books.authenticated_patron_from_request() + ) identifier_type = Identifier.GUTENBERG_ID identifier = "1234567890" edition, _ = selected_book_fixture.db.edition( @@ -128,13 +100,14 @@ def test_select_unselect_success(self, selected_book_fixture: SelectBooksFixture [entry] = feed.entries assert "selected" not in entry - def test_detail(self, selected_book_fixture: SelectBooksFixture): """ Test that a selected book's details are fetched successfully. """ - with selected_book_fixture.request_context_with_library("/", headers=dict(Authorization=selected_book_fixture.valid_auth)): + with selected_book_fixture.request_context_with_library( + "/", headers=dict(Authorization=selected_book_fixture.valid_auth) + ): # A patron first selects a work selected_book_fixture.manager.select_books.authenticated_patron_from_request() identifier_type = Identifier.GUTENBERG_ID @@ -166,7 +139,9 @@ def test_fetch_selected_books_feed(self, selected_book_fixture: SelectBooksFixtu Test that the selected books acquisition feed is fetched successfully. """ - with selected_book_fixture.request_context_with_library("/", headers=dict(Authorization=selected_book_fixture.valid_auth)): + with selected_book_fixture.request_context_with_library( + "/", headers=dict(Authorization=selected_book_fixture.valid_auth) + ): # A patron first selects a work selected_book_fixture.manager.select_books.authenticated_patron_from_request() identifier_type = Identifier.GUTENBERG_ID From 11e7055c1cefd35bc56468e50ed4662d222d118c Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 23 Dec 2024 13:55:15 +0200 Subject: [PATCH 34/35] EKIRJASTO-131 Add tests for new routes --- tests/api/test_routes.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/api/test_routes.py b/tests/api/test_routes.py index 3d5197d6a..22ef7aea7 100644 --- a/tests/api/test_routes.py +++ b/tests/api/test_routes.py @@ -326,6 +326,44 @@ def test_related_books(self, fixture: RouteTestFixture): ) +class TestSelectedBooksController: + CONTROLLER_NAME = "selected_book_controller" + + @pytest.fixture(scope="function") + def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture: + route_test.set_controller_name(self.CONTROLLER_NAME) + return route_test + + def test_select(self, fixture: RouteTestFixture): + url = "/works///select_book" + fixture.assert_authenticated_request_calls( + url, fixture.controller.select, "", "", None, None # type: ignore[union-attr] + ) + fixture.assert_supported_methods(url, "POST") + + def test_unselect(self, fixture: RouteTestFixture): + url = "/works///unselect_book" + fixture.assert_authenticated_request_calls( + url, fixture.controller.unselect, "", "", None, None # type: ignore[union-attr] + ) + fixture.assert_supported_methods(url, "DELETE") + + def test_detail(self, fixture: RouteTestFixture): + url = "/selected_books//" + fixture.assert_authenticated_request_calls( + url, fixture.controller.detail, "", "", None, None # type: ignore[union-attr] + ) + fixture.assert_supported_methods(url, "GET", "DELETE") + + def test_selected_books(self, fixture: RouteTestFixture): + url = "/selected_books" + fixture.assert_authenticated_request_calls( + url, + fixture.controller.fetch_books, # type: ignore[union-attr] + ) + fixture.assert_supported_methods(url, "GET") + + class TestAnalyticsController: CONTROLLER_NAME = "analytics_controller" From 312aa7c51b4445c38e32e9980932622c2c9856b6 Mon Sep 17 00:00:00 2001 From: Kaisa Kuivalahti Date: Mon, 30 Dec 2024 11:30:35 +0200 Subject: [PATCH 35/35] EKIRJASTO-131 Fix new route tests --- tests/api/test_routes.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/api/test_routes.py b/tests/api/test_routes.py index 22ef7aea7..0053e48d9 100644 --- a/tests/api/test_routes.py +++ b/tests/api/test_routes.py @@ -327,7 +327,7 @@ def test_related_books(self, fixture: RouteTestFixture): class TestSelectedBooksController: - CONTROLLER_NAME = "selected_book_controller" + CONTROLLER_NAME = "select_books" @pytest.fixture(scope="function") def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture: @@ -337,21 +337,31 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture: def test_select(self, fixture: RouteTestFixture): url = "/works///select_book" fixture.assert_authenticated_request_calls( - url, fixture.controller.select, "", "", None, None # type: ignore[union-attr] + url, + fixture.controller.select, # type: ignore[union-attr] + "", + "", + http_method="POST", ) - fixture.assert_supported_methods(url, "POST") def test_unselect(self, fixture: RouteTestFixture): url = "/works///unselect_book" fixture.assert_authenticated_request_calls( - url, fixture.controller.unselect, "", "", None, None # type: ignore[union-attr] + url, + fixture.controller.unselect, # type: ignore[union-attr] + "", + "", + http_method="DELETE", ) - fixture.assert_supported_methods(url, "DELETE") def test_detail(self, fixture: RouteTestFixture): url = "/selected_books//" - fixture.assert_authenticated_request_calls( - url, fixture.controller.detail, "", "", None, None # type: ignore[union-attr] + fixture.assert_request_calls_method_using_identifier( + url, + fixture.controller.detail, # type: ignore[union-attr] + "", + "", + authenticated=True, ) fixture.assert_supported_methods(url, "GET", "DELETE")