Bugzilla – Attachment 90632 Details for
Bug 22563
Convert lost handling to use 'status' instead of multiple accounttypes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22563: (QA follow-up) Use issue_id in chargelostitem
Bug-22563-QA-follow-up-Use-issueid-in-chargelostit.patch (text/plain), 6.86 KB, created by
Martin Renvoize (ashimema)
on 2019-06-14 20:04:28 UTC
(
hide
)
Description:
Bug 22563: (QA follow-up) Use issue_id in chargelostitem
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2019-06-14 20:04:28 UTC
Size:
6.86 KB
patch
obsolete
>From 096ac5508eb11c9c3f4dd1fe10ae49766138e950 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Fri, 14 Jun 2019 20:05:27 +0100 >Subject: [PATCH] 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: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > C4/Accounts.pm | 7 ++++--- > t/db_dependent/Accounts.t | 25 ++++++++++++++++++++----- > 2 files changed, 24 insertions(+), 8 deletions(-) > >diff --git a/C4/Accounts.pm b/C4/Accounts.pm >index 4c01194c97..5601291d7a 100644 >--- a/C4/Accounts.pm >+++ b/C4/Accounts.pm >@@ -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( >diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t >index 1e214fa7fc..4a450632eb 100644 >--- a/t/db_dependent/Accounts.t >+++ b/t/db_dependent/Accounts.t >@@ -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"); >-- >2.20.1
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 22563
:
86913
|
86914
|
87092
|
87093
|
87094
|
87637
|
87638
|
87639
|
87640
|
87641
|
87642
|
87801
|
87802
|
87803
|
87804
|
87805
|
87806
|
88279
|
88280
|
88281
|
88282
|
88283
|
88284
|
88285
|
88323
|
88324
|
88325
|
88326
|
88327
|
88328
|
88720
|
88721
|
88722
|
88723
|
88724
|
88725
|
89462
|
89463
|
89464
|
89465
|
89466
|
89467
|
90221
|
90222
|
90223
|
90224
|
90225
|
90226
|
90227
|
90307
|
90308
|
90309
|
90310
|
90311
|
90312
|
90330
|
90331
|
90332
|
90333
|
90334
|
90335
|
90632
|
90637
|
90667
|
91338
|
91339
|
91340
|
91341
|
91342
|
91343
|
91344
|
91345
|
91346
|
91347
|
91348