Add another test for hold

src/palace/manager/api/odl/api.py

@@ -387,6 +387,7 @@ def _checkout(
                     # patrons in position 1 in the hold queue, but this will be resolved next time
                     # the hold queue is recalculated.
                     hold.position = 1
+                    hold.end = None
                 # Update the pool and the next holds in the queue when a license is reserved.
                 licensepool.update_availability_from_licenses()
                 raise NoAvailableCopies()
@@ -437,8 +438,12 @@ def _checkout(
         # We also need to update the remaining checkouts for the license.
         license.checkout()
 
-        # Update the pool to reflect the new loan.
-        licensepool.update_availability_from_licenses(holds_adjustment=1 if hold else 0)
+        # If there was a hold CirculationAPI will take care of deleting it. So we just need to
+        # update the license pool to reflect the loan. Since update_availability_from_licenses
+        # takes into account holds, we need to tell it to ignore the hold about to be deleted.
+        licensepool.update_availability_from_licenses(
+            ignored_holds={hold} if hold else None
+        )
         return loan
 
     def fulfill(
@@ -652,8 +657,11 @@ def release_hold(self, patron: Patron, pin: str, licensepool: LicensePool) -> No
         if not hold:
             raise NotOnHold()
 
-        # Update the license pool to reflect the released hold.
-        hold.license_pool.update_availability_from_licenses(holds_adjustment=1)
+        # The hold itself will be deleted by the caller (usually CirculationAPI),
+        # so we just need to update the license pool to reflect the released hold.
+        # Since we are calling this before the hold is deleted, we need to pass the
+        # hold as an ignored hold to get the correct count.
+        hold.license_pool.update_availability_from_licenses(ignored_holds={hold})
 
     def update_availability(self, licensepool: LicensePool) -> None:
         pass

src/palace/manager/sqlalchemy/model/licensing.py

@@ -697,7 +697,7 @@ def needs_update(self):
     def update_availability_from_licenses(
         self,
         as_of: datetime.datetime | None = None,
-        holds_adjustment: int = 0,
+        ignored_holds: set[Hold] | None = None,
     ):
         """
         Update the LicensePool with new availability information, based on the
@@ -716,7 +716,9 @@ def update_availability_from_licenses(
             if l.currently_available_loans is not None
         )
 
-        patrons_in_hold_queue = max(len(self.get_active_holds()) - holds_adjustment, 0)
+        ignored_holds_ids = {h.id for h in (ignored_holds or set())}
+        active_holds_ids = {h.id for h in self.get_active_holds()}
+        patrons_in_hold_queue = len(active_holds_ids - ignored_holds_ids)
         if patrons_in_hold_queue > licenses_available:
             licenses_reserved = licenses_available
             licenses_available = 0

tests/manager/api/odl/test_api.py

@@ -704,6 +704,50 @@ def test_checkout_no_available_copies_unknown_to_us(
         assert license.license_pool.licenses_available == 0
         assert license.checkouts_available == 0
 
+    def test_checkout_ready_hold_no_available_copies(
+        self,
+        db: DatabaseTransactionFixture,
+        opds2_with_odl_api_fixture: OPDS2WithODLApiFixture,
+    ) -> None:
+        """
+        We think there is a hold ready for us, but we are out of sync with the distributor,
+        so there actually isn't a copy ready for our hold.
+        """
+        # We think there is a copy available for this hold.
+        hold, _ = opds2_with_odl_api_fixture.pool.on_hold_to(
+            opds2_with_odl_api_fixture.patron,
+            start=utc_now() - datetime.timedelta(days=1),
+            end=utc_now() + datetime.timedelta(days=1),
+            position=0,
+        )
+        opds2_with_odl_api_fixture.setup_license(concurrency=1, available=1)
+
+        assert opds2_with_odl_api_fixture.pool.licenses_available == 0
+        assert opds2_with_odl_api_fixture.pool.licenses_reserved == 1
+        assert opds2_with_odl_api_fixture.pool.patrons_in_hold_queue == 1
+
+        # But the distributor says there are no available copies.
+        opds2_with_odl_api_fixture.mock_http.queue_response(
+            400,
+            "application/api-problem+json",
+            content=opds2_with_odl_api_fixture.files.sample_text("unavailable.json"),
+        )
+
+        with pytest.raises(NoAvailableCopies):
+            opds2_with_odl_api_fixture.api_checkout()
+
+        assert db.session.query(Loan).count() == 0
+        assert db.session.query(Hold).count() == 1
+
+        # The availability has been updated.
+        assert opds2_with_odl_api_fixture.pool.licenses_available == 0
+        assert opds2_with_odl_api_fixture.pool.licenses_reserved == 0
+        assert opds2_with_odl_api_fixture.pool.patrons_in_hold_queue == 1
+
+        # The hold has been updated to reflect the new availability.
+        assert hold.position == 1
+        assert hold.end is None
+
     def test_checkout_failures(
         self,
         db: DatabaseTransactionFixture,