Bug 29990 introduced a feature to show appropriate heading use in authority search results. This should be available for Elasticsearch as well.
Created attachment 148791 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS
Created attachment 150440 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS
With this patch applied and ShowHeadingUse set to 'Allow' I notice that unit tests are failing: kohadev-koha@kohadevbox:koha(master)$ prove -v /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t .. 1..2 Name "Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX" used only once: possible typo at /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t line 55. ok 1 - use Koha::SearchEngine::Elasticsearch::Search; # Subtest: search_auth_compat 1..2 ok 1 - Creating a new Search object # Looks like you planned 2 tests but ran 1. not ok 2 - search_auth_compat # Failed test 'search_auth_compat' # at /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t line 63. Can't call method "data" on an undefined value at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 252. # Looks like your test exited with 255 just after 2. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 1/2 subtests
Created attachment 150479 [details] [review] Bug 33348: (follow-up) Fix if no 008 defined And copy fix across to Zebra search as well.
Created attachment 150481 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 150482 [details] [review] Bug 33348: (follow-up) Fix if no 008 defined And copy fix across to Zebra search as well. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 152570 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 152571 [details] [review] Bug 33348: (follow-up) Fix if no 008 defined And copy fix across to Zebra search as well. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Missing unit tests, failing QA for now.
Created attachment 159066 [details] [review] Bug 33348: Tests prove Koha/SearchEngine/Elasticsearch/Search.t
*** Bug 36069 has been marked as a duplicate of this bug. ***
From my testing: 1. The tests fail after the patches are applied - see [4]. 2. Authority finder plugin when editing a record and trying to find an authority term (for example for a 650 entry): does not display any results, so can't confirm that it works. May be affected by the CSRF changes - see step 6 of my testing notes. Testing notes using KTD: 1. Start up KTD with Elasticsearch: ktd --es8 up 2. Apply the patches, update the database (updatedatabase), and restart services (restart_all). 3. Search system preferences for ShowHeadingUse - it should be set to 'Don't show'. 4. Do an authority search, for example: perl 5. Confirm it works as expected and no new information is shown in the search results, as ShowHeadingUse is disabled. [1] 6. Search for and edit a bibliographic record (for example: Programming perl, record = 146): . Go to one of the authority fields (1xx, 6xx or 7xx) (for example: 650) . Repeat the tag (such as 650) . Launch the authority finder plugin . Do an authority search and confirm it works as expected and no new information is shown - see [3], not currently working in master - this may be because of the CSRF changes (bugs 34478 and 36192) 7. Set the ShowHeadingUse system preference to 'Show'. 8. Repeat steps 4 to 6 and confirm that the 'Heading use' column now shows, and correctly displays what the heading is used for based on 008/14-16. [2] 9. Edit tag 008 for a record (for example: Perl (Computer language), authid=1525): . change 14-16 to '|- No attempt to code' . in the authority search for that term, these entries are displayed in the heading use column: x Main/Added Entry x Subject x Series Title 10. Tests should pass: prove t/Koha/SearchEngine/Elasticsearch/Search.t 11. Bonus test: Test steps 7-9 using the Zebra search engine. 12. Bonus test: Test with Elasticsearch before the patches are applied. The heading use column is displayed, but the values are not correctly displayed - that is, they all display as follows irrespective of the values in 008/14-16: x Main/Added Entry x Subject x Series Title [1] What is shown for authority search results with ShowHeadingUse set to 'Don't show': Standard search results are shown for an authority search with details listed under these columns: . Summary: lists terms . Heading type: lists the type of term, such as 'Topical Term' . Used: shows how many records the term is used in, for example '1 record(s), with a link to the record(s) . Action column: Actions button - dropdown displays actions available [2] What is shown for authority search results with ShowHeadingUse set to 'Show': Same as in [1], with an extra column before the actions column: . Heading use: lists what the heading can be used for, for example for Perl (Computer language): ✔ Main/Added Entry ✔ Subject x Series Title [3] Expected the authority finder plug to show standard authority search results, similar to [1]. However, on the current master no search results are displayed. Maybe this is [4] Tests fail for me after the patches are applied (they passed before they were applied): prove t/Koha/SearchEngine/Elasticsearch/Search.t t/Koha/SearchEngine/Elasticsearch/Search.t .. 1/2 # Failed test 'Valid main heading with ShowHeadingUse' # at t/Koha/SearchEngine/Elasticsearch/Search.t line 95. # got: undef # expected: '1' # Failed test 'Valid main heading with ShowHeadingUse' # at t/Koha/SearchEngine/Elasticsearch/Search.t line 98. # got: undef # expected: '1' # Looks like you failed 2 tests of 7. t/Koha/SearchEngine/Elasticsearch/Search.t .. 2/2 # Failed test 'search_auth_compat' # at t/Koha/SearchEngine/Elasticsearch/Search.t line 99. # Looks like you failed 1 test of 2. t/Koha/SearchEngine/Elasticsearch/Search.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/2 subtests Test Summary Report ------------------- t/Koha/SearchEngine/Elasticsearch/Search.t (Wstat: 256 Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 1 Files=1, Tests=2, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.85 cusr 0.12 csys = 0.99 CPU) Result: FAIL
(In reply to David Nind from comment #12) > 2. Authority finder plugin when editing a record and trying to find an > authority term (for example for a 650 entry): does not display any results, > so can't confirm that it works. May be affected by the CSRF changes - see > step 6 of my testing notes. I can confirm that there is a problem with displaying authorities search result from within plugin--but it dos not seem to be related with the patch. This issue occurs on clean master. > Test Summary Report > ------------------- > t/Koha/SearchEngine/Elasticsearch/Search.t (Wstat: 256 Tests: 2 Failed: 1) > Failed test: 2 > Non-zero exit status: 1 > Files=1, Tests=2, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.85 cusr 0.12 > csys = 0.99 CPU) > Result: FAIL As to the test: Aleisha, there are two problems in the test script: 1) 'aba' is placed on wrong position in the 008 field; 2) the MARC record should be base64-encoded while simulating ES search result. My correction proposal would be: @@ -20,6 +20,8 @@ use Modern::Perl; use Test::More tests => 2; use Test::MockModule; use t::lib::Mocks; +use Encode qw( encode ); +use MIME::Base64 qw( encode_base64 ); use_ok('Koha::SearchEngine::Elasticsearch::Search'); @@ -67,9 +69,10 @@ subtest 'search_auth_compat' => sub { ); $marc_record->append_fields( MARC::Field->new( - '008', '100803t2009 aba||||| ||| 00| 0 d' + '008', '950121n||a| nnaban |a aaa |d' ), ); + my $marc_data = encode_base64(encode('UTF-8', $marc_record->as_usmarc())); return { hits => { hits => [ @@ -77,7 +80,7 @@ subtest 'search_auth_compat' => sub { '_id' => 8675309, '_source' => { 'local-number' => ['Wrong001Number'], - 'marc_data' => $marc_record, + 'marc_data' => $marc_data, 'marc_format' => 'base64ISO2709', }, }
Created attachment 171601 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 171602 [details] [review] Bug 33348: (follow-up) Fix if no 008 defined And copy fix across to Zebra search as well. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 171603 [details] [review] Bug 33348: Tests prove Koha/SearchEngine/Elasticsearch/Search.t
Since there was no action from Aleisha's side for almost six months, I have made corrections to the unit test (as described in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33348#c13) and also some tidiness reformatting according to qa scripts, and have resubmitted the patches. The problems with the plugin mentioned by David were not related to the patch itself and are not currently occurring.
(In reply to Janusz Kaczmarek from comment #17) > Since there was no action from Aleisha's side for almost six months, I have > made corrections to the unit test (as described in > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33348#c13) and > also some tidiness reformatting according to qa scripts, and have > resubmitted the patches. > > The problems with the plugin mentioned by David were not related to the > patch itself and are not currently occurring. Thanks for the follow-up. This fell down my priority list.
Created attachment 171686 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Created attachment 171687 [details] [review] Bug 33348: (follow-up) Fix if no 008 defined And copy fix across to Zebra search as well. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Created attachment 171688 [details] [review] Bug 33348: Tests prove Koha/SearchEngine/Elasticsearch/Search.t Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Tests are failing for me with Elasticsearch Version: 7.17.20 with ShowHeadingUse set to 'Don't show': # Failed test 'Valid main heading with ShowHeadingUse' # at /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t line 96. # got: undef # expected: '1' # Failed test 'Valid main heading with ShowHeadingUse' # at /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t line 101. # got: undef # expected: '1' # Looks like you failed 2 tests of 7. # Failed test 'search_auth_compat' # at /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t line 102. # Looks like you failed 1 test of 2. /kohadevbox/koha/t/Koha/SearchEngine/Elasticsearch/Search.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/2 subtests
Created attachment 171875 [details] [review] Bug 33348: Tests prove t/Koha/SearchEngine/Elasticsearch/Search.t
(In reply to Lucas Gass from comment #22) > Tests are failing for me with Elasticsearch Version: 7.17.20 with > ShowHeadingUse set to 'Don't show': Thanks, Lucas--mock_preference was originally misplaced.
Created attachment 171876 [details] [review] Bug 33348: Tests prove t/Koha/SearchEngine/Elasticsearch/Search.t Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Created attachment 173987 [details] [review] Bug 33348: Show authority heading use with Elasticsearch This patch ensures the ShowHeadingUse system preference and feature works as expected when using Elasticsearch as the searchengine. To test, follow the test plan at Bug 29990 Comment 9. Ensure the SearchEngine system preference is set to Elasticsearch. Sponsored-by: Education Services Australia SCIS Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Thomas Klausner <domm@plix.at>
Created attachment 173988 [details] [review] Bug 33348: (follow-up) Fix if no 008 defined And copy fix across to Zebra search as well. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Thomas Klausner <domm@plix.at>
Created attachment 173989 [details] [review] Bug 33348: Tests prove t/Koha/SearchEngine/Elasticsearch/Search.t Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Thomas Klausner <domm@plix.at>
I followed the excellent test plan provided by David Nind in Comment 12, everything works (showing Heading Use in auth search and auth finder; unit test also works; After changing 008/14-16 the changes are reflected in the search result) Nice work!
Pushed for 24.11! Well done everyone, thank you!