Bug 25872

Summary: Advanced search on OPAC with limiter but no search term fails when re-sorted
Product: Koha Reporter: Andrew Fuerste-Henry <andrewfh>
Component: Searching - ElasticsearchAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: aleisha, barbara.johnson, heather_hernandez, lisettepalouse+koha, lucas
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03
Bug Depends on: 22592    
Bug Blocks:    
Attachments: Bug 25872: Unit tests
Bug 25872: Fix scoping of $limits
Bug 25872: Unit tests
Bug 25872: Fix scoping of $limits
Bug 25872: Unit tests
Bug 25872: Fix scoping of $limits
Bug 25872: Fix scoping of $limits

Description Andrew Fuerste-Henry 2020-06-24 21:01:29 UTC
To recreate:
- go to adv search on the OPAC
- check a limiter box under CCODE, LOC, or ITYPE
- do not enter any search terms
- click search, get results
- use the Sort dropdown to change the sort
- Koha kicks you back to the search page
Comment 1 Barbara Johnson 2020-08-06 20:54:08 UTC
We're using some dropdown lists of search options based on shelving location codes that patrons can use to easily get to new items.  

<h1>Just Arrived</h1>
<p><form name="searchform" method="get" action="/cgi-bin/koha/opac-search.pl">
<input name="idx" value="kw" type="hidden">
<input name="sort_by" value="acqdate_dsc" type="hidden">
<input name="do" value="OK" type="hidden">
<select name="limit" onchange="this.form.submit()">
<option>-- Please choose --</option>
<option value="mc-loc:FN">New Fiction</option>
<option value="mc-loc:FMN">New Mystery</option>
<option value="mc-loc:FYAN">New YA Fiction</option>
</select>
</form>
</p>

We also use some jQuery for an item type limiter that can be used when searching.

//BEGIN Add Search Limits to Main Search Box opac-main.pl
    $(".mastheadsearch #masthead_search").after(" <select name='limit' id='item_type_dropdown'><option value=''>All Items</option><option id='book_limit' value='mc-itype,phr:BOOK'>Books</option><option id='dvdmovie_limit' value='mc-itype,phr:DVDMOVIE'>DVDs (Movies & TV)</option><option id='dvdnf_limit' value='mc-itype,phr:DVDNF'>DVDs (Non-Fiction)</option></select>");

Because the code doesn't have search terms in it the list of results can't be sorted.  We could change everything to use direct urls but that takes a lot of time.  Being able to use locations and item types in the code is easier and cleaner.
Comment 2 Nick Clemens 2020-08-13 12:58:47 UTC
Created attachment 108176 [details] [review]
Bug 25872: Unit tests
Comment 3 Nick Clemens 2020-08-13 12:58:50 UTC
Created attachment 108177 [details] [review]
Bug 25872: Fix scoping of $limits

To test:
1 - Enable Elasticsearch
2 - On OPAC perform advanced search, selecting only an itype, ccode, or LOC limit
3 - Attempt to sort results
4 - You are returned to Advanced search
5 - Apply patch
6 - Repeat
7 - It sorts!
Comment 4 ByWater Sandboxes 2020-08-13 18:33:34 UTC
Created attachment 108211 [details] [review]
Bug 25872: Unit tests

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 5 ByWater Sandboxes 2020-08-13 18:33:37 UTC
Created attachment 108212 [details] [review]
Bug 25872: Fix scoping of $limits

To test:
1 - Enable Elasticsearch
2 - On OPAC perform advanced search, selecting only an itype, ccode, or LOC limit
3 - Attempt to sort results
4 - You are returned to Advanced search
5 - Apply patch
6 - Repeat
7 - It sorts!

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 6 Lisette Scheer 2020-08-13 18:34:11 UTC
Worked great!
Comment 7 Katrin Fischer 2020-08-14 11:15:06 UTC
Created attachment 108257 [details] [review]
Bug 25872: Unit tests

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2020-08-14 11:15:14 UTC
Created attachment 108258 [details] [review]
Bug 25872: Fix scoping of $limits

To test:
1 - Enable Elasticsearch
2 - On OPAC perform advanced search, selecting only an itype, ccode, or LOC limit
3 - Attempt to sort results
4 - You are returned to Advanced search
5 - Apply patch
6 - Repeat
7 - It sorts!

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2020-08-14 11:15:55 UTC
Please remember to include a short description of the fix (what was wrong?) in your commit message as well.
Comment 10 Nick Clemens 2020-08-18 13:43:09 UTC
Created attachment 108507 [details] [review]
Bug 25872: Fix scoping of $limits

The code in build_query_compat contained a 'my' in the assigning of limits for the search in a conditional

This meant the limits were being set correctly during the conditional, but we blanked when passed to the rest
of the code. The effect was that the searches worked, however, the template params to repeat the search were
incomplete.

Removing the my ensures the same limits are applied during search and on re-sorting

To test:
1 - Enable Elasticsearch
2 - On OPAC perform advanced search, selecting only an itype, ccode, or LOC limit
3 - Attempt to sort results
4 - You are returned to Advanced search
5 - Apply patch
6 - Repeat
7 - It sorts!

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Jonathan Druart 2020-08-18 15:47:03 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 12 Lucas Gass 2020-09-03 15:38:39 UTC
backported to 20.05.x for 20.05.03
Comment 13 Aleisha Amohia 2020-09-09 04:39:09 UTC
couldn't reproduce on 19.11.x. not backported