Bugzilla – Attachment 81133 Details for
Bug 18677
issue_id is not added to accountlines for lost item fees
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18677: issue_id is not added to accountlines for lost item fees
Bug-18677-issueid-is-not-added-to-accountlines-for.patch (text/plain), 16.90 KB, created by
Kyle M Hall (khall)
on 2018-10-25 12:18:16 UTC
(
hide
)
Description:
Bug 18677: issue_id is not added to accountlines for lost item fees
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2018-10-25 12:18:16 UTC
Size:
16.90 KB
patch
obsolete
>From 9a8e9aadd8b1da7310659a9e373c4d91f4f44b8f Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatetsolutions.com> >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 >--- > 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 f087ba6b1a..6d906db713 100644 >--- a/C4/Accounts.pm >+++ b/C4/Accounts.pm >@@ -130,7 +130,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; >@@ -158,6 +158,7 @@ sub chargelostitem{ > my $accountline = Koha::Account::Line->new( > { > borrowernumber => $borrowernumber, >+ issue_id => $issue_id, > accountno => getnextacctno($borrowernumber), > date => \'NOW()', > amount => $processfee, >@@ -198,6 +199,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 9c8d5ba13e..a81ac0a9fd 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -3672,7 +3672,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 26a2957706..6bcab8a848 100644 >--- a/t/db_dependent/Accounts.t >+++ b/t/db_dependent/Accounts.t >@@ -491,7 +491,7 @@ subtest 'balance' => sub { > }; > > subtest "Koha::Account::chargelostitem tests" => sub { >- plan tests => 32; >+ plan tests => 40; > > my $lostfine; > my $procfee; >@@ -521,112 +521,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 817461b2de..bdbdc6f045 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 119; >+use Test::More tests => 120; > > 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.17.1 (Apple Git-112)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18677
:
74022
|
74165
|
81133
|
81732
|
81823
|
81824
|
81976
|
81977
|
81978
|
82386
|
82387
|
82388
|
82389