Bugzilla – Attachment 124781 Details for
Bug 27032
CanBookBeRenewed is not understandable and needs refactoring
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27032: Move auto renewal code out of CanBookBeRenewed
Bug-27032-Move-auto-renewal-code-out-of-CanBookBeR.patch (text/plain), 9.38 KB, created by
Kyle M Hall (khall)
on 2021-09-10 16:10:32 UTC
(
hide
)
Description:
Bug 27032: Move auto renewal code out of CanBookBeRenewed
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2021-09-10 16:10:32 UTC
Size:
9.38 KB
patch
obsolete
>From ce808ee73ad21dddd790cb399b701ea819e1df9a Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi> >Date: Mon, 16 Nov 2020 15:49:07 +0200 >Subject: [PATCH] Bug 27032: Move auto renewal code out of CanBookBeRenewed >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This moves the checks related to whether auto renewal can be done on >the checkout to its own function _CanBookBeAutoRenewed(). This makes >it more clear which parts of the code are related to auto renewal >checking and which are not. > >To test: > 1) Make sure prove t/db_dependent/Circulation.t still passes > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Circulation.pm | 178 ++++++++++++++++++++++++++-------------------- > 1 file changed, 102 insertions(+), 76 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 5f3a279514..0b61c7181e 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2757,10 +2757,7 @@ sub CanBookBeRenewed { > branchcode => $branchcode, > rules => [ > 'renewalsallowed', >- 'no_auto_renewal_after', >- 'no_auto_renewal_after_hard_limit', > 'lengthunit', >- 'norenewalbefore', > 'unseen_renewals_allowed' > ] > } >@@ -2786,79 +2783,10 @@ sub CanBookBeRenewed { > return ( 0, 'overdue'); > } > >- if ( $issue->auto_renew && $patron->autorenew_checkouts ) { >- >- if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { >- return ( 0, 'auto_account_expired' ); >- } >- >- if ( defined $issuing_rule->{no_auto_renewal_after} >- and $issuing_rule->{no_auto_renewal_after} ne "" ) { >- # Get issue_date and add no_auto_renewal_after >- # If this is greater than today, it's too late for renewal. >- my $maximum_renewal_date = dt_from_string($issue->issuedate, 'sql'); >- $maximum_renewal_date->add( >- $issuing_rule->{lengthunit} => $issuing_rule->{no_auto_renewal_after} >- ); >- my $now = dt_from_string; >- if ( $now >= $maximum_renewal_date ) { >- return ( 0, "auto_too_late" ); >- } >- } >- if ( defined $issuing_rule->{no_auto_renewal_after_hard_limit} >- and $issuing_rule->{no_auto_renewal_after_hard_limit} ne "" ) { >- # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal >- if ( dt_from_string >= dt_from_string( $issuing_rule->{no_auto_renewal_after_hard_limit} ) ) { >- return ( 0, "auto_too_late" ); >- } >- } >- >- if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) { >- my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals"); >- my $amountoutstanding = >- C4::Context->preference("OPACFineNoRenewalsIncludeCredit") >- ? $patron->account->balance >- : $patron->account->outstanding_debits->total_outstanding; >- if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) { >- return ( 0, "auto_too_much_oweing" ); >- } >- } >- } >- >- if ( defined $issuing_rule->{norenewalbefore} >- and $issuing_rule->{norenewalbefore} ne "" ) >- { >- >- # Calculate soonest renewal by subtracting 'No renewal before' from due date >- my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract( >- $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} ); >- >- # Depending on syspref reset the exact time, only check the date >- if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date' >- and $issuing_rule->{lengthunit} eq 'days' ) >- { >- $soonestrenewal->truncate( to => 'day' ); >- } >- >- if ( $soonestrenewal > dt_from_string() ) >- { >- $auto_renew = ($issue->auto_renew && $patron->autorenew_checkouts) ? "auto_too_soon" : "too_soon"; >- } >- elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) { >- $auto_renew = "ok"; >- } >- } >- >- # Fallback for automatic renewals: >- # If norenewalbefore is undef, don't renew before due date. >- if ( $issue->auto_renew && $auto_renew eq "no" && $patron->autorenew_checkouts ) { >- my $now = dt_from_string; >- if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){ >- $auto_renew = "ok"; >- } else { >- $auto_renew = "auto_too_soon"; >- } >- } >+ $auto_renew = _CanBookBeAutoRenewed($borrowernumber, $itemnumber); >+ return ( 0, $auto_renew ) if $auto_renew =~ 'auto_account_expired'; >+ return ( 0, $auto_renew ) if $auto_renew =~ 'auto_too_late'; >+ return ( 0, $auto_renew ) if $auto_renew =~ 'auto_too_much_oweing'; > } > > my ( $resfound, $resrec, $possible_reserves ) = C4::Reserves::CheckReserves($itemnumber); >@@ -4306,6 +4234,104 @@ sub _CalculateAndUpdateFine { > } > } > >+sub _CanBookBeAutoRenewed { >+ my ( $borrowernumber, $itemnumber ) = @_; >+ >+ my $item = Koha::Items->find($itemnumber); >+ my $issue = $item->checkout; >+ my $patron = $issue->patron; >+ my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); >+ >+ 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', >+ ] >+ } >+ ); >+ >+ if ( $issue->auto_renew && $patron->autorenew_checkouts ) { >+ >+ if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { >+ return 'auto_account_expired'; >+ } >+ >+ if ( defined $issuing_rule->{no_auto_renewal_after} >+ and $issuing_rule->{no_auto_renewal_after} ne "" ) { >+ # Get issue_date and add no_auto_renewal_after >+ # If this is greater than today, it's too late for renewal. >+ my $maximum_renewal_date = dt_from_string($issue->issuedate, 'sql'); >+ $maximum_renewal_date->add( >+ $issuing_rule->{lengthunit} => $issuing_rule->{no_auto_renewal_after} >+ ); >+ my $now = dt_from_string; >+ if ( $now >= $maximum_renewal_date ) { >+ return "auto_too_late"; >+ } >+ } >+ if ( defined $issuing_rule->{no_auto_renewal_after_hard_limit} >+ and $issuing_rule->{no_auto_renewal_after_hard_limit} ne "" ) { >+ # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal >+ if ( dt_from_string >= dt_from_string( $issuing_rule->{no_auto_renewal_after_hard_limit} ) ) { >+ return "auto_too_late"; >+ } >+ } >+ >+ if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) { >+ my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals"); >+ my $amountoutstanding = >+ C4::Context->preference("OPACFineNoRenewalsIncludeCredit") >+ ? $patron->account->balance >+ : $patron->account->outstanding_debits->total_outstanding; >+ if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) { >+ return "auto_too_much_oweing"; >+ } >+ } >+ } >+ >+ if ( defined $issuing_rule->{norenewalbefore} >+ and $issuing_rule->{norenewalbefore} ne "" ) >+ { >+ >+ # Calculate soonest renewal by subtracting 'No renewal before' from due date >+ my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract( >+ $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} ); >+ >+ # Depending on syspref reset the exact time, only check the date >+ if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date' >+ and $issuing_rule->{lengthunit} eq 'days' ) >+ { >+ $soonestrenewal->truncate( to => 'day' ); >+ } >+ >+ if ( $soonestrenewal > dt_from_string() ) >+ { >+ return ($issue->auto_renew && $patron->autorenew_checkouts) ? "auto_too_soon" : "too_soon"; >+ } >+ elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) { >+ return "ok"; >+ } >+ } >+ >+ # Fallback for automatic renewals: >+ # If norenewalbefore is undef, don't renew before due date. >+ if ( $issue->auto_renew && $patron->autorenew_checkouts ) { >+ my $now = dt_from_string; >+ if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){ >+ return "ok"; >+ } else { >+ return "auto_too_soon"; >+ } >+ } >+ return "no"; >+} >+ > sub _item_denied_renewal { > my ($params) = @_; > >-- >2.30.1 (Apple Git-130)
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 27032
:
113666
|
113667
|
113668
|
118593
|
118594
|
118595
|
121492
|
121493
|
121494
|
124780
| 124781 |
124782