Skip to content

Commit

Permalink
Axis 360 loan sync exception (PP-1910) (#2165)
Browse files Browse the repository at this point in the history
- Update LoanInfo to remove the fulfillment attribute since it is confusing, and only used in the Axis360 API code.
- Add AxisLoanInfo which extends LoanInfo with the extra fulfillment attribute, and update the Axis360 API code to use this new class.
- Add some type hints to licensing.py that would have caught this issue if they had existed before.
  • Loading branch information
jonathangreen authored Nov 15, 2024
1 parent a4ecf50 commit 05748ac
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 22 deletions.
24 changes: 18 additions & 6 deletions src/palace/manager/api/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import urllib
from abc import ABC, abstractmethod
from collections.abc import Callable, Generator, Mapping, Sequence
from dataclasses import dataclass
from datetime import timedelta
from typing import Any, Generic, Literal, Optional, TypeVar, Union, cast
from urllib.parse import urlparse
Expand Down Expand Up @@ -481,7 +482,7 @@ def fulfill(
self.internal_format(delivery_mechanism),
)
for loan in activities:
if not isinstance(loan, LoanInfo):
if not isinstance(loan, AxisLoanInfo):
continue
if not (
loan.identifier_type == identifier.type
Expand Down Expand Up @@ -547,7 +548,7 @@ def patron_activity(
pin: str | None,
identifier: Identifier | None = None,
internal_format: str | None = None,
) -> list[LoanInfo | HoldInfo]:
) -> list[AxisLoanInfo | HoldInfo]:
if identifier:
assert identifier.identifier is not None
title_ids = [identifier.identifier]
Expand Down Expand Up @@ -1517,7 +1518,18 @@ def process_one(
return True


class AvailabilityResponseParser(XMLResponseParser[Union[LoanInfo, HoldInfo]]):
@dataclass(kw_only=True)
class AxisLoanInfo(LoanInfo):
"""
An extension of the normal LoanInfo dataclass that includes some Axis 360-specific
information, since the Axis 360 API uses this object to get information about
loan fulfillment in addition to checkout.
"""

fulfillment: Fulfillment | None


class AvailabilityResponseParser(XMLResponseParser[Union[AxisLoanInfo, HoldInfo]]):
def __init__(self, api: Axis360API, internal_format: str | None = None) -> None:
"""Constructor.
Expand All @@ -1544,7 +1556,7 @@ def xpath_expression(self) -> str:

def process_one(
self, e: _Element, ns: dict[str, str] | None
) -> LoanInfo | HoldInfo | None:
) -> AxisLoanInfo | HoldInfo | None:
# Figure out which book we're talking about.
axis_identifier = self.text_of_subtag(e, "axis:titleId", ns)
availability = self._xpath1(e, "axis:availability", ns)
Expand All @@ -1554,7 +1566,7 @@ def process_one(
checked_out = self._xpath1_boolean(availability, "axis:isCheckedout", ns)
on_hold = self._xpath1_boolean(availability, "axis:isInHoldQueue", ns)

info: LoanInfo | HoldInfo | None = None
info: AxisLoanInfo | HoldInfo | None = None
if checked_out:
start_date = self._xpath1_date(availability, "axis:checkoutStartDate", ns)
end_date = self._xpath1_date(availability, "axis:checkoutEndDate", ns)
Expand Down Expand Up @@ -1599,7 +1611,7 @@ def process_one(
else:
# We're out of luck -- we can't fulfill this loan.
fulfillment = None
info = LoanInfo(
info = AxisLoanInfo(
collection_id=self.collection_id,
identifier_type=self.id_type,
identifier=axis_identifier,
Expand Down
12 changes: 2 additions & 10 deletions src/palace/manager/api/circulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ class LoanInfo(LoanAndHoldInfoMixin):
identifier: str
start_date: datetime.datetime | None = None
end_date: datetime.datetime | None
fulfillment: Fulfillment | None = None
external_identifier: str | None = None
locked_to: DeliveryMechanismInfo | None = None
license_identifier: str | None = None
Expand All @@ -360,7 +359,6 @@ def from_license_pool(
*,
start_date: datetime.datetime | None = None,
end_date: datetime.datetime | None,
fulfillment: Fulfillment | None = None,
external_identifier: str | None = None,
locked_to: DeliveryMechanismInfo | None = None,
license_identifier: str | None = None,
Expand All @@ -377,23 +375,17 @@ def from_license_pool(
identifier=identifier,
start_date=start_date,
end_date=end_date,
fulfillment=fulfillment,
external_identifier=external_identifier,
locked_to=locked_to,
license_identifier=license_identifier,
)

def __repr__(self) -> str:
if self.fulfillment:
fulfillment = " Fulfilled by: " + repr(self.fulfillment)
else:
fulfillment = ""
return "<LoanInfo for {}/{}, start={} end={}>{}".format(
return "<LoanInfo for {}/{}, start={} end={}>".format(
self.identifier_type,
self.identifier,
self.start_date.isoformat() if self.start_date else self.start_date,
self.end_date.isoformat() if self.end_date else self.end_date,
fulfillment,
)

def create_or_update(
Expand All @@ -402,6 +394,7 @@ def create_or_update(
session = Session.object_session(patron)
license_pool = license_pool or self.license_pool(session)

loanable: LicensePool | License
if self.license_identifier is not None:
loanable = session.execute(
select(License).where(
Expand All @@ -416,7 +409,6 @@ def create_or_update(
patron,
start=self.start_date,
end=self.end_date,
fulfillment=self.fulfillment,
external_identifier=self.external_identifier,
)

Expand Down
21 changes: 15 additions & 6 deletions src/palace/manager/sqlalchemy/model/licensing.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,17 @@ def is_available_for_borrowing(self) -> bool:
and self.checkouts_available > 0
)

def loan_to(self, patron: Patron, **kwargs) -> tuple[Loan, bool]:
loan, is_new = self.license_pool.loan_to(patron, **kwargs)
def loan_to(
self,
patron: Patron,
start: datetime.datetime | None = None,
end: datetime.datetime | None = None,
fulfillment: LicensePoolDeliveryMechanism | None = None,
external_identifier: str | None = None,
) -> tuple[Loan, bool]:
loan, is_new = self.license_pool.loan_to(
patron, start, end, fulfillment, external_identifier
)
loan.license = self
return loan, is_new

Expand Down Expand Up @@ -1050,10 +1059,10 @@ def _part(message, args, string, old_value, new_value):
def loan_to(
self,
patron: Patron,
start=None,
end=None,
fulfillment=None,
external_identifier=None,
start: datetime.datetime | None = None,
end: datetime.datetime | None = None,
fulfillment: LicensePoolDeliveryMechanism | None = None,
external_identifier: str | None = None,
) -> tuple[Loan, bool]:
_db = Session.object_session(patron)
kwargs = dict(start=start or utc_now(), end=end)
Expand Down

0 comments on commit 05748ac

Please sign in to comment.