Bug 28286 - Place hold button not displayed when biblio has only Ordered items
Summary: Place hold button not displayed when biblio has only Ordered items
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: Peter Vashchuk
QA Contact: Testopia
URL:
Keywords:
Depends on: 26139
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-04 13:53 UTC by Joonas Kylmälä
Modified: 2021-05-25 13:04 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06


Attachments
Bug 28286: show "Place hold" button if item is holdable (2.18 KB, patch)
2021-05-05 13:49 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 28286: show "Place hold" button if item is holdable (2.29 KB, patch)
2021-05-06 12:42 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 28286: tests for "filter_by_for_hold" method (1.53 KB, patch)
2021-05-06 12:43 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 28286: show "Place hold" button if item is holdable (3.89 KB, patch)
2021-05-07 09:51 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 28286: show "Place hold" button if item is holdable (3.96 KB, patch)
2021-05-07 10:57 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28286: show "Place hold" button if item is holdable (4.02 KB, patch)
2021-05-07 11:01 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 28286: show "Place hold" button if item is holdable (4.08 KB, patch)
2021-05-07 15:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28286: Replace < 1 with <= 0 (861 bytes, patch)
2021-05-10 12:08 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 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