From 096ac5508eb11c9c3f4dd1fe10ae49766138e950 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 14 Jun 2019 20:05:27 +0100 Subject: [PATCH] Bug 22563: (QA follow-up) Use issue_id in chargelostitem C4::Accounts::chargelostitem contained a FIXME which asked if an item should be charged for it lost, returned and then lost again. We add handling for that case here. Signed-off-by: Martin Renvoize --- C4/Accounts.pm | 7 ++++--- t/db_dependent/Accounts.t | 25 ++++++++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index 4c01194c97..5601291d7a 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -71,7 +71,7 @@ FIXME : if no replacement price, borrower just doesn't get charged? =cut -sub chargelostitem{ +sub chargelostitem { my $dbh = C4::Context->dbh(); my ($borrowernumber, $itemnumber, $amount, $description) = @_; my $itype = Koha::ItemTypes->find({ itemtype => Koha::Items->find($itemnumber)->effective_itemtype() }); @@ -83,6 +83,8 @@ sub chargelostitem{ if ($usedefaultreplacementcost && $amount == 0 && $defaultreplacecost){ $replacementprice = $defaultreplacecost; } + my $checkout = Koha::Checkouts->find({ itemnumber => $itemnumber }); + my $issue_id = $checkout ? $checkout->issue_id : undef; my $account = Koha::Account->new({ patron_id => $borrowernumber }); # first make sure the borrower hasn't already been charged for this item @@ -92,13 +94,12 @@ sub chargelostitem{ { itemnumber => $itemnumber, accounttype => 'LOST', + issue_id => $issue_id } )->count(); # OK, they haven't unless ($existing_charges) { - my $checkout = Koha::Checkouts->find({ itemnumber => $itemnumber }); - my $issue_id = $checkout ? $checkout->issue_id : undef; #add processing fee if ($processfee && $processfee > 0){ my $accountline = $account->add_debit( diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 1e214fa7fc..4a450632eb 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -32,6 +32,8 @@ use Koha::Notice::Messages; use Koha::Notice::Templates; use Koha::DateUtils qw( dt_from_string ); +use C4::Circulation qw( MarkIssueReturned ); + BEGIN { use_ok('C4::Accounts'); use_ok('Koha::Object'); @@ -569,12 +571,13 @@ subtest "C4::Accounts::chargelostitem tests" => sub { my $cli_issue_id_2 = $builder->build({ source => 'Issue', value => { borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2 } })->{issue_id}; my $cli_issue_id_3 = $builder->build({ source => 'Issue', value => { borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3 } })->{issue_id}; my $cli_issue_id_4 = $builder->build({ source => 'Issue', value => { borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4 } })->{issue_id}; + my $cli_issue_id_4X = undef; my $lostfine; my $procfee; subtest "fee application tests" => sub { - plan tests => 40; + plan tests => 44; t::lib::Mocks::mock_preference('item-level_itypes', '1'); t::lib::Mocks::mock_preference('useDefaultReplacementCost', '0'); @@ -688,8 +691,20 @@ subtest "C4::Accounts::chargelostitem tests" => sub { is( $lostfine->amount, "6.120000", "Lost fine equals replacementcost when pref on and default set"); is( $procfee->amount, "2.040000", "Processing fee if processing fee"); is( $procfee->issue_id, $cli_issue_id_4, "Processing fee issue id is correct" ); - $lostfine->delete(); - $procfee->delete(); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 6.12, "Perdedor"); + my $lostfines = Koha::Account::Lines->search({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'LOST' }); + my $procfees = Koha::Account::Lines->search({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); + ok( $lostfines->count == 1 , "Lost fine cannot be double charged for the same issue_id"); + ok( $procfees->count == 1, "Processing fee cannot be double charged for the same issue_id"); + MarkIssueReturned($cli_borrowernumber, $cli_itemnumber4); + $cli_issue_id_4X = $builder->build({ source => 'Issue', value => { borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4 } })->{issue_id}; + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 6.12, "Perdedor"); + $lostfines = Koha::Account::Lines->search({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'LOST' }); + $procfees = Koha::Account::Lines->search({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); + ok( $lostfines->count == 2 , "Lost fine can be charged twice for the same item if they are distinct issue_id's"); + ok( $procfees->count == 2, "Processing fee can be charged twice for the same item if they are distinct issue_id's"); + $lostfines->delete(); + $procfees->delete(); }; subtest "basic fields tests" => sub { @@ -702,7 +717,7 @@ subtest "C4::Accounts::chargelostitem tests" => sub { $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'LOST' }); ok($lostfine, "Lost fine created"); is($lostfine->manager_id, $staff_id, "Lost fine manager_id set correctly"); - is($lostfine->issue_id, $cli_issue_id_4, "Lost fine issue_id set correctly"); + is($lostfine->issue_id, $cli_issue_id_4X, "Lost fine issue_id set correctly"); is($lostfine->description, "Perdedor", "Lost fine issue_id set correctly"); is($lostfine->note, '', "Lost fine does not contain a note"); is($lostfine->branchcode, $branchcode, "Lost fine branchcode set correctly"); @@ -711,7 +726,7 @@ subtest "C4::Accounts::chargelostitem tests" => sub { $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); ok($procfee, "Processing fee created"); is($procfee->manager_id, $staff_id, "Processing fee manager_id set correctly"); - is($procfee->issue_id, $cli_issue_id_4, "Processing fee issue_id set correctly"); + is($procfee->issue_id, $cli_issue_id_4X, "Processing fee issue_id set correctly"); is($procfee->description, "Perdedor", "Processing fee issue_id set correctly"); is($procfee->note, C4::Context->preference("ProcessingFeeNote"), "Processing fee contains note matching ProcessingFeeNote"); is($procfee->branchcode, $branchcode, "Processing fee branchcode set correctly"); -- 2.20.1