From 38b25d635bb04a49d224a35dff08fb56892062b9 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 --- 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 5760ebc05c..05f2c13a85 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1118,14 +1118,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 }, @@ -1134,13 +1146,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 62e2f5639a..6a6371945c 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 @@ -974,26 +974,37 @@ [% IF ( current_branch == '*' ) %] [% IF ( defaultRefundRule == 'refund' ) %] + + + + + [% ELSIF ( defaultRefundRule == 'refund_unpaid' ) %] + + [% ELSIF ( defaultRefundRule == 'charge' ) %] + [% ELSIF ( defaultRefundRule == 'restore' ) %] + [% ELSIF ( defaultRefundRule == 0 ) %] + [% ELSE %] + @@ -1007,6 +1018,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 restore overdue fine) [% ELSIF defaultRefundRule == 'restore' %] @@ -1017,27 +1030,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 cf54a4f720..a242a83186 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -3138,7 +3138,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" }); @@ -3199,6 +3199,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 ); @@ -3209,6 +3224,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.30.2