Bug 30528

Summary: Limits are not correctly parsed when query contains CCL
Product: Koha Reporter: Nick Clemens <nick>
Component: Searching - ZebraAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Joonas Kylmälä <joonas.kylmala>
Severity: normal    
Priority: P5 - low CC: joonas.kylmala, lucas, magnus, martin.renvoize, victor
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17170
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5274
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.01
Attachments: Bug 30528: Unit tests
Bug 30528: Process limits before handling CCL query
Bug 30528: Unit tests
Bug 30528: Process limits before handling CCL query
Bug 30528: Unit tests
Bug 30528: Process limits before handling CCL query

Description Nick Clemens 2022-04-13 14:53:19 UTC
This was made more apparent by bug 17170 - when a special limit that needs parsing is passed along witha query containing CCL indexes then the limits are not parsed (with exception of 'available' which is handled)

This is due to a clause the prevents the normal parsing in C4::Search::buildQuery

To test:
1 - Perform advanced search in staff client
2 - Select 'More options'
3 - In first box with keyword as index type 'ti:a'
4 - Enter a year into Limits
5 - Search
6 - You see only 'ti:a' in description, and search fails

with bug 17170
1 - Save a search filter visible in staff client
2 - Perform a search like 'ti:a'
3 - Attempt to filter using custom filter
4 - In plack intranet error log:
[2022/04/13 14:14:51] [WARN] WARNING: query problem with ti=a and search_filter=94 ZOOM error 10014 "CCL parsing error" (addinfo: "Unknown qualifier") from diag-set 'ZOOM' at /kohadevbox/koha/C4/Search.pm line 352.
Comment 1 Nick Clemens 2022-04-14 12:23:19 UTC
Created attachment 133306 [details] [review]
Bug 30528: Unit tests
Comment 2 Nick Clemens 2022-04-14 12:23:22 UTC
Created attachment 133307 [details] [review]
Bug 30528: Process limits before handling CCL query

This patch moves the code that handles limit processing before that for CCL queries to ensure that special limit cases (branch, multibranch, etc) are handled before adding the limits to the query string

To test:
1 - Apply unit tests patch only
2 - prove -v t/db_dependent/Search.t
3 - It fails
4 - Apply this patch
5 - It passes!
6 - Test searching and confirm things work as epxected
Comment 3 Magnus Enger 2022-04-29 07:33:37 UTC
*** Bug 30527 has been marked as a duplicate of this bug. ***
Comment 4 Magnus Enger 2022-04-29 07:38:48 UTC
I'm seeing this on 21.11.04, when looking at a detail view in the intranet: 

==> /var/log/koha/hig/plack-error.log <==
CCL parsing error (10014) Unknown qualifier ZOOM for query: Host-item=(Psychology and law =) at /usr/share/koha/lib/C4/Search.pm line 242.
Warning from simple_search_compat: 'CCL parsing error (10014) Unknown qualifier ZOOM' at /usr/share/koha/lib/Koha/Biblio.pm line 540.

In the staff client, this message is displayed above the title of the record:

"There was an error searching for analytic records, please see the logs for details."

Not sure if this is related.
Comment 5 Nick Clemens 2022-05-10 11:50:12 UTC
(In reply to Magnus Enger from comment #4)
> I'm seeing this on 21.11.04, when looking at a detail view in the intranet: 
> 
> ==> /var/log/koha/hig/plack-error.log <==
> CCL parsing error (10014) Unknown qualifier ZOOM for query:
> Host-item=(Psychology and law =) at /usr/share/koha/lib/C4/Search.pm line
> 242.
> Warning from simple_search_compat: 'CCL parsing error (10014) Unknown
> qualifier ZOOM' at /usr/share/koha/lib/Koha/Biblio.pm line 540.
> 
> In the staff client, this message is displayed above the title of the record:
> 
> "There was an error searching for analytic records, please see the logs for
> details."
> 
> Not sure if this is related.

This is related, but a different bug, see the ones below:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29418
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27575
Comment 6 Martin Renvoize 2022-05-11 11:28:22 UTC
Created attachment 134867 [details] [review]
Bug 30528: Unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2022-05-11 11:28:25 UTC
Created attachment 134868 [details] [review]
Bug 30528: Process limits before handling CCL query

This patch moves the code that handles limit processing before that for CCL queries to ensure that special limit cases (branch, multibranch, etc) are handled before adding the limits to the query string

To test:
1 - Apply unit tests patch only
2 - prove -v t/db_dependent/Search.t
3 - It fails
4 - Apply this patch
5 - It passes!
6 - Test searching and confirm things work as epxected

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2022-05-11 11:29:08 UTC
OK, took me a while to wrap my head around, but I agree with this change now.. and yes, we should chase the clean_query stuff in the other bugs.

Signing off
Comment 9 Joonas Kylmälä 2022-05-26 09:23:27 UTC
Created attachment 135361 [details] [review]
Bug 30528: Unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 10 Joonas Kylmälä 2022-05-26 09:23:31 UTC
Created attachment 135362 [details] [review]
Bug 30528: Process limits before handling CCL query

This patch moves the code that handles limit processing before that for CCL queries to ensure that special limit cases (branch, multibranch, etc) are handled before adding the limits to the query string

To test:
1 - Apply unit tests patch only
2 - prove -v t/db_dependent/Search.t
3 - It fails
4 - Apply this patch
5 - It passes!
6 - Test searching and confirm things work as epxected

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 11 Tomás Cohen Arazi 2022-06-01 18:01:56 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 12 Lucas Gass 2022-06-06 15:45:12 UTC
Pushed to 22.05.x for 22.05.01
Comment 13 Victor Grousset/tuxayo 2022-06-25 21:48:34 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.