Checkout expired (but not canceled) hold leads to error status

Bug #1668682 reported by Bill Erickson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned
2.10
Fix Released
Medium
Unassigned
2.11
Fix Released
Medium
Unassigned

Bug Description

Evergreen 2.12 -- affects all versions.

Steps to reproduce.

1. Disable hold targeter (or test on a system where the targeter takes some time to run).
2. Capture a hold.
3. Manually set the expire time on the hold to now() - '5 minutes'::interval (or similar).
4. Checkout the captured copy to the patron for whom the hold was captured.
5. View patron holds for the patron shows status Error / -1.

This happens because the checkin code is unable to find the hold to mark it as fulfilled. So the hold persists as captured, unfulfilled, and not-yet-canceled, with a current copy that's checked out.

The disconnect here is that the checkin code is looking at both the cancel_time and the expire_time of the hold. However, a hold is not truly expired in EG until the targeter marks it as canceled.

This can (theoretically) be resolved by teaching the checkout code to avoid filtering holds on their expire date and relying solely on the cancel state of the hold.

Revision history for this message
Bill Erickson (berick) wrote :

Fix pushed here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1668682-hold-expire-not-canceled

To confirm, follow the steps from the bug description. After the final checkout (step 4), confirm the hold no longer appears in the patron's holds list and the fulfillment_time is correctly set on the hold in the database.

Note the checkout code still inspects expire_time when searching for "similar" holds to capture. I left this as-is since these holds do not directly target the copy in question, so no error condition occurs, and will soon be canceled anyway when the targeter catches up to them. I could see an argument for changing that logic for consistency, though.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.12-rc
Revision history for this message
Chris Sharp (chrissharp123) wrote :
tags: added: signedoff
Michele Morgan (mmorgan)
Changed in evergreen:
assignee: nobody → Michele Morgan (mmorgan)
Michele Morgan (mmorgan)
Changed in evergreen:
assignee: Michele Morgan (mmorgan) → nobody
Revision history for this message
Kathy Lussier (klussier) wrote :

Thank you Bill and Chris. Merged to master, release 2.11 and release 2.10.

Changed in evergreen:
status: New → Fix Committed
importance: Undecided → Medium
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.