Bugzilla – Attachment 157896 Details for
Bug 34924
Add ability to send 'final auto renewal notice'
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed
Bug-34924-Add-autorenewfinal-and-autounseenfinal-r.patch (text/plain), 9.66 KB, created by
Martin Renvoize (ashimema)
on 2023-10-26 08:52:28 UTC
(
hide
)
Description:
Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-10-26 08:52:28 UTC
Size:
9.66 KB
patch
obsolete
>From 18d14d746fec9339c5ff53784a35ba25db02de88 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 26 Sep 2023 15:03:45 +0000 >Subject: [PATCH] Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' > return to CanBookBeRenewed > >There is a desire for auto_renewals to treat the final renewal differently. We would like to notify the patron of the final renewal - but not again when the next renewal fails. This patch adds the new return value and tests. > >To test: >prove -v t/db_dependent/Circulation.t > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > C4/Circulation.pm | 12 ++++- > misc/cronjobs/automatic_renewals.pl | 42 +++++++++--------- > t/db_dependent/Circulation.t | 69 ++++++++++++++++++++++++++++- > 3 files changed, 99 insertions(+), 24 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 2e9ed52bdcd..baab76fef6b 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2988,7 +2988,10 @@ sub CanBookBeRenewed { > return ( 0, 'item_issued_to_other_patron') if $issue->borrowernumber != $patron->borrowernumber; > return ( 0, 'item_denied_renewal') if $item->is_denied_renewal; > >- # override_limit will override anything else except on_reserve >+ my $final_renewal = 0; >+ my $final_unseen_renewal = 0; >+ >+ # override_limit will override anything else except on_reserve > unless ( $override_limit ){ > my $branchcode = _GetCircControlBranch( $item, $patron ); > my $issuing_rule = Koha::CirculationRules->get_effective_rules( >@@ -3012,6 +3015,10 @@ sub CanBookBeRenewed { > looks_like_number($issuing_rule->{unseen_renewals_allowed}) && > $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals; > >+ $final_renewal = $issuing_rule->{renewalsallowed} == ( $issue->renewals_count + 1 ) ? 1 : 0; >+ $final_unseen_renewal = ( C4::Context->preference('UnseenRenewals') >+ && $issuing_rule->{unseen_renewals_allowed} == ( $issue->unseen_renewals + 1 ) ) ? 1 : 0; >+ > my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); > my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing'); > my $restricted = $patron->is_debarred; >@@ -3123,7 +3130,8 @@ sub CanBookBeRenewed { > return (0, "too_soon", { soonest_renew_date => $soonest } ) unless $override_limit; > } > >- return ( 1, "auto_renew" ) if $auto_renew eq "ok" || $auto_renew eq "auto_too_soon" && !$override_limit; # 0 if auto-renewal should not succeed >+ my $auto_renew_code = $final_renewal ? 'auto_renew_final' : $final_unseen_renewal ? 'auto_unseen_final' : 'auto_renew'; >+ return ( 1, $auto_renew_code ) if $auto_renew eq "ok" || $auto_renew eq "auto_too_soon" && !$override_limit; > > return ( 1, undef ); > } >diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl >index c6098d10fb2..72ed54fb314 100755 >--- a/misc/cronjobs/automatic_renewals.pl >+++ b/misc/cronjobs/automatic_renewals.pl >@@ -153,7 +153,7 @@ while ( my $auto_renew = $auto_renews->next ) { > print "examining item '" . $auto_renew->itemnumber . "' to auto renew\n" if $verbose; > > my ( $borrower_preferences, $wants_messages, $wants_digest ) = ( undef, 0, 0 ); >- if ( $send_notices_pref eq 'preferences' ){ >+ if ( $send_notices_pref eq 'preferences' ) { > $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( > { > borrowernumber => $auto_renew->borrowernumber, >@@ -166,13 +166,13 @@ while ( my $auto_renew = $auto_renews->next ) { > $wants_digest = 1 > if $wants_messages > && $borrower_preferences->{wants_digest}; >- } else { # Preference is never or cron >+ } else { # Preference is never or cron > $wants_messages = $send_notices; >- $wants_digest = 0; >+ $wants_digest = 0; > } > >- my ( $success, $error, $updated ) = $auto_renew->attempt_auto_renew({ confirm => $confirm }); >- if ( $success ) { >+ my ( $success, $error, $updated ) = $auto_renew->attempt_auto_renew( { confirm => $confirm } ); >+ if ($success) { > if ($verbose) { > say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.", > $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, >@@ -182,27 +182,29 @@ while ( my $auto_renew = $auto_renews->next ) { > push @item_renewal_ids, $auto_renew->itemnumber; > } > push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew >- if ( $wants_messages ) && !$wants_digest; >+ if ($wants_messages) && !$wants_digest; > } elsif ( >+ > # FIXME Do we need to list every status? Why not simply else? >- $error eq 'too_many' || >- $error eq 'on_reserve' || >- $error eq 'restriction' || >- $error eq 'overdue' || >- $error eq 'too_unseen' || >- $error eq 'auto_account_expired' || >- $error eq 'auto_too_late' || >- $error eq 'auto_too_much_oweing' || >- $error eq 'auto_too_soon' || >- $error eq 'item_denied_renewal' || >- $error eq 'item_issued_to_other_patron' >- ) { >- if ( $verbose ) { >+ $error eq 'too_many' >+ || $error eq 'on_reserve' >+ || $error eq 'restriction' >+ || $error eq 'overdue' >+ || $error eq 'too_unseen' >+ || $error eq 'auto_account_expired' >+ || $error eq 'auto_too_late' >+ || $error eq 'auto_too_much_oweing' >+ || $error eq 'auto_too_soon' >+ || $error eq 'item_denied_renewal' >+ || $error eq 'item_issued_to_other_patron' >+ ) >+ { >+ if ($verbose) { > say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)", > $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, > $confirm ? 'will' : 'would', $error; > } >- if ( $updated ) { >+ if ($updated) { > push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew > if $error ne 'auto_too_soon' && ( $wants_messages && !$wants_digest ); # Do not notify if it's too soon > } >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index fcd68e9bbe8..fc6e96b7356 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -431,7 +431,7 @@ subtest "GetIssuingCharges tests" => sub { > > my ( $reused_itemnumber_1, $reused_itemnumber_2 ); > subtest "CanBookBeRenewed tests" => sub { >- plan tests => 112; >+ plan tests => 113; > > C4::Context->set_preference('ItemsDeniedRenewal',''); > # Generate test biblio >@@ -1412,8 +1412,72 @@ subtest "CanBookBeRenewed tests" => sub { > ); > > }; >- # Too many renewals > >+ subtest "auto_renew_final" => sub { >+ plan tests => 6; >+ my $item_to_auto_renew = $builder->build_sample_item(); >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => $item_to_auto_renew->itype, >+ rules => { >+ norenewalbefore => undef, >+ renewalsallowed => 1, >+ } >+ } >+ ); >+ >+ my $ten_days_before = dt_from_string->add( days => -10 ); >+ my $ten_days_ahead = dt_from_string->add( days => 10 ); >+ my $issue = AddIssue( >+ $renewing_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, >+ undef, { auto_renew => 1 } >+ ); >+ $issue->renewals_count(0)->store; >+ >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); >+ is( $renewokay, 1, 'Success for auto_renewal' ); >+ is( $error, 'auto_renew_final', 'Auto renewal allowed, but it is the last one' ); >+ >+ # Final unseen renewal >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => $item_to_auto_renew->itype, >+ rules => { >+ unseen_renewals_allowed => 2, >+ renewalsallowed => 10, >+ } >+ } >+ ); >+ t::lib::Mocks::mock_preference( 'UnseenRenewals', 1 ); >+ $issue->unseen_renewals(1)->renewals_count(1)->store; >+ >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); >+ is( $renewokay, 1, 'Success for auto_renewal' ); >+ is( $error, 'auto_unseen_final', 'Final unseen renewal reported, not final overall' ); >+ >+ # Final unseen renewal >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => $item_to_auto_renew->itype, >+ rules => { >+ unseen_renewals_allowed => 2, >+ renewalsallowed => 2, >+ } >+ } >+ ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); >+ is( $renewokay, 1, 'Success for auto_renewal' ); >+ is( $error, 'auto_renew_final', 'Final auto renewal reported' ); >+ >+ }; >+ >+ # Too many renewals > # set policy to forbid renewals > Koha::CirculationRules->set_rules( > { >@@ -1444,6 +1508,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > t::lib::Mocks::mock_preference('UnseenRenewals', 1); >+ > $issue_1->unseen_renewals(2)->store; > > ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); >-- >2.41.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=157896">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 34924
:
156239
|
156240
|
156241
|
157290
|
157291
|
157292
|
157687
|
157688
|
157689
|
157690
|
157895
|
157896
|
157897
|
157898
|
157929
|
157930
|
157931
|
157932
|
157933
|
168642