From cde7a8e72c04ad31bdda66908e5e0482276bdcb8 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 11 Apr 2018 14:08:57 -0400 Subject: [PATCH] Bug 18677: issue_id is not added to accountlines for lost item fees Test Plan: 1) Apply this patch 2) prove t/db_dependent/Accounts.t Signed-off-by: Martin Renvoize Signed-off-by: Michal Denar [EDIT:] Patch should have increased the number of tests obviously. Signed-off-by: Marcel de Rooy Signed-off-by: Nick Clemens --- C4/Accounts.pm | 4 ++- C4/Circulation.pm | 4 ++- t/db_dependent/Accounts.t | 48 +++++++++++++++++++++++------------- t/db_dependent/Circulation.t | 8 +++++- 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index e41ce7904b..1baecda2d9 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -97,7 +97,7 @@ FIXME : if no replacement price, borrower just doesn't get charged? sub chargelostitem{ my $dbh = C4::Context->dbh(); - my ($borrowernumber, $itemnumber, $amount, $description) = @_; + my ($borrowernumber, $itemnumber, $issue_id, $amount, $description) = @_; my $itype = Koha::ItemTypes->find({ itemtype => Koha::Items->find($itemnumber)->effective_itemtype() }); my $replacementprice = $amount; my $defaultreplacecost = $itype->defaultreplacecost; @@ -125,6 +125,7 @@ sub chargelostitem{ my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, + issue_id => $issue_id, accountno => getnextacctno($borrowernumber), date => \'NOW()', amount => $processfee, @@ -165,6 +166,7 @@ sub chargelostitem{ my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, + issue_id => $issue_id, accountno => getnextacctno($borrowernumber), date => \'NOW()', amount => $replacementprice, diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 12460efbf0..548266e768 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3697,7 +3697,9 @@ sub LostItem{ defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!"; # zero is OK, check defined } if (C4::Context->preference('WhenLostChargeReplacementFee')){ - C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "Lost Item $issues->{'title'} $issues->{'barcode'}"); + my $checkout = Koha::Checkouts->find({ itemnumber => $itemnumber }); + my $checkout_id = $checkout ? $checkout->id : undef; + C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $checkout_id, $issues->{'replacementprice'}, "Lost Item $issues->{'title'} $issues->{'barcode'}"); #FIXME : Should probably have a way to distinguish this from an item that really was returned. #warn " $issues->{'borrowernumber'} / $itemnumber "; } diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 4773d63e31..faba445489 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -489,7 +489,7 @@ subtest 'balance' => sub { }; subtest "Koha::Account::chargelostitem tests" => sub { - plan tests => 32; + plan tests => 40; my $lostfine; my $procfee; @@ -519,112 +519,126 @@ subtest "Koha::Account::chargelostitem tests" => sub { my $cli_itemnumber2 = $builder->build({ source => 'Item', value => { itype => $itype_replace_no_fee->{itemtype} } })->{'itemnumber'}; my $cli_itemnumber3 = $builder->build({ source => 'Item', value => { itype => $itype_no_replace_fee->{itemtype} } })->{'itemnumber'}; my $cli_itemnumber4 = $builder->build({ source => 'Item', value => { itype => $itype_replace_fee->{itemtype} } })->{'itemnumber'}; + + my $cli_issue_id_1 = $builder->build({ source => 'Issue', value => { borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1 } })->{issue_id}; + 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 $duck = Koha::Items->find({itemnumber=>$cli_itemnumber1}); t::lib::Mocks::mock_preference('item-level_itypes', '1'); t::lib::Mocks::mock_preference('useDefaultReplacementCost', '0'); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, $cli_issue_id_1, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'PF' }); ok( !$lostfine, "No lost fine if no replacementcost or default when pref off"); ok( !$procfee, "No processing fee if no processing fee"); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, $cli_issue_id_1, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'PF' }); ok( $lostfine->amount == 6.12, "Lost fine equals replacementcost when pref off and no default set"); ok( !$procfee, "No processing fee if no processing fee"); $lostfine->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, $cli_issue_id_2, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'PF' }); ok( !$lostfine, "No lost fine if no replacementcost but default set when pref off"); ok( !$procfee, "No processing fee if no processing fee"); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, $cli_issue_id_2, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'PF' }); ok( $lostfine->amount == 6.12 , "Lost fine equals replacementcost when pref off and default set"); ok( !$procfee, "No processing fee if no processing fee"); $lostfine->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, $cli_issue_id_3, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'PF' }); ok( !$lostfine, "No lost fine if no replacementcost and no default set when pref off"); ok( $procfee->amount == 8.16, "Processing fee if processing fee"); + is( $procfee->issue_id, $cli_issue_id_3, "Processing fee issue id is correct" ); $procfee->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, $cli_issue_id_3, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'PF' }); ok( $lostfine->amount == 6.12 , "Lost fine equals replacementcost when pref off and no default set"); ok( $procfee->amount == 8.16, "Processing fee if processing fee"); + is( $procfee->issue_id, $cli_issue_id_3, "Processing fee issue id is correct" ); $lostfine->delete(); $procfee->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, $cli_issue_id_4, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); ok( !$lostfine, "No lost fine if no replacementcost but default set when pref off"); ok( $procfee->amount == 2.04, "Processing fee if processing fee"); + is( $procfee->issue_id, $cli_issue_id_4, "Processing fee issue id is correct" ); $procfee->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, $cli_issue_id_4, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); ok( $lostfine->amount == 6.12 , "Lost fine equals replacementcost when pref off and default set"); ok( $procfee->amount == 2.04, "Processing fee if processing fee"); + is( $procfee->issue_id, $cli_issue_id_4, "Processing fee issue id is correct" ); $lostfine->delete(); $procfee->delete(); t::lib::Mocks::mock_preference('useDefaultReplacementCost', '1'); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, $cli_issue_id_1, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'PF' }); ok( !$lostfine, "No lost fine if no replacementcost or default when pref on"); ok( !$procfee, "No processing fee if no processing fee"); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, $cli_issue_id_1, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber1, accounttype => 'PF' }); is( $lostfine->amount, "6.120000", "Lost fine equals replacementcost when pref on and no default set"); ok( !$procfee, "No processing fee if no processing fee"); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, $cli_issue_id_2, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'PF' }); is( $lostfine->amount(), "16.320000", "Lost fine is default if no replacementcost but default set when pref on"); ok( !$procfee, "No processing fee if no processing fee"); $lostfine->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, $cli_issue_id_2, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber2, accounttype => 'PF' }); is( $lostfine->amount, "6.120000" , "Lost fine equals replacementcost when pref on and default set"); ok( !$procfee, "No processing fee if no processing fee"); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, $cli_issue_id_3, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'PF' }); ok( !$lostfine, "No lost fine if no replacementcost and default not set when pref on"); is( $procfee->amount, "8.160000", "Processing fee if processing fee"); + is( $procfee->issue_id, $cli_issue_id_3, "Processing fee issue id is correct" ); $procfee->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, $cli_issue_id_3, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber3, accounttype => 'PF' }); is( $lostfine->amount, "6.120000", "Lost fine equals replacementcost when pref on and no default set"); is( $procfee->amount, "8.160000", "Processing fee if processing fee"); + is( $procfee->issue_id, $cli_issue_id_3, "Processing fee issue id is correct" ); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, $cli_issue_id_4, 0, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); is( $lostfine->amount, "4.080000", "Lost fine is default if no replacementcost but default set when pref on"); 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"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, $cli_issue_id_4, 6.12, "Perdedor"); $lostfine = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'L' }); $procfee = Koha::Account::Lines->find({ borrowernumber => $cli_borrowernumber, itemnumber => $cli_itemnumber4, accounttype => 'PF' }); 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" ); }; subtest "Koha::Account::non_issues_charges tests" => sub { diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 3b1da09b3c..f1f7a5903d 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 120; +use Test::More tests => 121; use Data::Dumper; use DateTime; @@ -904,6 +904,12 @@ C4::Context->dbh->do("DELETE FROM accountlines"); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber7); is( $renewokay, 0, '(Bug 8236), Cannot renew, one of the items is overdue'); + t::lib::Mocks::mock_preference('WhenLostChargeReplacementFee','1'); + $checkout = Koha::Checkouts->find( { itemnumber => $itemnumber3 } ); + LostItem( $itemnumber3, 'test', 0 ); + my $accountline = Koha::Account::Lines->find( { itemnumber => $itemnumber3 } ); + is( $accountline->issue_id, $checkout->id, "Issue id added for lost replacement fee charge" ); + } { -- 2.39.5