From a3b31ea66a8816f99bb1445777fb1b059d571836 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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