Bugzilla – Attachment 127906 Details for
Bug 29474
Automatic renewals cronjob is slow on systems with large numbers of reserves
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29474: Pass rules/branchcode to subroutines, further simplify logic
Bug-29474-Pass-rulesbranchcode-to-subroutines-furt.patch (text/plain), 5.79 KB, created by
Joonas Kylmälä
on 2021-11-20 12:14:19 UTC
(
hide
)
Description:
Bug 29474: Pass rules/branchcode to subroutines, further simplify logic
Filename:
MIME Type:
Creator:
Joonas Kylmälä
Created:
2021-11-20 12:14:19 UTC
Size:
5.79 KB
patch
obsolete
>From 9d000dbb844d3fd6f58dc4e19a8fab4893449437 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Mon, 15 Nov 2021 12:07:29 +0000 >Subject: [PATCH] Bug 29474: Pass rules/branchcode to subroutines, further > simplify logic >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >If we are going to decouple 'too_soon' from auto_renewals we should only call GetSoonestDate once > >Additionally, we can fetch all needed rules at once and pass to subroutines as needed > >Rather than check in the sub, let's not call _CanBookBeAutoRenewed if we aren't set for auto_renew > >Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> >--- > C4/Circulation.pm | 53 ++++++++++++++++++++--------------------------- > 1 file changed, 23 insertions(+), 30 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 1e302c0e48..0791c7f7c2 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2752,7 +2752,7 @@ sub CanBookBeRenewed { > return ( 0, 'item_denied_renewal') if _item_denied_renewal({ item => $item }); > > my $patron = $issue->patron or return; >- >+ my $soonest_renew_date; > # override_limit will override anything else except on_reserve > unless ( $override_limit ){ > my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); >@@ -2764,10 +2764,14 @@ sub CanBookBeRenewed { > rules => [ > 'renewalsallowed', > 'lengthunit', >- 'unseen_renewals_allowed' >+ 'unseen_renewals_allowed', >+ 'no_auto_renewal_after', >+ 'no_auto_renewal_after_hard_limit', >+ 'norenewalbefore' > ] > } > ); >+ $soonest_renew_date = GetSoonestRenewDate($borrowernumber, $itemnumber, $branchcode, $issuing_rule); > > return ( 0, "too_many" ) > if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals; >@@ -2789,12 +2793,16 @@ sub CanBookBeRenewed { > return ( 0, 'overdue'); > } > >+ > $auto_renew = _CanBookBeAutoRenewed({ > patron => $patron, > item => $item, > branchcode => $branchcode, >- issue => $issue >- }); >+ issue => $issue, >+ issuing_rule => $issuing_rule, >+ soonest => $soonest_renew_date >+ }) if $issue->auto_renew && $patron->autorenew_checkouts; >+ > return ( 0, $auto_renew ) if $auto_renew =~ 'auto_too_soon' && $cron; > # cron wants 'too_soon' over 'on_reserve' for performance and to avoid > # extra notices being sent. Cron also implies no override >@@ -2863,12 +2871,11 @@ sub CanBookBeRenewed { > } > > return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found >- return ( 0, $auto_renew ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok"; >- if ( GetSoonestRenewDate($borrowernumber, $itemnumber) > dt_from_string() ){ >- return (0, "too_soon") unless $override_limit; >- } >+ return ( 0, $auto_renew ) if $auto_renew =~ 'auto_too_soon'; >+ return (0, "too_soon") if !$override_limit && $soonest_renew_date > dt_from_string(); > >- return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed >+ return ( 0, "auto_renew" ) if $auto_renew eq "ok"; >+ # scheduled for automatic renewal, if overridden auto_renew is 'no' > > return ( 1, undef ); > } >@@ -3161,7 +3168,7 @@ cannot be found. > =cut > > sub GetSoonestRenewDate { >- my ( $borrowernumber, $itemnumber ) = @_; >+ my ( $borrowernumber, $itemnumber, $branchcode, $issuing_rule ) = @_; > > my $dbh = C4::Context->dbh; > >@@ -3172,8 +3179,8 @@ sub GetSoonestRenewDate { > my $patron = Koha::Patrons->find( $borrowernumber ) > or return; > >- my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); >- my $issuing_rule = Koha::CirculationRules->get_effective_rules( >+ $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ) unless $branchcode; >+ $issuing_rule = Koha::CirculationRules->get_effective_rules( > { categorycode => $patron->categorycode, > itemtype => $item->effective_itemtype, > branchcode => $branchcode, >@@ -3182,7 +3189,7 @@ sub GetSoonestRenewDate { > 'lengthunit', > ] > } >- ); >+ ) unless $issuing_rule; > > my $now = dt_from_string; > return $now unless $issuing_rule; >@@ -4262,22 +4269,8 @@ sub _CanBookBeAutoRenewed { > my $item = $params->{item}; > my $branchcode = $params->{branchcode}; > my $issue = $params->{issue}; >- >- return "no" unless $issue->auto_renew && $patron->autorenew_checkouts; >- >- my $issuing_rule = Koha::CirculationRules->get_effective_rules( >- { >- categorycode => $patron->categorycode, >- itemtype => $item->effective_itemtype, >- branchcode => $branchcode, >- rules => [ >- 'no_auto_renewal_after', >- 'no_auto_renewal_after_hard_limit', >- 'lengthunit', >- 'norenewalbefore', >- ] >- } >- ); >+ my $soonest_renew_date = $params->{soonest}; >+ my $issuing_rule = $params->{issuing_rule}; > > if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { > return 'auto_account_expired'; >@@ -4317,7 +4310,7 @@ sub _CanBookBeAutoRenewed { > > if ( defined $issuing_rule->{norenewalbefore} > and $issuing_rule->{norenewalbefore} ne "" ) { >- if ( GetSoonestRenewDate($patron->id, $item->id) > dt_from_string()) { >+ if ( $soonest_renew_date > dt_from_string()) { > return "auto_too_soon"; > } else { > return "ok"; >-- >2.20.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 29474
:
127610
|
127611
|
127612
|
127613
|
127614
|
127615
|
127616
|
127617
|
127632
|
127633
|
127634
|
127635
|
127658
|
127903
|
127904
|
127905
|
127906
|
127907
|
127949
|
127950
|
127951
|
127952
|
127953
|
128554
|
128555
|
128556