Bug 23719 - Record matching for authorities using defined fields is broken under ES
Summary: Record matching for authorities using defined fields is broken under ES
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-02 11:17 UTC by Nick Clemens
Modified: 2020-11-30 21:47 UTC (History)
6 users (show)

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


Attachments
Bug 23719: Allow searching specific fields for matching authorities in ES (3.49 KB, patch)
2019-10-02 11:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23719: Allow searching specific fields for matching authorities in ES (3.55 KB, patch)
2019-10-24 14:49 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23719: (follow-up) Add warn when passed invalid search field in marclist (1.43 KB, patch)
2019-10-31 14:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23719: (follow-up) Add warn when passed invalid search field in marclist (5.15 KB, patch)
2019-11-04 12:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23719: Allow searching specific fields for matching authorities in ES (3.62 KB, patch)
2019-11-06 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23719: (follow-up) Add warn when passed invalid search field in marclist (5.22 KB, patch)
2019-11-06 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23719: [19.05.x] Allow searching specific fields for matching authorities in ES (6.72 KB, patch)
2020-02-27 18:57 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-10-02 11:17:00 UTC
build_auth_query_compat for ES accepts only a few marclist parameters. In Zebra we pass through any indexes that we have not mapped. ES should do the same
Comment 1 Nick Clemens 2019-10-02 11:20:20 UTC
Created attachment 93414 [details] [review]
Bug 23719: Allow searching specific fields for matching authorities in ES

To test:
1 - Export your authorities via Tools->Export data
2 - Define a record matching rule in Admin->Record matchign rules
    Use index: LC-card-number
    field: 010$a
3 - Stage the exported records for import and use the rule created above for matching
4 - The process does not complete
5 - Check intranet error logs - exception on unknown marclist
6 - Apply patch
7 - Repeat
8 - Success!
9 - prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
Comment 2 ByWater Sandboxes 2019-10-24 14:49:19 UTC
Created attachment 94680 [details] [review]
Bug 23719: Allow searching specific fields for matching authorities in ES

To test:
1 - Export your authorities via Tools->Export data
2 - Define a record matching rule in Admin->Record matchign rules
    Use index: LC-card-number
    field: 010$a
3 - Stage the exported records for import and use the rule created above for matching
4 - The process does not complete
5 - Check intranet error logs - exception on unknown marclist
6 - Apply patch
7 - Repeat
8 - Success!
9 - prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Andrew Fuerste-Henry 2019-10-24 14:50:11 UTC
Signed off on behalf of Ron Houk, rhouk@ottumwapubliclibrary.org
Comment 4 Ron Houk 2019-10-24 15:51:39 UTC
I followed the test plan, and everything worked as it was supposed to.
Comment 5 Jonathan Druart 2019-10-31 13:21:52 UTC
The current behavior is definitely wrong.

I am wondering if we should not have the error displayed somewhere, if the search field is not defined in the ES mapping.

With Zebra we get:
  indexoAuth error: Unsupported Use attribute (114) pouet Bib-1

With ES (and this patch), we do not get anything.

Easier to test this patch with:

Existing search field:
http://pro.kohadev.org/cgi-bin/koha/authorities/authorities-home.pl?op=do_search&type=intranet&marclist=Personal-name&and_or=and&operator=contains&value=d&orderby=HeadingAsc

non-existing search field:
http://pro.kohadev.org/cgi-bin/koha/authorities/authorities-home.pl?op=do_search&type=intranet&marclist=pouet&and_or=and&operator=contains&value=d&orderby=HeadingAsc
Comment 6 Nick Clemens 2019-10-31 14:35:18 UTC
Created attachment 94913 [details] [review]
Bug 23719: (follow-up) Add warn when passed invalid search field in marclist
Comment 7 Martin Renvoize 2019-11-01 10:34:58 UTC
Does the extra patch here resolve your worries Jonathan?
Comment 8 Jonathan Druart 2019-11-04 09:54:32 UTC
(In reply to Martin Renvoize from comment #7)
> Does the extra patch here resolve your worries Jonathan?

I do not think so:

Unknown search field personal-name in marclist at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 478.

http://pro.kohadev.org/cgi-bin/koha/authorities/authorities-home.pl?op=do_search&type=intranet&marclist=Personal-name&and_or=and&operator=contains&value=d&orderby=HeadingAsc
Comment 9 Nick Clemens 2019-11-04 12:33:53 UTC
Created attachment 95001 [details] [review]
Bug 23719: (follow-up) Add warn when passed invalid search field in marclist

Note: I also remove warnings for undefined operation in this patch, is a trivial fix

To test:
prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
Comment 10 Nick Clemens 2019-11-04 12:35:03 UTC
(In reply to Nick Clemens from comment #6)
> Created attachment 94913 [details] [review] [review]
> Bug 23719: (follow-up) Add warn when passed invalid search field in marclist

Heh, helps if you get the order of the hashes correct.
Comment 11 Jonathan Druart 2019-11-06 09:31:36 UTC
Created attachment 95090 [details] [review]
Bug 23719: Allow searching specific fields for matching authorities in ES

To test:
1 - Export your authorities via Tools->Export data
2 - Define a record matching rule in Admin->Record matchign rules
    Use index: LC-card-number
    field: 010$a
3 - Stage the exported records for import and use the rule created above for matching
4 - The process does not complete
5 - Check intranet error logs - exception on unknown marclist
6 - Apply patch
7 - Repeat
8 - Success!
9 - prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2019-11-06 09:31:40 UTC
Created attachment 95091 [details] [review]
Bug 23719: (follow-up) Add warn when passed invalid search field in marclist

Note: I also remove warnings for undefined operation in this patch, is a trivial fix

To test:
prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Martin Renvoize 2019-11-06 17:10:25 UTC
Nice work!

Pushed to master for 19.11.00
Comment 14 Nick Clemens 2020-02-27 18:57:01 UTC
Created attachment 99714 [details] [review]
Bug 23719: [19.05.x] Allow searching specific fields for matching authorities in ES

To test:
1 - Export your authorities via Tools->Export data
2 - Define a record matching rule in Admin->Record matchign rules
    Use index: LC-card-number
    field: 010$a
3 - Stage the exported records for import and use the rule created above for matching
4 - The process does not complete
5 - Check intranet error logs - exception on unknown marclist
6 - Apply patch
7 - Repeat
8 - Success!
9 - prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 23719: (follow-up) Add warn when passed invalid search field in marclist

Note: I also remove warnings for undefined operation in this patch, is a trivial fix

To test:
prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Lucas Gass 2020-03-04 22:10:26 UTC
backported to 19.05.x for 19.05.09

thanks for the rebase kidclamp!