From e166f7612f68e3daeeb05e58c73360d5c32d3e82 Mon Sep 17 00:00:00 2001 From: Stefan Berndtsson Date: Thu, 30 Sep 2021 15:11:59 +0200 Subject: [PATCH] Bug 20262: Add ability to refund lost item fees without creating credits Some libraries handle refunding of paid lost fees at a financial office and not within Koha. To facilitate this, it would be good for Koha to have the option to not generate lost returned credits by skipping fully paid lost items, and only refunding the outstanding balance on partially paid lost fees. Test Plan: 1) Apply this patch 2) Set your lost item refund on return policies to 'Only if unpaid' 3) Mark an item lost, charging the lost fee 4) Return the item, a full refund should occur 5) Mark another item lost, charging the lost fee 6) Make a partial payment on this lost fee 7) Return the item 8) The remaining balance of that fee should be 0, but the patron should not recieve a credit for the already paid balance 8) Mark another item lost, charging the lost fee 9) Fully pay the lost fee 10) Return the item. The paid lost fee should be unaffected. 11) Run tests in t/db_dependent/Circulation.t Signed-off-by: Lisette Scheer Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- Koha/Item.pm | 30 ++- .../prog/en/modules/admin/smart-rules.tt | 24 +++ t/db_dependent/Circulation.t | 192 +++++++++++++++++- 3 files changed, 237 insertions(+), 9 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index f25ae42c57..04a64c1fe4 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1190,14 +1190,26 @@ sub _set_found_trigger { if ( $patron ) { my $account = $patron->account; - my $total_to_refund = 0; - # Use cases - if ( $lost_charge->amount > $lost_charge->amountoutstanding ) { + # Credit outstanding amount + my $credit_total = $lost_charge->amountoutstanding; + # Use cases + if ( + $lost_charge->amount > $lost_charge->amountoutstanding && + $lostreturn_policy ne "refund_unpaid" + ) { # some amount has been cancelled. collect the offsets that are not writeoffs # this works because the only way to subtract from this kind of a debt is # using the UI buttons 'Pay' and 'Write off' + + # We don't credit any payments if return policy is + # "refund_unpaid" + # + # In that case only unpaid/outstanding amount + # will be credited wich settles the debt without + # creating extra credits + my $credit_offsets = $lost_charge->debit_offsets( { 'credit_id' => { '!=' => undef }, @@ -1206,13 +1218,15 @@ sub _set_found_trigger { { join => 'credit' } ); - $total_to_refund = ( $credit_offsets->count > 0 ) - ? $credit_offsets->total * -1 # credits are negative on the DB - : 0; + my $total_to_refund = ( $credit_offsets->count > 0 ) ? + # credits are negative on the DB + $credit_offsets->total * -1 : + 0; + # Credit the outstanding amount, then add what has been + # paid to create a net credit for this amount + $credit_total += $total_to_refund; } - my $credit_total = $lost_charge->amountoutstanding + $total_to_refund; - my $credit; if ( $credit_total > 0 ) { my $branchcode = diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index dc0936efe2..ff0a54a56b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -1080,26 +1080,37 @@ [% IF ( current_branch == '*' ) %] [% IF ( defaultRefundRule == 'refund' ) %] + + + + + [% ELSIF ( defaultRefundRule == 'refund_unpaid' ) %] + + [% ELSIF ( defaultRefundRule == 'charge' ) %] + [% ELSIF ( defaultRefundRule == 'restore' ) %] + [% ELSIF ( defaultRefundRule == 0 ) %] + [% ELSE %] + @@ -1113,6 +1124,8 @@ [% END %] [% IF defaultRefundRule == 'refund' %] Use default (Refund lost item charge) + [% ELSIF defaultRefundRule == 'refund_unpaid' %] + Use default (Refund lost item charge (only if unpaid)) [% ELSIF defaultRefundRule == 'charge' %] Use default (Refund lost item charge and charge new overdue fine) [% ELSIF defaultRefundRule == 'restore' %] @@ -1123,27 +1136,38 @@ [% IF ( not refundLostItemFeeRule ) %] + [% ELSE %] [% IF ( refundLostItemFeeRule.rule_value == 'refund' ) %] + + [% ELSIF ( refundLostItemFeeRule.rule_value == 'refund_unpaid' ) %] + + + + + [% ELSIF ( refundLostItemFeeRule.rule_value == 'charge' ) %] + [% ELSIF ( refundLostItemFeeRule.rule_value == 'restore' ) %] + [% ELSIF ( refundLostItemFeeRule.rule_value == 0 ) %] + diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 3c69d684a0..38c8c08997 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -3191,7 +3191,7 @@ subtest 'AddReturn | is_overdue' => sub { }; subtest 'enh 23091 | Lost item return policies' => sub { - plan tests => 4; + plan tests => 5; my $manager = $builder->build_object({ class => "Koha::Patrons" }); @@ -3252,6 +3252,21 @@ subtest 'AddReturn | is_overdue' => sub { } ); + my $branchcode_refund_unpaid = + $builder->build( { source => 'Branch' } )->{branchcode}; + my $specific_rule_refund_unpaid = $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => $branchcode_refund_unpaid, + categorycode => undef, + itemtype => undef, + rule_name => 'lostreturn', + rule_value => 'refund_unpaid' + } + } + ); + my $replacement_amount = 99.00; t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); @@ -3262,6 +3277,181 @@ subtest 'AddReturn | is_overdue' => sub { t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', undef ); + subtest 'lostreturn | refund_unpaid' => sub { + plan tests => 21; + + t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $branchcode_refund_unpaid }); + + my $item = $builder->build_sample_item( + { + replacementprice => $replacement_amount + } + ); + + # Issue the item + my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + + # Mark item as lost + $item->itemlost(3)->store; + C4::Circulation::LostItem( $item->itemnumber, 1 ); + + my $lost_fee_lines = Koha::Account::Lines->search( + { + borrowernumber => $patron->id, + itemnumber => $item->itemnumber, + debit_type_code => 'LOST' + } + ); + is( $lost_fee_lines->count, 1, 'Lost item fee produced' ); + my $lost_fee_line = $lost_fee_lines->next; + is( int($lost_fee_line->amount), + $replacement_amount, 'The right LOST amount is generated' ); + is( int($lost_fee_line->amountoutstanding), + $replacement_amount, + 'The right LOST amountoutstanding is generated' ); + is( $lost_fee_line->status, undef, 'The LOST status was not set' ); + + is( + int($patron->account->balance), + $replacement_amount , + "Account balance equals the replacement amount after being charged lost fee when no payments has been made" + ); + + # Return lost item without any payments having been made + my ( $returned, $message ) = AddReturn( $item->barcode, $branchcode_refund_unpaid ); + + $lost_fee_line->discard_changes; + + is( int($lost_fee_line->amount), $replacement_amount, 'The LOST amount is left intact' ); + is( int($lost_fee_line->amountoutstanding) , 0, 'The LOST amountoutstanding is zero' ); + is( $lost_fee_line->status, 'FOUND', 'The FOUND status was set' ); + is( + int($patron->account->balance), + 0, + 'Account balance should be zero after returning item with lost fee when no payments has been made' + ); + + # Create a second item + $item = $builder->build_sample_item( + { + replacementprice => $replacement_amount + } + ); + + # Issue the item + $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + + # Mark item as lost + $item->itemlost(3)->store; + C4::Circulation::LostItem( $item->itemnumber, 1 ); + + $lost_fee_lines = Koha::Account::Lines->search( + { + borrowernumber => $patron->id, + itemnumber => $item->itemnumber, + debit_type_code => 'LOST' + } + ); + is( $lost_fee_lines->count, 1, 'Lost item fee produced' ); + $lost_fee_line = $lost_fee_lines->next; + + # Make partial payment + $patron->account->payin_amount({ + type => 'PAYMENT', + interface => 'intranet', + payment_type => 'CASH', + user_id => $patron->borrowernumber, + amount => 39.00, + lines => [$lost_fee_line] + }); + + $lost_fee_line->discard_changes; + + is( int($lost_fee_line->amountoutstanding), + 60, + 'The LOST amountoutstanding is the expected amount after partial payment of lost fee' + ); + + is( + int($patron->account->balance), + 60, + 'Account balance is the expected amount after partial payment of lost fee' + ); + + # Return lost item with partial payment having been made + ( $returned, $message ) = AddReturn( $item->barcode, $branchcode_refund_unpaid ); + + $lost_fee_line->discard_changes; + + is( int($lost_fee_line->amountoutstanding) , 0, 'The LOST amountoutstanding is zero after returning lost item with partial payment' ); + is( $lost_fee_line->status, 'FOUND', 'The FOUND status was set for lost item with partial payment' ); + is( + int($patron->account->balance), + 0, + 'Account balance should be zero after returning item with lost fee when partial payment has been made' + ); + + # Create a third item + $item = $builder->build_sample_item( + { + replacementprice => $replacement_amount + } + ); + + # Issue the item + $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + + # Mark item as lost + $item->itemlost(3)->store; + C4::Circulation::LostItem( $item->itemnumber, 1 ); + + $lost_fee_lines = Koha::Account::Lines->search( + { + borrowernumber => $patron->id, + itemnumber => $item->itemnumber, + debit_type_code => 'LOST' + } + ); + is( $lost_fee_lines->count, 1, 'Lost item fee produced' ); + $lost_fee_line = $lost_fee_lines->next; + + # Make full payment + $patron->account->payin_amount({ + type => 'PAYMENT', + interface => 'intranet', + payment_type => 'CASH', + user_id => $patron->borrowernumber, + amount => $replacement_amount, + lines => [$lost_fee_line] + }); + + $lost_fee_line->discard_changes; + + is( int($lost_fee_line->amountoutstanding), + 0, + 'The LOST amountoutstanding is the expected amount after partial payment of lost fee' + ); + + is( + int($patron->account->balance), + 0, + 'Account balance is the expected amount after partial payment of lost fee' + ); + + # Return lost item with partial payment having been made + ( $returned, $message ) = AddReturn( $item->barcode, $branchcode_refund_unpaid ); + + $lost_fee_line->discard_changes; + + is( int($lost_fee_line->amountoutstanding) , 0, 'The LOST amountoutstanding is zero after returning lost item with full payment' ); + is( $lost_fee_line->status, 'FOUND', 'The FOUND status was set for lost item iwth partial payment' ); + is( + int($patron->account->balance), + 0, + 'Account balance should be zero after returning item with lost fee when full payment has been made' + ); + }; + subtest 'lostreturn | false' => sub { plan tests => 12; -- 2.39.5