gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 33444: Update AddRenewal to take a hashref of params 

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Removed skip_record_index => 1 from automatic_renewals.pl. See BZ.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Nick Clemens 2023-04-07 14:54:36 +00:00 committed by Tomas Cohen Arazi
parent 67916fc23a
commit 7afbba200d
No known key found for this signature in database
GPG Key ID: 0A272EA1B2F3C15F
14 changed files with 287 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
C4/Circulation.pm
View File

@ -1540,11 +1540,14 @@ sub AddIssue {
if ( $actualissue and $actualissue->borrowernumber eq $borrower->{'borrowernumber'}
and not $switch_onsite_checkout ) {
$datedue = AddRenewal(
$borrower->{'borrowernumber'},
$item_object->itemnumber,
$branchcode,
$datedue,
$issuedate, # here interpreted as the renewal date
{
borrowernumber => $borrower->{'borrowernumber'},
itemnumber => $item_object->itemnumber,
branch => $branchcode,
datedue => $datedue,
lastreneweddate =>
$issuedate, # here interpreted as the renewal date
}
);
$issue = $item_object->checkout;
}
@ -3094,9 +3097,18 @@ sub CanBookBeRenewed {
=head2 AddRenewal
&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate], [$seen], [$automatic]);
$new_date_due = AddRenewal({
borrowernumber => $borrowernumber,
itemnumber => $itemnumber,
branch => $branch,
[datedue => $datedue],
[lastreneweddate => $lastreneweddate],
[seen => $seen],
[automatic => $automatic],
[skip_record_index => $skip_record_index]
});
Renews a loan.
Renews a loan, returns the updated due date upon success.
C<$borrowernumber> is the borrower number of the patron who currently
has the item.
@ -3131,15 +3143,19 @@ should be skipped for this renewal.
=cut
sub AddRenewal {
my $borrowernumber = shift;
my $itemnumber = shift or return;
my $branch = shift;
my $datedue = shift;
my $lastreneweddate = shift || dt_from_string();
my $skipfinecalc = shift;
my $seen = shift;
my $automatic = shift;
my $skip_record_index = shift;
my $params = shift;
my $borrowernumber = $params->{borrowernumber};
my $itemnumber = $params->{itemnumber};
return unless $itemnumber;
my $branch = $params->{branch};
my $datedue = $params->{datedue};
my $lastreneweddate = $params->{lastreneweddate} // dt_from_string();
my $skipfinecalc = $params->{skipfinecalc};
my $seen = $params->{seen};
my $automatic = $params->{automatic};
my $skip_record_index = $params->{skip_record_index};
# Fallback on a 'seen' renewal
$seen = defined $seen && $seen == 0 ? 0 : 1;

11
C4/ILSDI/Services.pm
View File

@ -691,8 +691,15 @@ sub RenewLoan {
# Add renewal if possible
my @renewal = CanBookBeRenewed( $patron, $issue );
if ( $renewal[0] ) { AddRenewal( $borrowernumber, $itemnumber, undef, undef, undef, undef, 0 ); }
if ( $renewal[0] ) {
AddRenewal(
{
borrowernumber => $borrowernumber,
itemnumber => $itemnumber,
seen => 0
}
);
}
# Hashref building
my $out;

13
Koha/Account/Line.pm
View File

@ -1027,13 +1027,12 @@ sub renew_item {
if ( $can_renew ) {
my $borrowernumber = $self->patron->borrowernumber;
my $due_date = C4::Circulation::AddRenewal(
$borrowernumber,
$itemnumber,
$self->{branchcode},
undef,
undef,
undef,
0
{
borrowernumber => $borrowernumber,
itemnumber => $itemnumber,
branch => $self->{branchcode},
seen => 0
}
);
return {
itemnumber => $itemnumber,

12
Koha/REST/V1/Checkouts.pm
View File

@ -350,12 +350,12 @@ sub renew {
}
AddRenewal(
$checkout->borrowernumber,
$checkout->itemnumber,
$checkout->branchcode,
undef,
undef,
$seen
{
borrowernumber => $checkout->borrowernumber,
itemnumber => $checkout->itemnumber,
branch => $checkout->branchcode,
seen => $seen
}
);
$checkout = Koha::Checkouts->find($checkout_id);

13
circ/renew.pl
View File

@ -98,13 +98,12 @@ if ($barcode) {
: $cgi->param('renewonholdduedate');
$date_due = AddRenewal(
undef,
$item->itemnumber(),
$branchcode,
$date_due,
undef,
undef,
!$unseen
{
itemnumber => $item->itemnumber(),
branch => $branchcode,
datedue => $date_due,
seen => !$unseen
}
);
$template->param( date_due => $date_due );
}

8
misc/cronjobs/automatic_renewals.pl
View File

@ -181,7 +181,13 @@ 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, 1 );
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;
}

10
offline_circ/process_koc.pl
View File

@ -202,11 +202,11 @@ sub kocIssueItem {
#warn "Item issued to this member already, renewing.";
C4::Circulation::AddRenewal(
$issue->borrowernumber, # borrowernumber
$item->itemnumber, # itemnumber
undef, # branch
undef, # datedue - let AddRenewal calculate it automatically
$circ->{'date'}, # issuedate
{
borrowernumber => $issue->borrowernumber,
itemnumber => $item->itemnumber,
lastreneweddate => $circ->{'date'},
}
) unless (DEBUG);
push @output, {

8
opac/opac-renew.pl
View File

@ -66,7 +66,13 @@ else {
my ( $status, $error ) =
CanBookBeRenewed( $patron, $issue );
if ( $status == 1 && $opacrenew == 1 ) {
AddRenewal( $borrowernumber, $issue->itemnumber, undef, undef, undef, undef, 0 );
AddRenewal(
{
borrowernumber => $borrowernumber,
itemnumber => $issue->itemnumber,
seen => 0
}
);
push( @renewed, $issue->itemnumber );
}
else {

8
opac/sco/sco-main.pl
View File

@ -281,7 +281,13 @@ if ( $patron && ( $op eq 'renew' ) ) {
if ( $patron->checkouts->find( { itemnumber => $item->itemnumber } ) ) {
my ($status,$renewerror) = CanBookBeRenewed( $patron, $item->checkout );
if ($status) {
AddRenewal( $patron->borrowernumber, $item->itemnumber, undef, undef, undef, undef, 1 );
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
seen => 1
}
);
push @newissueslist, $barcode;
$template->param( renewed => 1 );
}

10
svc/renew
View File

@ -72,7 +72,15 @@ if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference
if ( $data->{renew_okay} || ( $seen && $data->{error} eq 'too_unseen') ) {
try{
$date_due = AddRenewal( $borrowernumber, $itemnumber, $branchcode, $date_due, undef, undef, $seen );
$date_due = AddRenewal(
{
borrowernumber => $borrowernumber,
itemnumber => $itemnumber,
branch => $branchcode,
datedue => $date_due,
seen => $seen
}
);
$data->{date_due} = output_pref( { dt => $date_due, as_due_date => 1 } );
$data->{renew_okay} = 1;
$data->{error} = undef;

129
t/db_dependent/Circulation.t
View File

@ -779,7 +779,13 @@ subtest "CanBookBeRenewed tests" => sub {
my $old_log_size = Koha::ActionLogs->count( \%params_renewal );
my $dt = dt_from_string();
Time::Fake->offset( $dt->epoch );
my $datedue1 = AddRenewal( $renewing_borrower_obj->borrowernumber, $item_7->itemnumber, $branch );
my $datedue1 = AddRenewal(
{
borrowernumber => $renewing_borrower_obj->borrowernumber,
itemnumber => $item_7->itemnumber,
branch => $branch
}
);
my $new_log_size = Koha::ActionLogs->count( \%params_renewal );
is ($new_log_size, $old_log_size, 'renew log not added because of the syspref RenewalLog');
isnt (DateTime->compare($datedue1, $dt), 0, "AddRenewal returned a good duedate");
@ -788,7 +794,13 @@ subtest "CanBookBeRenewed tests" => sub {
t::lib::Mocks::mock_preference('RenewalLog', 1);
$date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } );
$old_log_size = Koha::ActionLogs->count( \%params_renewal );
AddRenewal( $renewing_borrower_obj->borrowernumber, $item_7->itemnumber, $branch );
AddRenewal(
{
borrowernumber => $renewing_borrower_obj->borrowernumber,
itemnumber => $item_7->itemnumber,
branch => $branch
}
);
$new_log_size = Koha::ActionLogs->count( \%params_renewal );
is ($new_log_size, $old_log_size + 1, 'renew log successfully added');
@ -4557,7 +4569,14 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub {
AddIssue( $patron->unblessed, $item->barcode );
throws_ok {
AddRenewal( $patron->borrowernumber, $item->itemnumber, $library->id, undef, {break=>"the_renewal"} );
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => $library->id,
lastreneweddate => { break => "the_renewal" }
}
);
} 'Koha::Exceptions::Checkout::FailedRenewal', 'Exception is thrown when renewal update to issues fails';
t::lib::Mocks::mock_preference( 'RenewalLog', 0 );
@ -4568,7 +4587,13 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub {
action => "RENEWAL",
);
my $old_log_size = Koha::ActionLogs->count( \%params_renewal );;
AddRenewal( $patron->id, $item->id, $library->id );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id
}
);
my $new_log_size = Koha::ActionLogs->count( \%params_renewal );
is( $new_log_size, $old_log_size, 'renew log not added because of the syspref RenewalLog' );
@ -4604,7 +4629,13 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub {
my $sth = $dbh->prepare("SELECT COUNT(*) FROM statistics WHERE itemnumber = ? AND branch = ?");
$sth->execute($item->id, $library->id);
my ($old_stats_size) = $sth->fetchrow_array;
AddRenewal( $patron->id, $item->id, $library->id );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id
}
);
$new_log_size = Koha::ActionLogs->count( \%params_renewal );
$sth->execute($item->id, $library->id);
my ($new_stats_size) = $sth->fetchrow_array;
@ -4613,7 +4644,14 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub {
AddReturn( $item->id, $library->id, undef, $date );
AddIssue( $patron->unblessed, $item->barcode, $now );
AddRenewal( $patron->id, $item->id, $library->id, undef, undef, 1 );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
skipfinecalc => 1
}
);
my $lines_skipped = Koha::Account::Lines->search({
borrowernumber => $patron->id,
itemnumber => $item->id
@ -4640,7 +4678,14 @@ subtest 'AddRenewal() adds to renewals' => sub {
is(ref($issue), 'Koha::Checkout', 'Issue added');
# Renew item
my $duedate = AddRenewal( $patron->id, $item->id, $library->id, undef, undef, undef, undef, 1 );
my $duedate = AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
automatic => 1
}
);
ok( $duedate, "Renewal added" );
@ -4729,7 +4774,15 @@ subtest 'Incremented fee tests' => sub {
"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 0"
);
$accountline->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
is(
$accountline->amount + 0,
@ -4750,7 +4803,15 @@ subtest 'Incremented fee tests' => sub {
"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1"
);
$accountline->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
is(
$accountline->amount + 0,
@ -4781,7 +4842,15 @@ subtest 'Incremented fee tests' => sub {
"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1 and closed $closed_day_name"
);
$accountline->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
is(
$accountline->amount + 0,
@ -4799,7 +4868,15 @@ subtest 'Incremented fee tests' => sub {
Koha::Account::Lines->search( { itemnumber => $item->id } );
is( $accountlines->count, '2', "Fixed charge and accrued charge recorded distinctly" );
$accountlines->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountlines = Koha::Account::Lines->search( { itemnumber => $item->id } );
is( $accountlines->count, '2', "Fixed charge and accrued charge recorded distinctly, for renewal" );
$accountlines->delete();
@ -4834,7 +4911,15 @@ subtest 'Incremented fee tests' => sub {
"Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 0 (168h * 0.25u)"
);
$accountline->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
is(
$accountline->amount + 0,
@ -4854,7 +4939,15 @@ subtest 'Incremented fee tests' => sub {
"Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 and closed $closed_day_name (168h - 24h * 0.25u)"
);
$accountline->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
is(
$accountline->amount + 0,
@ -4874,7 +4967,15 @@ subtest 'Incremented fee tests' => sub {
"Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 (168h - 0h * 0.25u"
);
$accountline->delete();
AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
AddRenewal(
{
borrowernumber => $patron->id,
itemnumber => $item->id,
branch => $library->id,
datedue => $dt_to_renew,
lastreneweddate => $dt_to
}
);
$accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
is(
$accountline->amount + 0,

61
t/db_dependent/Circulation/issue.t
View File

@ -223,7 +223,15 @@ $se->mock( 'interface', sub {return 'intranet'});
# Mocking userenv with a different branchcode
t::lib::Mocks::mock_userenv({ patron => $patron_2, branchcode => $branchcode_3 });
my $datedue3 = AddRenewal( $borrower_id1, $item_id1, $branchcode_1, $datedue1, $daysago10 );
my $datedue3 = AddRenewal(
{
borrowernumber => $borrower_id1,
itemnumber => $item_id1,
branch => $branchcode_1,
datedue => $datedue1,
lastreneweddate => $daysago10
}
);
# Restoring the userenv with the original branchcode
t::lib::Mocks::mock_userenv({ patron => $patron_1});
@ -265,8 +273,15 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub {
my $opac_renew_issue =
C4::Circulation::AddIssue( $patron->unblessed, $item->barcode );
AddRenewal( $patron->borrowernumber, $item->itemnumber,
"Stavromula", $datedue1, $daysago10 );
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => "Stavromula",
datedue => $datedue1,
lastreneweddate => $daysago10
}
);
my $stat = Koha::Statistics->search(
{ itemnumber => $item->itemnumber, type => 'renew' } )->next;
@ -283,8 +298,15 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub {
my $opac_renew_issue =
C4::Circulation::AddIssue( $patron->unblessed, $item->barcode );
AddRenewal( $patron->borrowernumber, $item->itemnumber,
"Stavromula", $datedue1, $daysago10 );
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => "Stavromula",
datedue => $datedue1,
lastreneweddate => $daysago10
}
);
my $stat = Koha::Statistics->search(
{ itemnumber => $item->itemnumber, type => 'renew' } )->next;
@ -357,8 +379,15 @@ is_deeply(
"With issuing rules (renewal allowed) and with a valid parameter, Getrenewcount of item1 returns 3 renews left"
);
AddRenewal( $borrower_id1, $item_id1, $branchcode_1,
$datedue3, $daysago10 );
AddRenewal(
{
borrowernumber => $borrower_id1,
itemnumber => $item_id1,
branch => $branchcode_1,
datedue => $datedue3,
lastreneweddate => $daysago10
}
);
@renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
is_deeply(
\@renewcount,
@ -534,12 +563,26 @@ my $unseen_issue = C4::Circulation::AddIssue( $unseen_patron->unblessed, $unseen
# Does an unseen renewal increment the issue's count
my ( $unseen_before ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
AddRenewal( $unseen_patron->borrowernumber, $unseen_item->itemnumber, $branchcode_1, undef, undef, undef, 0 );
AddRenewal(
{
borrowernumber => $unseen_patron->borrowernumber,
itemnumber => $unseen_item->itemnumber,
branch => $branchcode_1,
seen => 0
}
);
my ( $unseen_after ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
is( $unseen_after, $unseen_before + 1, 'unseen_renewals increments' );
# Does a seen renewal reset the unseen count
AddRenewal( $unseen_patron->borrowernumber, $unseen_item->itemnumber, $branchcode_1, undef, undef, undef, 1 );
AddRenewal(
{
borrowernumber => $unseen_patron->borrowernumber,
itemnumber => $unseen_item->itemnumber,
branch => $branchcode_1,
seen => 1
}
);
my ( $unseen_reset ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
is( $unseen_reset, 0, 'seen renewal resets the unseen count' );

10
t/db_dependent/Koha/Plugins/Circulation_hooks.t
View File

@ -87,7 +87,15 @@ subtest 'after_circ_action() hook tests' => sub {
subtest 'AddRenewal' => sub {
plan tests => 1;
warning_like { AddRenewal( $patron->borrowernumber, $item_1->id, $patron->branchcode ); }
warning_like {
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item_1->id,
branch => $patron->branchcode
}
);
}
qr/after_circ_action called with action: renewal, ref: Koha::Checkout/,
'AddRenewal calls the after_circ_action hook';
};

19
t/db_dependent/Koha/SearchEngine/Indexer.t
View File

@ -284,10 +284,25 @@ subtest 'Test AddRenewal indexer call' => sub {
warnings_are{
AddRenewal($patron->borrowernumber, $item->itemnumber, $item->homebranch, undef, undef, undef, 0);
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => $item->homebranch,
seen => 0
}
);
} [$engine,"C4::Circulation"], "index_records is called for $engine when adding a renewal (AddRenewal())";
warnings_are{
AddRenewal($patron->borrowernumber, $item->itemnumber, undef, undef, undef, undef, 0, 1, 1);
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
seen => 0,
automatic => 1,
skip_record_index => 1
}
);
} undef, "index_records is not called for $engine when adding a renewal (AddRenewal()) with skip_record_index";
}