From 5d313a70829dea0060e8087c87e277040df4c5b2 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 26 Sep 2023 13:53:44 +0000 Subject: [PATCH] Bug 34924: Add Koha::Checkout->attempt_auto_renew 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 --- Koha/Checkout.pm | 44 +++++++++++++++++- misc/cronjobs/automatic_renewals.pl | 20 ++------- t/db_dependent/Koha/Checkouts.t | 70 ++++++++++++++++++++++++++++- 3 files changed, 114 insertions(+), 20 deletions(-) diff --git a/Koha/Checkout.pm b/Koha/Checkout.pm index 178a8c0da4..dab86db7b2 100644 --- a/Koha/Checkout.pm +++ b/Koha/Checkout.pm @@ -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 diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 4fa83566a8..d509305381 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -171,31 +171,19 @@ 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 ( + # FIXME Do we need to list every status? Why not simply else? $error eq 'too_many' || $error eq 'on_reserve' || $error eq 'restriction' || @@ -212,9 +200,7 @@ while ( my $auto_renew = $auto_renews->next ) { 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; 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/Koha/Checkouts.t b/t/db_dependent/Koha/Checkouts.t index 4373904142..c776985fb1 100755 --- a/t/db_dependent/Koha/Checkouts.t +++ b/t/db_dependent/Koha/Checkouts.t @@ -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 Koha::Checkouts; @@ -406,4 +408,68 @@ subtest 'automatic_checkin' => sub { is( dt_from_string($searched->returndate), $yesterday, 'old checkout for odue_ac_item has the right return date' ); $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; +}; -- 2.30.2