Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement LoanCheckout model for gathering history #9

Merged
merged 1 commit into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
"""Create loancheckouts table

Revision ID: d7ef6948af4e
Revises: cc084e35e037
Create Date: 2024-02-05 08:45:20.164531+00:00

"""
import sqlalchemy as sa

from alembic import op

# revision identifiers, used by Alembic.
revision = "d7ef6948af4e"
down_revision = "cc084e35e037"
branch_labels = None
depends_on = None


def upgrade() -> None:
op.create_table(
"loancheckouts",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("patron_id", sa.Integer(), nullable=True),
sa.Column("license_pool_id", sa.Integer(), nullable=True),
sa.Column("timestamp", sa.DateTime(timezone=True), nullable=True),
sa.ForeignKeyConstraint(
["license_pool_id"],
["licensepools.id"],
),
sa.ForeignKeyConstraint(["patron_id"], ["patrons.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("id"),
)
op.create_index(
op.f("ix_loancheckouts_license_pool_id"),
"loancheckouts",
["license_pool_id"],
unique=False,
)
op.create_index(
op.f("ix_loancheckouts_patron_id"), "loancheckouts", ["patron_id"], unique=False
)
op.create_index(
op.f("ix_loancheckouts_timestamp"), "loancheckouts", ["timestamp"], unique=False
)
# ### end Alembic commands ###


def downgrade() -> None:
op.drop_index(op.f("ix_loancheckouts_timestamp"), table_name="loancheckouts")
op.drop_index(op.f("ix_loancheckouts_patron_id"), table_name="loancheckouts")
op.drop_index(op.f("ix_loancheckouts_license_pool_id"), table_name="loancheckouts")
op.drop_table("loancheckouts")
# ### end Alembic commands ###
14 changes: 14 additions & 0 deletions api/circulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
get_one,
)
from core.model.integration import IntegrationConfiguration
from core.model.patron import LoanCheckout
from core.util.datetime_helpers import utc_now
from core.util.log import LoggerMixin

Expand Down Expand Up @@ -880,6 +881,18 @@ def _collect_event(
library, licensepool, name, neighborhood=neighborhood
)

# Finland
def _collect_checkout_history(
self, patron: Patron, license_pool: LicensePool
) -> None:
"""Save history for checkout, for later use in loan history or analytics"""
__transaction = self._db.begin_nested()
checkout = LoanCheckout(
patron=patron, license_pool=license_pool, timestamp=utc_now()
)
self._db.add(checkout)
__transaction.commit()

def _collect_checkout_event(self, patron: Patron, licensepool: LicensePool) -> None:
"""A simple wrapper around _collect_event for handling checkouts.

Expand Down Expand Up @@ -1093,6 +1106,7 @@ def borrow(
# Send out an analytics event to record the fact that
# a loan was initiated through the circulation
# manager.
self._collect_checkout_history(patron, licensepool)
self._collect_checkout_event(patron, licensepool)
return loan, None, new_loan_record

Expand Down
1 change: 1 addition & 0 deletions core/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ def _bulk_operation(self):
Hold,
Loan,
LoanAndHoldMixin,
LoanCheckout,
Patron,
PatronProfileStorage,
)
Expand Down
26 changes: 26 additions & 0 deletions core/model/patron.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ class Patron(Base):
loans: Mapped[List[Loan]] = relationship(
"Loan", backref="patron", cascade="delete", uselist=True
)

loan_checkouts: Mapped[List[LoanCheckout]] = relationship(
"LoanCheckout", backref="patron", cascade="delete", uselist=True
)

holds: Mapped[List[Hold]] = relationship(
"Hold",
back_populates="patron",
Expand Down Expand Up @@ -571,6 +576,27 @@ def until(self, default_loan_period):
return start + default_loan_period


# Finland
class LoanCheckout(Base, LoanAndHoldMixin):
"""A model to keep track of loan history, i.e. past checkouts. Similar to `Loan` model with some fields omitted and timestamp added"""

__tablename__ = "loancheckouts"
id = Column(Integer, primary_key=True)

patron_id = Column(
Integer, ForeignKey("patrons.id", ondelete="CASCADE"), index=True
)
patron: Patron

license_pool_id = Column(Integer, ForeignKey("licensepools.id"), index=True)
license_pool: Mapped[LicensePool] = relationship("LicensePool")

timestamp = Column(DateTime(timezone=True), index=True)

def __lt__(self, other):
return self.timestamp < other.timestamp


class Hold(Base, LoanAndHoldMixin):
"""A patron is in line to check out a book."""

Expand Down
45 changes: 45 additions & 0 deletions tests/api/test_circulationapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,51 @@ def test_borrow_sends_analytics_event(self, circulation_api: CirculationAPIFixtu
loan, hold, is_new = self.borrow(circulation_api)
assert 3 == circulation_api.analytics.count

# Finland
def test_borrow_is_added_to_checkout_history(
self, circulation_api: CirculationAPIFixture
):
now = utc_now()
loaninfo = LoanInfo(
circulation_api.pool.collection,
circulation_api.pool.data_source,
circulation_api.pool.identifier.type,
circulation_api.pool.identifier.identifier,
now,
now + timedelta(seconds=3600),
external_identifier=circulation_api.db.fresh_str(),
)
circulation_api.remote.queue_checkout(loaninfo)
now = utc_now()

loan, hold, is_new = self.borrow(circulation_api)

# A checkout history row was created
assert 1 == len(circulation_api.patron.loan_checkouts)

# Try to 'borrow' the same book again.
circulation_api.remote.queue_checkout(AlreadyCheckedOut())
loan, hold, is_new = self.borrow(circulation_api)

assert loaninfo.external_identifier == loan.external_identifier

# Since the loan already existed, no new history item was created.
assert 1 == len(circulation_api.patron.loan_checkouts)

# Now try to renew the book.
circulation_api.remote.queue_checkout(loaninfo)
loan, hold, is_new = self.borrow(circulation_api)

# Renewals are counted as checkouts
assert 2 == len(circulation_api.patron.loan_checkouts)

# Loans of open-access books go through a different code
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include open-access books in checkout history?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't know if there will be such titles in the library collection, at least not at the beginning.

# path, but they count as loans nonetheless.
circulation_api.pool.open_access = True
circulation_api.remote.queue_checkout(loaninfo)
loan, hold, is_new = self.borrow(circulation_api)
assert 3 == len(circulation_api.patron.loan_checkouts)

def test_attempt_borrow_with_existing_remote_loan(
self, circulation_api: CirculationAPIFixture
):
Expand Down
Loading