Librarians have expressed that it would be useful to have the number of holds a record has on it display in the search results, preferably next to the "Holds" link for each record ( e.g. "Holds (3)" ).
Idea: 1) Create new module Koha::Template::Plugin::Holds with subroutine count( $biblionumber ) 2) Modify koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt to call this plugin to display the count of holds per record
Created attachment 53402 [details] [review] Bug 14876 - Show number of holds on the staff side next to 'holds' link This patch adds a a Holds plugin for the templates to count existing holds on a record. We then use this in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt to display the current number of holds on a record. To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - prove -v t/db_dependent/Template/Plugin/Holds.t 5 - Do search in staff that returns records with some holds (i.e. place some holds) 6 - Note the number of holds now displays next to the text of the link: Holds (#)
Created attachment 53465 [details] [review] [SIGNED-OFF] Bug 14876 - Show number of holds on the staff side next to 'holds' link This patch adds a a Holds plugin for the templates to count existing holds on a record. We then use this in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt to display the current number of holds on a record. To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - prove -v t/db_dependent/Template/Plugin/Holds.t 5 - Do search in staff that returns records with some holds (i.e. place some holds) 6 - Note the number of holds now displays next to the text of the link: Holds (#) Signed-off-by: Liz Rea <liz@catalyst.net.nz>
QA comments: 1/ POD of the plugin is wrong (c/p and not updated from another one). 2/ Since this plugin is only used once, why did you choose to use a TT plugin?
Created attachment 54578 [details] [review] Bug 14876 - (QA Followup) Fix pod
(In reply to Jonathan Druart from comment #4) > 2/ Since this plugin is only used once, why did you choose to use a TT > plugin? If we don't use this we'd have to iterate through the search results and counts the holds for each before display, this way we only check for those that are being currently shown. We could also alter detail.tt to use this function
(In reply to Nick Clemens from comment #6) > (In reply to Jonathan Druart from comment #4) > > 2/ Since this plugin is only used once, why did you choose to use a TT > > plugin? > > If we don't use this we'd have to iterate through the search results and > counts the holds for each before display, this way we only check for those > that are being currently shown. I am not sure I am following you, the results returned by searchResults are always displayed, isn't? Since we already iterate in catalogue/search.pl it's not a big deal to do this job over there. I don't think it's the job of a plugin to calculate the number of holds for a given record. Feel free to ask for another QA point of view ;)
I also think the plugin is a good solution. It keeps the code DRY. There is a distinct likelihood that we will want to display the holds count in other places in the future. The plugin means we only need to modify half the files we would need to otherwise.
CCing Katrin, Tomas, Marcel and Martin. It's not clear to me when we need a TT plugin. Sometimes it's obvious (get branch names, get patron category description, filter (json, date, price), pref values, etc.) But sometimes it's not: Borrowers.IsDebarred, Borrowers.HasOverdues. Holds.count is part of this second group. I don't think plugins should be a lazy solution for developers. We need to define when we can use a plugin, otherwise any methods from Koha modules could have their plugin wrapper. And we will have to maintain: Koha::Holds->count({biblionumber => $biblionumber }); Koha::Template::Plugin::Holds->count($biblionumber); and Koha::REST::V1::Hold->count($biblionumber); # does not exist (yet) (I don't switch to in discussion otherwise you will hate me)
From catalogue/detail.pl: #we only need to pass the number of holds to the template my $holds = C4::Reserves::GetReservesFromBiblionumber({ biblionumber => $biblionumber, all_dates => 1 }); $template->param( holdcount => scalar ( @$holds ) );
I was also wondering why my $nb_holds = Koha::Holds->count({ biblionumber => $biblionumber })->count; and not something like my $nb_holds = Koha::Biblios->find(biblionumber)->get_holds->count
(In reply to Jonathan Druart from comment #11) > and not something like > my $nb_holds = Koha::Biblios->find(biblionumber)->get_holds->count Hm. Does not really look nice to me. Apart from looking up the biblio record without using it..
ping Tomas, Katrin and Martin!
Created attachment 55889 [details] [review] Bug 14876 - Show number of holds per record on the search results This patch edits C4/Search.pm to pass back the count of holds along with other data for display in results.tt To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - Do search in staff that returns records with some holds (i.e. place some holds) 5 - Note the number of holds now displays next to the text of the link: Holds (#)
Updated patch to remove use of tt plugin
Created attachment 55892 [details] [review] Bug 14876 - Show number of holds per record on the search results This patch edits C4/Search.pm to pass back the count of holds along with other data for display in results.tt To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - Do search in staff that returns records with some holds (i.e. place some holds) 5 - Note the number of holds now displays next to the text of the link: Holds (#) Signed-off-by: Marc Véron <veron@veron.ch>
Nick, Since this is only used on the search result on the staff interface, I'd prefer to get the number of holds from catalogue/search.pl (unless you plan to display this info somewhere else).
Created attachment 55910 [details] [review] Bug 14876 - Show number of holds per record on the search results This patch edits catalogue/search.pl to pass back the count of holds along with other data for display in results.tt To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - Do search in staff that returns records with some holds (i.e. place some holds) 5 - Note the number of holds now displays next to the text of the link: Holds (#) Signed-off-by: Marc Véron <veron@veron.ch>
Nick, the parenthesis are displayed even if there are no holds on the record.
I prefer to show 'Holds (0)' when there are no holds, this is how it is currently done on the details page. If we want to display nothing then we should alter the details page to be 'Holds' instead of 'Holds (0)' Happy to do it either way, but want it to be consistent.
Created attachment 56082 [details] [review] Bug 14876 (QA Followup) Don't show (0) as holds counts when no holds
So it seems that you now use something that I did not want but was already pushed (and I QAed) You created Koha::Template::Plugin::Hold->acount when Koha::Template::Plugin::Biblio->HoldsCount already existed...
But you are using the plugin for biblio-view-menu.inc but for not results.tt. I am at lost here....
(In reply to Jonathan Druart from comment #23) > But you are using the plugin for biblio-view-menu.inc but for not results.tt. > I am at lost here.... s/but for not/but not for
Created attachment 57000 [details] [review] Bug 14876 - Show number of holds per record on the search results This patch sues the existing plugin Koha::Template::Plugin::Biblio->HoldsCount To be consistent with current behaviour 'Holds (0)' is shown when no holds have been placed on an item To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - prove -v t/db_dependent/Template/Plugin/Holds.t 5 - Do search in staff that returns records with some holds (i.e. place some holds) 6 - Note the number of holds now displays next to the text of the link: Holds (#)
Created attachment 58013 [details] [review] Bug 14876 - Show number of holds per record on the search results This patch sues the existing plugin Koha::Template::Plugin::Biblio->HoldsCount To be consistent with current behaviour 'Holds (0)' is shown when no holds have been placed on an item To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - prove -v t/db_dependent/Template/Plugin/Holds.t 5 - Do search in staff that returns records with some holds (i.e. place some holds) 6 - Note the number of holds now displays next to the text of the link: Holds (#) Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Created attachment 58153 [details] [review] Bug 14876 - Show number of holds per record on the search results This patch sues the existing plugin Koha::Template::Plugin::Biblio->HoldsCount To be consistent with current behaviour 'Holds (0)' is shown when no holds have been placed on an item To test: 1 - Search in staff 2 - Note holds link has no extra information 3 - Apply patch 4 - prove -v t/db_dependent/Template/Plugin/Holds.t 5 - Do search in staff that returns records with some holds (i.e. place some holds) 6 - Note the number of holds now displays next to the text of the link: Holds (#) Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Pushed to master for 17.05, thanks Nick!
This won't get backported to 16.11.x as it is an enhancement.