Bug 36063 - Add holds to pull notification to staff interface home page
Summary: Add holds to pull notification to staff interface home page
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Lucas Gass (lukeg)
QA Contact: Testopia
URL:
Keywords:
Depends on: 30849
Blocks: 36073 41513
  Show dependency treegraph
 
Reported: 2024-02-09 23:14 UTC by Lucas Gass (lukeg)
Modified: 2026-01-21 14:02 UTC (History)
5 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a "Holds to pull: All libraries: X" notification to the staff interface home page. Notes: - This only shows if the staff patron has the right permissions. - Currently it only shows the holds to pull for all libraries, and doesn't show the number for the library you are logged in as
Version(s) released in:
Circulation function:


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 (lukeg)
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
Bug 36063: Add local_holds_to_pull and adjust filters (3.90 KB, patch)
2026-01-02 15:26 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36063: Add local_holds_to_pull and adjust filters (4.05 KB, patch)
2026-01-02 15:37 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36063: Add local_holds_to_pull and adjust filters (4.46 KB, patch)
2026-01-02 15:42 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36063: Add local_holds_to_pull and adjust filters (4.46 KB, patch)
2026-01-02 17:27 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36063: Add local_holds_to_pull and adjust filters (6.78 KB, patch)
2026-01-02 17:49 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36063: Add local_holds_to_pull and adjust filters (6.84 KB, patch)
2026-01-02 18:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 36063: Add local_holds_to_pull and adjust filters (6.88 KB, patch)
2026-01-02 19:13 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 (lukeg) 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 (lukeg) 2024-02-09 23:30:56 UTC Comment hidden (obsolete)
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 (lukeg) 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 (lukeg) 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 (lukeg) 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.
Comment 9 Lucas Gass (lukeg) 2026-01-02 15:26:31 UTC
Created attachment 190830 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters
Comment 10 Lucas Gass (lukeg) 2026-01-02 15:37:10 UTC
Created attachment 190833 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters
Comment 11 Lucas Gass (lukeg) 2026-01-02 15:42:11 UTC
Created attachment 190834 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters

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 12 Lucas Gass (lukeg) 2026-01-02 17:27:26 UTC
Created attachment 190843 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters

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 13 Lucas Gass (lukeg) 2026-01-02 17:49:38 UTC
Created attachment 190844 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters

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 14 Owen Leonard 2026-01-02 18:58:57 UTC
Created attachment 190846 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters

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: Owen Leonard <oleonard@myacpl.org>
Comment 15 David Nind 2026-01-02 19:13:20 UTC
Created attachment 190848 [details] [review]
Bug 36063: Add local_holds_to_pull and adjust filters

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: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 16 David Nind 2026-01-02 19:23:01 UTC
Have added my sign-off as well, in case Owen's can count as the QA. 8-)
Comment 17 Nick Clemens (kidclamp) 2026-01-21 14:02:29 UTC
 - Commit title seems to be squashed?
 - $reserves_to_pull_from - maybe $date_to_pull_holds_from - not clear its a date with current name
 - 'item.homebranch' => C4::Context->userenv()->{'branch'}, - in a floating system this feels inaccurate - maybe it should be holding branch? And what about items in transit?