diff --git a/src/palace/manager/api/circulation.py b/src/palace/manager/api/circulation.py index 2e69379311..d9292cb5d4 100644 --- a/src/palace/manager/api/circulation.py +++ b/src/palace/manager/api/circulation.py @@ -793,10 +793,6 @@ def local_loans_or_holds( items_by_identifier = {} for item in items: license_pool = item.license_pool - if license_pool is None: - self.log.error("Active loan or hold (%r) with no license pool", item) - continue - if license_pool.identifier is None: self.log.error( "Active loan or hold (%r) on license pool (%r), which has no identifier.", diff --git a/tests/manager/celery/tasks/test_patron_activity.py b/tests/manager/celery/tasks/test_patron_activity.py index 69465a12cd..447d721054 100644 --- a/tests/manager/celery/tasks/test_patron_activity.py +++ b/tests/manager/celery/tasks/test_patron_activity.py @@ -1,4 +1,4 @@ -from unittest.mock import create_autospec, patch +from unittest.mock import MagicMock, create_autospec, patch import pytest @@ -46,6 +46,10 @@ def __init__( self.services_fixture.services.integration_registry.license_providers.override( self.mock_registry ) + self.mock_collection_api = MockPatronActivityCirculationAPI( + self.db.session, self.collection + ) + self.mock_registry.from_collection.return_value = self.mock_collection_api @pytest.fixture @@ -140,6 +144,7 @@ def test_not_supported( self, sync_task_fixture: SyncTaskFixture, caplog: pytest.LogCaptureFixture ): caplog.set_level(LogLevel.info) + sync_task_fixture.mock_registry.from_collection.return_value = MagicMock() sync_patron_activity.apply_async( (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin") @@ -155,13 +160,6 @@ def test_not_supported( ) def test_success(self, sync_task_fixture: SyncTaskFixture): - mock_collection_api = MockPatronActivityCirculationAPI( - sync_task_fixture.db.session, sync_task_fixture.collection - ) - sync_task_fixture.mock_registry.from_collection.return_value = ( - mock_collection_api - ) - sync_patron_activity.apply_async( (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin") ).wait() @@ -173,8 +171,40 @@ def test_success(self, sync_task_fixture: SyncTaskFixture): sync_task_fixture.mock_registry.from_collection.assert_called_once_with( sync_task_fixture.db.session, sync_task_fixture.collection ) - assert len(mock_collection_api.patron_activity_calls) == 1 - assert mock_collection_api.patron_activity_calls[0] == ( + assert len(sync_task_fixture.mock_collection_api.patron_activity_calls) == 1 + assert sync_task_fixture.mock_collection_api.patron_activity_calls[0] == ( sync_task_fixture.patron, "pin", ) + + def test_force( + self, sync_task_fixture: SyncTaskFixture, caplog: pytest.LogCaptureFixture + ): + # The task has been marked as failed. Normally, this means we don't need to run it again + # until the status expires. + caplog.set_level(LogLevel.info) + sync_task_fixture.redis_record.lock() + sync_task_fixture.redis_record.fail() + + sync_patron_activity.apply_async( + (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin") + ).wait() + + assert "Patron activity sync not needed (state: FAILED)" in caplog.text + sync_task_fixture.mock_registry.from_collection.assert_not_called() + + # But if we force it, we should run it again. + caplog.clear() + sync_patron_activity.apply_async( + (sync_task_fixture.collection.id, sync_task_fixture.patron.id, "pin"), + {"force": True}, + ).wait() + assert "Patron activity sync not needed" not in caplog.text + sync_task_fixture.mock_registry.from_collection.assert_called_once_with( + sync_task_fixture.db.session, sync_task_fixture.collection + ) + + # And it will update the state when it's done. + task_status = sync_task_fixture.redis_record.status() + assert task_status is not None + assert task_status.state == PatronActivityStatus.State.SUCCESS diff --git a/tests/manager/service/redis/models/test_patron_activity.py b/tests/manager/service/redis/models/test_patron_activity.py index a077a116bc..fc87c470c6 100644 --- a/tests/manager/service/redis/models/test_patron_activity.py +++ b/tests/manager/service/redis/models/test_patron_activity.py @@ -113,6 +113,46 @@ def test_to_redis(self): assert status.to_redis() == "F::2024-06-11T01:24:00::test-123" + def test___eq__(self): + timestamp = utc_now() + task_id = "test::123456" + + status = PatronActivityStatus( + state=PatronActivityStatus.State.FAILED, + task_id=task_id, + timestamp=timestamp, + ) + + # We cannot compare with a different type + assert not (status == "foo") + + # We can compare with the same instance + assert status == status + + # Or a different instance, with same data + assert status == PatronActivityStatus( + state=PatronActivityStatus.State.FAILED, + task_id=task_id, + timestamp=timestamp, + ) + + # But a different instance with different data will not be equal + assert status != PatronActivityStatus( + state=PatronActivityStatus.State.FAILED, + task_id="different", + timestamp=timestamp, + ) + assert status != PatronActivityStatus( + state=PatronActivityStatus.State.SUCCESS, + task_id=task_id, + timestamp=timestamp, + ) + assert status != PatronActivityStatus( + state=PatronActivityStatus.State.SUCCESS, + task_id=task_id, + timestamp=datetime_utc(2010, 1, 1), + ) + class PatronActivityFixture: def __init__(self, db: DatabaseTransactionFixture, redis_fixture: RedisFixture):