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

Bug 34924: Add Koha::Checkout->attempt_auto_renew

Browse source 
This patch moves the actual renewal out of the auto_renewals cronjob script and into the object and adds tests. The logic for notices is still handled in the script.

To test:
1 - prove -v t/db_dependent/Koha/Checkouts.t
2 - Add a circ rule with auto_renew checked
3 - Checkout an item to a patron and set due date in the past
4 - Checkout an item to a patron and set due date in the future
5 - perl misc/cronjobs/automatic_renewals.pl -v
6 - Confirm one would be renewed and the other is too_soon
7 - perl misc/cronjobs/automatic_renewals.pl -v --confirm
8 - Confirm the expected issue is successfully renewed

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 13:53:44 +00:00 committed by Tomas Cohen Arazi
parent 0f3e68f75f
commit 95f6015d43
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
3 changed files with 130 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
Koha/Checkout.pm
View file

@ -23,7 +23,7 @@ use Modern::Perl;
use DateTime;
use Try::Tiny qw( catch try );
use C4::Circulation qw( LostItem MarkIssueReturned );
use C4::Circulation qw( AddRenewal CanBookBeRenewed LostItem MarkIssueReturned );
use Koha::Checkouts::Renewals;
use Koha::Checkouts::ReturnClaims;
use Koha::Database;
@ -151,6 +151,48 @@ sub renewals {
return Koha::Checkouts::Renewals->_new_from_dbic( $renewals_rs );
}
=head3 attempt_auto_renew
my ($success, $error, $updated) = $checkout->auto_renew({ confirm => 1 });
Attempt to automatically renew a book. Return error reason if it cannot be renewed.
Also return whether a change has been made to avoid notifying on more than one attempt.
If not passed confirm, we will only report and no changes will be made.
=cut
sub attempt_auto_renew {
my ( $self, $params ) = @_;
my $confirm = $params->{confirm} // 0;
# CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script
my ( $ok, $error ) = C4::Circulation::CanBookBeRenewed( $self->patron, $self, undef, 1 );
if ( $error eq 'auto_renew' ) {
if ($confirm) {
my $date_due = C4::Circulation::AddRenewal(
{
borrowernumber => $self->borrowernumber,
itemnumber => $self->itemnumber,
branch => $self->branchcode,
seen => 0,
automatic => 1,
}
);
$self->auto_renew_error(undef)->store;
}
return ( 1, undef, 1 );
} else {
my $updated = 0;
if ( !$self->auto_renew_error || $error ne $self->auto_renew_error ) {
$self->auto_renew_error($error)->store if $confirm;
$updated = 1;
}
return ( 0, $error, $updated );
}
}
=head3 to_api_mapping
This method returns the mapping for representing a Koha::Checkout object

52
misc/cronjobs/automatic_renewals.pl
View file

@ -171,52 +171,38 @@ while ( my $auto_renew = $auto_renews->next ) {
$wants_digest = 0;
}
# CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script
my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->patron, $auto_renew, undef, 1 );
my $updated;
if ( $error eq 'auto_renew' ) {
$updated = 1;
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,
$confirm ? 'will' : 'would';
}
if ($confirm) {
my $date_due = AddRenewal(
{
borrowernumber => $auto_renew->borrowernumber,
itemnumber => $auto_renew->itemnumber,
branch => $auto_renew->branchcode,
seen => 0,
automatic => 1,
}
);
push @item_renewal_ids, $auto_renew->itemnumber;
$auto_renew->auto_renew_error(undef)->store;
}
push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
if ($wants_messages) && !$wants_digest;
} elsif ( $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 'too_soon'
|| $error eq 'item_denied_renewal'
|| $error eq 'item_issued_to_other_patron' )
{
if ($verbose) {
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 ) {
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;
}
$updated = 1 if ( !$auto_renew->auto_renew_error || $error ne $auto_renew->auto_renew_error );
if ($updated) {
$auto_renew->auto_renew_error($error)->store if $confirm;
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
}

70
t/db_dependent/Koha/Checkouts.t
View file

@ -19,7 +19,9 @@
use Modern::Perl;
use Test::More tests => 11;
use Test::More tests => 12;
use Test::MockModule;
use Test::Warn;
use C4::Circulation qw( MarkIssueReturned AddReturn );
use C4::Reserves qw( AddReserve );
@ -481,4 +483,68 @@ subtest 'automatic_checkin' => sub {
};
$schema->storage->txn_rollback;
}
};
subtest 'attempt_auto_renew' => sub {
plan tests => 17;
$schema->storage->txn_begin;
my $renew_error = 'auto_renew';
my $module = Test::MockModule->new('C4::Circulation');
$module->mock( 'CanBookBeRenewed', sub { return ( 1, $renew_error ) } );
my $around_now = dt_from_string();
$module->mock( 'AddRenewal', sub { warn "AddRenewal called" } );
my $checkout = $builder->build_object(
{
class => 'Koha::Checkouts',
value => {
date_due => '2023-01-01 23:59:59',
returndate => undef,
auto_renew => 1,
auto_renew_error => undef,
onsite_checkout => 0,
renewals_count => 0,
}
}
);
my ( $success, $error, $updated );
warning_is {
( $success, $error, $updated ) = $checkout->attempt_auto_renew();
}
undef, "AddRenewal not called without confirm";
ok( $success, "Issue is renewed when error is 'auto_renew'" );
is( $error, undef, "No error when renewed" );
ok( $updated, "Issue reported as updated when renewed" );
warning_is {
( $success, $error, $updated ) = $checkout->attempt_auto_renew( { confirm => 1 } );
}
"AddRenewal called", "AddRenewal called when confirm is passed";
ok( $success, "Issue is renewed when error is 'auto_renew'" );
is( $error, undef, "No error when renewed" );
ok( $updated, "Issue reported as updated when renewed" );
$renew_error = 'anything_else';
( $success, $error, $updated ) = $checkout->attempt_auto_renew();
ok( !$success, "Success is untrue for any other status" );
is( $error, 'anything_else', "The error is passed through" );
ok( $updated, "Issue reported as updated when status changes" );
$checkout->discard_changes();
is( $checkout->auto_renew_error, undef, "Error not updated if confirm not passed" );
( $success, $error, $updated ) = $checkout->attempt_auto_renew( { confirm => 1 } );
ok( !$success, "Success is untrue for any other status" );
is( $error, 'anything_else', "The error is passed through" );
ok( $updated, "Issue updated when confirm passed" );
$checkout->discard_changes();
is( $checkout->auto_renew_error, 'anything_else', "Error updated if confirm passed" );
# Error now equals 'anything_else'
( $success, $error, $updated ) = $checkout->attempt_auto_renew();
ok( !$updated, "Issue not reported as updated when status has not changed" );
$schema->storage->txn_rollback;
};