From 82836b0ae0e1d4d152743a396bbe2002fc416d73 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. --- C4/Circulation.pm | 26 +++++----- .../prog/en/modules/admin/smart-rules.tt | 31 +++++++++-- t/db_dependent/Circulation.t | 60 +++++++++++++++++++++- t/db_dependent/RefundLostItemFeeRule.t | 8 +-- 4 files changed, 102 insertions(+), 23 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 4b39a2d..3709b40 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1374,18 +1374,16 @@ sub AddIssue { ## If item was lost, it has now been found, reverse any list item charges if necessary. if ( $item->{'itemlost'} ) { - if ( - Koha::RefundLostItemFeeRules->should_refund( - { - current_branch => C4::Context->userenv->{branch}, - item_home_branch => $item->{homebranch}, - item_holding_branch => $item->{holdingbranch} - } - ) - ) - { - _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, - $item->{'barcode'} ); + my $should_refund = Koha::RefundLostItemFeeRules->should_refund( + { + current_branch => C4::Context->userenv->{branch}, + item_home_branch => $item->{homebranch}, + item_holding_branch => $item->{holdingbranch} + } + ); + + if ($should_refund) { + _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'}, $should_refund == 2 ); } } @@ -2366,6 +2364,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; # check for charge made for lost book my $accountline = Koha::Account::Lines->search( @@ -2380,6 +2379,7 @@ sub _FixAccountForLostAndReturned { return unless $accountline; return if $accountline->accounttype eq 'W'; # Written off + return if $skip_paid && $accountline->amountoutstanding <= 0; $accountline->accounttype('LR'); $accountline->store(); @@ -2387,7 +2387,7 @@ sub _FixAccountForLostAndReturned { my $account = Koha::Account->new( { patron_id => $accountline->borrowernumber } ); my $credit_id = $account->pay( { - amount => $accountline->amount, + amount => $skip_paid ? $accountline->amountoutstanding : $accountline->amount, description => "Item Returned " . $item_id, account_type => 'CR', offset_type => 'Lost Item Return', 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 e1a5839..edf2885 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 @@ -534,20 +534,29 @@ diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index f4ff177..48aba76 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -1827,7 +1827,7 @@ subtest 'AddReturn | is_overdue' => sub { }; subtest '_FixAccountForLostAndReturned' => sub { - plan tests => 2; + plan tests => 8; # Generate test biblio my $title = 'Koha for Dummies'; @@ -1865,6 +1865,64 @@ subtest '_FixAccountForLostAndReturned' => sub { 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 )'); + + # 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'); + + ## 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'); + + ## 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 84a373a..c6057bd 100755 --- a/t/db_dependent/RefundLostItemFeeRule.t +++ b/t/db_dependent/RefundLostItemFeeRule.t @@ -130,7 +130,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { source => 'RefundLostItemFeeRule', value => { branchcode => '*', - refund => 1 + refund => 2 } }); my $specific_rule_false = $builder->build({ @@ -153,7 +153,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; @@ -243,7 +243,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { my $specific_rule_true = $builder->build({ source => 'RefundLostItemFeeRule', value => { - refund => 1 + refund => 2 } }); # Make sure we have an unused branchcode @@ -264,7 +264,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.10.2