Bug 24236

Summary: Using quotes in a cataloging search, resulting in multiple pages, will not allow you to advance page
Product: Koha Reporter: Todd Goatley <tgoatley>
Component: CatalogingAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: normal    
Priority: P5 - low CC: 1joynelson, george, hayleypelham, jonathan.druart, kyle, lucas, m.de.rooy, mkstephens, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03
Attachments: Bug 24236: Fix pagination to use query_cgi
Bug 24236: Fix pagination to use query_cgi
Bug 24236: Fix pagination to use query_cgi
Bug 24236: Fix pagination to use query_cgi

Description Todd Goatley 2019-12-12 23:17:08 UTC
When you do a search that has multiple page results in cataloging, if you do that search in quotations it will not allow you to advance the page.
Comment 1 Kyle M Hall 2019-12-13 12:26:29 UTC
Using double quotes, subsequent pages were 'blank'. Singles quotes did not break the paging, but did make the reservoir search come up empty.

https://monosnap.com/file/H3n9b1Xncd7UAGhTrkcOzv0XgSc2sH
Comment 2 Nick Clemens 2019-12-13 16:12:42 UTC
Created attachment 96264 [details] [review]
Bug 24236: Fix pagination to use query_cgi

This is still wrong, we should use page-numbers.inc, but that is a larger refactoring
This patch fixes non-QueryParser searches by simple getting an using the escaped query (query_cgi)

For QueryParser, we do what the QP code does, just uri escape the query - the way that SimpleSearch does the query parsing instead of the usual buildQuery suggest to me that we should probably switch all of this to use the general Zebra search.

The issues with reservoir should be their own bug

To test:
1 - Search for cat in cataloging search
2 - Get results and see you can paginate
3 - Search for "cat"
4 - Get results but subsequent pages empty
5 - Apply patch
6 - Repeat search for "cat"
7 - You can paginate
8 - Enable UseQueryParser syspref (don't worry about installing)
9 - Confirm can still search with quotes and paginate
Comment 3 Myka Kennedy Stephens 2020-01-15 00:49:20 UTC
I cannot replicate the problem. Please provide further direction for testing.

Testing system = koha-testing-docker
Versions tested = master, 19.11.x, 19.05.x
Search engine = Zebra
Search query = cat >> can page through results
Search query = "cat" >> can page through results
Comment 4 Nick Clemens 2020-01-15 13:35:28 UTC
(In reply to Myka Kennedy Stephens from comment #3)
> I cannot replicate the problem. Please provide further direction for testing.
> 
> Testing system = koha-testing-docker
> Versions tested = master, 19.11.x, 19.05.x
> Search engine = Zebra
> Search query = cat >> can page through results
> Search query = "cat" >> can page through results

I I try  search for "a" I end up at this URL:
http://localhost:8081/cgi-bin/koha/cataloguing/addbooks.pl?q=%22a%22

I cannot page forward from there, I get blank pages

If i hit:
http://localhost:8081/cgi-bin/koha/cataloguing/addbooks.pl?q=a

then I can page
Comment 5 ByWater Sandboxes 2020-01-15 18:43:24 UTC
Created attachment 97407 [details] [review]
Bug 24236: Fix pagination to use query_cgi

This is still wrong, we should use page-numbers.inc, but that is a larger refactoring
This patch fixes non-QueryParser searches by simple getting an using the escaped query (query_cgi)

For QueryParser, we do what the QP code does, just uri escape the query - the way that SimpleSearch does the query parsing instead of the usual buildQuery suggest to me that we should probably switch all of this to use the general Zebra search.

The issues with reservoir should be their own bug

To test:
1 - Search for cat in cataloging search
2 - Get results and see you can paginate
3 - Search for "cat"
4 - Get results but subsequent pages empty
5 - Apply patch
6 - Repeat search for "cat"
7 - You can paginate
8 - Enable UseQueryParser syspref (don't worry about installing)
9 - Confirm can still search with quotes and paginate

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 ByWater Sandboxes 2020-01-15 19:23:44 UTC
Created attachment 97413 [details] [review]
Bug 24236: Fix pagination to use query_cgi

This is still wrong, we should use page-numbers.inc, but that is a larger refactoring
This patch fixes non-QueryParser searches by simple getting an using the escaped query (query_cgi)

For QueryParser, we do what the QP code does, just uri escape the query - the way that SimpleSearch does the query parsing instead of the usual buildQuery suggest to me that we should probably switch all of this to use the general Zebra search.

The issues with reservoir should be their own bug

To test:
1 - Search for cat in cataloging search
2 - Get results and see you can paginate
3 - Search for "cat"
4 - Get results but subsequent pages empty
5 - Apply patch
6 - Repeat search for "cat"
7 - You can paginate
8 - Enable UseQueryParser syspref (don't worry about installing)
9 - Confirm can still search with quotes and paginate

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: George Williams <george@nekls.org>
Comment 7 Jonathan Druart 2020-01-21 12:22:48 UTC
Created attachment 97657 [details] [review]
Bug 24236: Fix pagination to use query_cgi

This is still wrong, we should use page-numbers.inc, but that is a larger refactoring
This patch fixes non-QueryParser searches by simple getting an using the escaped query (query_cgi)

For QueryParser, we do what the QP code does, just uri escape the query - the way that SimpleSearch does the query parsing instead of the usual buildQuery suggest to me that we should probably switch all of this to use the general Zebra search.

The issues with reservoir should be their own bug

To test:
1 - Search for cat in cataloging search
2 - Get results and see you can paginate
3 - Search for "cat"
4 - Get results but subsequent pages empty
5 - Apply patch
6 - Repeat search for "cat"
7 - You can paginate
8 - Enable UseQueryParser syspref (don't worry about installing)
9 - Confirm can still search with quotes and paginate

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: George Williams <george@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Martin Renvoize 2020-01-23 09:10:47 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 9 Joy Nelson 2020-02-06 22:55:04 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 10 Lucas Gass 2020-02-13 20:15:52 UTC
backported to 19.05.x for 19.05.08
Comment 11 Hayley Pelham 2020-02-14 03:09:09 UTC
Is someone able to also test 18.11.x for the presence of this bug?

I have tested, and I am able to paginate with results still coming up even when searching with "cat".
Comment 12 Jonathan Druart 2020-02-14 08:25:17 UTC
(In reply to Hayley Mapley from comment #11)
> Is someone able to also test 18.11.x for the presence of this bug?
> 
> I have tested, and I am able to paginate with results still coming up even
> when searching with "cat".

I do recreate with "cat" with the double quote :)

Page 2 link is /cgi-bin/koha/cataloguing/addbooks.pl?q=

With the patch (it applies correctly on 18.11.x), I get
/cgi-bin/koha/cataloguing/addbooks.pl?idx=kw&q=%22cat%22&page=2