From aa4971a0f75e2e0d62b65abb7b47abef6d394c25 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 20 Jan 2023 14:57:21 +0000 Subject: [PATCH] Bug 30642: (QA follow-up) Do not rely on script names in modules, add unit test Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi Edit: Kyle, stop impersonating John Doe --- C4/Circulation.pm | 8 +++++--- misc/cronjobs/automatic_renewals.pl | 2 +- t/db_dependent/Circulation.t | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 068093e7a65..1421c7869b8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3020,7 +3020,7 @@ sub CanBookBeRenewed { =head2 AddRenewal - &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen]); + &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen], [$automatic]); Renews a loan. @@ -3049,6 +3049,8 @@ C<$seen> is a boolean flag indicating if the item was seen or not during the ren informs the incrementing of the unseen_renewals column. If this flag is not supplied, we fallback to a true value +C<$automatic> is a boolean flag indicating the renewal was triggered automatically and not by a person ( librarian or patron ) + =cut sub AddRenewal { @@ -3059,6 +3061,7 @@ sub AddRenewal { my $lastreneweddate = shift || dt_from_string(); my $skipfinecalc = shift; my $seen = shift; + my $automatic = shift; # Fallback on a 'seen' renewal $seen = defined $seen && $seen == 0 ? 0 : 1; @@ -3068,8 +3071,7 @@ sub AddRenewal { my $issue = $item_object->checkout; my $item_unblessed = $item_object->unblessed; - my ($package, $filename, $line) = caller; - my $renewal_type = $filename =~ m/automatic_renewals.pl/ ? "Automatic" : "Manual"; + my $renewal_type = $automatic ? "Automatic" : "Manual"; my $dbh = C4::Context->dbh; diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 4884eee2f34..0a843843ab7 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -180,7 +180,7 @@ while ( my $auto_renew = $auto_renews->next ) { $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would'; } if ($confirm){ - my $date_due = AddRenewal( $auto_renew->borrowernumber, $auto_renew->itemnumber, $auto_renew->branchcode, undef, undef, undef, 0 ); + my $date_due = AddRenewal( $auto_renew->borrowernumber, $auto_renew->itemnumber, $auto_renew->branchcode, undef, undef, undef, 0, 1 ); $auto_renew->auto_renew_error(undef)->store; } push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index d06b8d688d0..2ecb44bec47 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -4614,7 +4614,7 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub { }; subtest 'AddRenewal() adds to renewals' => sub { - plan tests => 4; + plan tests => 5; my $library = $builder->build_object({ class => 'Koha::Libraries' }); my $patron = $builder->build_object({ @@ -4631,7 +4631,7 @@ subtest 'AddRenewal() adds to renewals' => sub { is(ref($issue), 'Koha::Checkout', 'Issue added'); # Renew item - my $duedate = AddRenewal( $patron->id, $item->id, $library->id ); + my $duedate = AddRenewal( $patron->id, $item->id, $library->id, undef, undef, undef, undef, 1 ); ok( $duedate, "Renewal added" ); @@ -4639,6 +4639,7 @@ subtest 'AddRenewal() adds to renewals' => sub { is($renewals->count, 1, 'One renewal added'); my $THE_renewal = $renewals->next; is( $THE_renewal->renewer_id, C4::Context->userenv->{'number'}, 'Renewer recorded from context' ); + is( $THE_renewal->renewal_type, 'Automatic', 'AddRenewal "automatic" parameter sets renewal type to "Automatic"'); }; subtest 'ProcessOfflinePayment() tests' => sub { -- 2.34.1