Bug 19540 - opac-reserve does not correctly warn of too_much reserves
Summary: opac-reserve does not correctly warn of too_much reserves
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 11160 27335 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-10-27 15:10 UTC by David Bourgault
Modified: 2022-10-26 09:41 UTC (History)
9 users (show)

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


Attachments
Bug 19540: Tell the patron why a hold cannot be placed on items (3.28 KB, patch)
2021-01-07 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19540: Tell the patron why a hold cannot be placed on items (3.24 KB, patch)
2022-05-06 18:17 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19540: Tell the patron why a hold cannot be placed on items (3.37 KB, patch)
2022-08-17 12:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Bourgault 2017-10-27 15:10:59 UTC
When attempting to place more holds than allowed, opac-reserve.pl only checks against the SysPref 'maxreserves' and not the issuingrules 'holdsallowed'. If the issuing rule's limitation is lower than maxreserve, the opac-reserve page shows a message saying you cannot place any more holds but the explanatory message does not show.

To reproduce:
0) Set MaxReserve to a high value (50 is default);
1) Set an issuing rule's "Holds allowed" to a low number (1 or 2);
2) Place as many holds as you can (for the corresponding (categorycode, itype, branchcode, etc). When you reach the maximum allowed by the issuing rule, you'll see a message telling you cannot place any more holds, but the explanatory message saying that you cannot place more than %d holds will not show. (It will show "No items available instead".

Bug is present on 16.05 and master
Comment 1 David Bourgault 2017-10-27 15:11:59 UTC
I think the problem affects all holds that are blocked by CanBookBeReserved() error codes. They are never handled/transmitted to the user.
Comment 2 Jonathan Druart 2017-11-02 13:41:37 UTC
*** Bug 11160 has been marked as a duplicate of this bug. ***
Comment 3 Katrin Fischer 2020-03-18 13:40:37 UTC
Still an issue.
Comment 4 Jonathan Druart 2021-01-06 10:26:22 UTC
*** Bug 27335 has been marked as a duplicate of this bug. ***
Comment 5 Jonathan Druart 2021-01-07 12:48:38 UTC
Created attachment 114919 [details] [review]
Bug 19540: Tell the patron why a hold cannot be placed on items

This patch adds an explanation when a hold cannot be placed, in some situation:
tooManyReserves
tooManyHoldsForThisRecord
tooManyReservesToday
itemAlreadyOnHold
Comment 6 Jonathan Druart 2021-01-07 12:49:03 UTC
Would this patch work?
Comment 7 Lucas Gass 2021-01-08 21:46:08 UTC
Jonathan,

For me this patch works. I followed a test plan like so:


1 Set an issuing rule's 'Holds allowed (total)'' to a low number (1 or 2);
2 Place more holds than you are allowed and note the error: 'There are no items that can be placed on hold.'
3. Set the issuing rule 'Holds allowed (total)' to 3 or 4 and set the issuing rule 'Holds allowed (daily)' to 1. Note the error 'There are no items that can be placed on hold.'
4. Set the issuning rule 'Holds per record (count)' to 1 then set 'Holds allowed (total)' and 'Holds allowed (daily)' to a high number.
6. Set the issuing rule 'OPAC item level holds' to allow.
7. Find an record with multiple items attached. Place a hold on 2 of the items in the record. Note the error 'There are no items that can be placed on hold.'
8. Make sure issuing rule 'OPAC item level holds' to allow and place an item level hold.
9. Try to place a 2nd hold on that same record. Note the error: 'There are no items that can be placed on hold.'

Apply patch 
repeat step 1: error message should read: 'This title cannot be requested because you reached the maximum number of holds.'
repeat step 3: error message should now read: 'his title cannot be requested because you reached the daily hold limit'.
repeat step 6 and 7: error message should no read 'This title cannot be requested because you exceeded max holds per record.'
repeat steps 8 and 9: error message should read: 'This title cannot be requested because you already have hold for this item.'
Comment 8 Katrin Fischer 2022-03-24 14:22:42 UTC
It looks like we kinda forgot about this one :( 

Is there a chance to revive?
Comment 9 Jonathan Druart 2022-03-24 14:27:35 UTC
Did you test the patch?
Comment 10 Owen Leonard 2022-05-06 18:17:48 UTC
Created attachment 134755 [details] [review]
Bug 19540: Tell the patron why a hold cannot be placed on items

This patch adds an explanation when a hold cannot be placed, in some situation:
tooManyReserves
tooManyHoldsForThisRecord
tooManyReservesToday
itemAlreadyOnHold

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 11 Tomás Cohen Arazi 2022-05-09 18:30:00 UTC
I like it, but it feels like several more use cases need to be considered.

Things like:
- pickupNotInHoldGroup
- libraryNotPickupLocation
- branchNotInHoldGroup

look like they should share the description, more or less.
Comment 12 Jonathan Druart 2022-06-08 14:34:50 UTC
(In reply to Tomás Cohen Arazi from comment #11)
> I like it, but it feels like several more use cases need to be considered.
> 
> Things like:
> - pickupNotInHoldGroup
> - libraryNotPickupLocation
> - branchNotInHoldGroup
> 
> look like they should share the description, more or less.

The patch fixes the problem that is described in comment 0.
Comment 13 Fridolin Somers 2022-08-16 22:12:51 UTC
Can this bug be on #c0 and a new bug report created for other cases ?

Many public libraries are asking for this.
Comment 14 Tomás Cohen Arazi 2022-08-17 12:58:29 UTC
Created attachment 139286 [details] [review]
Bug 19540: Tell the patron why a hold cannot be placed on items

This patch adds an explanation when a hold cannot be placed, in some situation:
tooManyReserves
tooManyHoldsForThisRecord
tooManyReservesToday
itemAlreadyOnHold

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2022-08-17 13:27:47 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 16 Lucas Gass 2022-10-03 22:27:17 UTC
Backported to 22.05.x for 22.05.06
Comment 17 Victor Grousset/tuxayo 2022-10-16 21:23:14 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 18 Arthur Suzuki 2022-10-22 22:06:44 UTC
Thanks!

Pushed to 21.11 for 21.11.12