Created attachment 64741 [details] limit here is limit=BN, not limit=holdingbranch%3ABN, which causes an item not from this library (Bison) showing in search results On the advanced search area, when you limit to a specific library in the availability limits, the resultant search string is: /cgi-bin/koha/catalogue/search.pl?idx=au&q=janet&op=and&idx=kw&op=and&idx=kw&limit=E&sort_by=relevance where the limit is: limit=E for holding branch...the limit should be /cgi-bin/koha/catalogue/search.pl?idx=au&q=evanovich+janet&op=and&idx=kw&op=and&idx=kw&limit=holdingbranch%3ABN&sort_by=relevance limit=holdingbranch%3ABN This means that currently a limit for a specific library will return books not in that library.
I ran into this problem today on 17.05.01. The workaround I came up with was to create a library group, and add to this group the specific library I wanted to search. Then in the search form, I selected that library group. I'm not familiar with the code, but I did take a quick look, and couldn't see a way to fix this just in advsearch.tt. The language limit is handled correctly there through the use of a FOREACH loop that inserts "ln,rtrn:" in front of the language name in the option value. But the branch limit is handled by a call to options_for_libraries. That function, in html_helpers.inc, doesn't seem to allow a prefix to be inserted in front of the option value. So it looks like the branch selector in advsearch.tt should have a name other than "limit", and in search.pl this param should be handled separately from the other "limit" params, by inserting the prefix "limit=holdingbranch:" in front of the branch name. Of course, due to my ignorance, I could be entirely wrong about all this :-) .
(In reply to Mark Alexander from comment #1) > I ran into this problem today on 17.05.01. > > The workaround I came up with was to create a library group, and add to this > group the specific library I wanted to search. Then in the search form, I > selected that library group. > > I'm not familiar with the code, but I did take a quick look, and couldn't > see a way to fix this just in advsearch.tt. The language limit is handled > correctly there through the use of a FOREACH loop that inserts "ln,rtrn:" in > front of the language name in the option value. But the branch limit is > handled by a call to options_for_libraries. That function, in > html_helpers.inc, doesn't seem to allow a prefix to be inserted in front of > the option value. > > So it looks like the branch selector in advsearch.tt should have a name > other than "limit", and in search.pl this param should be handled separately > from the other "limit" params, by inserting the prefix > "limit=holdingbranch:" in front of the branch name. > > Of course, due to my ignorance, I could be entirely wrong about all this :-) > . It's definitely a regression introduced by Bug 15758: - [% FOREACH branchloo IN branchloop %] - [% IF ( branchloo.selected ) %] - <option value="branch:[% branchloo.value %]" selected="selected">[% branchloo.branchname %]</option> - [% ELSE %] - <option value="branch:[% branchloo.value %]">[% branchloo.branchname %]</option> - [% END %] - [% END %] + [% PROCESS options_for_libraries libraries => Branches.all() %] I think I'll just write a patch to add an optional prefix to html_helpers.inc to restore the original syntax.
Created attachment 67858 [details] [review] Bug 18884 - Advanced search on staff client, Availability limit not properly limiting Bug 15758 accidentally removed the branch: prefix off the individual library limits on the advanced search in the staff client. This patch adds an optional prefix to the html_helpers.inc file, so that advsearch.tt can provide the required "branch:" prefix. _TEST PLAN_ Before applying: 0) Make sure you have branches in your Koha (e.g. select all sample data via web installer initially) 1) View the source on the staff client advanced search, and note that the "branch:" prefix is missing from the individual library limit: <option value="CPL">Centerville</option> 3) Apply the patch 4) Refresh the advanced search page 5) View the source on the staff client advanced search, and note that the "branch:" prefix now appears for the individual library limit: <option value="branch:CPL">Centerville</option>
Note that the fix I've done isn't exactly the one that the OP asked for. "branch" is actually mapped to "homebranch" in ccl.properties, and the "Individual libraries" limit in the staff client advanced search has always used "branch:". If one wanted to differentiate between holding branch and home branch, I think that would be an enhancement, whilst my patch simply fixes a regression.
Created attachment 67985 [details] [review] Bug 18884 - Advanced search on staff client, Availability limit not properly limiting Patch applies and functions as described. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
(In reply to Dilan Johnpullé from comment #5) > Created attachment 67985 [details] [review] [review] > Bug 18884 - Advanced search on staff client, Availability limit not properly > limiting > > Patch applies and functions as described. > Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Dilan, you are not supposed to remove the test plan from the original patch :) Thanks for your signoff!
Upping severity a bit, it impacts stable releases.
Created attachment 68683 [details] [review] Bug 18884 - Advanced search on staff client, Availability limit not properly limiting Patch applies and functions as described. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.06
Patch was pushed to 16.11.x, will be in 16.11.14