Bug 26957 - Find duplicate removes operators from the middle of search terms
Summary: Find duplicate removes operators from the middle of search terms
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-06 16:13 UTC by Nick Clemens
Modified: 2021-02-11 22:11 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.02,20.05.09


Attachments
Bug 26957: Unit tests (2.81 KB, patch)
2020-11-23 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26957: Don't remove words that are also operators from search terms (1.81 KB, patch)
2020-11-23 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26957: Don't remove words that are also operators from search terms (1.93 KB, patch)
2020-11-23 18:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Test records (2.78 KB, text/plain)
2020-11-23 18:45 UTC, Nick Clemens
Details
Bug 26957: Don't remove words that are also operators from search terms (1.99 KB, patch)
2020-12-29 10:43 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26957: Unit tests (2.87 KB, patch)
2021-01-12 12:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26957: Don't remove words that are also operators from search terms (2.06 KB, patch)
2021-01-12 12:18 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-11-06 16:13:45 UTC
To test:
1 - Edit your koha-conf.xml and set zebra debug levels to include request
    <zebra_loglevels>none,fatal,warn,request</zebra_loglevels>
2 - Restart all the things
3 - Tail the zebra output log
    tail -f /var/log/koha/kohadev/zebra-output.log
4 - Try to create a new purchase suggestion for the author: Morgenstern
5 - Note the logged search is for Mgenstern
Comment 1 Nick Clemens 2020-11-23 18:44:08 UTC
Created attachment 113933 [details] [review]
Bug 26957: Unit tests
Comment 2 Nick Clemens 2020-11-23 18:44:10 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2020-11-23 18:45:23 UTC
Created attachment 113935 [details] [review]
Bug 26957: Don't remove words that are also operators from search terms

FindDuplicate has a hardcoded regex to remove 'and', 'or', and 'not' from any search fields, however,
is does this with no regard for these terms being embedded in the middle of a word or phrase.

E.g.:
'Coupland' becomes 'Coupl'
'Frog and toad' becomes 'frog toad'
etc.

This patch simplay removes the substitutions as we already quote the terms as well

To test:
1 - Add records to your system like:
    The night circus by Erin Morgenstern
    Frog and toad all year
    Nothing ever happens
2 - Attempt to add purchase suggestions like
    title: the night circus, author:Morgenstern, Erin
    title: For and toad all year
    title:nothing ever happens (case is important)
3 - No warnings about existing biblios
4 - Apply patches
5 - Delete existing suggestions
6 - Repeat step 2
7 - Each attempt should warn you of existing biblio
Comment 4 Nick Clemens 2020-11-23 18:45:51 UTC
Created attachment 113936 [details]
Test records
Comment 5 Fridolin Somers 2020-12-29 10:43:50 UTC
Created attachment 114716 [details] [review]
Bug 26957: Don't remove words that are also operators from search terms

FindDuplicate has a hardcoded regex to remove 'and', 'or', and 'not' from any search fields, however,
is does this with no regard for these terms being embedded in the middle of a word or phrase.

E.g.:
'Coupland' becomes 'Coupl'
'Frog and toad' becomes 'frog toad'
etc.

This patch simplay removes the substitutions as we already quote the terms as well

To test:
1 - Add records to your system like:
    The night circus by Erin Morgenstern
    Frog and toad all year
    Nothing ever happens
2 - Attempt to add purchase suggestions like
    title: the night circus, author:Morgenstern, Erin
    title: For and toad all year
    title:nothing ever happens (case is important)
3 - No warnings about existing biblios
4 - Apply patches
5 - Delete existing suggestions
6 - Repeat step 2
7 - Each attempt should warn you of existing biblio

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 6 Fridolin Somers 2020-12-29 10:46:07 UTC
Whaou what an ugly bug ^^.

I confirm finding duplicate works even with title containing operators like 'and'.
For example 'War and'.
Comment 7 Martin Renvoize 2021-01-12 12:18:46 UTC
Created attachment 115079 [details] [review]
Bug 26957: Unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-01-12 12:18:50 UTC
Created attachment 115080 [details] [review]
Bug 26957: Don't remove words that are also operators from search terms

FindDuplicate has a hardcoded regex to remove 'and', 'or', and 'not' from any search fields, however,
is does this with no regard for these terms being embedded in the middle of a word or phrase.

E.g.:
'Coupland' becomes 'Coupl'
'Frog and toad' becomes 'frog toad'
etc.

This patch simplay removes the substitutions as we already quote the terms as well

To test:
1 - Add records to your system like:
    The night circus by Erin Morgenstern
    Frog and toad all year
    Nothing ever happens
2 - Attempt to add purchase suggestions like
    title: the night circus, author:Morgenstern, Erin
    title: For and toad all year
    title:nothing ever happens (case is important)
3 - No warnings about existing biblios
4 - Apply patches
5 - Delete existing suggestions
6 - Repeat step 2
7 - Each attempt should warn you of existing biblio

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2021-01-12 12:19:26 UTC
What a crazy bug!

Fix works well and no regressions found.

Passing QA
Comment 10 Jonathan Druart 2021-01-12 15:48:16 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 11 Fridolin Somers 2021-01-18 14:32:16 UTC
Pushed to 20.11.x for 20.11.02
Comment 12 Andrew Fuerste-Henry 2021-01-29 14:27:12 UTC
Pushed to 20.05.x for 20.05.09
Comment 13 Victor Grousset/tuxayo 2021-02-01 16:54:55 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.