From 7283429840403bb8d95a3073e2b51b1f2b077097 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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ä --- 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