From b0427c0430743354325e868b0b107e4f98a9db26 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 12 Feb 2020 12:55:51 -0500 Subject: [PATCH] Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost This adds the ability to not refund lost item fees on return if the item has been lost for more than a given number of days. Test Plan: 1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days 2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded 3) Return the item 4) Note no refund on the lost item fee was processed, the fee remains unchanged 5) prove t/db_dependent/Circulation.t Signed-off-by: Deb Stephenson --- C4/Circulation.pm | 32 +- .../data/mysql/atomicupdate/bug_20815.perl | 10 + installer/data/mysql/sysprefs.sql | 1 + .../admin/preferences/circulation.pref | 5 + t/db_dependent/Circulation.t | 302 +++++++++++++++++- 5 files changed, 346 insertions(+), 4 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_20815.perl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e439250933..f8b356bb72 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1447,11 +1447,24 @@ sub AddIssue { ## If item was lost, it has now been found, reverse any list item charges if necessary. if ( $item_object->itemlost ) { + my $refund = 1; + my $no_refund_after_days = C4::Context->preference('NoRefundOnLostReturnedItemsAge'); + if ($no_refund_after_days) { + my $today = dt_from_string(); + my $lost_age_in_days = + dt_from_string( $item_object->itemlost_on ) + ->delta_days($today) + ->in_units('days'); + + $refund = 0 unless ( $lost_age_in_days < $no_refund_after_days ); + } + if ( - Koha::RefundLostItemFeeRules->should_refund( + $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, } ) @@ -2031,7 +2044,20 @@ sub AddReturn { if ( $item->itemlost ) { $messages->{'WasLost'} = 1; unless ( C4::Context->preference("BlockReturnOfLostItems") ) { + my $refund = 1; + my $no_refund_after_days = C4::Context->preference('NoRefundOnLostReturnedItemsAge'); + if ($no_refund_after_days) { + my $today = dt_from_string(); + my $lost_age_in_days = + dt_from_string( $item->itemlost_on ) + ->delta_days($today) + ->in_units('days'); + + $refund = 0 unless ( $lost_age_in_days < $no_refund_after_days ); + } + if ( + $refund && Koha::RefundLostItemFeeRules->should_refund( { current_branch => C4::Context->userenv->{branch}, diff --git a/installer/data/mysql/atomicupdate/bug_20815.perl b/installer/data/mysql/atomicupdate/bug_20815.perl new file mode 100644 index 0000000000..50c7ef54ce --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20815.perl @@ -0,0 +1,10 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do(q{ + INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('NoRefundOnLostReturnedItemsAge','','','Do not refund lost item fees if item is lost for more than this number of days','Integer') + }); + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug XXXXX - description)\n"; +} diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 4ad73ef827..08d193b8aa 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -332,6 +332,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('noissuescharge','5','','Define maximum amount withstanding before check outs are blocked','Integer'), ('NoIssuesChargeGuarantees','','','Define maximum amount withstanding before check outs are blocked','Integer'), ('noItemTypeImages','0',NULL,'If ON, disables itemtype images in the staff interface','YesNo'), +('NoRefundOnLostReturnedItemsAge','','','Do not refund lost item fees if item is lost for more than this number of days','Integer'), ('NoRenewalBeforePrecision','exact_time','date|exact_time','Calculate "No renewal before" based on date only or exact time of due date','Choice'), ('NotesBlacklist','',NULL,'List of notes fields that should not appear in the title notes/description separator of details','free'), ('NotHighlightedWords','and|or|not',NULL,'List of words to NOT highlight when OpacHitHighlight is enabled','free'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 92137896dc..b34cacf89b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -930,6 +930,11 @@ Circulation: yes: Forgive no: "Don't Forgive" - the fines on an item when it is lost. + - + - "Don't refund lost fees if a lost item is returned more than" + - pref: NoRefundOnLostReturnedItemsAge + class: integer + - days after it was marked lost. - - pref: WhenLostChargeReplacementFee choices: diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index e9f28b55d9..667a97f10f 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 48; +use Test::More tests => 52; use Test::MockModule; use Test::Deep qw( cmp_deeply ); @@ -3926,6 +3926,306 @@ subtest 'Filling a hold should cancel existing transfer' => sub { is( Koha::Item::Transfers->search({ itemnumber => $item->itemnumber, datearrived => undef })->count, 0, "No outstanding transfers when hold is waiting"); }; +subtest 'Tests for NoRefundOnLostReturnedItemsAge = undef' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', undef ); + + my $lost_on = dt_from_string->subtract( days => 7 )->date; + + my $library = $builder->build( { source => 'Branch' } ); + my $patron = $builder->build( + { + source => 'Borrower', + value => { categorycode => $patron_category->{categorycode} } + } + ); + + my $biblionumber = $builder->build_sample_biblio( + { + branchcode => $library->{branchcode}, + } + )->biblionumber; + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->{branchcode}, + replacementprice => '42.00', + } + ); + + # And the circulation rule + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + issuelength => 14, + lengthunit => 'days', + } + } + ); + $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'refund', + rule_value => 1 + } + } + ); + + # Test with NoRefundOnLostReturnedItemsAge disabled + my $issue = AddIssue( $patron, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->find( + { + itemnumber => $item->id, + borrowernumber => $patron->{borrowernumber} + } + ); + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); + $a = Koha::Account::Lines->find( $a->id ); + is( $a->amountoutstanding, '0.000000', "Lost fee was refunded" ); +}; + +subtest 'Tests for NoRefundOnLostReturnedItemsAge > length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 6 )->date; + + my $library = $builder->build( { source => 'Branch' } ); + my $patron = $builder->build( + { + source => 'Borrower', + value => { categorycode => $patron_category->{categorycode} } + } + ); + + my $biblionumber = $builder->build_sample_biblio( + { + branchcode => $library->{branchcode}, + } + )->biblionumber; + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->{branchcode}, + replacementprice => '42.00', + } + ); + + # And the circulation rule + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + issuelength => 14, + lengthunit => 'days', + } + } + ); + $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'refund', + rule_value => 1 + } + } + ); + + # Test with NoRefundOnLostReturnedItemsAge disabled + my $issue = AddIssue( $patron, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->find( + { + itemnumber => $item->id, + borrowernumber => $patron->{borrowernumber} + } + ); + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); + $a = Koha::Account::Lines->find( $a->id ); + is( $a->amountoutstanding, '0.000000', "Lost fee was refunded" ); +}; + +subtest 'Tests for NoRefundOnLostReturnedItemsAge = length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 7 )->date; + + my $library = $builder->build( { source => 'Branch' } ); + my $patron = $builder->build( + { + source => 'Borrower', + value => { categorycode => $patron_category->{categorycode} } + } + ); + + my $biblionumber = $builder->build_sample_biblio( + { + branchcode => $library->{branchcode}, + } + )->biblionumber; + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->{branchcode}, + replacementprice => '42.00', + } + ); + + # And the circulation rule + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + issuelength => 14, + lengthunit => 'days', + } + } + ); + $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'refund', + rule_value => 1 + } + } + ); + + # Test with NoRefundOnLostReturnedItemsAge disabled + my $issue = AddIssue( $patron, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->find( + { + itemnumber => $item->id, + borrowernumber => $patron->{borrowernumber} + } + ); + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); + $a = Koha::Account::Lines->find( $a->id ); + is( $a->amountoutstanding, '42.000000', "Lost fee was not refunded" ); +}; + +subtest 'Tests for NoRefundOnLostReturnedItemsAge < length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 8 )->date; + + my $library = $builder->build( { source => 'Branch' } ); + my $patron = $builder->build( + { + source => 'Borrower', + value => { categorycode => $patron_category->{categorycode} } + } + ); + + my $biblionumber = $builder->build_sample_biblio( + { + branchcode => $library->{branchcode}, + } + )->biblionumber; + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->{branchcode}, + replacementprice => '42.00', + } + ); + + # And the circulation rule + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + issuelength => 14, + lengthunit => 'days', + } + } + ); + $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'refund', + rule_value => 1 + } + } + ); + + # Test with NoRefundOnLostReturnedItemsAge disabled + my $issue = AddIssue( $patron, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->find( + { + itemnumber => $item->id, + borrowernumber => $patron->{borrowernumber} + } + ); + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); + $a = Koha::Account::Lines->find( $a->id ); + is( $a->amountoutstanding, '42.000000', "Lost fee was not refunded" ); +}; + $schema->storage->txn_rollback; C4::Context->clear_syspref_cache(); $cache->clear_from_cache('single_holidays'); -- 2.24.2 (Apple Git-127)