From 2ba1493e2e8b48a4ca277ed2cea3f31d54c9e45d Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 4 Mar 2020 13:25:48 +0000 Subject: [PATCH] Bug 23051: (QA follow-up) Missing curly and tabs and fix test Some rebase issues, accounttype no longer exists, circ rules make CanBookBeRenewed fail, so we mock that too interface must be passed as a hashref Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 2 +- Koha/Account.pm | 2 +- Koha/Account/Line.pm | 6 ++++-- .../prog/en/includes/renew_results.inc | 16 ++++++++-------- t/db_dependent/Circulation.t | 2 +- t/db_dependent/Koha/Account.t | 3 ++- t/db_dependent/Koha/Account/Line.t | 14 ++++++++------ 7 files changed, 25 insertions(+), 20 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0fb39e80b4..405528675e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2951,7 +2951,7 @@ sub AddRenewal { my $schema = Koha::Database->schema; $schema->txn_do(sub{ - if ( !skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) { + if ( !$skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) { _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } ); } _FixOverduesOnReturn( $borrowernumber, $itemnumber, undef, 'RENEWED' ); diff --git a/Koha/Account.pm b/Koha/Account.pm index 48d75cd95f..81f7a344ce 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -125,7 +125,7 @@ sub pay { # We're ignoring the definition of $interface above, by all # accounts we can't rely on C4::Context::interface, so here # we're only using what we've been explicitly passed - my $outcome = $fine->renew_item($params->{interface}); + my $outcome = $fine->renew_item({ interface => $interface }); push @{$renew_outcomes}, $outcome if $outcome; } diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index ef53eddba4..8a6f03091a 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -769,6 +769,8 @@ sub to_api_mapping { note => 'internal_note', }; +} + =head3 renewable my $bool = $line->renewable; @@ -780,8 +782,8 @@ sub renewable { return ( $self->amountoutstanding == 0 && - $self->accounttype && - $self->accounttype eq 'OVERDUE' && + $self->debit_type_code && + $self->debit_type_code eq 'OVERDUE' && $self->status && $self->status eq 'UNRETURNED' ) ? 1 : 0; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc index 4c4ab3c056..21945cb915 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_results.inc @@ -1,12 +1,12 @@ [% IF renew_results && renew_results.size > 0 %]
- The fines on the following items were paid off, renewal results are displayed below: - [% FOREACH result IN renew_results %] - [% IF result.success %] -

[% INCLUDE 'biblio-title.inc' biblio=result.item.biblio %] ( [% result.item.barcode | html %] ): Renewed - due [% result.info | html %]

- [% ELSE %] -

[% INCLUDE 'biblio-title.inc' biblio=result.item.biblio %] ( [% result.item.barcode | html %] ): Not renewed - [% INCLUDE 'renew_strings.inc' error=result.info %]

- [% END %] - [% END %] + The fines on the following items were paid off, renewal results are displayed below: + [% FOREACH result IN renew_results %] + [% IF result.success %] +

[% INCLUDE 'biblio-title.inc' biblio=result.item.biblio %] ( [% result.item.barcode | html %] ): Renewed - due [% result.info | html %]

+ [% ELSE %] +

[% INCLUDE 'biblio-title.inc' biblio=result.item.biblio %] ( [% result.item.barcode | html %] ): Not renewed - [% INCLUDE 'renew_strings.inc' error=result.info %]

+ [% END %] + [% END %]
[% END %] diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 1cbfe51cd5..66c3617638 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -3326,7 +3326,7 @@ subtest 'AddReturn should clear items.onloan for unissued items' => sub { subtest 'AddRenewal and AddIssuingCharge tests' => sub { - plan tests => 11; + plan tests => 12; t::lib::Mocks::mock_preference('item-level_itypes', 1); diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index 1e3426d476..5c4f25f199 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -941,7 +941,7 @@ subtest 'pay() renews items when appropriate' => sub { borrowernumber => $patron->id, itemnumber => $item->id, date => \'NOW()', - accounttype => 'OVERDUE', + debit_type_code => 'OVERDUE', status => 'UNRETURNED', interface => 'cli', amount => '1', @@ -954,6 +954,7 @@ subtest 'pay() renews items when appropriate' => sub { my $called = 0; my $module = new Test::MockModule('C4::Circulation'); $module->mock('AddRenewal', sub { $called = 1; }); + $module->mock('CanBookBeRenewed', sub { return 1; }); $account->pay( { amount => '1', diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index 9f33f3e40e..0b35a590c6 100644 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -274,7 +274,7 @@ subtest 'apply() tests' => sub { itemnumber => $item->id, branchcode => $library->id, date => \'NOW()', - accounttype => 'OVERDUE', + debit_type_code => 'OVERDUE', status => 'UNRETURNED', interface => 'cli', amount => '1', @@ -287,6 +287,7 @@ subtest 'apply() tests' => sub { my $called = 0; my $module = new Test::MockModule('C4::Circulation'); $module->mock('AddRenewal', sub { $called = 1; }); + $module->mock('CanBookBeRenewed', sub { return 1; }); my $credit_renew = $account->add_credit({ amount => 100, user_id => $patron->id, interface => 'commandline' }); my $debits_renew = Koha::Account::Lines->search({ accountlines_id => $accountline->id })->as_list; $credit_renew->apply( { debits => $debits_renew, offset_type => 'Manual Credit' } ); @@ -336,7 +337,7 @@ subtest 'Keep account info when related patron, staff, item or cash_register is $patron->delete; $line = $line->get_from_storage; - is( $line->borro1wernumber, undef, "The account line should not be deleted when the related patron is delete"); + is( $line->borrowernumber, undef, "The account line should not be deleted when the related patron is delete"); $register->delete; $line = $line->get_from_storage; @@ -370,7 +371,7 @@ subtest 'Renewal related tests' => sub { borrowernumber => $patron->borrowernumber, manager_id => $staff->borrowernumber, itemnumber => $item->itemnumber, - accounttype => "OVERDUE", + debit_type_code => "OVERDUE", status => "UNRETURNED", amountoutstanding => 0, interface => 'commandline', @@ -380,15 +381,15 @@ subtest 'Renewal related tests' => sub { $line->amountoutstanding(5); is( $line->renewable, 0, "Item is returned as unrenewable when it has outstanding fine" ); $line->amountoutstanding(0); - $line->accounttype("VOID"); + $line->debit_type_code("VOID"); is( $line->renewable, 0, "Item is returned as unrenewable when it has the wrong account type" ); - $line->accounttype("OVERDUE"); + $line->debit_type_code("OVERDUE"); $line->status("RETURNED"); is( $line->renewable, 0, "Item is returned as unrenewable when it has the wrong account status" ); t::lib::Mocks::mock_preference( 'RenewAccruingItemWhenPaid', 0 ); - is ($line->renew_item, 0, 'Attempt to renew fails when syspref is not set'); + is ($line->renew_item, undef, 'Attempt to renew fails when syspref is not set'); t::lib::Mocks::mock_preference( 'RenewAccruingItemWhenPaid', 1 ); is_deeply( $line->renew_item, @@ -414,6 +415,7 @@ subtest 'Renewal related tests' => sub { my $called = 0; my $module = new Test::MockModule('C4::Circulation'); $module->mock('AddRenewal', sub { $called = 1; }); + $module->mock('CanBookBeRenewed', sub { return 1; }); $line->renew_item; is( $called, 1, 'Attempt to renew succeeds when conditions are met' ); -- 2.39.5