Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 34924: Add 'auto_renew_final' and 'auto_unseen_final' return to CanBookBeRenewed

Browse source 
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>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Nick Clemens 2023-09-26 15:03:45 +00:00 committed by Tomas Cohen Arazi
parent 95f6015d43
commit 7ecb05f4ec
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
3 changed files with 99 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
C4/Circulation.pm
View file

@ -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 );
}

42
misc/cronjobs/automatic_renewals.pl
View file

@ -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
}

69
t/db_dependent/Circulation.t
View file

@ -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);