[Bug 17872] New: Fix small error in GetBudgetHierarchy and one of its calls
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Bug ID: 17872 Summary: Fix small error in GetBudgetHierarchy and one of its calls Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 58723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58723&action=edit Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls In aqbudgetperiods.pl a commented line is removed that contains a wrong userenv hash key. Should be branch instead of branchcode. In aqbudgets GetBudgetHierarchy is called with the same wrong userenv hash key. Should be userenv->{branch}. This made another bug visible: if you call GetBudgetHierarchy with a branch and without owner, the where clause should take into account that the branchcode can be empty (not null). Test plan: [1] Run Budgets.t [2] Run aqbudgets.pl from Administration [3] Add the show_mine=1 parameter in the URL [4] Change owner of one of the funds and repeat step 2 and 3. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | Patch complexity|--- |Small patch Severity|enhancement |minor -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Academy CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 58723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58723 Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls Review of attachment 58723: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17872&attachment=58723) ----------------------------------------------------------------- ::: admin/aqbudgets.pl @@ +237,4 @@
);
my @budgets = @{ + GetBudgetHierarchy( $$period{budget_period_id}, C4::Context->userenv->{branch}, ( $show_mine ? $borrower_id : 0 ))
Really don't like C4::Context->userenv->{branch}! Can this code be triggered by a test which isn't "logged in"? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to M. Tompsett from comment #2)
Comment on attachment 58723 [details] [review] Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls
Review of attachment 58723 [details] [review]: -----------------------------------------------------------------
::: admin/aqbudgets.pl @@ +237,4 @@
);
my @budgets = @{ + GetBudgetHierarchy( $$period{budget_period_id}, C4::Context->userenv->{branch}, ( $show_mine ? $borrower_id : 0 ))
Really don't like C4::Context->userenv->{branch}! Can this code be triggered by a test which isn't "logged in"?
Corrects the even worse userenv->{branchcode}. This code cannot be reached when you are not logged in. Would you feel better with: my $userenv = C4::Context->userenv; my $branch = $userenv ? $userenv->{branch} : undef; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to M. Tompsett from comment #2)
Really don't like C4::Context->userenv->{branch}! Can this code be triggered by a test which isn't "logged in"?
You have to be logged in to see this view. That would be a completely unnecessary test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58723|0 |1 is obsolete| | --- Comment #5 from Marc Véron <veron@veron.ch> --- Created attachment 62064 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62064&action=edit Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls In aqbudgetperiods.pl a commented line is removed that contains a wrong userenv hash key. Should be branch instead of branchcode. In aqbudgets GetBudgetHierarchy is called with the same wrong userenv hash key. Should be userenv->{branch}. This made another bug visible: if you call GetBudgetHierarchy with a branch and without owner, the where clause should take into account that the branchcode can be empty (not null). Test plan: [1] Run Budgets.t [2] Run aqbudgets.pl from Administration [3] Add the show_mine=1 parameter in the URL [4] Change owner of one of the funds and repeat step 2 and 3. t/Budgets.t returns green. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62064|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 62066 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62066&action=edit Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls In aqbudgetperiods.pl a commented line is removed that contains a wrong userenv hash key. Should be branch instead of branchcode. In aqbudgets GetBudgetHierarchy is called with the same wrong userenv hash key. Should be userenv->{branch}. This made another bug visible: if you call GetBudgetHierarchy with a branch and without owner, the where clause should take into account that the branchcode can be empty (not null). Test plan: [1] Run Budgets.t [2] Run aqbudgets.pl from Administration [3] Add the show_mine=1 parameter in the URL [4] Change owner of one of the funds and repeat step 2 and 3. t/Budgets.t returns green. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #6)
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Thanks, Marc and Jonathan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Pushed to Master |Pushed to Stable --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This patch has been pushed to 16.11.x and will be in 16.11.07. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #10 from Julian Maurice <julian.maurice@biblibre.com> --- Pushed to 3.22.x for 3.22.20 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17872 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #11 from Mason James <mtj@kohaaloha.com> --- Pushed to 16.05.x, for 16.05.12 release -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org