From d6c4fa251de53a0b2a1295b54a5ecddbf117b6bc Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 31 Oct 2022 18:38:04 +0000 Subject: [PATCH] Bug 30254: Don't charge overdue fines unless some fine exists We need to determine if a book was lost by a patron, the clues we have are previous charges. If we don't find any, we shouldn't charge a new fine To test: 1 - set Lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine", turn on FinesMode, make sure your circ rules charge fines 2 - have an itemtype / patron combo that charges an overdue fine 3 - check item out (with a due date in the future) and then right back in again 4 - confirm patron doesn't have a fine because the item was not late 5 - set the item to Lost 6 - in the database, edit the date_due of your checkout to a date in the past 7 - check the item in, it is marked found 8 - confirm your patron now has a fine 9 - Apply patch 10 - Repeat with a new item and patron 11 - Confirm no charges --- Koha/Item.pm | 92 +++++++++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 3ef1e04194..402cb716f3 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1264,58 +1264,56 @@ sub _set_found_trigger { } } - # restore fine for lost book - if ( $lostreturn_policy eq 'restore' ) { - my $lost_overdue = Koha::Account::Lines->search( - { - itemnumber => $self->itemnumber, - debit_type_code => 'OVERDUE', - status => 'LOST' - }, - { - order_by => { '-desc' => 'date' }, - rows => 1 - } - )->single; - - if ( $lost_overdue ) { - - my $patron = $lost_overdue->patron; - if ($patron) { - my $account = $patron->account; + # possibly restore fine for lost book + my $lost_overdue = Koha::Account::Lines->search( + { + itemnumber => $self->itemnumber, + debit_type_code => 'OVERDUE', + status => 'LOST' + }, + { + order_by => { '-desc' => 'date' }, + rows => 1 + } + )->single; + if ( $lostreturn_policy eq 'restore' && $lost_overdue ) { - # Update status of fine - $lost_overdue->status('FOUND')->store(); + my $patron = $lost_overdue->patron; + if ($patron) { + my $account = $patron->account; - # Find related forgive credit - my $refund = $lost_overdue->credits( + # Update status of fine + $lost_overdue->status('FOUND')->store(); + + # Find related forgive credit + my $refund = $lost_overdue->credits( + { + credit_type_code => 'FORGIVEN', + itemnumber => $self->itemnumber, + status => [ { '!=' => 'VOID' }, undef ] + }, + { order_by => { '-desc' => 'date' }, rows => 1 } + )->single; + + if ( $refund ) { + # Revert the forgive credit + $refund->void({ interface => 'trigger' }); + $self->add_message( { - credit_type_code => 'FORGIVEN', - itemnumber => $self->itemnumber, - status => [ { '!=' => 'VOID' }, undef ] - }, - { order_by => { '-desc' => 'date' }, rows => 1 } - )->single; - - if ( $refund ) { - # Revert the forgive credit - $refund->void({ interface => 'trigger' }); - $self->add_message( - { - type => 'info', - message => 'lost_restored', - payload => { refund_id => $refund->id } - } - ); - } - - # Reconcile balances if required - if ( C4::Context->preference('AccountAutoReconcile') ) { - $account->reconcile_balance; - } + type => 'info', + message => 'lost_restored', + payload => { refund_id => $refund->id } + } + ); + } + + # Reconcile balances if required + if ( C4::Context->preference('AccountAutoReconcile') ) { + $account->reconcile_balance; } } - } elsif ( $lostreturn_policy eq 'charge' ) { + + } elsif ( $lostreturn_policy eq 'charge' && ( $lost_overdue || $lost_charge ) ) { $self->add_message( { type => 'info', -- 2.30.2