Bug 13654 - Cannot place a hold for biblio with too many items to display unless viewing them all
Summary: Cannot place a hold for biblio with too many items to display unless viewing ...
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
: 29756 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-02-02 15:58 UTC by Olli-Antti Kivilahti
Modified: 2025-03-07 08:09 UTC (History)
9 users (show)

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


Attachments
Bug 13654 - Cannot place a hold for biblio with too many Items to display unless viewing them all. (873 bytes, patch)
2015-02-02 15:59 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 13654: Show 'Place hold' when there are too many items? (1.42 KB, patch)
2018-11-12 13:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2015-02-02 15:58:20 UTC
REPLICATE:

1. Find a Biblio with many Items.
2. Set the system preference "OpacMaxItemsToDisplay" to something less than the amount of Items the Biblio has, but more than 0.
3. Go to the OPAC-details.pl -page for the Biblio.
4. Observe the notification "This record has many physical items (??). Click here to view them all." and the missing "Place hold" on the rightmost frame.
5. Clicking the "view them all"-button enables the "Place hold"-button.

Quick hack is provided.
Comment 1 Olli-Antti Kivilahti 2015-02-02 15:59:16 UTC
Created attachment 35656 [details] [review]
Bug 13654 - Cannot place a hold for biblio with too many Items to display unless viewing them all.

Certainly not a complete fix, but makes it possible to place holds on Serial Items again.
Comment 2 Owen Leonard 2015-02-02 16:03:41 UTC
Please remember to include the bug number, description, and test plan in your patch's commit message.
Comment 3 Olli-Antti Kivilahti 2015-02-02 17:29:40 UTC
Thanks for the tip Owen, but I think this patch is far from being included to the master. I am just too busy now with stuff breaking around me to provide proper unit tests etc.

The status was left to "New" on purpose.

Take care.
Comment 4 Olli-Antti Kivilahti 2015-02-02 17:31:19 UTC
There must be some consequences to this modification I made. They shouldn't be dangerous, so I am waiting for feedback from our librarians.

So far so good.
Comment 5 Nick Clemens (kidclamp) 2016-05-12 23:51:11 UTC
Did you get any feedback Olli?
Comment 6 George Williams (NEKLS) 2016-11-09 16:08:20 UTC
Any chance of re-writing this since it failed QA?  I'd really like to see this work.
Comment 7 Marcel de Rooy 2018-11-12 13:03:36 UTC
(In reply to George Williams (NEKLS) from comment #6)
> Any chance of re-writing this since it failed QA?  I'd really like to see
> this work.

I was just opening a new report when I saw this one. I have a patch with one line more for it?
Comment 8 Marcel de Rooy 2018-11-12 13:07:13 UTC
Currently, we do not show Place hold on opac-detail if there are too many items. The user can click View all items to refresh the detail page with all items. At that time we show Place hold (if possible).

If we just show Place hold if there are too many items, clicking Place hold could perhaps generate a refusal but will probably most often save another click on View all items (which even seems to confuse some users).
Comment 9 Marcel de Rooy 2018-11-12 13:16:02 UTC
Created attachment 82223 [details] [review]
Bug 13654: Show 'Place hold' when there are too many items?

Currently, we do not show Place hold on opac-detail if there are too many items. The user can click View all items to refresh the detail page with all items. At that time we show Place hold (if possible).

If we just show Place hold if there are too many items, clicking Place hold could perhaps generate a refusal but will probably most often save another click on View all items (which even seems to confuse some users).

Test plan:
Pick a biblio with too many items or lower OpacMaxItemsToDisplay.
With this patch, you now see Place hold in the side bar.
Comment 10 Marcel de Rooy 2018-11-12 13:17:25 UTC
Not for purists, but highly pragmatical approach..
Comment 11 Charles Farmer 2019-01-04 22:59:43 UTC
In the case where neither CountItemsIssued nor $biblio->has_items_waiting_or_intransit return 1, pushing AllowOnShelfHolds => 1 to the template doesn't seem enough, since the ReservableItems variable wouldn't have been initialized in opac-details.pl. On my end, at least, this means that I'm still not able to see the 'Place hold' button after applying the patch.

I would suggest either also sending ReservableItems to the template, or simply setting $allow_onshelf_holds = 1.

Still a bit too early to put this as Failed QA, as I'm not a QA person myself, but the patch doesn't feel quite right.
Comment 12 Joelle Hannert 2020-05-12 17:29:15 UTC
I see that this hasn't been commented on in about a year, so I wanted to mention that this bug is coming up for our library; not just with serials but also in-demand equipment (Chromebooks/hotspots) for which we have many items.
Comment 13 Owen Leonard 2022-01-04 17:09:20 UTC
*** Bug 29756 has been marked as a duplicate of this bug. ***
Comment 14 Christopher Brannon 2022-01-04 18:26:46 UTC
Why is the status on this bug blocked?
Comment 15 Marcel de Rooy 2022-01-10 08:38:54 UTC
(In reply to Christopher Brannon from comment #14)
> Why is the status on this bug blocked?

Comment11
Comment 16 Owen Leonard 2025-03-06 21:10:12 UTC
This bug has been reported at least three times now. Why was it changed to WONTFIX?
Comment 17 Christopher Brannon 2025-03-06 22:42:02 UTC
(In reply to Owen Leonard from comment #16)
> This bug has been reported at least three times now. Why was it changed to
> WONTFIX?

Good question.  Seems like if it is not going to be fixed, it should be explained why.  Otherwise, I'm just going to be a pest, and nobody wants that.  :)
Comment 18 David Cook 2025-03-06 23:26:34 UTC
(In reply to Christopher Brannon from comment #17)
> (In reply to Owen Leonard from comment #16)
> > This bug has been reported at least three times now. Why was it changed to
> > WONTFIX?
> 
> Good question.  Seems like if it is not going to be fixed, it should be
> explained why.  Otherwise, I'm just going to be a pest, and nobody wants
> that.  :)

I'm going to guess maybe Marcel closed it because of Comment 8 and Comment 11. But I don't know either.

--

One way to fix this one would be to remove the "Place hold" HTML and logic from the opac-detail.pl page and instead handle it via an API call. The template could show a loading graphic and once the async API request has determined holdability then it could render the "Place hold" or go from a disabled "Place hold" to a "Place hold" link. 

In theory, that should provide good usability, good page load performance, and an accurate view.
Comment 19 David Cook 2025-03-06 23:50:35 UTC
Actually... looking at this one again...

It's gated off ReservableItems... which is populated based off this:

my $holdable_items = $biblio->items->filter_by_for_hold->count;
my $can_holds_be_placed = $patron ? 0 : $holdable_items;

So actually... if you have "OpacMaxItemsToDisplay" active on a record, "Place hold" WILL appear if you're not logged in (if it's widely holdable). But if WILL NOT appear if you're logged in.

Honestly, I think you could just use $holdable_items to populate $can_holds_by_placed if the threshold for "OpacMaxItemsToDisplay" is met even if you're logged in.

But... technically that's wrong too. 

For instance, if an item is holdable for STAFF but not for the general public, and you're logged in as STAFF and "OpacMaxItemsToDisplay" is active on the record, "Place hold" won't appear even with the above "fix" since $holdable_items is based off the circ rules that don't include branch and categorycode. 

So to do get an accurate determination... we'd either have to loop all the items in opac-detail.pl for holdability or do the async API call.

I've waffled on the performance impact of looping through a large number of items for holdability. Sometimes it seems like it's a huge performance impact and other times it seems not great but not the worst thing ever. 

--

Overall, all things holdings/items needs a re-work to be more scalable really...
Comment 20 David Cook 2025-03-06 23:51:12 UTC
I'd say this should be re-opened but I don't know that anyone will work on this any time soon either...
Comment 21 David Cook 2025-03-06 23:52:45 UTC
These days, the holdings in the staff interface are loaded via API. It would be great to get that in the OPAC too, so then we wouldn't need "OpacMaxItemsToDisplay" anymore.

If we did that... would we do an API call for holdabilty or would we do the calculation server-side?
Comment 22 Katrin Fischer 2025-03-07 08:09:38 UTC
CLOSED was the semi-automatism we do after releases, RESOLVED WONTFIX was by Marcel, I imagine because he was not planning to work on this anymore? Reopening.