Bug 23086 - Search for collection is broken
Summary: Search for collection is broken
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Zebra (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Alex Arnaud
URL:
Keywords:
: 24575 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-06-10 08:25 UTC by Katrin Fischer
Modified: 2020-09-18 20:33 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 19.11.09, 19.05.15


Attachments
Bug 23086 - Search for collection is broken (1.06 KB, patch)
2019-09-18 14:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23086 - Search for collection is broken (1.92 KB, patch)
2020-07-08 19:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23086 - Search for collection is broken (2.01 KB, patch)
2020-07-08 21:03 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 23086: Search for collection is broken (2.13 KB, patch)
2020-07-13 21:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23086: Unit test (1.26 KB, patch)
2020-07-15 15:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23086: Search for collection is broken (2.13 KB, patch)
2020-07-15 15:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23086: Unit test (1.32 KB, patch)
2020-07-18 16:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23086: Search for collection is broken (2.19 KB, patch)
2020-07-18 16:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23086: Unit test (1.37 KB, patch)
2020-07-24 14:28 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 23086: Search for collection is broken (2.24 KB, patch)
2020-07-24 14:28 UTC, Alex Arnaud
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2019-06-10 08:25:44 UTC
We have seen this issue in a 17.11 installation, but not in a newer 18.11. I believe that Zebra version and using/not using ICU might play a role here.

When looking into it, we were able to fix the issue removing the lines adding quotes to the search in Search.pm:

http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Search.pm;h=4191cbc2de0e8530637cb2027b2a3de8374e0eef;hb=8427b684148944b9306fd4786c75c53527037024#l1722

The quotes appear to be the issue, as a simple search using mc-ccode: without quotes works.

This problem has also been reported on the mailing list:

https://lists.katipo.co.nz/pipermail/koha/2019-June/053081.html

Setting to major as this effects a major feature in the OPAC.
Comment 1 Liz Rea 2019-09-17 17:56:12 UTC
I have just seen this in an 18.11.08 with ICU enabled. Not quoting the queries fixes it.

My thought was to say if syspref ICU is on, don't quote the ccode queries. What do you think? 

Or is there something that needs changing in the ICU chains config?
Comment 2 Kyle M Hall 2019-09-18 14:28:48 UTC
Created attachment 92940 [details] [review]
Bug 23086 - Search for collection is broken
Comment 3 Katrin Fischer 2019-09-24 05:23:51 UTC
Hi Kyle, could you add a bit of a description of the fix and a test plan please? Thx for taking this up!
Comment 4 Katrin Fischer 2020-03-15 18:03:50 UTC
*** Bug 24575 has been marked as a duplicate of this bug. ***
Comment 5 Nick Clemens 2020-07-08 19:11:53 UTC
Created attachment 106664 [details] [review]
Bug 23086 - Search for collection is broken

It appears that we are quoting the ccode values deep in the search code.
Under ICU chains this breaks searching by limits

To recreate
 1 - Setup Koha using Zebra and icuchains
 2 - Add ccode to AdvancedSearchTypes
 3 - In koha-conf.xml set zebra debug level to include request
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 4 - Set some items into different ccodes
 5 - On opac perform a search for:
    ccode:NFIC
 6 - It works
 7 - tail -n 50 /var/log/koha/kohadev/zebra-output.log
 8 - Note search request like:
    Search biblios OK 26 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
 9 - On opac go to advanced search, select Collection, and limit to smae code a s above
10 - No results
11 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 'NFIC'
12 - Apply patch
13 - Restart all
14 - Repeat search by collection limit
15 - Success!
Comment 6 Liz Rea 2020-07-08 21:03:57 UTC
Created attachment 106668 [details] [review]
Bug 23086 - Search for collection is broken

It appears that we are quoting the ccode values deep in the search code.
Under ICU chains this breaks searching by limits

To recreate
 1 - Setup Koha using Zebra and icuchains
 2 - Add ccode to AdvancedSearchTypes
 3 - In koha-conf.xml set zebra debug level to include request
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 4 - Set some items into different ccodes
 5 - On opac perform a search for:
    ccode:NFIC
 6 - It works
 7 - tail -n 50 /var/log/koha/kohadev/zebra-output.log
 8 - Note search request like:
    Search biblios OK 26 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
 9 - On opac go to advanced search, select Collection, and limit to smae code a s above
10 - No results
11 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 'NFIC'
12 - Apply patch
13 - Restart all
14 - Repeat search by collection limit
15 - Success!

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=23806
Comment 7 Katrin Fischer 2020-07-13 21:47:16 UTC
Created attachment 106855 [details] [review]
Bug 23086: Search for collection is broken

It appears that we are quoting the ccode values deep in the search code.
Under ICU chains this breaks searching by limits

To recreate
 1 - Setup Koha using Zebra and icuchains
 2 - Add ccode to AdvancedSearchTypes
 3 - In koha-conf.xml set zebra debug level to include request
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 4 - Set some items into different ccodes
 5 - On opac perform a search for:
    ccode:NFIC
 6 - It works
 7 - tail -n 50 /var/log/koha/kohadev/zebra-output.log
 8 - Note search request like:
    Search biblios OK 26 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
 9 - On opac go to advanced search, select Collection, and limit to smae code a s above
10 - No results
11 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 'NFIC'
12 - Apply patch
13 - Restart all
14 - Repeat search by collection limit
15 - Success!

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=23806

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2020-07-13 21:51:16 UTC
The fix we use in production is a bit more radical than yours :)

-            if ( $k !~ /mc-i(tem)?type/ ) {
-                # in case the mc-ccode value has complicating chars like ()'s inside it we wrap in quotes
-                $this_limit =~ tr/"//d;
-                $this_limit = $k.":'".$v."'";
-            }
Comment 9 Jonathan Druart 2020-07-15 15:12:45 UTC
Can we expect a unit test to cover this change?
Comment 10 Nick Clemens 2020-07-15 15:50:02 UTC
Created attachment 106935 [details] [review]
Bug 23086: Unit test
Comment 11 Nick Clemens 2020-07-15 15:50:04 UTC
Created attachment 106936 [details] [review]
Bug 23086: Search for collection is broken

It appears that we are quoting the ccode values deep in the search code.
Under ICU chains this breaks searching by limits

To recreate
 1 - Setup Koha using Zebra and icuchains
 2 - Add ccode to AdvancedSearchTypes
 3 - In koha-conf.xml set zebra debug level to include request
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 4 - Set some items into different ccodes
 5 - On opac perform a search for:
    ccode:NFIC
 6 - It works
 7 - tail -n 50 /var/log/koha/kohadev/zebra-output.log
 8 - Note search request like:
    Search biblios OK 26 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
 9 - On opac go to advanced search, select Collection, and limit to smae code a s above
10 - No results
11 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 'NFIC'
12 - Apply patch
13 - Restart all
14 - Repeat search by collection limit
15 - Success!
16 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
17 - Add a new ccode value: N)N
18 - Set some items to that ccode
19 - Confirm searching by that ccode works
Comment 12 Nick Clemens 2020-07-15 15:51:14 UTC
(In reply to Jonathan Druart from comment #9)
> Can we expect a unit test to cover this change?

Added and updated patch

Previous version didn't handle bad characters in the ccode well, using double quotes seems to work
Comment 13 Katrin Fischer 2020-07-18 16:07:01 UTC
Created attachment 107055 [details] [review]
Bug 23086: Unit test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2020-07-18 16:07:05 UTC
Created attachment 107056 [details] [review]
Bug 23086: Search for collection is broken

It appears that we are quoting the ccode values deep in the search code.
Under ICU chains this breaks searching by limits

To recreate
 1 - Setup Koha using Zebra and icuchains
 2 - Add ccode to AdvancedSearchTypes
 3 - In koha-conf.xml set zebra debug level to include request
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 4 - Set some items into different ccodes
 5 - On opac perform a search for:
    ccode:NFIC
 6 - It works
 7 - tail -n 50 /var/log/koha/kohadev/zebra-output.log
 8 - Note search request like:
    Search biblios OK 26 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
 9 - On opac go to advanced search, select Collection, and limit to smae code a s above
10 - No results
11 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 'NFIC'
12 - Apply patch
13 - Restart all
14 - Repeat search by collection limit
15 - Success!
16 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
17 - Add a new ccode value: N)N
18 - Set some items to that ccode
19 - Confirm searching by that ccode works

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Alex Arnaud 2020-07-24 14:28:18 UTC
Created attachment 107355 [details] [review]
Bug 23086: Unit test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 16 Alex Arnaud 2020-07-24 14:28:23 UTC
Created attachment 107356 [details] [review]
Bug 23086: Search for collection is broken

It appears that we are quoting the ccode values deep in the search code.
Under ICU chains this breaks searching by limits

To recreate
 1 - Setup Koha using Zebra and icuchains
 2 - Add ccode to AdvancedSearchTypes
 3 - In koha-conf.xml set zebra debug level to include request
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 4 - Set some items into different ccodes
 5 - On opac perform a search for:
    ccode:NFIC
 6 - It works
 7 - tail -n 50 /var/log/koha/kohadev/zebra-output.log
 8 - Note search request like:
    Search biblios OK 26 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
 9 - On opac go to advanced search, select Collection, and limit to smae code a s above
10 - No results
11 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 'NFIC'
12 - Apply patch
13 - Restart all
14 - Repeat search by collection limit
15 - Success!
16 - Check the zebra-output.log:
    Search biblios OK 0 1 1+0 RPN @attrset Bib-1 @attr 1=8009 NFIC
17 - Add a new ccode value: N)N
18 - Set some items to that ccode
19 - Confirm searching by that ccode works

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 17 Alex Arnaud 2020-07-24 14:34:08 UTC
I have to say that TUs on t/db_dependent/Search.t fail on master with Zebra set up with ICU.

So i launched the new TU after setting back Zebra to CHR:
New test without the code failed: OK
New test with the code passed: OK

Passed QA
Comment 18 Jonathan Druart 2020-07-27 15:25:50 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 19 Lucas Gass 2020-07-31 21:06:36 UTC
backported to 20.05.x for 20.05.03
Comment 20 Aleisha Amohia 2020-08-03 23:51:29 UTC
backported to 19.11.x for 19.11.09
Comment 21 Victor Grousset/tuxayo 2020-09-18 20:33:49 UTC
Backported to 19.05.x branch for 19.05.15