From 087f0ee4a9a7cf9552d193d9810738f9decaa7d4 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 21 Feb 2018 11:10:02 -0500 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 ocurr 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. Signed-off-by: Kyle M Hall Signed-off-by: Martha Fuerst --- C4/Circulation.pm | 45 ++++++++------- .../prog/en/modules/admin/smart-rules.tt | 29 ++++++++-- t/db_dependent/Circulation.t | 67 +++++++++++++++++++++- t/db_dependent/RefundLostItemFeeRule.t | 8 +-- 4 files changed, 117 insertions(+), 32 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 014c018628..839c4d693d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1436,19 +1436,18 @@ sub AddIssue { ## If item was lost, it has now been found, reverse any list item charges if necessary. if ( $item_object->itemlost ) { - if ( - Koha::RefundLostItemFeeRules->should_refund( + my $should_refund = Koha::RefundLostItemFeeRules->should_refund( { - current_branch => C4::Context->userenv->{branch}, - item_home_branch => $item_object->homebranch, + current_branch => C4::Context->userenv->{branch}, + item_home_branch => $item_object->homebranch, item_holding_branch => $item_object->holdingbranch, } - ) - ) - { - _FixAccountForLostAndReturned( $item_object->itemnumber, undef, - $item_object->barcode ); - } + ); + + if ($should_refund) { + _FixAccountForLostAndReturned( $item_object->itemnumber, + undef , $item_object->barcode, $should_refund eq '2' ); + } } ModItem( @@ -2008,18 +2007,17 @@ sub AddReturn { if ( $item->itemlost ) { $messages->{'WasLost'} = 1; unless ( C4::Context->preference("BlockReturnOfLostItems") ) { - if ( - Koha::RefundLostItemFeeRules->should_refund( - { - current_branch => C4::Context->userenv->{branch}, - item_home_branch => $item->homebranch, - item_holding_branch => $item_holding_branch - } - ) - ) - { + my $should_refund = Koha::RefundLostItemFeeRules->should_refund( + { + current_branch => C4::Context->userenv->{branch}, + item_home_branch => $item->homebranch, + item_holding_branch => $item_holding_branch + } + ); + + if ($should_refund) { _FixAccountForLostAndReturned( $item->itemnumber, - $borrowernumber, $barcode ); + $borrowernumber, $barcode, $should_refund eq '2' ); $messages->{'LostItemFeeRefunded'} = 1; } } @@ -2405,6 +2403,7 @@ sub _FixAccountForLostAndReturned { my $itemnumber = shift or return; my $borrowernumber = @_ ? shift : undef; my $item_id = @_ ? shift : $itemnumber; # Send the barcode if you want that logged in the description + my $skip_paid = @_ ? shift : 0; my $credit; @@ -2424,6 +2423,8 @@ sub _FixAccountForLostAndReturned { my $accountline = $accountlines->next; my $total_to_refund = 0; + return if $skip_paid && $accountline->amountoutstanding <= 0; + return unless $accountline->borrowernumber; my $patron = Koha::Patrons->find( $accountline->borrowernumber ); return unless $patron; # Patron has been deleted, nobody to credit the return to @@ -2447,7 +2448,7 @@ sub _FixAccountForLostAndReturned { : 0; } - my $credit_total = $accountline->amountoutstanding + $total_to_refund; + my $credit_total = $accountline->amountoutstanding + ( $skip_paid ? 0 : $total_to_refund ); if ( $credit_total > 0 ) { my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; 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 ed8bc3034e..af257d15e3 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 @@ -639,20 +639,28 @@ diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index b7dca93851..0b29e3e770 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -2078,7 +2078,7 @@ subtest 'AddReturn | is_overdue' => sub { subtest '_FixAccountForLostAndReturned' => sub { - plan tests => 5; + plan tests => 14; t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); t::lib::Mocks::mock_preference( 'WhenLostForgiveFine', 0 ); @@ -2464,6 +2464,71 @@ subtest '_FixAccountForLostAndReturned' => sub { my $manual_debit = Koha::Account::Lines->search({ borrowernumber => $patron->id, accounttype => 'OVERDUE', status => 'UNRETURNED' })->next; is( $manual_debit->amountoutstanding + 0, $manual_debit_amount - $payment_amount, 'reconcile_balance was called' ); }; + + + is( $accountline->amountoutstanding, '0.000000', 'Lost fee has no outstanding amount' ); + is( $accountline->accounttype, 'LR', 'Lost fee now has account type of LR ( Lost Returned )' ); + is( $patron->account->balance, 0, 'Account balance should be 0 after return of lost item' ); + + # Tests for only if unpaid + ## Unpaid + $accountline->delete(); + $accountline = Koha::Account::Line->new( + { + borrowernumber => $patron->borrowernumber, + accounttype => 'L', + itemnumber => $itemnumber, + amount => 99.00, + amountoutstanding => 99.00, + } + )->store(); + + C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber, undef, 1 ); + + $accountline->_result()->discard_changes(); + + is( $accountline->amountoutstanding, '0.000000', 'Lost fee has no outstanding amount, with skip_paid = 1 and unpaid' ); + is( $accountline->accounttype, 'LR', 'Lost fee now has account type of LR ( Lost Returned ), with skip_paid = 1 and unpaid'); + is( $patron->account->balance, 0, 'Account balance should be 0 after return of unpaid lost item' ); + + ## Paid + $accountline->delete(); + $accountline = Koha::Account::Line->new( + { + borrowernumber => $patron->borrowernumber, + accounttype => 'L', + itemnumber => $itemnumber, + amount => 99.00, + amountoutstanding => 0.00, + } + )->store(); + + C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber, undef, 1 ); + + $accountline->_result()->discard_changes(); + + is( $accountline->amountoutstanding, '0.000000', 'Lost fee still has outstanding amount of 0, with skip_paid = 1 and already paid' ); + is( $accountline->accounttype, 'L', 'Lost fee now has account type of L ( Lost ), with skip_paid = 1 and already paid'); + is( $patron->account->balance, 0, 'Account balance should be 0 after return of partially paid lost item' ); + + ## Partially paid + $accountline->delete(); + $accountline = Koha::Account::Line->new( + { + borrowernumber => $patron->borrowernumber, + accounttype => 'L', + itemnumber => $itemnumber, + amount => 99.00, + amountoutstanding => 33.00, + } + )->store(); + + C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber, undef, 1 ); + + $accountline->_result()->discard_changes(); + + is( $accountline->amountoutstanding, '0.000000', 'Lost fee has no outstanding amount, with skip_paid = 1 and partially paid' ); + is( $accountline->accounttype, 'LR', 'Lost fee now has account type of L ( Lost ), with skip_paid = 1 and partially paid'); }; subtest '_FixOverduesOnReturn' => sub { diff --git a/t/db_dependent/RefundLostItemFeeRule.t b/t/db_dependent/RefundLostItemFeeRule.t index 4599165961..548afcd8ee 100755 --- a/t/db_dependent/RefundLostItemFeeRule.t +++ b/t/db_dependent/RefundLostItemFeeRule.t @@ -181,7 +181,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { categorycode => undef, itemtype => undef, rule_name => 'refund', - rule_value => 1, + rule_value => 2, } } ); @@ -215,7 +215,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { # Delete specific rules Koha::RefundLostItemFeeRules->find({ branchcode => $specific_rule_false->{ branchcode } })->delete; is( Koha::RefundLostItemFeeRules->_effective_branch_rule( $specific_rule_false->{ branchcode } ), - 1,'No specific rule defined, fallback to global (true)'); + 2,'No specific rule defined, fallback to global (true)'); # Rollback transaction $schema->storage->txn_rollback; @@ -319,7 +319,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { categorycode => undef, itemtype => undef, rule_name => 'refund', - rule_value => 1 + rule_value => 2 } } ); @@ -356,7 +356,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); is( Koha::RefundLostItemFeeRules->should_refund( $params ), - 1,'Specific rule is applied (true)'); + 2,'Specific rule is applied (true)'); t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHomeBranch' ); is( Koha::RefundLostItemFeeRules->should_refund( $params ), -- 2.11.0