Skip to content

Commit

Permalink
Add test for hold converted to loan and test for loan
Browse files Browse the repository at this point in the history
converted to hold.
  • Loading branch information
dbernstein committed Nov 19, 2024
1 parent 19c006d commit 4173f00
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 10 deletions.
6 changes: 5 additions & 1 deletion src/palace/manager/api/circulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,11 @@ def borrow(
self._collect_event(patron, licensepool, CirculationEvent.CM_HOLD_PLACE)

if existing_loan:
self._collect_event(patron, licensepool, CirculationEvent.CM_LOAN_REVOKED)
# Send out analytics event capturing the unusual circumstance that a loan was converted to a hold
# TODO: Do we know what the conditions under which this situation can occur?
self._collect_event(
patron, licensepool, CirculationEvent.CM_LOAN_CONVERTED_TO_HOLD
)
self._db.delete(existing_loan)
__transaction.commit()
return None, hold, is_new
Expand Down
2 changes: 1 addition & 1 deletion src/palace/manager/sqlalchemy/model/circulationevent.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class CirculationEvent(Base):
CM_HOLD_READY_FOR_CHECKOUT = "circulation_manager_hold_ready"
CM_LOAN_EXPIRED = "circulation_manager_loan_expired"
CM_HOLD_CONVERTED_TO_LOAN = "circulation_manager_hold_converted_to_loan"
CM_LOAN_REVOKED = "circulation_manager_loan_revoked"
CM_LOAN_CONVERTED_TO_HOLD = "circulation_manager_loan_converted_to_hold"
CM_FULFILL = "circulation_manager_fulfill"

# Events that we hear about from a distributor.
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/api/test_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ def test_get_or_create_patron(
assert "2" == patron.authorization_identifier
assert default_library == patron.library
assert True == is_new
assert CirculationEvent.NEW_PATRON == analytics.event_type
assert CirculationEvent.NEW_PATRON == analytics.last_event_type
assert 1 == analytics.count

# Patron.neighborhood was set, even though there is no
Expand Down
43 changes: 38 additions & 5 deletions tests/manager/api/test_circulationapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def test_borrow_sends_analytics_event(self, circulation_api: CirculationAPIFixtu

# An analytics event was created.
assert 1 == circulation_api.analytics.count
assert CirculationEvent.CM_CHECKOUT == circulation_api.analytics.event_type
assert CirculationEvent.CM_CHECKOUT == circulation_api.analytics.last_event_type

# Try to 'borrow' the same book again.
circulation_api.remote.queue_checkout(AlreadyCheckedOut())
Expand Down Expand Up @@ -363,7 +363,9 @@ def test_hold_sends_analytics_event(self, circulation_api: CirculationAPIFixture

# An analytics event was created.
assert 1 == circulation_api.analytics.count
assert CirculationEvent.CM_HOLD_PLACE == circulation_api.analytics.event_type
assert (
CirculationEvent.CM_HOLD_PLACE == circulation_api.analytics.last_event_type
)

# Try to 'borrow' the same book again.
circulation_api.remote.queue_checkout(AlreadyOnHold())
Expand All @@ -374,6 +376,34 @@ def test_hold_sends_analytics_event(self, circulation_api: CirculationAPIFixture
# sent.
assert 1 == circulation_api.analytics.count

def test_hold_is_ready_converts_to_loan_on_borrow(
self, circulation_api: CirculationAPIFixture
):
now = utc_now()
loaninfo = LoanInfo.from_license_pool(
circulation_api.pool,
start_date=now,
end_date=now + timedelta(seconds=3600),
external_identifier=circulation_api.db.fresh_str(),
)
circulation_api.remote.queue_checkout(loaninfo)
circulation_api.pool.on_hold_to(patron=circulation_api.patron, position=0)
loan, hold, is_new = self.borrow(circulation_api)

# The Hold is gone and there is a new loan.
assert loan is not None
assert hold is None
assert is_new is True

assert circulation_api.analytics.count == 2
# A hold converted analytics event was recorded
assert (
circulation_api.analytics.event_types[0]
== CirculationEvent.CM_HOLD_CONVERTED_TO_LOAN
)
# A check event was recorded
assert circulation_api.analytics.event_types[1] == CirculationEvent.CM_CHECKOUT

def test_borrow_with_expired_card_fails(
self, circulation_api: CirculationAPIFixture
):
Expand Down Expand Up @@ -813,7 +843,7 @@ def test_fulfill(self, circulation_api: CirculationAPIFixture):

# An analytics event was created.
assert 1 == circulation_api.analytics.count
assert CirculationEvent.CM_FULFILL == circulation_api.analytics.event_type
assert CirculationEvent.CM_FULFILL == circulation_api.analytics.last_event_type

def test_fulfill_without_loan(self, circulation_api: CirculationAPIFixture):
# By default, a title cannot be fulfilled unless there is an active
Expand Down Expand Up @@ -857,7 +887,7 @@ def test_revoke_loan(self, circulation_api: CirculationAPIFixture, open_access):

# An analytics event was created.
assert 1 == circulation_api.analytics.count
assert CirculationEvent.CM_CHECKIN == circulation_api.analytics.event_type
assert CirculationEvent.CM_CHECKIN == circulation_api.analytics.last_event_type

@pytest.mark.parametrize("open_access", [True, False])
def test_release_hold(self, circulation_api: CirculationAPIFixture, open_access):
Expand All @@ -873,7 +903,10 @@ def test_release_hold(self, circulation_api: CirculationAPIFixture, open_access)

# An analytics event was created.
assert 1 == circulation_api.analytics.count
assert CirculationEvent.CM_HOLD_RELEASE == circulation_api.analytics.event_type
assert (
CirculationEvent.CM_HOLD_RELEASE
== circulation_api.analytics.last_event_type
)

def test__collect_event(self, circulation_api: CirculationAPIFixture):
# Test the _collect_event method, which gathers information
Expand Down
24 changes: 23 additions & 1 deletion tests/manager/api/test_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
LoanReaper,
)
from palace.manager.api.opds_for_distributors import OPDSForDistributorsAPI
from palace.manager.sqlalchemy.model.circulationevent import CirculationEvent
from palace.manager.sqlalchemy.model.datasource import DataSource
from palace.manager.sqlalchemy.model.patron import Annotation
from palace.manager.sqlalchemy.util import get_one_or_create
from palace.manager.util.datetime_helpers import utc_now
from tests.fixtures.database import DatabaseTransactionFixture
from tests.fixtures.services import ServicesFixture


class TestLoanlikeReaperMonitor:
Expand All @@ -26,7 +28,9 @@ def test_source_of_truth_protocols(self):
OPDSForDistributorsAPI.label()
]

def test_reaping(self, db: DatabaseTransactionFixture):
def test_reaping(
self, db: DatabaseTransactionFixture, services_fixture: ServicesFixture
):
# This patron stopped using the circulation manager a long time
# ago.
inactive_patron = db.patron()
Expand Down Expand Up @@ -152,6 +156,7 @@ def test_reaping(self, db: DatabaseTransactionFixture):

# Now we fire up the loan reaper.
monitor = LoanReaper(db.session)
monitor.services.analytics = services_fixture.analytics_fixture.analytics_mock
monitor.run()

# All of the inactive patron's loans have been reaped,
Expand All @@ -171,8 +176,18 @@ def test_reaping(self, db: DatabaseTransactionFixture):
assert 2 == len(current_patron.loans)
assert 2 == len(current_patron.holds)

call_args_list = (
services_fixture.analytics_fixture.analytics_mock.collect_event.call_args_list
)
assert len(call_args_list) == 2
for call_args in call_args_list:
assert call_args.kwargs["event_type"] == CirculationEvent.CM_LOAN_EXPIRED

# Now fire up the hold reaper.
hold_monitor = HoldReaper(db.session)
hold_monitor.services.analytics = (
services_fixture.analytics_fixture.analytics_mock
)
hold_monitor.run()

# All of the inactive patron's holds have been reaped,
Expand All @@ -181,6 +196,13 @@ def test_reaping(self, db: DatabaseTransactionFixture):
assert [sot_hold] == inactive_patron.holds
assert 2 == len(current_patron.holds)

call_args_list = (
services_fixture.analytics_fixture.analytics_mock.collect_event.call_args_list
)
assert len(call_args_list) == 2
for call_args in call_args_list:
assert call_args.kwargs["event_type"] == CirculationEvent.CM_HOLD_EXPIRED


class TestIdlingAnnotationReaper:
def test_where_clause(self, db: DatabaseTransactionFixture):
Expand Down
5 changes: 4 additions & 1 deletion tests/mocks/analytics_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@ def __init__(self, integration=None, services=None, library=None):
"""
self.count = 0
self.event = None
self.event_types = []
self.last_event_type = None
if integration:
self.url = integration.url

def collect_event(self, library, lp, event_type, time=None, **kwargs):
self.count = self.count + 1
self.event_type = event_type
self.last_event_type = event_type
self.event_types.append(event_type)
self.time = time

0 comments on commit 4173f00

Please sign in to comment.