[Bug 28622] New: selected branchcode not passed to adv search
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Bug ID: 28622 Summary: selected branchcode not passed to adv search Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 15758 From commit df97814f3034a3d5015546c4eca21f08ae951e37 Bug 15758: Koha::Libraries - Remove GetBranches selected_branchcode => ( C4::Context->IsSuperLibrarian ? C4::Context->userenv : '' ), We must pass C4::Context->userenv->{branch} Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122339&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|selected branchcode not |Selected branchcode not |passed to adv search |passed to adv search? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122339|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 122467 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122467&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thing is, I think as this wasn't reported - it's very likely it would be an unwanted change, don't you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Tested on 16.05.x and 'all libraries" was selected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122467|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122598 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122598&action=edit 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". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Selected branchcode not |Selected branchcode |passed to adv search? |incorrectly passed to adv | |search -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thx for testing and rewriting! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122598|0 |1 is obsolete| | --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 122750 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122750&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #13 from 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' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122750|0 |1 is obsolete| | --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 122871 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122871&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Version(s)|21.11.00 |21.11.00,21.05.03 released in| | --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Depends on| |23271 --- Comment #17 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 23271 which added do_not_select_my_library param Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23271 [Bug 23271] Koha::Patron::Category should use Koha::Object::Limit::Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 23271 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org