we have checked that at staff interface when you try to broswe authorities, you always get the same result list, except one for the last result. we have checked at 18.05.01 and 18.05.03 from our servers and demo servers, and same result. test plan 1. login into staff interface 2. Go to authorities 3. Search full authotities (get more than one page results) 4. go to second page... the same results stills there
Created attachment 79948 [details] [review] Bug 21405: Fix authorities pagination for Zebra and ES This patch corrects a problem introduced by bug 20261 The underlying problem still exists - the two search_auth_compat methods don't expect the same information. Using the 'options' parameter allows us to work around this for ES. Additionally code is added to avoid asking for more results form ES than are returned. To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correctly 11 - Clicking on last page hsould return last results
Nick, bug 19365 includes a fix for search_auth_compat in ES.
I have tested with zebra and it works fine, but I am not able to test on ES.. waitting for other tester
(In reply to Ere Maijala from comment #2) > Nick, bug 19365 includes a fix for search_auth_compat in ES. Indeed - it looks like there is no conflict - this patch fixes it for now and still works after your patch We still need this to restore passing the offset directly and handling 10000+ results - whichever gets pushed first will need to rebase on the other
Created attachment 80157 [details] [review] Bug 21405: (follow-up) C4::Auth is imported twice In authorities/authorities-home.pl C4::Auth is imported twice.
Created attachment 80158 [details] [review] Bug 21405: Fix authorities pagination for Zebra and ES This patch corrects a problem introduced by bug 20261 The underlying problem still exists - the two search_auth_compat methods don't expect the same information. Using the 'options' parameter allows us to work around this for ES. Additionally code is added to avoid asking for more results form ES than are returned. To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correctly 11 - Clicking on last page hsould return last results Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 80159 [details] [review] Bug 21405: (follow-up) C4::Auth is imported twice In authorities/authorities-home.pl C4::Auth is imported twice. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
(In reply to Hugo Agud from comment #3) > I have tested with zebra and it works fine, but I am not able to test on > ES.. waitting for other tester Works fine with ES !
Should there not be a test accompanying this for Koha/SearchEngine/Elasticsearch/Search.pm search_auth_compat to ensure we don't get future regressions?
*** Bug 21527 has been marked as a duplicate of this bug. ***
*** Bug 21636 has been marked as a duplicate of this bug. ***
Hi! I just wanted to chime in that as a cataloger, I want to thank everyone who is working on this bug! This bug is causing me a lot of extra work: I have to do all my authority searching in the public OPAC, then switch back to the staff client to search the authority record(s) in ways that I know they will be retrieved in order to edit, update, etc., or even just look at the MARC record in an efficient way. If I don't take all that time & forget to do all my searching in the public OPAC, then I run the risk of importing duplicate authority records, creating duplicate authority records, etc. My workflow to clean up our headings & authority file is not just impacted, but my workflow for every single title that I catalog is impacted, since every, single authority controlled access point requires searching the authority file in the OPAC...switching to staff client...etc., etc., etc. So my hearty thanks to all of you working to fix this bug--I'm cheering you on and really appreciate your efforts!! --Grateful librarian h2.
Created attachment 82824 [details] [review] Bug 21405 - Fix auhtorities pagination for Zebra and ES Bug 20261 introduced an error because search_auth_compat responded differently for ES and Zebra, now bug 19365 fixed the underlying difference. This patch restores previous code to fix pagination. Additionally we add a fix for 10000+ results in ES and remove a double import of C4::Auth To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correct 11 - Clicking on last page should return last results
(In reply to Martin Renvoize from comment #9) > Should there not be a test accompanying this for > Koha/SearchEngine/Elasticsearch/Search.pm search_auth_compat to ensure we > don't get future regressions? Rebased on top of 19365 - that added some basic tests, I have talked with Ere and we agree we need to work towrds actual search testing, then we can add selenium for this, but for now I think we need to fix this issue asap
I pretty much agree with the above.. perhaps a more concerted effort for increasing search test coverage elsewhere and get this particular bug pushed through.. I'm happy to go with that, and defer to Ere as ES Topic Expert (and of course the RM, Nick)
Created attachment 82830 [details] [review] Bug 21405 - Fix authorities pagination for Zebra and ES Bug 20261 introduced an error because search_auth_compat responded differently for ES and Zebra, now bug 19365 fixed the underlying difference. This patch restores previous code to fix pagination. Additionally we add a fix for 10000+ results in ES and remove a double import of C4::Auth To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correct 11 - Clicking on last page should return last results Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
I took the liberty of removing addition of "use POSIX qw(ceil);" since it wasn't actually used.
Oops, I didn't test the pagination fix properly. It's not working. I get 1822 pages of results and trying to go to the last page causes an error. It seems that $hits_to_paginate is not being used.
Created attachment 82838 [details] [review] Bug 21405 - Fix authorities pagination for Zebra and ES Bug 20261 introduced an error because search_auth_compat responded differently for ES and Zebra, now bug 19365 fixed the underlying difference. This patch restores previous code to fix pagination. Additionally we add a fix for 10000+ results in ES and remove a double import of C4::Auth To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correct 11 - Clicking on last page should return last results
Created attachment 82839 [details] [review] Bug 21405 - Fix authorities pagination for Zebra and ES Bug 20261 introduced an error because search_auth_compat responded differently for ES and Zebra, now bug 19365 fixed the underlying difference. This patch restores previous code to fix pagination. Additionally we add a fix for 10000+ results in ES and remove a double import of C4::Auth To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correct 11 - Clicking on last page should return last results Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Created attachment 82960 [details] [review] Bug 21405 - Fix authorities pagination for Zebra and ES Bug 20261 introduced an error because search_auth_compat responded differently for ES and Zebra, now bug 19365 fixed the underlying difference. This patch restores previous code to fix pagination. Additionally we add a fix for 10000+ results in ES and remove a double import of C4::Auth To test: 1 - Perform an authorities search with 1+ pages using Zebra 2 - Page through results, note you are only offseeting by 1 each time 3 - Perform an authorities search with 10,000+ results in ES 4 - Click on the last page and get an error 5 - Apply patch 6 - Retry Zebra search 7 - Results should paginate correctly 8 - Clicking on last page should return last results 9 - Retry ES results 10 - Results should paginate correct 11 - Clicking on last page should return last results Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Forgot this one, Nick: * Commit title does not start with 'Bug XXXXX: ' - 736809b
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.01
Pushed to 18.05.x for 18.05.07
Depends on Bug 20261 not in 17.11.x