Bug 33348 - Show authority heading use with Elasticsearch
Summary: Show authority heading use with Elasticsearch
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact:
URL:
Keywords:
: 36069 (view as bug list)
Depends on: 29990
Blocks:
  Show dependency treegraph
 
Reported: 2023-03-28 02:11 UTC by Aleisha Amohia
Modified: 2024-04-04 13:38 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 33348: Show authority heading use with Elasticsearch (1.85 KB, patch)
2023-03-28 02:13 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33348: Show authority heading use with Elasticsearch (1.79 KB, patch)
2023-04-30 22:33 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33348: (follow-up) Fix if no 008 defined (3.14 KB, patch)
2023-05-01 22:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33348: Show authority heading use with Elasticsearch (1.84 KB, patch)
2023-05-01 22:49 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33348: (follow-up) Fix if no 008 defined (3.19 KB, patch)
2023-05-01 22:49 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33348: Show authority heading use with Elasticsearch (1.90 KB, patch)
2023-06-22 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33348: (follow-up) Fix if no 008 defined (3.25 KB, patch)
2023-06-22 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33348: Tests (1.81 KB, patch)
2023-11-16 21:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2023-03-28 02:11:03 UTC
Bug 29990 introduced a feature to show appropriate heading use in authority search results.

This should be available for Elasticsearch as well.
Comment 1 Aleisha Amohia 2023-03-28 02:13:33 UTC
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
Comment 2 Aleisha Amohia 2023-04-30 22:33:07 UTC
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
Comment 3 Lucas Gass 2023-05-01 17:51:59 UTC
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
Comment 4 Aleisha Amohia 2023-05-01 22:39:47 UTC
Created attachment 150479 [details] [review]
Bug 33348: (follow-up) Fix if no 008 defined

And copy fix across to Zebra search as well.
Comment 5 Lucas Gass 2023-05-01 22:49:31 UTC
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>
Comment 6 Lucas Gass 2023-05-01 22:49:33 UTC
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>
Comment 7 Martin Renvoize 2023-06-22 15:31:35 UTC
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>
Comment 8 Martin Renvoize 2023-06-22 15:31:38 UTC
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>
Comment 9 Martin Renvoize 2023-06-22 15:31:54 UTC
Missing unit tests, failing QA for now.
Comment 10 Aleisha Amohia 2023-11-16 21:39:14 UTC
Created attachment 159066 [details] [review]
Bug 33348: Tests

prove Koha/SearchEngine/Elasticsearch/Search.t
Comment 11 Phil Ringnalda 2024-02-13 15:49:14 UTC
*** Bug 36069 has been marked as a duplicate of this bug. ***
Comment 12 David Nind 2024-03-23 07:08:43 UTC
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
Comment 13 Janusz Kaczmarek 2024-04-04 13:38:11 UTC
(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',
                             },
                         }