Bug 21932

Summary: reservetype and reserveexpiration needed for holds (reserves and old_reserves tables)
Product: Koha Reporter: Christopher Brannon <cbrannon>
Component: Hold requestsAssignee: Bugs List <koha-bugs>
Status: RESOLVED DUPLICATE QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, katrin.fischer, lisetteslatah
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 21932: Fix REST API mapping

Description Christopher Brannon 2018-12-01 20:02:52 UTC
I see an inherent flaw in the reserves table and process.  If a hold is waiting, and someone changes the status from waiting back to a priority level, the waiting status is removed, but the expiration remains.  Also, if the hold was a first available hold, it is now changed to an item level hold for the item it was on hold for.

I would suggest that a new field be created for the holds table, such as reservetype, and is marked as 'ANY' or 'SPECIFIC'.

I would suggest that a new field be created for the reservation expiration called reserveexpiration, separate from the waiting expiration, as to maintain a reservation cutoff date if the status is changed back to a priority level.  This would make the expirationdate exclusive to waiting items.

I would suggest that when priority level is changed from waiting to a priority level, the reservetype is checked, and if it is marked as ANY, the item number be removed.  If it is SPECIFIC, the item number is kept.

I would suggest that the expirationdate be cleared if changed from a waiting status to a priority level.

I would suggest that all pages that show the "Expiration date" as the reservationexpiration, if exists, or superseded by expirationdate, if exists.  No need to have a separate cell for both.

I would suggest that when a hold is triggered and marked as waiting, the expiration date is calculated, or limited to the reservationexpiration, whichever is shorter.  No sense in holding it longer than the patron was willing to wait for it.
Comment 1 Christopher Brannon 2018-12-01 20:16:21 UTC
To test flaws:

1) Place a first available hold for the next available item, and set the hold expires date to a short time, like tomorrow.
2) Make sure your ReservesMaxPickUpDelay setting is set for longer that a day.  Say 7 days, for example.
3) View the hold.  Note the expiration date is set.
4) Check in the item and trigger the hold.  See that the item is waiting.  Note the expiration is set to the patron's preferred expiration date.
5) Set the priority back to 1, and update the hold.  Note that the hold is now an item specific hold instead of a next available hold.
6) Note the expiration stayed.  This is okay in this instance.
7) Cancel the hold and start over.
8) Place a first available hold for the next available item, but do not set the hold expires date.
9) View the hold.  Note the expiration date is not set.
10) Check in the item and trigger the hold.  See that the item is waiting.  Not the expiration date is set.
11) Set the priority back to 1, and update the hold.  Note that the hold is now an item specific hold instead of a next available hold.
12) Note the expiration stayed.  This is not okay in this instance.  The patron's hold will disappear after the date!
Comment 2 Christopher Brannon 2018-12-01 20:18:06 UTC
It is noted that the waiting expiration date DOES adhere to the patron's suggested expiration, but I have spelled it out in my suggestions to maintain that behavior if the dates are recorded separately.
Comment 3 Katrin Fischer 2019-07-13 10:11:00 UTC
Hi Christopher,

partly good news here:

The issue about reverting a hold to the right type (item-level, biblio-level) has been resolved: bug 9834

I'll mark this as duplicate of bug 21729, which discusses the remaining issue with the double-use of expirationdate. I agree with you that the right fix would be a separate database field in reserves and old_reserves.

*** This bug has been marked as a duplicate of bug 21729 ***
Comment 4 Jonathan Druart 2021-10-13 09:41:56 UTC
Created attachment 126163 [details] [review]
Bug 21932: Fix REST API mapping

It seems correct to not let third-party apps bypass the expiration date
settings and so let them set patron_expiration_date but the calculated expirationdate.