Add a select field to the search results page (both in staff client and opac) where user can select how many results per page should be shown. For instance: 20/40/60/80/100/all
Created attachment 41949 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page".
Created attachment 42003 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
I would prefer to see a version of this patch using sessionStorage instead of cookies.
I have now investigated the option of using sessionStorage instead of a cookie. However this would, what I can see, either: require to send the sessionStorage variable to the server for every search, which means modifying all scripts requesting the (opac-)search.pl files. In this case it would not be much different from using a cookie. or require the (opac-)search.pl scripts to get the maximum number of search results ("all", or perhaps limit to 100) and then filter them on client side. This would require a lot of extra unnecessary work for the server. Neither option seems very attractiv to me. Perhaps there's a third option which I'm missing?
Martin, I am really sorry, this was a stupid suggestion! I had a look again at your patch and I have just two concerns: 1/ If the number of results is large (say more than 1000), we don't want to allow the user to display all of the results. The solution would be to add a syspref, to let the choice to the library, or not display this entry. 2/ The value of the [OPAC]numSearchResults pref should appear in the options. Marked as Failed QA.
Created attachment 42742 [details] [review] Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults.
Created attachment 42743 [details] [review] Bug 14715: database updates
Created attachment 56002 [details] [review] Bug 14715: Fixing merge conflicts etc. To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all)
Created attachment 56003 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page".
Created attachment 56004 [details] [review] Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults.
Created attachment 56005 [details] [review] Bug 14715: database updates
These patches should now apply cleanly and are ready for testing. To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all)
Hi! If the results of the search is bigger than the page size, the pagination appears but it doesnt work.
Wanted to test, but sorry, does not apply.
Created attachment 56575 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page".
Created attachment 56576 [details] [review] Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults.
Created attachment 56577 [details] [review] Bug 14715: database updates
Hi! The pagination doesnt work if the result of the search is bigger than the results_per_page.
Created attachment 56610 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page".
Created attachment 56611 [details] [review] Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults.
Created attachment 56612 [details] [review] Bug 14715: database updates
Patches reattached to fix merge conflicts. (In reply to Jokin Perez de Viñaspre from comment #18) > Hi! > > The pagination doesnt work if the result of the search is bigger than the > results_per_page. Are you running updatedatabase.pl ?
Hi! Yes, I runned updatedatabase.pl To solve the problem of pagination I change one line in a/opac/opac-search.pl(line 527) and a/catalogue/search.pl(line 456) of your patch my $offset = $results_per_page != 'all' ? ($page-1)*$results_per_page : 0; to: my $offset = $params->{'offset'} || 0; With that change works fine.
Created attachment 57870 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems
I made two searchs. It worked just fine for both of them.
I do not think it is a good idea to provide 'all' at the OPAC. With only one query you can kill a server for several minutes :)
From logs: Argument "all" isn't numeric in numeric eq (==) at /home/vagrant/kohaclone/opac/opac-search.pl line 637.
Failing for comment 27, please answer to comment 26.
(In reply to Jonathan Druart from comment #28) > Failing for comment 27, please answer to comment 26. I think you're probably right about this - I will remove the 'all' option for the OPAC.
Created attachment 61721 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems
Sorry, QA tools complain: FAIL koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt FAIL tt_valid lines 368, 369, 370, 371, 372, 373 FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt FAIL tt_valid lines 151, 152, 153, 154, 155 Suppose it is because of: https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML
Created attachment 64250 [details] [review] Bug 14715: [FOLLOW-UP] Fixing qa-tools issues and merge conflicts
Patch doesn't apply anymore, but this sounds like an interesting feature. Can someone rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14715: results per page setting for catalog search Using index info to reconstruct a base tree... M C4/Search.pm M catalogue/search.pl M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt M koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt M opac/opac-search.pl Falling back to patching base and 3-way merge... Auto-merging opac/opac-search.pl CONFLICT (content): Merge conflict in opac/opac-search.pl Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt Auto-merging catalogue/search.pl CONFLICT (content): Merge conflict in catalogue/search.pl Auto-merging C4/Search.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14715: results per page setting for catalog search The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14715-results-per-page-setting-for-catalog-sea-nZTnir.patch
Created attachment 67912 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems
Created attachment 67913 [details] [review] Bug 14715: [FOLLOW-UP] Fixing qa-tools issues and merge conflicts
Patches rebased, ready for testing.
Created attachment 68000 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 68001 [details] [review] Bug 14715: [FOLLOW-UP] Fixing qa-tools issues and merge conflicts Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to Jonathan Druart from comment #26) > I do not think it is a good idea to provide 'all' at the OPAC. With only one > query you can kill a server for several minutes :) For the same reason I would prefer to remove it for Staff too.
FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt OK forbidden patterns OK git manipulation SKIP spelling FAIL tt_valid lines 151, 152, 153, 154, 155 OK valid_template
Created attachment 68305 [details] [review] Bug 14715: [FOLLOW-UP] Removing 'all' option, fixing QA tools issues
Created attachment 68327 [details] [review] Bug 14715: [FOLLOW-UP] Removing 'all' option, fixing QA tools issues Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
QA Comment: Why do you add in catalogue/search.pl: +use String::Random; +@results_per_page_options = do { my %seen; grep { !$seen{$_}++ } @results_per_page_options }; Could you 'do' this without a do statement? Do you need uniq from List::More ? Is it really needed btw? Do you expect duplicate numbers in this pref.. results.tt Why are you changing the lines with respect to list counts? What is the relation with this patch? opac-search.pl -$offset = ($page-1)*$results_per_page if $page>1; Why do you remove it ? - ($error, $results_hashref, $facets) = C4::Search::pazGetRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,undef,$query_type,$scan); + ($error, $results_hashref, $facets) = C4::Search::pazGetRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$query_type,$scan); You remove a undef parameter. Why? Is this a bug on its own? What is the relation with this report? Please clarify or adjust.
Created attachment 75938 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 75939 [details] [review] Bug 14715: [FOLLOW-UP] Fixing qa-tools issues and merge conflicts Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 75940 [details] [review] Bug 14715: [FOLLOW-UP] Removing 'all' option, fixing QA tools issues Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Created attachment 75941 [details] [review] Bug 14715: [FOLLOW-UP] Rebase and fix up I noticed that the two sysprefs initially created with this patch weren't really being used, so I removed them. Instead I've added two new sysprefs: numSearchResultsDropdown OPACnumSearchResultsDropdown that control whether the dropdown shows or not. Otherwise the defaults in numSearchResults and OPACnumSearchResults are used to decide number of search results per page. When testing: Run updatedatabase.pl Test with values in numSearchResults and OPACnumSearchResults. Use original test plan in Comment 12 but test with the dropdown sysprefs off and on. Sponsored-by: Region Halland
(In reply to Marcel de Rooy from comment #43) > QA Comment: > > Why do you add in catalogue/search.pl: > +use String::Random; > > +@results_per_page_options = do { my %seen; grep { !$seen{$_}++ } > @results_per_page_options }; > Could you 'do' this without a do statement? Do you need uniq from List::More > ? > Is it really needed btw? Do you expect duplicate numbers in this pref.. > This is now gone in latest follow-up. > results.tt > Why are you changing the lines with respect to list counts? What is the > relation with this patch? > This seems unrelated to this patch but is no longer affected in the follow-up. > opac-search.pl > -$offset = ($page-1)*$results_per_page if $page>1; > Why do you remove it ? > Offset is dealt with later. > - ($error, $results_hashref, $facets) = > C4::Search::pazGetRecords($query,$simple_query,\@sort_by,\@servers, > $results_per_page,$offset,$expanded_facet,undef,$query_type,$scan); > + ($error, $results_hashref, $facets) = > C4::Search::pazGetRecords($query,$simple_query,\@sort_by,\@servers, > $results_per_page,$offset,$expanded_facet,$query_type,$scan); > You remove a undef parameter. > Why? Is this a bug on its own? What is the relation with this report? > > Please clarify or adjust. Have put undef param back. Not sure why this was removed initially. Ready for testing.
Tested in Biblibre Sandbox 05. Results appear correctly in the intranet when set *manually* on the results page to 20, 40, et. No results displayed initially, however. This is true whether numSearchResults preference is blank or whether it is set to 20. Unable to test in the OPAC as all searches lead to "No results found!"
My tests turned out exactly the same as Cab's in comment 49.
Created attachment 78230 [details] [review] Bug 14715: [FOLLOW-UP] Fix results not showing This fixes the display issue for me. Can someone please test?
Patch tested with a sandbox, by Cab Vinton <bibliwho@gmail.com>
Created attachment 78298 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com>
Created attachment 78299 [details] [review] Bug 14715: [FOLLOW-UP] Fixing qa-tools issues and merge conflicts Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com>
Created attachment 78300 [details] [review] Bug 14715: [FOLLOW-UP] Removing 'all' option, fixing QA tools issues Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com> Signed-off-by: Cab Vinton <bibliwho@gmail.com>
Created attachment 78301 [details] [review] Bug 14715: [FOLLOW-UP] Rebase and fix up I noticed that the two sysprefs initially created with this patch weren't really being used, so I removed them. Instead I've added two new sysprefs: numSearchResultsDropdown OPACnumSearchResultsDropdown that control whether the dropdown shows or not. Otherwise the defaults in numSearchResults and OPACnumSearchResults are used to decide number of search results per page. When testing: Run updatedatabase.pl Test with values in numSearchResults and OPACnumSearchResults. Use original test plan in Comment 12 but test with the dropdown sysprefs off and on. Sponsored-by: Region Halland Signed-off-by: Cab Vinton <bibliwho@gmail.com>
Created attachment 78302 [details] [review] Bug 14715: [FOLLOW-UP] Fix results not showing This fixes the display issue for me. Can someone please test? Signed-off-by: Cab Vinton <bibliwho@gmail.com>
Signed off in PTFS Sandbox 0. But note that the dropdown setting appears to control. In other words, once the dropdown is displayed, that setting sticks, even if the preference setting is changed or if the user logs out of the staff client or OPAC. This is also true if the dropdown is subsequently hidden. So, once the dropdown has been displayed, that is where the Results per page must be set & the system preference does not control.
Hi Aleisha, 1) some complaints from the QA script: FAIL installer/data/mysql/atomicupdate/bug_14715-searchresults_syspref.sql FAIL git manipulation The file has been added and deleted in the same patchset FAIL installer/data/mysql/atomicupdate/bug_14715_-_results_per_page_dropdown_sysprefs.sql OK git manipulation FAIL semicolon simicolon found instead of comma at line 3 FAIL sysprefs_order Not blocker: Sysprefs numSearchResultsDropdown is bad placed (see bug 10610) It appears the change to sysprefs.sql is missing? 2) I think I would just remove the display of 'library default' in the OPAC and staff. I think we don't need the extra entry in the pull down and it's more confusing as the difference between Library default:20 and 20 is not clear. 3) It appears to break when Elasticsearch is activated in a non-good way. Switching the number of results made the OPAC load forever for me showing a white page. Then it started to work, but the number of pages doesn't change. Can you please check? If we can't make this work, we should at least add a note to the pref and keep it from showing up with Elasticsearch active. Note for later: New cookie needs to be documented in the wiki, once this has been pushed: https://wiki.koha-community.org/wiki/Use_of_Cookies
Created attachment 79796 [details] [review] Bug 14715: (follow-up) some code changes 1) adds sysprefs to sysprefs.sql 2) removes a merge conflict error that should not have been added
Created attachment 79797 [details] [review] Bug 14715: (follow-up) some code changes 1) adds sysprefs to sysprefs.sql 2) removes a merge conflict error that should not have been added
Created attachment 79988 [details] [review] Bug 14715: (follow-up) Note about elasticsearch I'm not sure how to hide the sysprefs if ElasticSearch is active so I've added some comments to the preferences. Ready to test
Tested with Zebra : works fine ! Tested with Elasticsearch : works fine except for "60 results per page" (UI + OPAC) : each time there is more than 10000 results, using "Last" button failed : "Error: Unable to perform your search. Please try again." As it's not supposed to work with ES, I don't know if I should change the status to "Failed" or sign the patch...
Created attachment 81203 [details] [review] Bug 14715: (follow-up) Edits for ElasticSearch Note added to preferences that the dropdown should not be used with ElasticSearch activated. Dropdowns do not show on search results on staff side or OPAC side if Elasticsearch enabled. Ready to test
(In reply to Séverine Queune from comment #63) > Tested with Zebra : works fine ! > > Tested with Elasticsearch : works fine except for "60 results per page" (UI > + OPAC) : each time there is more than 10000 results, using "Last" button > failed : "Error: Unable to perform your search. Please try again." > > As it's not supposed to work with ES, I don't know if I should change the > status to "Failed" or sign the patch... Hi there! Can you please test again - i have hidden the dropdown if Elasticsearch is enabled to avoid this error.
I'm going to sign off on this despite these two things: FAIL installer/data/mysql/atomicupdate/bug_14715-searchresults_syspref.sql FAIL git manipulation The file has been added and deleted in the same patchset ^^ I couldn't figure out how to resolve that, someone share plz. FAIL installer/data/mysql/atomicupdate/bug_14715_-_results_per_page_dropdown_sysprefs.sql OK git manipulation OK semicolon FAIL sysprefs_order Not blocker: Sysprefs numSearchResultsDropdown is bad placed (see bug 10610) ^^ isn't a blocker so I'm not letting it block me.
Created attachment 88097 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88102 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88103 [details] [review] Bug 14715: [QA Follow up] Fixing qa-tools issues and merge conflicts Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88104 [details] [review] Bug 14715: [QA Follow up] Removing 'all' option, fixing QA tools issues Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88105 [details] [review] Bug 14715: [QA Follow up] Rebase and fix up I noticed that the two sysprefs initially created with this patch weren't really being used, so I removed them. Instead I've added two new sysprefs: numSearchResultsDropdown OPACnumSearchResultsDropdown that control whether the dropdown shows or not. Otherwise the defaults in numSearchResults and OPACnumSearchResults are used to decide number of search results per page. When testing: Run updatedatabase.pl Test with values in numSearchResults and OPACnumSearchResults. Use original test plan in Comment 12 but test with the dropdown sysprefs off and on. Sponsored-by: Region Halland Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88106 [details] [review] Bug 14715: (QA Follow up) Fix results not showing This fixes the display issue for me. Can someone please test? Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88107 [details] [review] Bug 14715: (QA follow-up) some code changes 1) adds sysprefs to sysprefs.sql 2) removes a merge conflict error that should not have been added Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88108 [details] [review] Bug 14715: (QA follow-up) Edits for ElasticSearch Note added to preferences that the dropdown should not be used with ElasticSearch activated. Dropdowns do not show on search results on staff side or OPAC side if Elasticsearch enabled. Ready to test Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88109 [details] [review] Bug 14715: (QA Follow up) More QA tool cleanup Tools are picking up more stuff now, so a few more things to fix! Signed-off-by: Liz Rea <wizzyrea@gmail.com>
System preference names don't match up: Database updates add 4 different prefs: OPACnumSearchResultsPerPage numSearchResultsPerPage OPACnumSearchResultsDropdown numSearchResultsDropdown syspref.sql adds only 2: numSearchResultsPerPage OPACnumSearchResultsPerPage yaml file has: OPACnumSearchResultsDropdown numSearchResultsDropdown Code appears to use the Dropdown version. Please, when cleaning this up: Database update SQL files should be merged and moved into a .perl file - this will make the RM much happier. For later: New cookie results_per_page needs documentation on the wiki.
Created attachment 88126 [details] [review] Bug 14715: (follow-up) Fixing database updates This patch removes the atomic update that adds the 'perPage' system preferences and keeps the 'dropdown' system preferences. Also sql file is moved into perl file.
Created attachment 88195 [details] [review] Bug 14715: (follow-up) Fixing database updates This patch removes the atomic update that adds the 'perPage' system preferences and keeps the 'dropdown' system preferences. Also sql file is moved into perl file. Signed-off-by: Liz Rea <wizzyrea@gmail.com>
we might want to rebase out the old .sql files, but I don't want to mess it up so I will leave that bit. That's the only QA fail I can see, the newest patch resolves Katrin's concerns, near as I can tell.
Hi Aleisha, I am sorry, can you please rebase?
Created attachment 88831 [details] [review] Bug 14715: results per page setting for catalog search Add new select field to catalog search results pages which allows user to set number of results to be shown per page. Setting saved in cookie "results_per_page". To test: 1) Do a catalog search in the intranet and OPAC 2) Confirm that results automatically update upon selection of results per page to show 3) Test for all options (20, 40, 60, 80, 100, all) Bug 14715: syspref for results per page options Remove the "all" option. Add syspref [OPAC]numSearchResultsPerPage for specifying options for numbers of results per page including [OPAC]numSearchResults. Bug 14715: database updates Bug 14715: Fixing pagination problems Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88832 [details] [review] Bug 14715: [QA Follow up] Fixing qa-tools issues and merge conflicts Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88833 [details] [review] Bug 14715: [QA Follow up] Removing 'all' option, fixing QA tools issues Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com> Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88834 [details] [review] Bug 14715: [QA Follow up] Rebase and fix up I noticed that the two sysprefs initially created with this patch weren't really being used, so I removed them. Instead I've added two new sysprefs: numSearchResultsDropdown OPACnumSearchResultsDropdown that control whether the dropdown shows or not. Otherwise the defaults in numSearchResults and OPACnumSearchResults are used to decide number of search results per page. When testing: Run updatedatabase.pl Test with values in numSearchResults and OPACnumSearchResults. Use original test plan in Comment 12 but test with the dropdown sysprefs off and on. Sponsored-by: Region Halland Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88835 [details] [review] Bug 14715: (QA Follow up) Fix results not showing This fixes the display issue for me. Can someone please test? Signed-off-by: Cab Vinton <bibliwho@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88836 [details] [review] Bug 14715: (QA follow-up) some code changes 1) adds sysprefs to sysprefs.sql 2) removes a merge conflict error that should not have been added Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88837 [details] [review] Bug 14715: (QA follow-up) Edits for ElasticSearch Note added to preferences that the dropdown should not be used with ElasticSearch activated. Dropdowns do not show on search results on staff side or OPAC side if Elasticsearch enabled. Ready to test Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88838 [details] [review] Bug 14715: (QA Follow up) More QA tool cleanup Tools are picking up more stuff now, so a few more things to fix! Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88839 [details] [review] Bug 14715: (follow-up) Fixing database updates This patch removes the atomic update that adds the 'perPage' system preferences and keeps the 'dropdown' system preferences. Also sql file is moved into perl file. Signed-off-by: Liz Rea <wizzyrea@gmail.com>
*** Bug 7260 has been marked as a duplicate of this bug. ***
The sandbox you've requested is not ready. Some problems occurred applying patches from bug 14715: <h1>Something went wrong !</h1>Applying: Bug 14715: results per page setting for catalog search Using index info to reconstruct a base tree... M C4/Search.pm M catalogue/search.pl M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt M koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt M opac/opac-search.pl Falling back to patching base and 3-way merge... Auto-merging opac/opac-search.pl CONFLICT (content): Merge conflict in opac/opac-search.pl Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt Auto-merging catalogue/search.pl CONFLICT (content): Merge conflict in catalogue/search.pl Auto-merging C4/Search.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14715: results per page setting for catalog search
Created attachment 95038 [details] [review] Bug 14715: Results per page dropdown for catalogue search This patch utilises an unused search facet to handle results shown per page. To test: 1) Enable patch and update database to apply atomic updates 2) Go to system preferences and set both 'numSearchResultsDropdown' and 'OPACnumSearchResultsDropdown' to 'Show' 3) Set both 'numSearchResults' and 'OPACnumSearchResults' system preferences to 10 so that it will be clear to see how the patch works 4) Do a catalogue search in the intranet using a search term that will bring lots of results 5) The results per page dropdown should show with the selected option as the library default, the value of numSearchResults. The number of results shown on the page should match numSearchResults. 6) Choose a different number of results per page in the dropdown. The search results will automatically refresh 7) Confirm that the correct number of results per page shows 8) Confirm that changing pages does not revert the number of results shown per page to the default value 9) Confirm that you are still able to use other search facets as expected 10) Repeat steps 4-9 on the OPAC (note OPAC uses OPACnumSearchResults as the default value) Sponsored-by: Region Halland
Created attachment 97004 [details] [review] Bug 14715: Results per page dropdown for catalogue search This patch utilises an unused search facet to handle results shown per page. Sponsored-by: Region Halland Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com>
Created attachment 98365 [details] [review] Bug 14715: Results per page dropdown for catalogue search This patch utilises an unused search facet to handle results shown per page. To test: 1) Enable patch and update database to apply atomic updates 2) Go to system preferences and set both 'numSearchResultsDropdown' and 'OPACnumSearchResultsDropdown' to 'Show' 3) Set both 'numSearchResults' and 'OPACnumSearchResults' system preferences to 10 so that it will be clear to see how the patch works 4) Do a catalogue search in the intranet using a search term that will bring lots of results 5) The results per page dropdown should show with the selected option as the library default, the value of numSearchResults. The number of results shown on the page should match numSearchResults. 6) Choose a different number of results per page in the dropdown. The search results will automatically refresh 7) Confirm that the correct number of results per page shows 8) Confirm that changing pages does not revert the number of results shown per page to the default value 9) Confirm that you are still able to use other search facets as expected 10) Repeat steps 4-9 on the OPAC (note OPAC uses OPACnumSearchResults as the default value) Sponsored-by: Region Halland Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com> Signed-off-by: David Nind <david@davidnind.com>
A couple of things I noticed during testing, not sure whether they are show stoppers or not: - OPAC and staff interface: the number of results resets to the default if you select any of the refine search options/facets - Staff interface: if you choose any option from the sort drop down this also resets the number of search results to the default Otherwise, it works very nicely!
Hi Aleisha, can you please take a look at David'd comment#95? I also got a lot of fails with the QA tools for missing filters :( FAIL koha-tmpl/intranet-tmpl/prog/en/includes/page-numbers.inc FAIL filters missing_filter at line 4 ( <li><a class="nav" href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %][% IF ( sort_by ) %]&sort_by=[% sort_by |url %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]"><i class="fa fa-fw fa-angle-double-left"></i> First</a></li>) missing_filter at line 6 ( <li><a class="nav" href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% previous_page_offset | html %][% IF ( sort_by ) %]&sort_by=[% sort_by |url %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]"><i class="fa fa-fw fa-angle-left"></i> Previous</a></li>) missing_filter at line 12 ( <li><a class="nav" href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% PAGE_NUMBER.offset | html %][% IF ( sort_by ) %]&sort_by=[% sort_by |url %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">[% PAGE_NUMBER.pg | html %]</a></li>) missing_filter at line 16 ( <li><a class="nav" href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% next_page_offset | html %][% IF ( sort_by ) %]&sort_by=[% sort_by |url %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">Next <i class="fa fa-fw fa-angle-right"></i></a></li>) missing_filter at line 17 ( <li><a class="nav" href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% last_page_offset | html %][% IF ( sort_by ) %]&sort_by=[% sort_by |url %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">Last <i class="fa fa-fw fa-angle-double-right"></i></a></li>) OK forbidden patterns OK git manipulation OK js_in_body OK spelling OK tt_valid OK valid_template FAIL koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt FAIL filters missing_filter at line 274 ( Results per page: [% results_per_page %] <span class="caret"></span>) missing_filter at line 281 ( [% END %] Library default: [% Koha.Preference('numSearchResults') %]) OK forbidden patterns OK git manipulation OK js_in_body OK spelling OK tt_valid OK valid_template FAIL koha-tmpl/opac-tmpl/bootstrap/en/includes/page-numbers.inc FAIL filters wrong_html_filter at line 6 ( <li class="page-first"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]"><i class="fa fa-fw fa-angle-double-left"></i> First</a></li>) missing_filter at line 6 ( <li class="page-first"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]"><i class="fa fa-fw fa-angle-double-left"></i> First</a></li>) wrong_html_filter at line 7 ( <li class="page-prev"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% previous_page_offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]"> <i class="fa fa-fw fa-angle-left"></i> Previous</a></li>) missing_filter at line 7 ( <li class="page-prev"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% previous_page_offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]"> <i class="fa fa-fw fa-angle-left"></i> Previous</a></li>) wrong_html_filter at line 13 ( <li class="page-num"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% PAGE_NUMBER.offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">[% PAGE_NUMBER.pg | html %]</a></li>) missing_filter at line 13 ( <li class="page-num"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% PAGE_NUMBER.offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">[% PAGE_NUMBER.pg | html %]</a></li>) wrong_html_filter at line 17 ( <li class="page-next"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% next_page_offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">Next <i class="fa fa-fw fa-angle-double-right"></i></a></li>) missing_filter at line 17 ( <li class="page-next"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% next_page_offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">Next <i class="fa fa-fw fa-angle-double-right"></i></a></li>) wrong_html_filter at line 18 ( <li class="page-last"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% last_page_offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">Last <i class="fa fa-fw fa-angle-double-right"></i></a></li>) missing_filter at line 18 ( <li class="page-last"><a href="/cgi-bin/koha/opac-search.pl?[% query_cgi | $raw %][% limit_cgi | $raw %]&offset=[% last_page_offset | uri %][% IF ( sort_by ) %]&sort_by=[% sort_by | uri %][% END %][% IF results_per_page %]&count=[% results_per_page %][% END %]">Last <i class="fa fa-fw fa-angle-double-right"></i></a></li>) OK forbidden patterns OK git manipulation OK js_in_body OK spelling OK tt_valid OK valid_template FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt FAIL filters missing_filter at line 172 ( <option value="[% Koha.Preference('OPACnumSearchResults') %]" selected="selected">Library default: [% Koha.Preference('OPACnumSearchResults') %]</option>) missing_filter at line 172 ( <option value="[% Koha.Preference('OPACnumSearchResults') %]" selected="selected">Library default: [% Koha.Preference('OPACnumSearchResults') %]</option>) missing_filter at line 174 ( <option value="[% Koha.Preference('OPACnumSearchResults') %]">Library default: [% Koha.Preference('OPACnumSearchResults') %]</option>) missing_filter at line 174 ( <option value="[% Koha.Preference('OPACnumSearchResults') %]">Library default: [% Koha.Preference('OPACnumSearchResults') %]</option>)
Missing filters can be added easily with misc/devel/add_missing_filters.pl
Created attachment 99607 [details] [review] Bug 14715: (follow-up) Keep results per page setting when selecting facets and sorting results Also adding missing filters
Aleisha, what about David's comment?
Aleisha, what about David's comment? can you please take a look at David'd comment#95?(In reply to Jonathan Druart from comment #99) > Aleisha, what about David's comment? Scratch that! I thought you only added the filters!
Thanks for all the changes Aleisha! When changing facets it now remembers the setting. However, the only thing thing that isn't working is on the staff interface. When you try to go back to the default (Library default: 10) nothing happens, example URL - the number doesn't change, the page doesn't automatically refredh and a # is added at the end of the URL: http://127.0.0.1:8081/cgi-bin/koha/catalogue/search.pl?count=20&idx=kw&q=cat#
Created attachment 99677 [details] [review] Bug 14715: (follow-up) Keep results per page setting when selecting facets and sorting results Also adding missing filters ALSO ensuring you can switch back to library default on staff side
Created attachment 99684 [details] [review] Bug 14715: Results per page dropdown for catalogue search This patch utilises an unused search facet to handle results shown per page. Sponsored-by: Region Halland Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99685 [details] [review] Bug 14715: (follow-up) Keep results per page setting when selecting facets and sorting results Also adding missing filters ALSO ensuring you can switch back to library default on staff side Signed-off-by: David Nind <david@davidnind.com>
What is the problem with elasticsearch? I have tried (removing the condition in the template), and it works as expected. I do not think we should continue to push Zebra-only features nowadays, especially if it works with elastic :)
>3) It appears to break when Elasticsearch is activated in a non-good way. >Switching the number of results made the OPAC load forever for me showing a >white page. Then it started to work, but the number of pages doesn't change. >Can you please check? If we can't make this work, we should at least add a note >to the pref and keep it from showing up with Elasticsearch active. I have retested and can't find any issues with Elasticsearch - I think the problems I found earlier were probably an issue in my installation. I am proposing a PQA with a follow-up removing the code that makes this Zebra only.
Created attachment 101775 [details] [review] Bug 14715: Results per page dropdown for catalogue search This patch utilises an unused search facet to handle results shown per page. Sponsored-by: Region Halland Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 101776 [details] [review] Bug 14715: (follow-up) Keep results per page setting when selecting facets and sorting results Also adding missing filters ALSO ensuring you can switch back to library default on staff side Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 101777 [details] [review] Bug 14715: (QA follow-up) Add missing filters Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 101778 [details] [review] Bug 14715: (QA follow-up) Remove limit on Zebra Found in testing that this appears to work well for Elasticsearch - so removing everything that made this Zebra only. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x
Hm, was looking to add information on the use of the cookie mentioned to https://wiki.koha-community.org/wiki/Use_of_Cookies, but didn't spot the code for it. Can you help?
Verified with Joubu - there is no cookie :)
I'm noticing something in situations where the OPAC is suppressing records that I would appreciate some feedback on. I walk through what I'm seeing below using a general example from a site I'm testing on. In the below example, the system preferences have been set so that the results per page dropdown appears and a search will show 10 results per page for both the OPAC and the staff side. 1. I use the advanced search on the OPAC to search for all items with a specific item type called "General items". 3. The results per page display shows that there should be 10 results on the first and all other pages. 4. There are actually 9 results on the first results page. 5. There are 10 results on all other pages. 6. Doing the same search on the staff side, the results per page display shows that there should be 10 results on the first and all other pages. 7. There are 10 results on the first page and all other pages. 8. The first result on the first page of the staff side does not appear on the OPAC. Looking at it, this seems to be because all items on the bibliographic record are marked as lost. 9. It is fine that the OPAC is not showing the suppressed record, but I would assume in cases like this, instead of showing nine results on the first page, it should show 10, moving the first result from the second page up one, and so on for all the other pages. To summarise, my understanding is that there should be 10 results on the first page of results, assuming that all subsequent pages have 10 or more results on them, even if one of the results for the first page is suppressed. Is anyone else seeing this? Would this require a new bug to be opened?
Hi Holly, when using OpacHiddenItems the results are filtered "after" paging, is what I was told, so it leaves holes in the result lists like you see. That cannot be changed easily it seems, which is why it was chosen to remove the numbering of results when OpacHiddenItems is used to hide this a bit. See also: bug 23923 and bug 18989
Wanted to add: it's not related to this bug or feature. And a tip: When the bug you think is related has already been closed/released, it's always better to open a new one with any findings and link them using depends or see also. Any patches/fixes will need a new bug number.