Bugzilla – Attachment 100180 Details for
Bug 19014
Patrons should not get an 'on_reserve' notification if the due date is far into the future
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19014: [19.11.x] Prevent auto_renew notices form sending if too early to renew
Bug-19014-1911x-Prevent-autorenew-notices-form-sen.patch (text/plain), 12.52 KB, created by
Nick Clemens (kidclamp)
on 2020-03-05 12:03:52 UTC
(
hide
)
Description:
Bug 19014: [19.11.x] Prevent auto_renew notices form sending if too early to renew
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-03-05 12:03:52 UTC
Size:
12.52 KB
patch
obsolete
>From 74c03a404a8a3aafd748787d938f4d61b1b1afe8 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Mon, 30 Dec 2019 19:27:26 +0000 >Subject: [PATCH] Bug 19014: [19.11.x] Prevent auto_renew notices form sending > if too early to renew > >Bug 19014: Unit tests > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Bug 19014: Return auto_too_soon before on_reserve > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Bug 19014: on_reserve blocks auto_renew > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > C4/Circulation.pm | 202 ++++++++++++++++++++++--------------------- > t/db_dependent/Circulation.t | 20 ++++- > 2 files changed, 123 insertions(+), 99 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 57fa54ba76..26e7f6dac0 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2672,6 +2672,7 @@ sub CanBookBeRenewed { > > my $dbh = C4::Context->dbh; > my $renews = 1; >+ my $auto_renew = 0; > > my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' ); > my $issue = $item->checkout or return ( 0, 'no_checkout' ); >@@ -2680,6 +2681,108 @@ sub CanBookBeRenewed { > > my $patron = $issue->patron or return; > >+ # override_limit will override anything else except on_reserve >+ unless ( $override_limit ){ >+ my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); >+ my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule( >+ { >+ categorycode => $patron->categorycode, >+ itemtype => $item->effective_itemtype, >+ branchcode => $branchcode, >+ } >+ ); >+ >+ return ( 0, "too_many" ) >+ if not $issuing_rule or $issuing_rule->renewalsallowed <= $issue->renewals; >+ >+ my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); >+ my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing'); >+ $patron = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful? >+ my $restricted = $patron->is_debarred; >+ my $hasoverdues = $patron->has_overdues; >+ >+ if ( $restricted and $restrictionblockrenewing ) { >+ return ( 0, 'restriction'); >+ } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) { >+ return ( 0, 'overdue'); >+ } >+ >+ if ( $issue->auto_renew ) { >+ >+ 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 > DateTime->now( time_zone => C4::Context->tz() ) ) >+ { >+ return ( 0, "auto_too_soon" ) if $issue->auto_renew; >+ return ( 0, "too_soon" ); >+ } >+ elsif ( $issue->auto_renew ) { >+ $auto_renew = 1; >+ } >+ } >+ >+ # Fallback for automatic renewals: >+ # If norenewalbefore is undef, don't renew before due date. >+ if ( $issue->auto_renew && !$auto_renew ) { >+ my $now = dt_from_string; >+ if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){ >+ $auto_renew = 1; >+ } else { >+ return ( 0, "auto_too_soon" ); >+ } >+ } >+ } >+ > my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber); > > # This item can fill one or more unfilled reserve, can those unfilled reserves >@@ -2746,104 +2849,7 @@ sub CanBookBeRenewed { > } > } > return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found >- >- return ( 1, undef ) if $override_limit; >- >- my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); >- my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule( >- { categorycode => $patron->categorycode, >- itemtype => $item->effective_itemtype, >- branchcode => $branchcode >- } >- ); >- >- return ( 0, "too_many" ) >- if not $issuing_rule or $issuing_rule->renewalsallowed <= $issue->renewals; >- >- my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); >- my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing'); >- $patron = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful? >- my $restricted = $patron->is_debarred; >- my $hasoverdues = $patron->has_overdues; >- >- if ( $restricted and $restrictionblockrenewing ) { >- return ( 0, 'restriction'); >- } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) { >- return ( 0, 'overdue'); >- } >- >- if ( $issue->auto_renew ) { >- >- 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 > DateTime->now( time_zone => C4::Context->tz() ) ) >- { >- return ( 0, "auto_too_soon" ) if $issue->auto_renew; >- return ( 0, "too_soon" ); >- } >- elsif ( $issue->auto_renew ) { >- return ( 0, "auto_renew" ); >- } >- } >- >- # Fallback for automatic renewals: >- # If norenewalbefore is undef, don't renew before due date. >- if ( $issue->auto_renew ) { >- my $now = dt_from_string; >- return ( 0, "auto_renew" ) >- if $now >= dt_from_string( $issue->date_due, 'sql' ); >- return ( 0, "auto_too_soon" ); >- } >+ return ( 0, "auto_renew" ) if $auto_renew && !$override_limit; # 0 if auto-renewal should not succeed > > return ( 1, undef ); > } >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 8bf9e56aff..10b41fdb07 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -277,7 +277,7 @@ $dbh->do( > > my ( $reused_itemnumber_1, $reused_itemnumber_2 ); > subtest "CanBookBeRenewed tests" => sub { >- plan tests => 71; >+ plan tests => 77; > > C4::Context->set_preference('ItemsDeniedRenewal',''); > # Generate test biblio >@@ -606,6 +606,24 @@ subtest "CanBookBeRenewed tests" => sub { > is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); > is( $error, 'auto_too_soon', > 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = undef (returned code is auto_too_soon)' ); >+ AddReserve( >+ $branch, $reserving_borrowernumber, $biblio->biblionumber, >+ $bibitems, $priority, $resdate, $expdate, $notes, >+ 'a title', $item_4->itemnumber, $found >+ ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ is( $renewokay, 0, 'Still should not be able to renew' ); >+ is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked' ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); >+ is( $renewokay, 0, 'Still should not be able to renew' ); >+ is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); >+ $dbh->do('UPDATE circulation_rules SET rule_value = 0 where rule_name = "norenewalbefore"'); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); >+ is( $renewokay, 0, 'Still should not be able to renew' ); >+ is( $error, 'on_reserve', 'returned code is on_reserve, auto_renew only happens if not on reserve' ); >+ ModReserveCancelAll($item_4->itemnumber, $reserving_borrowernumber); >+ >+ > > # Bug 7413 > # Test premature manual renewal >-- >2.11.0
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 19014
:
65590
|
86960
|
90509
|
95348
|
95851
|
95852
|
96688
|
96689
|
97374
|
97395
|
97396
|
97397
|
98704
|
98705
|
98706
|
98717
|
98718
|
98719
|
98720
| 100180