Bug 36063 - Add Holds to Pull info to intranet-main page
Summary: Add Holds to Pull info to intranet-main page
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 36064 36073
  Show dependency treegraph
 
Reported: 2024-02-09 23:14 UTC by Lucas Gass
Modified: 2024-02-13 20:03 UTC (History)
2 users (show)

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


Attachments
Bug 36063: Add holds to pull info to staff interface main page (5.04 KB, patch)
2024-02-09 23:30 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 36063: Add holds to pull info to staff interface main page (5.08 KB, patch)
2024-02-10 02:23 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2024-02-09 23:14:15 UTC
We often use JS to add a display to the intranet-main page when there are holds to pull. It would be nice to just add that into Koha.
Comment 1 Lucas Gass 2024-02-09 23:30:56 UTC
Created attachment 161997 [details] [review]
Bug 36063: Add holds to pull info to staff interface main page

To test:
1. APPLY PATCH, restart_all
2. Make some holds
3. Go to the staff interface home page, you should see 'New holds to pull: X'.
4. Check those holds in so they are waiting. Now they should not be counted against the number you see on the staff main page.
5. Log in as a staff member with Staff access but without circulate_remaining_permissions.
6. You should not see the 'New holds to pull: X' line.
Comment 2 David Nind 2024-02-10 02:23:58 UTC
Created attachment 162000 [details] [review]
Bug 36063: Add holds to pull info to staff interface main page

To test:
1. APPLY PATCH, restart_all
2. Make some holds
3. Go to the staff interface home page, you should see 'New holds to pull: X'.
4. Check those holds in so they are waiting. Now they should not be counted against the number you see on the staff main page.
5. Log in as a staff member with Staff access but without circulate_remaining_permissions.
6. You should not see the 'New holds to pull: X' line.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Christopher Brannon 2024-02-10 17:30:47 UTC
I placed a couple holds for the patron.  One that was available at the current logged in library, and one that was only at another library.  Both holds show up in the counter.  I never liked the holds to pull list, because in this case it shows everything regardless of location.  The counter should only be based on the logged in library, and you should either base it off the holds queue or, if you are going to use the holds to pull list, filter the list for the current library.

Personally, we hide this list because of the above reason.  The queue for the logged in library would be more reliable.
Comment 4 Lucas Gass 2024-02-12 14:18:03 UTC
(In reply to Christopher Brannon from comment #3)
> I placed a couple holds for the patron.  One that was available at the
> current logged in library, and one that was only at another library.  Both
> holds show up in the counter.  I never liked the holds to pull list, because
> in this case it shows everything regardless of location.  The counter should
> only be based on the logged in library, and you should either base it off
> the holds queue or, if you are going to use the holds to pull list, filter
> the list for the current library.
> 
> Personally, we hide this list because of the above reason.  The queue for
> the logged in library would be more reliable.

I would expect both to always show up. Since the Holds to pull report ( pendingreserves.pl ) cannot be filtered by branch I would not expect this to filter by branch either.
Comment 5 Christopher Brannon 2024-02-12 20:50:41 UTC
Which is why I recommend basing this off the holds queue.  This is what I am doing via jQuery right now.  This is also consistent with how the rest of the alerts work.  Staff don't need to be looking at alerts that don't concern them.
Comment 6 Lucas Gass 2024-02-12 21:35:20 UTC
(In reply to Christopher Brannon from comment #5)
> Which is why I recommend basing this off the holds queue.  This is what I am
> doing via jQuery right now.  This is also consistent with how the rest of
> the alerts work.  Staff don't need to be looking at alerts that don't
> concern them.

I filed Bug 36073, as it would be good to add this for the 'Holds queue' as well. I don't love the idea of adding a sys pref to show or hide each of these from the main page and it can be hidden with CSS if needed. 

Setting this back to 'Signed off'.
Comment 7 Owen Leonard 2024-02-13 11:19:57 UTC
In my system with custom test data I'm seeing a mismatch between the count on the home page and the number of holds returned by pendingreserves.pl. I see that there is a lot of extra filtering going on in that script, so I wonder if the simple 'Koha::Holds->search( found => undef )' is not counting too many holds.

The other issue I see is that pendingreserves.pl has a default date range which makes the user's experience of what the home page reports different from what they see on that page by default.

In my system the home page reports 59 pending holds, pendingreserves.pl reports zero holds by default, and with an all-inclusive date range returns 31 holds.

- I think the home page count should definitely use the same (non-date) limits, but should it also use the same date limits?
- Alternatively, should we remove the default date limit on pendingreserves.pl?
Comment 8 Lucas Gass 2024-02-13 20:03:28 UTC
(In reply to Owen Leonard from comment #7)
> In my system with custom test data I'm seeing a mismatch between the count
> on the home page and the number of holds returned by pendingreserves.pl. I
> see that there is a lot of extra filtering going on in that script, so I
> wonder if the simple 'Koha::Holds->search( found => undef )' is not counting
> too many holds.
> 
> The other issue I see is that pendingreserves.pl has a default date range
> which makes the user's experience of what the home page reports different
> from what they see on that page by default.
> 
> In my system the home page reports 59 pending holds, pendingreserves.pl
> reports zero holds by default, and with an all-inclusive date range returns
> 31 holds.
> 
> - I think the home page count should definitely use the same (non-date)
> limits, but should it also use the same date limits?
> - Alternatively, should we remove the default date limit on
> pendingreserves.pl?

Thanks Owen. You're right, there needs to be more filters. That makes me want to move some of the logic from pendingreserves.pl into Koha/Holds.pm ( something like 'filter_by_holds_to_pull') so it can be reused on mainpage.pl and covered by tests. I will work up a new patch.