From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), We must pass C4::Context->userenv->{branch}
Created attachment 122339 [details] [review] Bug 28622: (bug 15758 follow-up) Remove incorrect branch selection in adv search From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches + selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), And - [% PROCESS options_for_libraries libraries => Branches.all() %] + [%# FIXME Should not we filter the libraries displayed? %] + [% PROCESS options_for_libraries libraries => Branches.all( selected => selected_branchcode, unfiltered => 1 ) %] The change in the pl is wrong as it should pass C4::Context->userenv->{branch}, but prior to this commit it seems that we didn't select the logged in library anyway. Looks like we should remove this code.
Created attachment 122467 [details] [review] Bug 28622: (bug 15758 follow-up) Remove incorrect branch selection in adv search From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches + selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), And - [% PROCESS options_for_libraries libraries => Branches.all() %] + [%# FIXME Should not we filter the libraries displayed? %] + [% PROCESS options_for_libraries libraries => Branches.all( selected => selected_branchcode, unfiltered => 1 ) %] The change in the pl is wrong as it should pass C4::Context->userenv->{branch}, but prior to this commit it seems that we didn't select the logged in library anyway. Looks like we should remove this code. Signed-off-by: David Nind <david@davidnind.com>
Testing notes (koha-testing-docker): - Before applying patch behavour is: . Set library in staff interface . Go to the advanced search interface . For 'Location and availability', 'All libraries' is the default choice - After patch is applied, it now defaults to whatever is set as the library - Changed library (used drop down list > Set library) and then went back to advanced search page: library should reflect the new library set
(In reply to David Nind from comment #3) > - After patch is applied, it now defaults to whatever is set as the library Heh, I wasn't expecting a behaviour change actually. But my commit message was confusing. I read the change and code again and, even if I haven't tried, I think that the dropdown list had the logged-in library selected prior to 15758. So yes, this is a bug fix and not simply a code cleanup. However I am surprised nobody reported the problem before.
I don't think the advanced search should limit by default to any library, afaik it was always set to "All libraries". - I just double checked in our 18.11 installations.
(In reply to Katrin Fischer from comment #5) > I don't think the advanced search should limit by default to any library, > afaik it was always set to "All libraries". - I just double checked in our > 18.11 installations. Bug 15758 has been pushed to 16.11.
Not sure I understand the last comment, but I am pretty sure Koha did not preselect the logged-in users library since I have been using Koha - 3.2 :) So can we pinpoint when there was a change or could there have been other code in place?
Thing is, I think as this wasn't reported - it's very likely it would be an unwanted change, don't you think?
Tested on 16.05.x and 'all libraries" was selected.
Created attachment 122598 [details] [review] Bug 28622: (bug 15758 follow-up) Remove incorrect branch selection in adv search From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches + selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), And - [% PROCESS options_for_libraries libraries => Branches.all() %] + [%# FIXME Should not we filter the libraries displayed? %] + [% PROCESS options_for_libraries libraries => Branches.all( selected => selected_branchcode, unfiltered => 1 ) %] The change in the pl is wrong as it should pass C4::Context->userenv->{branch}, but prior to this commit it seems that we didn't select the logged in library anyway. Looks like we should remove this code but preserve the current behavior and select "All libraries".
Thx for testing and rewriting!
Created attachment 122750 [details] [review] Bug 28622: (bug 15758 follow-up) Remove incorrect branch selection in adv search From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches + selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), And - [% PROCESS options_for_libraries libraries => Branches.all() %] + [%# FIXME Should not we filter the libraries displayed? %] + [% PROCESS options_for_libraries libraries => Branches.all( selected => selected_branchcode, unfiltered => 1 ) %] The change in the pl is wrong as it should pass C4::Context->userenv->{branch}, but prior to this commit it seems that we didn't select the logged in library anyway. Looks like we should remove this code but preserve the current behavior and select "All libraries". Signed-off-by: David Nind <david@davidnind.com>
Testing notes (koha-testing-docker): - Before and after apply the patch the behavour is: . Set library in staff interface . Go to the advanced search interface . For 'Location and availability', 'All libraries' is the default choice . If the library is changed (drop down list > Set library) the default for 'Location and availability' should remain as 'All libraries'
Created attachment 122871 [details] [review] Bug 28622: (bug 15758 follow-up) Remove incorrect branch selection in adv search From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches + selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), And - [% PROCESS options_for_libraries libraries => Branches.all() %] + [%# FIXME Should not we filter the libraries displayed? %] + [% PROCESS options_for_libraries libraries => Branches.all( selected => selected_branchcode, unfiltered => 1 ) %] The change in the pl is wrong as it should pass C4::Context->userenv->{branch}, but prior to this commit it seems that we didn't select the logged in library anyway. Looks like we should remove this code but preserve the current behavior and select "All libraries". Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 21.11, thanks to everybody involved!
Pushed to 21.05.x for 21.05.03
Depends on Bug 23271 which added do_not_select_my_library param
Depends on Bug 23271 not in 20.11.x