From 30b372c522008cc6995c4b73b0fbc8037bac5bdf Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 31 Oct 2022 18:38:04 +0000 Subject: [PATCH] Bug 30254: [22.05.x] 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 Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Bug 30254: Unit tests Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Bug 30254: (QA follow-up) Remove warn from tests Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall https://bugs.koha-community.org/show_bug.cgi?id=32054 --- Koha/Item.pm | 92 +++++++++++++++++++------------------- t/db_dependent/Koha/Item.t | 10 ++++- 2 files changed, 54 insertions(+), 48 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 9384b11670..fecc718c56 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1187,58 +1187,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', diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index c64e83b761..2a895e5caa 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -1206,7 +1206,7 @@ subtest 'store() tests' => sub { subtest '_set_found_trigger() tests' => sub { - plan tests => 6; + plan tests => 7; $schema->storage->txn_begin; @@ -1253,6 +1253,14 @@ subtest 'store() tests' => sub { is( $message_2->message, 'lost_charge', 'message is correct' ); is( $message_2->payload, undef, 'no payload' ); + + # Let's build a new item + $item = $builder->build_sample_item({ itemlost => 1, itemlost_on => dt_from_string() }); + $item->set( { itemlost => 0 } )->store; + + $messages = $item->object_messages; + is( scalar @{$messages}, 0, 'This item has no history, no associated lost fines, presumed not lost by patron, no messages returned'); + $schema->storage->txn_rollback; }; -- 2.30.2