Bug 28286

Summary: Place hold button not displayed when biblio has only Ordered items
Product: Koha Reporter: Joonas Kylmälä <joonas.kylmala>
Component: Hold requestsAssignee: Peter Vashchuk <stalkernoid>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrewfh, fridolin.somers, gmcharlt, jonathan.druart, nick, nugged, sally.healey, stalkernoid
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06
Bug Depends on: 26139    
Bug Blocks:    
Attachments: Bug 28286: show "Place hold" button if item is holdable
Bug 28286: show "Place hold" button if item is holdable
Bug 28286: tests for "filter_by_for_hold" method
Bug 28286: show "Place hold" button if item is holdable
Bug 28286: show "Place hold" button if item is holdable
Bug 28286: show "Place hold" button if item is holdable
Bug 28286: show "Place hold" button if item is holdable
Bug 28286: Replace < 1 with <= 0

Description Joonas Kylmälä 2021-05-04 13:53:45 UTC
The "Place hold" button doesn't show up if there are only Ordered items, but it should since it is possible to place a hold for Ordered item. To reproduce go to /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX and add first 1 item with notforloan = 0 and notice the "Place hold" button shows, then change the notforloan value to -1 / Ordered and notice the hold button disappears.

The bug happens because in 

koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc

we decide whether to show the button based on if there are loanable items and not holdable items:

>     [% IF Context.Scalar(Context.Scalar(items, "filter_by_for_loan"), "count") %]

The call here should be changed to "filter_by_for_hold" in order to fix the issue.
Comment 1 Peter Vashchuk 2021-05-05 13:49:13 UTC Comment hidden (obsolete)
Comment 2 Peter Vashchuk 2021-05-06 12:42:16 UTC Comment hidden (obsolete)
Comment 3 Peter Vashchuk 2021-05-06 12:43:57 UTC Comment hidden (obsolete)
Comment 4 Joonas Kylmälä 2021-05-07 06:41:03 UTC
Please remove the old filter_by_for_loan() function completely because it becomes unused after your change.
Comment 5 Joonas Kylmälä 2021-05-07 06:43:44 UTC
The comment just above your changes:

>    [%# biblio.items.filter_by_for_loan.count %]

should also be removed.
Comment 6 Peter Vashchuk 2021-05-07 09:51:51 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2021-05-07 10:57:20 UTC Comment hidden (obsolete)
Comment 8 PTFS Europe Sandboxes 2021-05-07 11:01:58 UTC
Created attachment 120674 [details] [review]
Bug 28286: show "Place hold" button if item is holdable

This patch adds "filter_by_for_hold" method in "Items.pm" and
uses it in "cat-toolbar.inc" instead of "filter_by_for_load".

Also this patch removes "filter_by_for_loan" method.

To reproduce the bug:
1) go to /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX	that has
item with notforloan value set as "Ordered" (-1)
2) see that button "Place hold" is not present
3) apply the patch
4) refresh the page and ensure that "Place hold" button appears even if
item is "Ordered"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 9 Sally 2021-05-07 11:02:29 UTC
Works as expected - important catch, this one.  :)
Comment 10 Nick Clemens 2021-05-07 15:03:08 UTC
Created attachment 120701 [details] [review]
Bug 28286: show "Place hold" button if item is holdable

This patch adds "filter_by_for_hold" method in "Items.pm" and
uses it in "cat-toolbar.inc" instead of "filter_by_for_load".

Also this patch removes "filter_by_for_loan" method.

To reproduce the bug:
1) go to /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX	that has
item with notforloan value set as "Ordered" (-1)
2) see that button "Place hold" is not present
3) apply the patch
4) refresh the page and ensure that "Place hold" button appears even if
item is "Ordered"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2021-05-07 15:06:32 UTC
Passing QA, but wondering if this method is really necessary?

The 'Place hold' button is the same link as the 'Holds' tab on the left hand side

It checks one indicator of whether the items may be holdable or not, but we don't really know holdability until we select a patron.

If the items are all not for loan, but AllowHoldPolicyOverride is set, we can place holds but don't see the button.

Can we just show the button all the time and remove this single use method?
Comment 12 Jonathan Druart 2021-05-10 12:08:25 UTC
Created attachment 120768 [details] [review]
Bug 28286: Replace < 1 with <= 0

It's the same but read more natural
Comment 13 Jonathan Druart 2021-05-10 14:19:32 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 14 Fridolin Somers 2021-05-12 14:43:51 UTC
Pushed to 20.11.x for 20.11.06
Comment 15 Andrew Fuerste-Henry 2021-05-25 13:04:50 UTC
missing dependencies for 20.05.x, no backport