From 419bddb305af8df9be9bd170f0e24fe96a97cf42 Mon Sep 17 00:00:00 2001
From: Daniel Bernstein <dabylon@gmail.com>
Date: Tue, 19 Nov 2024 09:46:30 -0800
Subject: [PATCH] Fix broken tests.

---
 src/palace/manager/api/monitor.py  | 31 +++++++++++++-----------------
 src/palace/manager/core/monitor.py |  7 ++-----
 tests/manager/api/test_monitor.py  | 25 ++++++++++++------------
 3 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/src/palace/manager/api/monitor.py b/src/palace/manager/api/monitor.py
index 9c5c54635..b22014a36 100644
--- a/src/palace/manager/api/monitor.py
+++ b/src/palace/manager/api/monitor.py
@@ -1,5 +1,3 @@
-from collections.abc import Callable
-
 from sqlalchemy import and_, or_
 
 from palace.manager.api.opds_for_distributors import OPDSForDistributorsAPI
@@ -47,23 +45,20 @@ def where_clause(self):
         )
         return ~self.MODEL_CLASS.id.in_(source_of_truth_subquery)
 
-    def post_delete_op(self, row: Loan | Hold) -> Callable:
-
-        def post_delete() -> None:
-            ce = CirculationEvent
-            event_type = (
-                CirculationEvent.CM_LOAN_EXPIRED
-                if isinstance(row, Loan)
-                else CirculationEvent.CM_HOLD_EXPIRED
-            )
-            self.services.analytics.collect_event(
-                library=row.library,
-                license_pool=row.license_pool,
-                event_type=event_type,
-                patron=row.patron,
-            )
+    def post_delete(self, row: Loan | Hold) -> None:
+        ce = CirculationEvent
+        event_type = (
+            CirculationEvent.CM_LOAN_EXPIRED
+            if isinstance(row, Loan)
+            else CirculationEvent.CM_HOLD_EXPIRED
+        )
 
-        return post_delete
+        self.services.analytics.collect_event(
+            library=row.library,
+            license_pool=row.license_pool,
+            event_type=event_type,
+            patron=row.patron,
+        )
 
 
 class LoanReaper(LoanlikeReaperMonitor):
diff --git a/src/palace/manager/core/monitor.py b/src/palace/manager/core/monitor.py
index 9182f2abd..914f61276 100644
--- a/src/palace/manager/core/monitor.py
+++ b/src/palace/manager/core/monitor.py
@@ -3,7 +3,6 @@
 import datetime
 import logging
 import traceback
-from collections.abc import Callable
 from typing import TYPE_CHECKING
 
 from sqlalchemy.exc import InvalidRequestError
@@ -884,10 +883,8 @@ def run_once(self, *args, **kwargs):
             post_delete_ops = []
             for i in qu.limit(self.BATCH_SIZE):
                 self.log.info("Deleting %r", i)
-                post_delete_op = self.post_delete_op(i)
-                if post_delete_op:
-                    post_delete_ops.append(post_delete_op)
                 self.delete(i)
+                self.post_delete(i)
                 rows_deleted += 1
 
             self._db.commit()
@@ -907,7 +904,7 @@ def delete(self, row):
         """
         self._db.delete(row)
 
-    def post_delete_op(self, row) -> Callable | None:
+    def post_delete(self, row) -> None:
         return None
 
     def query(self):
diff --git a/tests/manager/api/test_monitor.py b/tests/manager/api/test_monitor.py
index fe74c15c2..3ecbec9a8 100644
--- a/tests/manager/api/test_monitor.py
+++ b/tests/manager/api/test_monitor.py
@@ -168,20 +168,13 @@ def test_reaping(
         assert {open_access_loan, sot_loan, unlimited_access_loan} == set(
             inactive_patron.loans
         )
-        assert 3 == len(inactive_patron.holds)
+        assert len(inactive_patron.holds) == 3
 
         # The active patron's loans and holds are unaffected, either
         # because they have not expired or because they have no known
         # expiration date and were created relatively recently.
-        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
+        assert len(current_patron.loans) == 2
+        assert len(current_patron.holds) == 2
 
         # Now fire up the hold reaper.
         hold_monitor = HoldReaper(db.session)
@@ -196,12 +189,18 @@ def test_reaping(
         assert [sot_hold] == inactive_patron.holds
         assert 2 == len(current_patron.holds)
 
+        # verify expected circ event count and order for two monitor operations.
         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
+        assert len(call_args_list) == 4
+        event_types = [call_args.kwargs["event_type"] for call_args in call_args_list]
+        assert event_types == [
+            CirculationEvent.CM_LOAN_EXPIRED,
+            CirculationEvent.CM_LOAN_EXPIRED,
+            CirculationEvent.CM_HOLD_EXPIRED,
+            CirculationEvent.CM_HOLD_EXPIRED,
+        ]
 
 
 class TestIdlingAnnotationReaper: