Bug 25872 - Advanced search on OPAC with limiter but no search term fails when re-sorted
Summary: Advanced search on OPAC with limiter but no search term fails when re-sorted
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on: 22592
Blocks:
  Show dependency treegraph
 
Reported: 2020-06-24 21:01 UTC by Andrew Fuerste-Henry
Modified: 2020-09-09 04:39 UTC (History)
5 users (show)

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


Attachments
Bug 25872: Unit tests (1.67 KB, patch)
2020-08-13 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25872: Fix scoping of $limits (1.32 KB, patch)
2020-08-13 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25872: Unit tests (1.73 KB, patch)
2020-08-13 18:33 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 25872: Fix scoping of $limits (1.37 KB, patch)
2020-08-13 18:33 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 25872: Unit tests (1.78 KB, patch)
2020-08-14 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25872: Fix scoping of $limits (1.43 KB, patch)
2020-08-14 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25872: Fix scoping of $limits (1.84 KB, patch)
2020-08-18 13:43 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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