From 03fa734ea2b77e52f1d97c8121c12bea94c10d52 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 Signed-off-by: Katrin Fischer --- C4/Circulation.pm | 32 ++- installer/data/mysql/atomicupdate/bug_20815.perl | 10 + installer/data/mysql/sysprefs.sql | 1 + .../en/modules/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 67a986b417..e1d6ff41e8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1456,11 +1456,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, } ) @@ -2040,7 +2053,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 d67a02b367..2bebabc5e2 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 f2379e797e..897db1c6c1 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 @@ -937,6 +937,11 @@ Circulation: 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: yes: Charge 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.11.0