Bug 18677: Remove new issue_id param from charlostitem
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 1 Nov 2018 14:47:47 +0000 (11:47 -0300)
committerFridolin Somers <fridolin.somers@biblibre.com>
Wed, 5 Dec 2018 08:00:10 +0000 (09:00 +0100)
We have the itemnumber no need to pass the issue_id, we can retrieve it
from chargelostitem

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 3475670881e75f47bc6b7e21f0563162e428b831)

Signed-off-by: Jesse Maseto <jesse@bywatersolution.com>
(cherry picked from commit 8b02c6c3a1aed55d44a721d44d6c606c0935ccb2)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

C4/Accounts.pm
C4/Circulation.pm
t/db_dependent/Accounts.t

index 0c644bb..702f525 100644 (file)
@@ -133,7 +133,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;
@@ -156,6 +156,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(
index 465a4a5..80db6b1 100644 (file)
@@ -3653,9 +3653,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 ";
         }
index 0562642..4b0f10a 100644 (file)
@@ -530,38 +530,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");
@@ -570,14 +570,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");
@@ -588,44 +588,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");
@@ -633,7 +633,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");