Bug 8365: Add unit tests and fix QA issues

This patch adds some unit tests for CalcDateDue and GetLoanLength

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.

Tests done:
- Checked update works correctly for existing circulation rules.
- Adding, deleting and overwriting circulation rules works.
- Renewals work for different circulation rules and changes
  to the holiday calendar.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

C4/Circulation.pm

@@ -1345,9 +1345,9 @@ sub GetLoanLength {
 
     # try to find issuelength & return the 1st available.
     # check with borrowertype, itemtype and branchcode, then without one of those parameters
-
     $sth->execute( $borrowertype, $itemtype, $branchcode );
     my $loanlength = $sth->fetchrow_hashref;
+
     return $loanlength
       if defined($loanlength) && $loanlength->{issuelength};
 
@@ -2421,8 +2421,6 @@ END_SQL
 
 Find out whether a borrowed item may be renewed.
 
-C<$dbh> is a DBI handle to the Koha database.
-
 C<$borrowernumber> is the borrower number of the patron who currently
 has the item on loan.
 
@@ -2432,7 +2430,7 @@ C<$override_limit>, if supplied with a true value, causes
 the limit on the number of times that the loan can be renewed
 (as controlled by the item type) to be ignored.
 
-C<$CanBookBeRenewed> returns a true value iff the item may be renewed. The
+C<$CanBookBeRenewed> returns a true value if the item may be renewed. The
 item must currently be on loan to the specified borrower; renewals
 must be allowed for the item's type; and the borrower must not have
 already renewed the loan. $error will contain the reason the renewal can not proceed
@@ -2994,6 +2992,7 @@ C<$startdate>   = C4::Dates object representing start date of loan period (assum
 C<$itemtype>  = itemtype code of item in question
 C<$branch>  = location whose calendar to use
 C<$borrower> = Borrower object
+C<$isrenewal> = Boolean: is true if we want to calculate the date due for a renewal. Else is false.
 
 =cut
 
@@ -3011,22 +3010,29 @@ sub CalcDateDue {
             : qq{issuelength};
 
     my $datedue;
+    if ( $startdate ) {
+        if (ref $startdate ne 'DateTime' ) {
+            $datedue = dt_from_string($datedue);
+        } else {
+            $datedue = $startdate->clone;
+        }
+    } else {
+        $datedue =
+          DateTime->now( time_zone => C4::Context->tz() )
+          ->truncate( to => 'minute' );
+    }
+
 
     # calculate the datedue as normal
     if ( C4::Context->preference('useDaysMode') eq 'Days' )
     {    # ignoring calendar
-        my $dt =
-          DateTime->now( time_zone => C4::Context->tz() )
-          ->truncate( to => 'minute' );
         if ( $loanlength->{lengthunit} eq 'hours' ) {
-            $dt->add( hours => $loanlength->{$length_key} );
+            $datedue->add( hours => $loanlength->{$length_key} );
         } else {    # days
-            $dt->add( days => $loanlength->{$length_key} );
+            $datedue->add( days => $loanlength->{$length_key} );
-            $dt->set_hour(23);
+            $datedue->set_hour(23);
-            $dt->set_minute(59);
+            $datedue->set_minute(59);
         }
-        # break
-        return $dt;
     } else {
         my $dur;
         if ($loanlength->{lengthunit} eq 'hours') {
@@ -3035,11 +3041,8 @@ sub CalcDateDue {
         else { # days
             $dur = DateTime::Duration->new( days => $loanlength->{$length_key});
         }
-        if (ref $startdate ne 'DateTime' ) {
-            $startdate = dt_from_string($startdate);
-        }
         my $calendar = Koha::Calendar->new( branchcode => $branch );
-        $datedue = $calendar->addDate( $startdate, $dur, $loanlength->{lengthunit} );
+        $datedue = $calendar->addDate( $datedue, $dur, $loanlength->{lengthunit} );
         if ($loanlength->{lengthunit} eq 'days') {
             $datedue->set_hour(23);
             $datedue->set_minute(59);
@@ -3063,6 +3066,7 @@ sub CalcDateDue {
         }
 
         # in all other cases, keep the date due as it is
+
     }
 
     # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate

koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt

@@ -149,7 +149,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                 <th>Overdue Fines Cap ($)</th>
                 <th>Suspension in days (day)</th>
                 <th>Renewals allowed (count)</th>
-                <th>Renewals period</th>
+                <th>Renewal period</th>
                 <th>Holds allowed (count)</th>
                 <th>Rental discount (%)</th>
                 <th colspan="2">&nbsp;</th>

t/db_dependent/Circulation_issuingrules.t

@@ -0,0 +1,123 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+
+use t::lib::Mocks::Context;
+use Test::More tests => 7;
+use Test::MockModule;
+use DBI;
+use DateTime;
+
+my $contextmodule = new Test::MockModule('C4::Context');
+$contextmodule->mock('_new_dbh', sub {
+    my $dbh = DBI->connect( 'DBI:Mock:', '', '' )
+    || die "Cannot create handle: $DBI::errstr\n";
+    return $dbh
+});
+
+use_ok('C4::Circulation');
+
+my $dbh = C4::Context->dbh();
+
+my $issuelength = 10;
+my $renewalperiod = 5;
+my $lengthunit = 'days';
+
+my $expected = {
+    issuelength => $issuelength,
+    renewalperiod => $renewalperiod,
+    lengthunit => $lengthunit
+};
+
+my $default = {
+    issuelength => 21,
+    renewalperiod => 21,
+    lengthunit => 'days'
+};
+
+my $loanlength;
+my $mock_undef = [
+    []
+];
+
+my $mock_loan_length = [
+    ['issuelength', 'renewalperiod', 'lengthunit'],
+    [$issuelength, $renewalperiod, $lengthunit]
+];
+
+my $categorycode = 'B';
+my $itemtype = 'MX';
+my $branchcode = 'FPL';
+
+#=== GetLoanLength
+$dbh->{mock_add_resultset} = $mock_loan_length;
+$loanlength = C4::Circulation::GetLoanLength($categorycode, $itemtype, $branchcode);
+is_deeply($loanlength, $expected, 'first matches');
+
+$dbh->{mock_add_resultset} = $mock_undef;
+$loanlength = C4::Circulation::GetLoanLength($categorycode, $itemtype, $branchcode);
+is_deeply($loanlength, $default, 'none matches');
+
+#=== CalcDateDue
+
+#Set syspref ReturnBeforeExpiry = 1 and useDaysMode = 'Days'
+$contextmodule->mock('preference', sub {
+    my ($self, $syspref) = @_;
+    given ( $syspref ) {
+        when ("ReturnBeforeExpiry"){ return 1; }
+        when ("useDaysMode"){ return 'Days'; }
+        default{ return; }
+    }
+});
+
+my $dateexpiry = '2013-01-01';
+
+my $borrower = {categorycode => 'B', dateexpiry => $dateexpiry};
+my $start_date = DateTime->new({year => 2013, month => 2, day => 9});
+$dbh->{mock_add_resultset} = $mock_loan_length;
+my $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower );
+is($date, $dateexpiry . 'T23:59:00', 'date expiry');
+
+$dbh->{mock_add_resultset} = $mock_loan_length;
+$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 );
+
+
+#Set syspref ReturnBeforeExpiry = 1 and useDaysMode != 'Days'
+$contextmodule->mock('preference', sub {
+    my ($self, $syspref) = @_;
+    given ( $syspref ) {
+        when ("ReturnBeforeExpiry"){ return 1; }
+        when ("useDaysMode"){ return 'noDays'; }
+        default{ return; }
+    }
+});
+
+$borrower = {categorycode => 'B', dateexpiry => $dateexpiry};
+$start_date = DateTime->new({year => 2013, month => 2, day => 9});
+$dbh->{mock_add_resultset} = $mock_loan_length;
+$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower );
+is($date, $dateexpiry . 'T23:59:00', 'date expiry');
+
+$dbh->{mock_add_resultset} = $mock_loan_length;
+$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 );
+
+
+#Set syspref ReturnBeforeExpiry = 0 and useDaysMode = 'Days'
+$contextmodule->mock('preference', sub {
+    my ($self, $syspref) = @_;
+    given ( $syspref ) {
+        when ("ReturnBeforeExpiry"){ return 0; }
+        when ("useDaysMode"){ return 'Days'; }
+        default{ return; }
+    }
+});
+
+$borrower = {categorycode => 'B', dateexpiry => $dateexpiry};
+$start_date = DateTime->new({year => 2013, month => 2, day => 9});
+$dbh->{mock_add_resultset} = $mock_loan_length;
+$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower );
+is($date, '2013-02-' . (9 + $issuelength) . 'T23:59:00', "date expiry ( 9 + $issuelength )");
+
+$dbh->{mock_add_resultset} = $mock_loan_length;
+$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 );
+is($date, '2013-02-' . (9 + $renewalperiod) . 'T23:59:00', "date expiry ( 9 + $renewalperiod )");