Bugzilla – Attachment 194457 Details for
Bug 29923
Do not generate overpayment refund from writeoff of fine
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29923: Prevent overpayment credit when written-off accruing fine is adjusted on check-in
a3b31ea.patch (text/plain), 8.17 KB, created by
Martin Renvoize (ashimema)
on 2026-03-04 16:05:08 UTC
(
hide
)
Description:
Bug 29923: Prevent overpayment credit when written-off accruing fine is adjusted on check-in
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-03-04 16:05:08 UTC
Size:
8.17 KB
patch
obsolete
>From a3b31ea66a8816f99bb1445777fb1b059d571836 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Wed, 4 Mar 2026 16:03:40 +0000 >Subject: [PATCH] Bug 29923: Prevent overpayment credit when written-off > accruing fine is adjusted on check-in > >When a staff member writes off an accruing fine and the item is >subsequently checked in (backdated or in fine-free mode), the fine >amount is recalculated downward via adjust(). The previous code >unconditionally created an OVERPAYMENT credit whenever the adjusted >outstanding went negative, regardless of whether the existing credits >were reversible payments or non-reversible writeoffs/discounts. > >This caused patrons to receive an erroneous credit equal to the >written-off amount, since writeoffs should simply be absorbed when >the underlying fine shrinks - no refund is due. > >The fix calculates the total non-reversible credits (writeoffs, >discounts, cancellations) applied to the debit using the existing >filter_by_non_reversible method, and only generates an OVERPAYMENT >credit for the portion attributable to reversible cash payments that >exceed the recalculated fine amount. > >Test plan: >1. Write off an accruing fine in full >2. Check the item in (backdated or fine-free) >3. Confirm no overpayment credit is generated for the patron >--- > Koha/Account/Line.pm | 35 +++++++--- > t/db_dependent/Koha/Account/Line.t | 108 ++++++++++++++++++++++++++++- > 2 files changed, 133 insertions(+), 10 deletions(-) > >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index f44761fcf48..807a54af8e4 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -848,15 +848,32 @@ sub adjust { > > # Catch cases that require patron refunds > if ( $new_outstanding < 0 ) { >- my $account = Koha::Patrons->find( $self->borrowernumber )->account; >- my $credit = $account->add_credit( >- { >- amount => $new_outstanding * -1, >- type => 'OVERPAYMENT', >- interface => $interface, >- ( $update_type eq 'overdue_update' ? ( item_id => $self->itemnumber ) : () ), >- } >- ); >+ >+ # Only generate an OVERPAYMENT credit for reversible payments >+ # (e.g. cash payments). Non-reversible credits such as writeoffs, >+ # discounts and cancellations should simply be absorbed when the >+ # fine is adjusted downward - they must not produce a refund >+ # credit for the patron. >+ my $total_non_reversible = 0; >+ if ( my $debit_offsets = $self->debit_offsets ) { >+ my $nr_offsets = $debit_offsets->filter_by_non_reversible; >+ $total_non_reversible += abs( $_->amount ) for $nr_offsets->as_list; >+ } >+ my $total_applied = $amount_before - $amount_outstanding_before; >+ my $total_reversible = $total_applied - $total_non_reversible; >+ my $overpayment_amount = $total_reversible - $amount; >+ >+ if ( $overpayment_amount > 0 ) { >+ my $account = Koha::Patrons->find( $self->borrowernumber )->account; >+ $account->add_credit( >+ { >+ amount => $overpayment_amount, >+ type => 'OVERPAYMENT', >+ interface => $interface, >+ ( $update_type eq 'overdue_update' ? ( item_id => $self->itemnumber ) : () ), >+ } >+ ); >+ } > $new_outstanding = 0; > } > >diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t >index 7655fb92d10..8fa9d016533 100755 >--- a/t/db_dependent/Koha/Account/Line.t >+++ b/t/db_dependent/Koha/Account/Line.t >@@ -20,7 +20,7 @@ > use Modern::Perl; > > use Test::NoWarnings; >-use Test::More tests => 16; >+use Test::More tests => 17; > use Test::Exception; > use Test::MockModule; > >@@ -726,6 +726,112 @@ subtest 'adjust() tests' => sub { > $schema->storage->txn_rollback; > }; > >+subtest 'adjust() tests (writeoff does not generate overpayment credit)' => sub { >+ >+ plan tests => 12; >+ >+ $schema->storage->txn_begin; >+ >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $account = $patron->account; >+ >+ # Scenario 1: Full writeoff then fine adjusted to 0 - no OVERPAYMENT credit >+ my $debit_1 = Koha::Account::Line->new( >+ { >+ borrowernumber => $patron->id, >+ debit_type_code => "OVERDUE", >+ status => "UNRETURNED", >+ amount => 100, >+ amountoutstanding => 100, >+ interface => 'commandline', >+ } >+ )->store; >+ >+ my $lines_count_before = $account->lines->count; >+ >+ # Write off the full fine >+ $account->pay( >+ { >+ amount => 100, >+ type => 'WRITEOFF', >+ lines => [$debit_1], >+ interface => 'commandline', >+ } >+ ); >+ $debit_1->discard_changes; >+ is( $debit_1->amountoutstanding * 1, 0, 'Fine fully written off' ); >+ >+ # Simulate check-in recalculating fine to 0 (backdated or fine-free) >+ $debit_1->adjust( { amount => 0, type => 'overdue_update', interface => 'commandline' } )->discard_changes; >+ >+ is( $debit_1->amount * 1, 0, 'Fine amount adjusted to 0' ); >+ is( $debit_1->amountoutstanding * 1, 0, 'Fine amountoutstanding remains 0' ); >+ >+ # No new OVERPAYMENT credit should have been generated >+ is( >+ $account->lines->count, >+ $lines_count_before + 1, # only the WRITEOFF credit, no OVERPAYMENT >+ 'No OVERPAYMENT credit generated when fine adjusted after writeoff' >+ ); >+ isnt( >+ $account->lines->last->credit_type_code, >+ 'OVERPAYMENT', >+ 'Last account line is not an OVERPAYMENT credit' >+ ); >+ >+ # Scenario 2: Partial payment + partial writeoff, fine drops below payment amount >+ # => OVERPAYMENT only for the payment portion >+ my $debit_2 = Koha::Account::Line->new( >+ { >+ borrowernumber => $patron->id, >+ debit_type_code => "OVERDUE", >+ status => "UNRETURNED", >+ amount => 100, >+ amountoutstanding => 100, >+ interface => 'commandline', >+ } >+ )->store; >+ >+ # Apply a $60 writeoff >+ $account->pay( >+ { >+ amount => 60, >+ type => 'WRITEOFF', >+ lines => [$debit_2], >+ interface => 'commandline', >+ } >+ ); >+ $debit_2->discard_changes; >+ is( $debit_2->amountoutstanding * 1, 40, 'Fine outstanding after writeoff' ); >+ >+ # Apply a $30 cash payment >+ my $payment = $account->add_credit( { amount => 30, type => 'PAYMENT', interface => 'commandline' } ); >+ my $debits = Koha::Account::Lines->search( { accountlines_id => $debit_2->id } ); >+ $payment->apply( { debits => [ $debits->as_list ] } ); >+ $debit_2->discard_changes; >+ is( $debit_2->amountoutstanding * 1, 10, 'Fine outstanding after payment' ); >+ >+ my $lines_count_before_2 = $account->lines->count; >+ >+ # Simulate check-in recalculating fine to $0 (below the $30 payment) >+ $debit_2->adjust( { amount => 0, type => 'overdue_update', interface => 'commandline' } )->discard_changes; >+ >+ is( $debit_2->amount * 1, 0, 'Fine amount adjusted to 0' ); >+ is( $debit_2->amountoutstanding * 1, 0, 'Fine amountoutstanding remains 0' ); >+ >+ # OVERPAYMENT credit should be generated only for the reversible payment amount ($30) >+ is( >+ $account->lines->count, >+ $lines_count_before_2 + 1, >+ 'One OVERPAYMENT credit generated (for cash payment only)' >+ ); >+ my $overpayment = $account->lines->last; >+ is( $overpayment->amount * 1, -30, 'OVERPAYMENT credit is for payment amount only, not writeoff' ); >+ is( $overpayment->credit_type_code, 'OVERPAYMENT', 'Credit type is OVERPAYMENT' ); >+ >+ $schema->storage->txn_rollback; >+}; >+ > subtest 'checkout() tests' => sub { > plan tests => 7; > >-- >2.53.0 >
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 29923
:
194457
|
194555
|
194556