Bug 23322

Summary: Elasticsearch - Record matching fails when multiple keys exist
Product: Koha Reporter: Nick Clemens <nick>
Component: Searching - ElasticsearchAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: major    
Priority: P5 - low CC: fridolin.somers, glasklas, johanna.raisa, lucas, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22545
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.03
Bug Depends on:    
Bug Blocks: 23324    
Attachments: Test record
Bug 23322: Correct case for boolean in ES search
Bug 23322: Correct case for boolean in ES search
Bug 23322: Correct case for boolean in ES search

Description Nick Clemens 2019-07-16 12:08:19 UTC
For a record with multipel matchpoints C4/Matcher.pm combines terms using 'or' however, ES expects 'OR' so the search fails

To test:
 0 - Set SearchEngine to ElasticSearch
 1 - Import the example record (a version is included in sample data in devbox/testing docker so you can skip)
 2 - Stage the attached example record
 3 - Match using 'ISBN' rule
 4 - No matches found
Comment 1 Nick Clemens 2019-07-16 12:08:39 UTC
Created attachment 91547 [details]
Test record
Comment 2 Nick Clemens 2019-07-16 12:11:27 UTC
Created attachment 91548 [details] [review]
Bug 23322: Correct case for boolean in ES search

I did not include tests only because this is a very small reasonable change and routine has no tests.
It does not affect behaviour, it only touches syntax lightly.

To test:
  0 - Set SearchEngine to ElasticSearch
  1 - Import the example record (a version is included in sample data in devbox/testing docker so you can skip)
  2 - Stage the attached example record
  3 - Match using 'ISBN' rule
  4 - No matches found
  5 - Apply patch
  6 - Restart all the things
  7 - Reapply matching with no rule
  8 - Reapply with ISBN matcing
  9 - It matches!
 10 - Set SearchEngine to Zebra
 11 - Reapply matching with no rule
 12 - Reapply with ISBN matching
 13 - Matching works as before in Zebra
Comment 3 Nick Clemens 2019-07-16 16:38:05 UTC
Created attachment 91556 [details] [review]
Bug 23322: Correct case for boolean in ES search

I did not include tests only because this is a very small reasonable change and routine has no tests.
It does not affect behaviour, it only touches syntax lightly.

To test:
  0 - Set SearchEngine to ElasticSearch
  1 - Import the example record (a version is included in sample data in devbox/testing docker so you can skip)
  2 - Stage the attached example record
  3 - Match using 'ISBN' rule
  4 - No matches found
  5 - Apply patch
  6 - Restart all the things
  7 - Reapply matching with no rule
  8 - Reapply with ISBN matcing
  9 - It matches!
 10 - Set SearchEngine to Zebra
 11 - Reapply matching with no rule
 12 - Reapply with ISBN matching
 13 - Matching works as before in Zebra

Signed-off-by: Ron Houk <rhouk@ottumwapubliclibrary.org>
Comment 4 Martin Renvoize 2019-08-08 11:48:02 UTC
Created attachment 92067 [details] [review]
Bug 23322: Correct case for boolean in ES search

I did not include tests only because this is a very small reasonable change and routine has no tests.
It does not affect behaviour, it only touches syntax lightly.

To test:
  0 - Set SearchEngine to ElasticSearch
  1 - Import the example record (a version is included in sample data in devbox/testing docker so you can skip)
  2 - Stage the attached example record
  3 - Match using 'ISBN' rule
  4 - No matches found
  5 - Apply patch
  6 - Restart all the things
  7 - Reapply matching with no rule
  8 - Reapply with ISBN matcing
  9 - It matches!
 10 - Set SearchEngine to Zebra
 11 - Reapply matching with no rule
 12 - Reapply with ISBN matching
 13 - Matching works as before in Zebra

Signed-off-by: Ron Houk <rhouk@ottumwapubliclibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2019-08-08 11:48:41 UTC
Trivial patch, works as expected.. would have been nice to have a unit test for it.. but as it's ES dependant I'm not going to fail it for that.

Passing QA
Comment 6 Martin Renvoize 2019-08-08 12:07:05 UTC
Nice work!

Pushed to master for 19.11.00
Comment 7 Fridolin Somers 2019-08-09 14:05:29 UTC
Pushed to 19.05.x for 19.05.03
Comment 8 Lucas Gass 2019-08-28 14:41:44 UTC
backported to 18.11.x for 18.11.09
Comment 9 Katrin Fischer 2019-10-14 22:03:15 UTC
*** Bug 22545 has been marked as a duplicate of this bug. ***
Comment 10 David Gustafsson 2020-05-28 11:15:46 UTC
*** Bug 19197 has been marked as a duplicate of this bug. ***