Bugzilla – Attachment 131396 Details for
Bug 30232
Acquisition logged in user performance
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30232: Logged in user performance for CanUserUseBudget() and CanUserModifyBudget()
Bug-30232-Logged-in-user-performance-for-CanUserUs.patch (text/plain), 16.92 KB, created by
Fridolin Somers
on 2022-03-05 06:22:00 UTC
(
hide
)
Description:
Bug 30232: Logged in user performance for CanUserUseBudget() and CanUserModifyBudget()
Filename:
MIME Type:
Creator:
Fridolin Somers
Created:
2022-03-05 06:22:00 UTC
Size:
16.92 KB
patch
obsolete
>From 62576b5f591736c5133aaa3ee6abccb4ad15c9a2 Mon Sep 17 00:00:00 2001 >From: Fridolin Somers <fridolin.somers@biblibre.com> >Date: Fri, 4 Mar 2022 16:48:16 -1000 >Subject: [PATCH] Bug 30232: Logged in user performance for CanUserUseBudget() > and CanUserModifyBudget() > >Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. >When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once. > >This patch optimises call for CanUserUseBudget() and CanUserModifyBudget(); > >You may benchmark on a user with a lot of budgets acess. > >Test plan : >Play with impacted pages >and check budgets list is the same than wihtout patch >--- > acqui/acqui-home.pl | 5 ++++- > acqui/addorderiso2709.pl | 8 ++++---- > acqui/basket.pl | 7 ++++--- > acqui/booksellers.pl | 2 +- > acqui/duplicate_orders.pl | 6 ++++-- > acqui/invoice.pl | 6 ++++-- > acqui/invoices.pl | 6 +++++- > acqui/neworderempty.pl | 7 ++++--- > acqui/orderreceive.pl | 6 ++++-- > acqui/parcels.pl | 6 +++++- > admin/aqbudgets.pl | 11 +++++++---- > admin/aqplan.pl | 5 ++++- > suggestion/suggestion.pl | 7 +++++-- > 13 files changed, 55 insertions(+), 27 deletions(-) > >diff --git a/acqui/acqui-home.pl b/acqui/acqui-home.pl >index 05eb16bf06..ea48eece42 100755 >--- a/acqui/acqui-home.pl >+++ b/acqui/acqui-home.pl >@@ -46,6 +46,9 @@ my ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( > } > ); > >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > my $status = $query->param('status') || "ASKED"; > # Get current branch count and total viewable count, if they don't match then pass > # both to template >@@ -77,7 +80,7 @@ my $totavail_active = 0; > > my @budget_loop; > foreach my $budget ( @{$budget_arr} ) { >- next unless (CanUserUseBudget($loggedinuser, $budget, $userflags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $budget, $userflags)); > > my $patron = Koha::Patrons->find( $budget->{budget_owner_id} ); > if ( $patron ) { >diff --git a/acqui/addorderiso2709.pl b/acqui/addorderiso2709.pl >index 658adb03da..746498e128 100755 >--- a/acqui/addorderiso2709.pl >+++ b/acqui/addorderiso2709.pl >@@ -68,6 +68,8 @@ my $op = $cgiparams->{'op'} || ''; > my $booksellerid = $input->param('booksellerid'); > my $allmatch = $input->param('allmatch'); > my $bookseller = Koha::Acquisition::Booksellers->find( $booksellerid ); >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; > > $template->param(scriptname => "/cgi-bin/koha/acqui/addorderiso2709.pl", > booksellerid => $booksellerid, >@@ -312,14 +314,13 @@ if ($op eq ""){ > } > } else { > # 3rd add order >- my $patron = Koha::Patrons->find( $loggedinuser ); > # get quantity in the MARC record (1 if none) > my $quantity = GetMarcQuantity($marcrecord, C4::Context->preference('marcflavour')) || 1; > my %orderinfo = ( > biblionumber => $biblionumber, > basketno => $cgiparams->{'basketno'}, > quantity => $c_quantity, >- branchcode => $patron->branchcode, >+ branchcode => $logged_in_patron->branchcode, > budget_id => $c_budget_id, > uncertainprice => 1, > sort1 => $c_sort1, >@@ -406,14 +407,13 @@ if ($op eq ""){ > my $budgets = GetBudgets(); > my $budget_id = @$budgets[0]->{'budget_id'}; > # build bookfund list >-my $patron = Koha::Patrons->find( $loggedinuser )->unblessed; > my $budget = GetBudget($budget_id); > > # build budget list > my $budget_loop = []; > my $budgets_hierarchy = GetBudgetHierarchy; > foreach my $r ( @{$budgets_hierarchy} ) { >- next unless (CanUserUseBudget($patron, $r, $userflags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $r, $userflags)); > push @{$budget_loop}, > { b_id => $r->{budget_id}, > b_txt => $r->{budget_name}, >diff --git a/acqui/basket.pl b/acqui/basket.pl >index 0710861946..f61127f83d 100755 >--- a/acqui/basket.pl >+++ b/acqui/basket.pl >@@ -85,7 +85,8 @@ our ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( > } > ); > >-my $logged_in_patron = Koha::Patrons->find( $loggedinuser ); >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; > > our $basket = GetBasket($basketno); > $booksellerid = $basket->{booksellerid} unless $booksellerid; >@@ -95,7 +96,7 @@ my $rs = $schema->resultset('VendorEdiAccount')->search( > { vendor_id => $booksellerid, } ); > $template->param( ediaccount => ($rs->count > 0)); > >-unless (CanUserManageBasket($logged_in_patron->unblessed, $basket, $userflags)) { >+unless (CanUserManageBasket($logged_in_patron_unblessed, $basket, $userflags)) { > $template->param( > cannot_manage_basket => 1, > basketno => $basketno, >@@ -370,7 +371,7 @@ if ( $op eq 'list' ) { > my $budgets = GetBudgetHierarchy; > my $has_budgets = 0; > foreach my $r (@{$budgets}) { >- next unless (CanUserUseBudget($loggedinuser, $r, $userflags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $r, $userflags)); > > $has_budgets = 1; > last; >diff --git a/acqui/booksellers.pl b/acqui/booksellers.pl >index 959f829d48..ce285c45f0 100755 >--- a/acqui/booksellers.pl >+++ b/acqui/booksellers.pl >@@ -108,7 +108,7 @@ my $userbranch = $userenv->{branch}; > my $budgets = GetBudgetHierarchy; > my $has_budgets = 0; > foreach my $r (@{$budgets}) { >- next unless (CanUserUseBudget($loggedinuser, $r, $userflags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $r, $userflags)); > > $has_budgets = 1; > last; >diff --git a/acqui/duplicate_orders.pl b/acqui/duplicate_orders.pl >index 21e382e72f..38010b1b16 100755 >--- a/acqui/duplicate_orders.pl >+++ b/acqui/duplicate_orders.pl >@@ -29,6 +29,7 @@ use Koha::Acquisition::Baskets; > use Koha::Acquisition::Currencies; > use Koha::Acquisition::Orders; > use Koha::DateUtils qw( dt_from_string output_pref ); >+use Koha::Patrons; > > my $input = CGI->new; > my $basketno = $input->param('basketno'); >@@ -49,7 +50,8 @@ output_and_exit( $input, $cookie, $template, 'unknown_basket' ) > unless $basket; > > my $vendor = $basket->bookseller; >-my $patron = Koha::Patrons->find($loggedinuser)->unblessed; >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; > > my $filters = { > basket => scalar $input->param('basket'), >@@ -102,7 +104,7 @@ elsif ( $op eq 'batch_edit' ) { > my $budgets_hierarchy = GetBudgetHierarchy; > foreach my $r ( @{$budgets_hierarchy} ) { > next >- unless ( C4::Budgets::CanUserUseBudget( $patron, $r, $userflags ) ); >+ unless ( C4::Budgets::CanUserUseBudget( $logged_in_patron_unblessed, $r, $userflags ) ); > > push @{$budget_loop}, > { >diff --git a/acqui/invoice.pl b/acqui/invoice.pl >index 7240d9319f..5da329e3ef 100755 >--- a/acqui/invoice.pl >+++ b/acqui/invoice.pl >@@ -52,7 +52,9 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( > } > ); > >-my $logged_in_patron = Koha::Patrons->find( $loggedinuser ); >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > my $invoiceid = $input->param('invoiceid'); > my $op = $input->param('op'); > >@@ -279,7 +281,7 @@ my $shipmentcost_budgetid = $details->{shipmentcost_budgetid}; > my $budget_loop = []; > my $budgets = GetBudgetHierarchy(); > foreach my $r ( @{$budgets} ) { >- next unless ( CanUserUseBudget( $loggedinuser, $r, $flags ) ); >+ next unless ( CanUserUseBudget( $logged_in_patron_unblessed, $r, $flags ) ); > > my $selected = $shipmentcost_budgetid ? $r->{budget_id} eq $shipmentcost_budgetid : 0; > >diff --git a/acqui/invoices.pl b/acqui/invoices.pl >index 9720ead33e..173eac24a4 100755 >--- a/acqui/invoices.pl >+++ b/acqui/invoices.pl >@@ -36,6 +36,7 @@ use C4::Acquisition qw( GetInvoices GetInvoice ); > use C4::Budgets qw( GetBudget GetBudgets CanUserUseBudget ); > use Koha::DateUtils qw( dt_from_string output_pref ); > use Koha::Acquisition::Booksellers; >+use Koha::Patrons; > > my $input = CGI->new; > my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( >@@ -47,6 +48,9 @@ my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( > } > ); > >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > my $invoicenumber = $input->param('invoicenumber'); > my $supplierid = $input->param('supplierid'); > my $shipmentdatefrom = $input->param('shipmentdatefrom'); >@@ -107,7 +111,7 @@ foreach (@suppliers) { > my $budgets = GetBudgets(); > my @budgets_loop; > foreach my $budget (@$budgets) { >- push @budgets_loop, $budget if CanUserUseBudget( $loggedinuser, $budget, $flags ); >+ push @budgets_loop, $budget if CanUserUseBudget( $logged_in_patron_unblessed, $budget, $flags ); > } > > my (@openedinvoices, @closedinvoices); >diff --git a/acqui/neworderempty.pl b/acqui/neworderempty.pl >index 7c443147b1..078f366bf4 100755 >--- a/acqui/neworderempty.pl >+++ b/acqui/neworderempty.pl >@@ -125,6 +125,9 @@ our ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( > } > ); > >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > our $marcflavour = C4::Context->preference('marcflavour'); > > if(!$basketno) { >@@ -328,14 +331,12 @@ $suggestion = GetSuggestionInfo($suggestionid) if $suggestionid; > my $active_currency = Koha::Acquisition::Currencies->get_active; > > # build bookfund list >-my $patron = Koha::Patrons->find( $loggedinuser )->unblessed; >- > my $budget = GetBudget($budget_id); > # build budget list > my $budget_loop = []; > my $budgets = GetBudgetHierarchy; > foreach my $r (@{$budgets}) { >- next unless (CanUserUseBudget($patron, $r, $userflags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $r, $userflags)); > push @{$budget_loop}, { > b_id => $r->{budget_id}, > b_txt => $r->{budget_name}, >diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl >index e1b76d3273..8671766edb 100755 >--- a/acqui/orderreceive.pl >+++ b/acqui/orderreceive.pl >@@ -103,6 +103,9 @@ unless ( $order ) { > exit; > } > >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > # prepare the form for receiving > my $basket = $order->basket; > my $currencies = Koha::Acquisition::Currencies->search; >@@ -169,7 +172,6 @@ if ( $suggestion ) { > $template->param( suggestion => $suggestion ); > } > >-my $patron = Koha::Patrons->find( $loggedinuser )->unblessed; > my @budget_loop; > my $periods = GetBudgetPeriods( ); > foreach my $period (@$periods) { >@@ -180,7 +182,7 @@ foreach my $period (@$periods) { > my $budget_hierarchy = GetBudgetHierarchy( $period->{'budget_period_id'} ); > my @funds; > foreach my $r ( @{$budget_hierarchy} ) { >- next unless ( CanUserUseBudget( $patron, $r, $userflags ) ); >+ next unless ( CanUserUseBudget( $logged_in_patron_unblessed, $r, $userflags ) ); > if ( !defined $r->{budget_amount} || $r->{budget_amount} == 0 ) { > next; > } >diff --git a/acqui/parcels.pl b/acqui/parcels.pl >index cd6e2562b1..c6f1f8168c 100755 >--- a/acqui/parcels.pl >+++ b/acqui/parcels.pl >@@ -76,6 +76,7 @@ use C4::Budgets qw( GetBudgetHierarchy GetBudget CanUserUseBudget ); > > use Koha::Acquisition::Booksellers; > use Koha::DateUtils qw( output_pref dt_from_string ); >+use Koha::Patrons; > > my $input = CGI->new; > my $booksellerid = $input->param('booksellerid'); >@@ -96,6 +97,9 @@ our ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user( > } > ); > >+my $logged_in_patron = Koha::Patrons->find($loggedinuser); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > my $invoicenumber = $input->param('invoice'); > my $shipmentcost = $input->param('shipmentcost'); > my $shipmentcost_budgetid = $input->param('shipmentcost_budgetid'); >@@ -179,7 +183,7 @@ if ($count_parcels) { > my $budget_loop = []; > my $budgets = GetBudgetHierarchy; > foreach my $r (@{$budgets}) { >- next unless (CanUserUseBudget($loggedinuser, $r, $flags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $r, $flags)); > push @{$budget_loop}, { > b_id => $r->{budget_id}, > b_txt => $r->{budget_name}, >diff --git a/admin/aqbudgets.pl b/admin/aqbudgets.pl >index 4ed48d47d1..3b2d5fdb79 100755 >--- a/admin/aqbudgets.pl >+++ b/admin/aqbudgets.pl >@@ -56,6 +56,9 @@ my ($template, $borrowernumber, $cookie, $staffflags ) = get_template_and_user( > } > ); > >+my $logged_in_patron = Koha::Patrons->find($borrowernumber); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > my $active_currency = Koha::Acquisition::Currencies->get_active; > if ( $active_currency ) { > $template->param( symbol => $active_currency->symbol, >@@ -115,7 +118,7 @@ if ($op eq 'add_form') { > my $dropbox_disabled; > if (defined $budget_id ) { ### MOD > $budget = GetBudget($budget_id); >- if (!CanUserModifyBudget($borrowernumber, $budget, $staffflags)) { >+ if (!CanUserModifyBudget($logged_in_patron_unblessed, $budget, $staffflags)) { > $template->param(error_not_authorised_to_modify => 1); > output_html_with_http_headers $input, $cookie, $template->output; > exit; >@@ -211,7 +214,7 @@ if ($op eq 'add_form') { > > my $budget_modified = 0; > if (defined $budget_id) { >- if (CanUserModifyBudget($borrowernumber, $budget_hash->{budget_id}, >+ if (CanUserModifyBudget($logged_in_patron_unblessed, $budget_hash->{budget_id}, > $staffflags) > ) { > ModBudget( $budget_hash ); >@@ -252,8 +255,8 @@ if ( $op eq 'list' ) { > my @budgets_to_display; > foreach my $budget (@budgets) { > # PERMISSIONS >- next unless CanUserUseBudget($borrowernumber, $budget, $staffflags); >- unless(CanUserModifyBudget($borrowernumber, $budget, $staffflags)) { >+ next unless CanUserUseBudget($logged_in_patron_unblessed, $budget, $staffflags); >+ unless(CanUserModifyBudget($logged_in_patron_unblessed, $budget, $staffflags)) { > $budget->{'budget_lock'} = 1; > } > >diff --git a/admin/aqplan.pl b/admin/aqplan.pl >index 64b6cd05ab..d5839f403d 100755 >--- a/admin/aqplan.pl >+++ b/admin/aqplan.pl >@@ -41,6 +41,7 @@ use C4::Context; > use C4::Output qw( output_html_with_http_headers ); > use C4::Auth qw( get_template_and_user ); > use Koha::Acquisition::Currencies; >+use Koha::Patrons; > > our $input = CGI->new; > #### $input >@@ -54,6 +55,8 @@ my ( $template, $borrowernumber, $cookie, $staff_flags ) = get_template_and_user > flagsrequired => { acquisition => 'planning_manage' }, > } > ); >+my $logged_in_patron = Koha::Patrons->find($borrowernumber); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; > > my $budget_period_id = $input->param('budget_period_id'); > # ' ------- get periods stuff ------------------' >@@ -315,7 +318,7 @@ my ( @budget_lines, %cell_hash ); > foreach my $budget (@budgets) { > my $budget_lock; > >- unless (CanUserUseBudget($borrowernumber, $budget, $staff_flags)) { >+ unless (CanUserUseBudget($logged_in_patron_unblessed, $budget, $staff_flags)) { > $budget_lock = 1 > } > >diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl >index 5372e00d5b..07de3cf51b 100755 >--- a/suggestion/suggestion.pl >+++ b/suggestion/suggestion.pl >@@ -122,6 +122,9 @@ $borrowernumber = $input->param('borrowernumber') if ( $input->param('borrowernu > $template->param('borrowernumber' => $borrowernumber); > my $branchfilter = $input->param('branchcode') || C4::Context->userenv->{'branch'}; > >+my $logged_in_patron = Koha::Patrons->find($borrowernumber); >+my $logged_in_patron_unblessed = $logged_in_patron->unblessed; >+ > ######################################### > ## Operations > ## >@@ -421,7 +424,7 @@ $template->param(patron_reason_loop=>$patron_reason_loop); > my $budgets = GetBudgets; > my @budgets_loop; > foreach my $budget ( @{$budgets} ) { >- next unless (CanUserUseBudget($borrowernumber, $budget, $userflags)); >+ next unless (CanUserUseBudget($logged_in_patron_unblessed, $budget, $userflags)); > > ## Please see file perltidy.ERR > $budget->{'selected'} = 1 >@@ -436,7 +439,7 @@ $template->param( budgetsloop => \@budgets_loop); > my $sugg_budget_loop = []; > my $sugg_budgets = GetBudgetHierarchy(); > foreach my $r ( @{$sugg_budgets} ) { >- next unless ( CanUserUseBudget( $borrowernumber, $r, $userflags ) ); >+ next unless ( CanUserUseBudget( $logged_in_patron_unblessed, $r, $userflags ) ); > my $selected = ( $$suggestion_ref{budgetid} && $r->{budget_id} eq $$suggestion_ref{budgetid} ) ? 1 : 0; > push @{$sugg_budget_loop}, > { >-- >2.35.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30232
:
131391
|
131392
|
131393
|
131395
| 131396