Bugzilla – Attachment 110640 Details for
Bug 8338
Add ability to remove fines with dropbox mode
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 8338: [19.11] Remove zero amount overdues on backdated returns
Bug-8338-1911-Remove-zero-amount-overdues-on-backd.patch (text/plain), 16.51 KB, created by
Martin Renvoize (ashimema)
on 2020-09-24 09:13:11 UTC
(
hide
)
Description:
Bug 8338: [19.11] Remove zero amount overdues on backdated returns
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-09-24 09:13:11 UTC
Size:
16.51 KB
patch
obsolete
>From b8662ce83d790dde07f93818004ed6e4f4a3af95 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Wed, 13 May 2020 14:11:16 +0100 >Subject: [PATCH] Bug 8338: [19.11] Remove zero amount overdues on backdated > returns > >[19.11.x] This is a squashed patchset ready for application to 19.11 > >* Remove zero amount overdues on backdated returns where appropriate > >This patch removes any overdues which would be reversed on a backdated >return if CalcFineOnBackdate is enabled and the user has not already >attempted to pay off the accruing fine. > >* Fix test > >This patch moves the previous test introduced with bug 24075 into the >same block as the rest of the AddReturn tests and updates it to test for >the new 'remove accountline' behaviour as well as the reduce and refund >behaviour. >--- > C4/Circulation.pm | 16 +- > t/db_dependent/Circulation.t | 236 +++++++++++++++++++++------ > t/db_dependent/Circulation/Returns.t | 45 +++++ > 3 files changed, 239 insertions(+), 58 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 0eaf6f3a7a..ba783a71e5 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2034,7 +2034,7 @@ sub AddReturn { > # fix up the overdues in accounts... > if ($borrowernumber) { > my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine, 'RETURNED' ); >- defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->itemnumber...) failed!"; # zero is OK, check defined >+ defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, ".$item->itemnumber."...) failed!"; # zero is OK, check defined > > if ( $issue and $issue->is_overdue($return_date) ) { > # fix fine days >@@ -2406,9 +2406,12 @@ sub _FixOverduesOnReturn { > return 0 unless $accountlines->count; # no warning, there's just nothing to fix > > my $accountline = $accountlines->next; >+ my $payments = $accountline->credits; > > my $amountoutstanding = $accountline->amountoutstanding; >- if ($exemptfine && ($amountoutstanding != 0)) { >+ if ( $accountline->amount == 0 && $payments->count == 0 ) { >+ $accountline->delete; >+ } elsif ($exemptfine && ($amountoutstanding != 0)) { > my $account = Koha::Account->new({patron_id => $borrowernumber}); > my $credit = $account->add_credit( > { >@@ -2423,16 +2426,17 @@ sub _FixOverduesOnReturn { > > $credit->apply({ debits => [ $accountline ], offset_type => 'Forgiven' }); > >- $accountline->status('FORGIVEN'); >- > if (C4::Context->preference("FinesLog")) { > &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item"); > } >+ >+ $accountline->status('FORGIVEN'); >+ $accountline->store(); > } else { > $accountline->status($status); >- } >+ $accountline->store(); > >- return $accountline->store(); >+ } > } > ); > >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 9b53952774..0144de6b4c 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > use utf8; > >-use Test::More tests => 47; >+use Test::More tests => 46; > use Test::MockModule; > > use Data::Dumper; >@@ -2034,7 +2034,7 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub { > > > subtest 'AddReturn | is_overdue' => sub { >- plan tests => 6; >+ plan tests => 8; > > t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1); > t::lib::Mocks::mock_preference('finesMode', 'production'); >@@ -2068,6 +2068,7 @@ subtest 'AddReturn | is_overdue' => sub { > > my $now = dt_from_string; > my $one_day_ago = $now->clone->subtract( days => 1 ); >+ my $two_days_ago = $now->clone->subtract( days => 2 ); > my $five_days_ago = $now->clone->subtract( days => 5 ); > my $ten_days_ago = $now->clone->subtract( days => 10 ); > $patron = Koha::Patrons->find( $patron->{borrowernumber} ); >@@ -2132,9 +2133,9 @@ subtest 'AddReturn | is_overdue' => sub { > Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; > }; > >- subtest 'bug 25417 | backdated return + exemptfine' => sub { >+ subtest 'bug 8338 | backdated return resulting in zero amount fine' => sub { > >- plan tests => 6; >+ plan tests => 17; > > t::lib::Mocks::mock_preference('CalculateFinesOnBackdate', 1); > >@@ -2156,20 +2157,194 @@ subtest 'AddReturn | is_overdue' => sub { > is( int( $patron->account->balance() ), > 1, "Overdue fine of 1 day overdue" ); > >- # Backdated return (dropbox mode example - charge should exist but be zero) >+ # Backdated return (dropbox mode example - charge should be removed) > AddReturn( $item->{barcode}, $library->{branchcode}, 1, $one_day_ago ); > is( int( $patron->account->balance() ), > 0, "Overdue fine should be annulled" ); > my $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); >- is( $lines->count, 1, "Overdue fine accountlines still exists"); >+ is( $lines->count, 0, "Overdue fine accountline has been removed"); >+ >+ $issue = AddIssue( $patron->unblessed, $item->{barcode}, $two_days_ago ); # date due was 2d ago >+ >+ # Fake fines cronjob on this checkout >+ ($fine) = >+ CalcFine( $item, $patron->categorycode, $library->{branchcode}, >+ $two_days_ago, $now ); >+ UpdateFine( >+ { >+ issue_id => $issue->issue_id, >+ itemnumber => $item->{itemnumber}, >+ borrowernumber => $patron->borrowernumber, >+ amount => $fine, >+ due => output_pref($one_day_ago) >+ } >+ ); >+ is( int( $patron->account->balance() ), >+ 2, "Overdue fine of 2 days overdue" ); >+ >+ # Payment made against fine >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); >+ my $debit = $lines->next; >+ my $credit = $patron->account->add_credit( >+ { >+ amount => 2, >+ type => 'PAYMENT', >+ interface => 'test', >+ } >+ ); >+ $credit->apply( >+ { debits => [ $debit ], offset_type => 'Payment' } ); >+ >+ is( int( $patron->account->balance() ), >+ 0, "Overdue fine should be paid off" ); >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); >+ is ( $lines->count, 2, "Overdue (debit) and Payment (credit) present"); > my $line = $lines->next; >- is($line->amount+0,0, "Overdue fine amount has been reduced to 0"); >- is($line->amountoutstanding+0,0, "Overdue fine amount outstanding has been reduced to 0"); >- is($line->status,'RETURNED', "Overdue fine was fixed"); >+ is( $line->amount+0, 2, "Overdue fine amount remains as 2 days"); >+ is( $line->amountoutstanding+0, 0, "Overdue fine amountoutstanding reduced to 0"); >+ >+ # Backdated return (dropbox mode example - charge should be removed) >+ AddReturn( $item->{barcode}, $library->{branchcode}, undef, $one_day_ago ); >+ is( int( $patron->account->balance() ), >+ -1, "Refund credit has been applied" ); >+ $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }, { order_by => { '-asc' => 'accountlines_id' }}); >+ is( $lines->count, 3, "Overdue (debit), Payment (credit) and Refund (credit) are all present"); >+ >+ $line = $lines->next; >+ is($line->amount+0,1, "Overdue fine amount has been reduced to 1"); >+ is($line->amountoutstanding+0,0, "Overdue fine amount outstanding remains at 0"); >+ is($line->status,'RETURNED', "Overdue fine is fixed"); >+ $line = $lines->next; >+ is($line->amount+0,-2, "Original payment amount remains as 2"); >+ is($line->amountoutstanding+0,0, "Original payment remains applied"); >+ $line = $lines->next; >+ is($line->amount+0,-1, "Refund amount correctly set to 1"); >+ is($line->amountoutstanding+0,-1, "Refund amount outstanding unspent"); >+ >+ # Cleanup >+ Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; >+ }; >+ >+ subtest 'bug 25417 | backdated return + exemptfine' => sub { >+ >+ plan tests => 2; >+ >+ t::lib::Mocks::mock_preference('CalculateFinesOnBackdate', 1); >+ >+ my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $one_day_ago ); # date due was 1d ago >+ >+ # Fake fines cronjob on this checkout >+ my ($fine) = >+ CalcFine( $item, $patron->categorycode, $library->{branchcode}, >+ $one_day_ago, $now ); >+ UpdateFine( >+ { >+ issue_id => $issue->issue_id, >+ itemnumber => $item->{itemnumber}, >+ borrowernumber => $patron->borrowernumber, >+ amount => $fine, >+ due => output_pref($one_day_ago) >+ } >+ ); >+ is( int( $patron->account->balance() ), >+ 1, "Overdue fine of 1 day overdue" ); >+ >+ # Backdated return (dropbox mode example - charge should no longer exist) >+ AddReturn( $item->{barcode}, $library->{branchcode}, 1, $one_day_ago ); >+ is( int( $patron->account->balance() ), >+ 0, "Overdue fine should be annulled" ); > > # Cleanup > Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; > }; >+ >+ subtest 'bug 24075 | backdated return with return datetime matching due datetime' => sub { >+ plan tests => 7; >+ >+ t::lib::Mocks::mock_preference( 'CalculateFinesOnBackdate', 1 ); >+ >+ my $due_date = dt_from_string; >+ my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $due_date ); >+ >+ # Add fine >+ UpdateFine( >+ { >+ issue_id => $issue->issue_id, >+ itemnumber => $item->{itemnumber}, >+ borrowernumber => $patron->borrowernumber, >+ amount => 0.25, >+ due => output_pref($due_date) >+ } >+ ); >+ is( $patron->account->balance(), >+ 0.25, 'Overdue fine of $0.25 recorded' ); >+ >+ # Backdate return to exact due date and time >+ my ( undef, $message ) = >+ AddReturn( $item->{barcode}, $library->{branchcode}, >+ undef, $due_date ); >+ >+ my $accountline = >+ Koha::Account::Lines->find( { issue_id => $issue->id } ); >+ ok( !$accountline, 'accountline removed as expected' ); >+ >+ # Re-issue >+ $issue = AddIssue( $patron->unblessed, $item->{barcode}, $due_date ); >+ >+ # Add fine >+ UpdateFine( >+ { >+ issue_id => $issue->issue_id, >+ itemnumber => $item->{itemnumber}, >+ borrowernumber => $patron->borrowernumber, >+ amount => .25, >+ due => output_pref($due_date) >+ } >+ ); >+ is( $patron->account->balance(), >+ 0.25, 'Overdue fine of $0.25 recorded' ); >+ >+ # Partial pay accruing fine >+ my $lines = Koha::Account::Lines->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ issue_id => $issue->id >+ } >+ ); >+ my $debit = $lines->next; >+ my $credit = $patron->account->add_credit( >+ { >+ amount => .20, >+ type => 'PAYMENT', >+ interface => 'test', >+ } >+ ); >+ $credit->apply( { debits => [$debit], offset_type => 'Payment' } ); >+ >+ is( $patron->account->balance(), .05, 'Overdue fine reduced to $0.05' ); >+ >+ # Backdate return to exact due date and time >+ ( undef, $message ) = >+ AddReturn( $item->{barcode}, $library->{branchcode}, >+ undef, $due_date ); >+ >+ $lines = Koha::Account::Lines->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ issue_id => $issue->id >+ } >+ ); >+ $accountline = $lines->next; >+ is( $accountline->amountoutstanding + 0, >+ 0, 'Partially paid fee amount outstanding was reduced to 0' ); >+ is( $accountline->amount + 0, >+ 0, 'Partially paid fee amount was reduced to 0' ); >+ is( $patron->account->balance(), -0.20, 'Patron refund recorded' ); >+ >+ # Cleanup >+ Koha::Account::Lines->search( >+ { borrowernumber => $patron->borrowernumber } )->delete; >+ }; > }; > > subtest '_FixAccountForLostAndReturned' => sub { >@@ -3461,49 +3636,6 @@ subtest 'CanBookBeIssued & RentalFeesCheckoutConfirmation' => sub { > $itemtype->rentalcharge_daily('0')->store; > }; > >-subtest "Test Backdating of Returns" => sub { >- plan tests => 2; >- >- my $branch = $library2->{branchcode}; >- my $biblio = $builder->build_sample_biblio(); >- my $item = $builder->build_sample_item( >- { >- biblionumber => $biblio->biblionumber, >- library => $branch, >- itype => $itemtype, >- } >- ); >- >- my %a_borrower_data = ( >- firstname => 'Kyle', >- surname => 'Hall', >- categorycode => $patron_category->{categorycode}, >- branchcode => $branch, >- ); >- my $borrowernumber = Koha::Patron->new(\%a_borrower_data)->store->borrowernumber; >- my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; >- >- my $due_date = dt_from_string; >- my $issue = AddIssue( $borrower, $item->barcode, $due_date ); >- UpdateFine( >- { >- issue_id => $issue->id(), >- itemnumber => $item->itemnumber, >- borrowernumber => $borrowernumber, >- amount => .25, >- amountoutstanding => .25, >- type => q{} >- } >- ); >- >- >- my ( undef, $message ) = AddReturn( $item->barcode, $branch, undef, $due_date ); >- >- my $accountline = Koha::Account::Lines->find( { issue_id => $issue->id } ); >- is( $accountline->amountoutstanding+0, 0, 'Fee amount outstanding was reduced to 0' ); >- is( $accountline->amount+0, 0, 'Fee amount was reduced to 0' ); >-}; >- > subtest 'Filling a hold should cancel existing transfer' => sub { > plan tests => 4; > >diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t >index 5def587366..345c06e107 100644 >--- a/t/db_dependent/Circulation/Returns.t >+++ b/t/db_dependent/Circulation/Returns.t >@@ -394,4 +394,49 @@ subtest 'BranchTransferLimitsType' => sub { > is( $doreturn, 1, 'AddReturn should have checkin the item if BranchTransferLimitsType=itemtype'); > }; > >+subtest 'Backdated returns should reduce fine if needed' => sub { >+ plan tests => 3; >+ >+ t::lib::Mocks::mock_preference( "CalculateFinesOnReturn", 0 ); >+ t::lib::Mocks::mock_preference( "CalculateFinesOnBackdate", 1 ); >+ >+ my $biblio = $builder->build_object( { class => 'Koha::Biblios' } ); >+ my $item = $builder->build_object( >+ { >+ class => 'Koha::Items', >+ value => { >+ biblionumber => $biblio->biblionumber, >+ notforloan => 0, >+ itemlost => 0, >+ withdrawn => 0, >+ } >+ } >+ ); >+ my $patron = $builder->build_object({class => 'Koha::Patrons'}); >+ my $checkout = AddIssue( $patron->unblessed, $item->barcode ); >+ my $fine = Koha::Account::Line->new({ >+ issue_id => $checkout->id, >+ borrowernumber => $patron->id, >+ itemnumber => $item->id, >+ date => dt_from_string(), >+ amount => 100, >+ amountoutstanding => 100, >+ debit_type_code => 'OVERDUE', >+ status => 'UNRETURNED', >+ timestamp => dt_from_string(), >+ manager_id => undef, >+ interface => 'cli', >+ branchcode => $patron->branchcode, >+ })->store(); >+ >+ my $account = $patron->account; >+ is( $account->balance+0, 100, "Account balance before return is 100"); >+ >+ my ( $doreturn, $messages, $issue ) = AddReturn($item->barcode, undef, undef, dt_from_string('1999-01-01') ); >+ is( $account->balance+0, 0, "Account balance after return is 0"); >+ >+ $fine = $fine->get_from_storage; >+ is( $fine, undef, "Fine was removed correctly with a backdated return" ); >+}; >+ > $schema->storage->txn_rollback; >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=110640">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8338
:
104841
|
104842
|
104900
|
104901
|
105496
|
105497
|
105498
|
106152
|
106153
|
106154
|
106155
|
106209
|
106523
|
106524
|
106525
|
106526
|
106527
|
107140
| 110640