When Elasticsearch is enable the authorities search from cataloging is using Zebra, we need to use the compat functions to select the correct engine
Created attachment 68984 [details] [review] Bug 19582 - Use compat routines for searching authorities in auth_finder.pl To test: 1 - Enable Zebra 2 - Perform an auth search 3 - note results 4 - Enable ES 5 - Repeat search, note (likely) diff results 6 - Open a record in cataloging and use the button to launch auth search 7 - Perform same search as above, note results match for eiher engine selected 8 - NOTE: Disbale sorting for ES search - this will be dealt with in another report
Created attachment 68992 [details] [review] Bug 19582 - Use compat routines for searching authorities in auth_finder.pl To test: 1 - Enable Zebra 2 - Perform an auth search 3 - note results 4 - Enable ES 5 - Repeat search, note (likely) diff results 6 - Open a record in cataloging and use the button to launch auth search 7 - Perform same search as above, note results match for eiher engine selected 8 - NOTE: Disable sorting for ES search - this will be dealt with in another report
Created attachment 68993 [details] [review] Bug 19582: Unit tests
Created attachment 69031 [details] [review] Bug 19582 - Use compat routines for searching authorities in auth_finder.pl To test: 1 - Enable Zebra 2 - Perform an auth search 3 - note results 4 - Enable ES 5 - Repeat search, note (likely) diff results 6 - Open a record in cataloging and use the button to launch auth search 7 - Perform same search as above, note results match for eiher engine selected 8 - NOTE: Disbale sorting for ES search - this will be dealt with in another report Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69032 [details] [review] Bug 19582: Unit tests Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Followed test plan, but it took me a long time you meant the auth search in the record editor... Before patch the search would actually straight-up fail for me, with patch I got the same results as a normal auth search. QA tools and test passed green.
Hello Nick, I have nothing to say from a QA point of view about the current changes in the code but i'm wondering if we should not do the same work for autocomplete feature (it still use C4::AuthoritiesMarc::SearchAuthorities) or do it in another BZ.
(In reply to Alex Arnaud from comment #7) > Hello Nick, > > I have nothing to say from a QA point of view about the current changes in > the code but i'm wondering if we should not do the same work for > autocomplete feature (it still use C4::AuthoritiesMarc::SearchAuthorities) > or do it in another BZ. Yes of course, every occurrence of SearchAuthorities must be replaced. Please open a new bug report and put on QA stamp on this one ;)
Hi Alex, can you please take another look here? Thx!
Hello Nick, Searching on entire record doesn't work for me. You may replace all occurences of "value_any" by "value_all" ?
Also, i think that "C4::AuthoritiesMarc;" is no longer needed
I'm not sure for value_any by value_all. It could be an issue related to my unimarc authorities mapping. Can anyone using marc21 confirm this issue?
Created attachment 72717 [details] [review] Bug 19582: (follow-up) get params in correct order We were ordering the values out of sync with the order of the mathclist param and so causing 'entire record' and 'all headings' to be reversed
Created attachment 72782 [details] [review] Bug 19582 - Use compat routines for searching authorities in auth_finder.pl To test: 1 - Enable Zebra 2 - Perform an auth search 3 - note results 4 - Enable ES 5 - Repeat search, note (likely) diff results 6 - Open a record in cataloging and use the button to launch auth search 7 - Perform same search as above, note results match for eiher engine selected 8 - NOTE: Disbale sorting for ES search - this will be dealt with in another report Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Current status: Signed Off
Created attachment 72783 [details] [review] Bug 19582: Unit tests Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Created attachment 72784 [details] [review] Bug 19582: (follow-up) get params in correct order We were ordering the values out of sync with the order of the mathclist param and so causing 'entire record' and 'all headings' to be reversed Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
with unimarc setting and mapping from 17373 $ perl t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t 1..2 # Subtest: build_authorities_query_compat() tests ok 1 - Creating new query builder object for authorities ok 2 ok 3 ok 4 ok 5 ok 6 ok 7 ok 8 ok 9 - Exception thrown on invalid value in the marclist param 1..9 ok 1 - build_authorities_query_compat() tests # Subtest: build query from form subtests 1..5 ok 1 - Expected search is populated ok 2 - Only defined search is populated ok 3 - First index searched as expected ok 4 - Second index searched when populated ok 5 - Only defined searches are populated ok 2 - build query from form subtests
Created attachment 72792 [details] [review] Bug 19582 - remove use of C4::AuthoritiesMarc
Created attachment 72793 [details] [review] Bug 19582 - Use compat routines for searching authorities in auth_finder.pl To test: 1 - Enable Zebra 2 - Perform an auth search 3 - note results 4 - Enable ES 5 - Repeat search, note (likely) diff results 6 - Open a record in cataloging and use the button to launch auth search 7 - Perform same search as above, note results match for eiher engine selected 8 - NOTE: Disbale sorting for ES search - this will be dealt with in another report Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Created attachment 72794 [details] [review] Bug 19582: Unit tests Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Created attachment 72795 [details] [review] Bug 19582: (follow-up) get params in correct order We were ordering the values out of sync with the order of the mathclist param and so causing 'entire record' and 'all headings' to be reversed Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Created attachment 72796 [details] [review] Bug 19582 - remove use of C4::AuthoritiesMarc Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Please take care of commit messages and author: Author: Koha User <support@biblibre.com> Bug 19582 - remove use of C4::AuthoritiesMarc Bug 19582 - Use compat routines for searching authorities in auth_finder.pl
Nick, please care about spaces and alignment: In your patch it looks like: - $query->param('value_any') || undef, - $query->param('value_match') || undef + $query->param('value_match') || undef, + $query->param('value_any') || undef With correct alignement: + $query->param('value_match') || undef, $query->param('value_any') || undef, - $query->param('value_match') || undef Much easier to read, right? Will be done on pushing.
Pushed to master for 18.05, thanks to everybody involved!
And I messed up with the bug number when pushing the last patch... Bug 19583: Remove use of C4::AuthoritiesMarc should have been Bug 19582: Remove use of C4::AuthoritiesMarc
Small(ish) change, considered as bug Pushed to stable for 17.11.06 Awesome work all!
Enhancement not pushed to 17.05.x Viva ElasticSearch ;)
Oups in GIT last commit message has the wrong bug id : Bug 19583 http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=f60bff7aa0e3ca932f5572b7837d912b8681ca2a