Bug 20815: Add ability to choose if lost fee is refunded based on length of time...

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 <DStephen@dubuque.lib.ia.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 4fa208f..34fe735 100644 (file)
--- 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 (file)
index 0000000..50c7ef5
--- /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 e73912d..ded9537 100644 (file)
--- a/installer/data/mysql/sysprefs.sql
+++ b/installer/data/mysql/sysprefs.sql
@@ -333,6 +333,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 b89fd8f..29c076d 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -942,6 +942,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 bd0b9a2..2be657d 100755 (executable)
--- 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 => 47;
+use Test::More tests => 51;
 use Test::MockModule;
 use Test::Deep qw( cmp_deeply );
 
@@ -4062,6 +4062,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();
 $branches = Koha::Libraries->search();