A couple more updates

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.",

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

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):