gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Martin Renvoize 2019-06-14 20:05:27 +01:00
parent d2c76ac420
commit 40c0ebb219
No known key found for this signature in database
GPG Key ID: 422B469130441A0F
2 changed files with 24 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
C4/Accounts.pm
View File

@ -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(

25
t/db_dependent/Accounts.t
View File

@ -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");