From fa5a5219bbe749195c26eb20b7e3dcf5150e4b58 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) --- suggestion/suggestion.pl | 57 ++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index cb6da835f3..2296865f8c 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.43.1