Bug 21932 - reservetype and reserveexpiration needed for holds (reserves and old_reserves tables)
Summary: reservetype and reserveexpiration needed for holds (reserves and old_reserves...
Status: RESOLVED DUPLICATE of bug 21729
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-01 20:02 UTC by Christopher Brannon
Modified: 2021-10-13 09:41 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21932: Fix REST API mapping (987 bytes, patch)
2021-10-13 09:41 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.