From 9eccf8cdef5a9ae32377904e0e878079a8295238 Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Fri, 23 Feb 2024 08:59:30 +0100 Subject: [PATCH] Bug 36155: Improve perfs of suggestion.pl when there are many budgets Page suggestion/suggestion.pl always calls GetBudgets() and GetBudgetHierarchy(). But the result are not always used in template. When there are many budgets and funds this takes some time. Also CanUserUseBudget() in a loop should use a unique Koha::Patron unblessed object of the logged in user. Test plan : 1) Create a user 'P1' with permissions 'catalogue', 'suggestions_manage' and all permissions on acquisition except 'budget_manage_all' 2) Create some funds and subfunds without restricted access 3) Create a fund 'F1' with owner other than 'P1' and restricted access on owner 4) Go to staff interface with patron 'P1' 5) Go to Acquisitions > Suggestions 6) Check you see all founds except F1 in Filter by > Acquisition information 7) Create a new suggestion 8) Check you see all founds except F1 in Fund 9) Select a fund and save 10) Edit this suggestion 11) Check you see all founds except F1 in Fund 12) Compare performance with and without patch with many budgets and funds (see test plan of Bug 35921) Signed-off-by: Pedro Amorim --- suggestion/suggestion.pl | 57 ++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index cb6da835f30..2296865f8c5 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -132,6 +132,7 @@ my ( $template, $borrowernumber, $cookie, $userflags ) = get_template_and_user( $borrowernumber = $input->param('borrowernumber') if ( $input->param('borrowernumber') ); $template->param('borrowernumber' => $borrowernumber); my $branchfilter = $input->param('branchcode') || C4::Context->userenv->{'branch'}; +my $loggedinborrower = Koha::Patrons->find($borrowernumber)->unblessed; ######################################### ## Operations @@ -472,36 +473,40 @@ my $patron_reason_loop = GetAuthorisedValues("OPAC_SUG"); $template->param(patron_reason_loop=>$patron_reason_loop); # Budgets for filtering -my $budgets = GetBudgets; -my @budgets_loop; -foreach my $budget ( @{$budgets} ) { - next unless (CanUserUseBudget($borrowernumber, $budget, $userflags)); - - ## Please see file perltidy.ERR - $budget->{'selected'} = 1 - if ($$suggestion_ref{'budgetid'} - && $budget->{'budget_id'} eq $$suggestion_ref{'budgetid'}); - - push @budgets_loop, $budget; +unless ( $op eq 'save' || $op eq 'show' ) { + my $budgets = GetBudgets; + my @budgets_loop; + foreach my $budget ( @{$budgets} ) { + next unless ( CanUserUseBudget( $loggedinborrower, $budget, $userflags ) ); + + ## Please see file perltidy.ERR + $budget->{'selected'} = 1 + if ( $$suggestion_ref{'budgetid'} + && $budget->{'budget_id'} eq $$suggestion_ref{'budgetid'} ); + + push @budgets_loop, $budget; + } + $template->param( budgetsloop => \@budgets_loop ); } -$template->param( budgetsloop => \@budgets_loop); # Budgets for suggestion add or edition -my $sugg_budget_loop = []; -my $sugg_budgets = GetBudgetHierarchy(); -foreach my $r ( @{$sugg_budgets} ) { - next unless ( CanUserUseBudget( $borrowernumber, $r, $userflags ) ); - my $selected = ( $$suggestion_ref{budgetid} && $r->{budget_id} eq $$suggestion_ref{budgetid} ) ? 1 : 0; - push @{$sugg_budget_loop}, - { - b_id => $r->{budget_id}, - b_txt => $r->{budget_name}, - b_active => $r->{budget_period_active}, - selected => $selected, - }; +if ( $op eq 'save' ) { + my $sugg_budget_loop = []; + my $sugg_budgets = GetBudgetHierarchy(); + foreach my $r ( @{$sugg_budgets} ) { + next unless ( CanUserUseBudget( $loggedinborrower, $r, $userflags ) ); + my $selected = ( $$suggestion_ref{budgetid} && $r->{budget_id} eq $$suggestion_ref{budgetid} ) ? 1 : 0; + push @{$sugg_budget_loop}, + { + b_id => $r->{budget_id}, + b_txt => $r->{budget_name}, + b_active => $r->{budget_period_active}, + selected => $selected, + }; + } + @{$sugg_budget_loop} = sort { uc( $a->{b_txt} ) cmp uc( $b->{b_txt} ) } @{$sugg_budget_loop}; + $template->param( sugg_budgets => $sugg_budget_loop ); } -@{$sugg_budget_loop} = sort { uc( $a->{b_txt} ) cmp uc( $b->{b_txt} ) } @{$sugg_budget_loop}; -$template->param( sugg_budgets => $sugg_budget_loop); if( $suggestion_ref->{STATUS} ) { $template->param( -- 2.30.2