From f5698b402ba4ddee3486ac71bc05cd846180f725 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 1 Nov 2018 11:47:47 -0300 Subject: [PATCH] Bug 18677: Remove new issue_id param from charlostitem We have the itemnumber no need to pass the issue_id, we can retrieve it from chargelostitem --- C4/Accounts.pm | 4 +++- C4/Circulation.pm | 4 +--- t/db_dependent/Accounts.t | 32 ++++++++++++++++---------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index de20fa77ed..eb8eb4c319 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -98,7 +98,7 @@ FIXME : if no replacement price, borrower just doesn't get charged? sub chargelostitem{ my $dbh = C4::Context->dbh(); - my ($borrowernumber, $itemnumber, $issue_id, $amount, $description) = @_; + my ($borrowernumber, $itemnumber, $amount, $description) = @_; my $itype = Koha::ItemTypes->find({ itemtype => Koha::Items->find($itemnumber)->effective_itemtype() }); my $replacementprice = $amount; my $defaultreplacecost = $itype->defaultreplacecost; @@ -121,6 +121,8 @@ sub chargelostitem{ # 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 = Koha::Account::Line->new( diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 921af8d372..23d0b5fb5c 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3694,9 +3694,7 @@ sub LostItem{ defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!"; # zero is OK, check defined } if (C4::Context->preference('WhenLostChargeReplacementFee')){ - 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'}"); + C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $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 8d995e5ce0..651b875090 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -531,38 +531,38 @@ subtest "Koha::Account::chargelostitem tests" => sub { t::lib::Mocks::mock_preference('item-level_itypes', '1'); t::lib::Mocks::mock_preference('useDefaultReplacementCost', '0'); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, $cli_issue_id_1, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 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, $cli_issue_id_1, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 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, $cli_issue_id_2, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 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, $cli_issue_id_2, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 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, $cli_issue_id_3, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 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, $cli_issue_id_3, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 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"); @@ -571,14 +571,14 @@ subtest "Koha::Account::chargelostitem tests" => sub { $lostfine->delete(); $procfee->delete(); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, $cli_issue_id_4, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 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, $cli_issue_id_4, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 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"); @@ -589,44 +589,44 @@ subtest "Koha::Account::chargelostitem tests" => sub { t::lib::Mocks::mock_preference('useDefaultReplacementCost', '1'); - C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, $cli_issue_id_1, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 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, $cli_issue_id_1, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber1, 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, $cli_issue_id_2, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 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, $cli_issue_id_2, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber2, 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, $cli_issue_id_3, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 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, $cli_issue_id_3, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber3, 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, $cli_issue_id_4, 0, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 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"); @@ -634,7 +634,7 @@ subtest "Koha::Account::chargelostitem tests" => sub { 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, $cli_issue_id_4, 6.12, "Perdedor"); + C4::Accounts::chargelostitem( $cli_borrowernumber, $cli_itemnumber4, 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"); -- 2.11.0