Bug 36063

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>

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.

(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.

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.

(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'.

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?

(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.