Checkout expired (but not canceled) hold leads to error status
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.
Changed in evergreen: | |
assignee: | nobody → Michele Morgan (mmorgan) |
Changed in evergreen: | |
assignee: | Michele Morgan (mmorgan) → nobody |
Changed in evergreen: | |
status: | Fix Committed → Fix Released |
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.