Created attachment 58723 [details] [review] 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.
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"?
(In reply to M. Tompsett from comment #2) > Comment on attachment 58723 [details] [review] [review] > Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls > > Review of attachment 58723 [details] [review] [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;
(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.
Created attachment 62064 [details] [review] 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>
Created attachment 62066 [details] [review] 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>
(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.
Pushed to master for 17.05, thanks Marcel!
This patch has been pushed to 16.11.x and will be in 16.11.07.
Pushed to 3.22.x for 3.22.20
Pushed to 16.05.x, for 16.05.12 release