Bug 15757: Make GetLoanLength defaults to 0 instead of 21

GetLoanLength arbitrary defaulted to 21. The expected behavior seems to
be to default on 0 (loan will be dued today).

IMPORTANT NOTE: This patch will introduce a change in the behaviors for
configuration with a 0 in issuelength. Before this patch, the rule with
a issuelength==0 was skipped, now it's used!

Test plan:
1/ Do not define any rule: the due date will be today (before this patch
was +21 days)
2/ Define some rules which does not match the patron category, itemtype
or branchcode: the due date will be today (before this patch was +21
days).
3/ Modify a rule to match the checkout and set issuelength=0: the due
date will be today (before this patch, the rule was skipped)
4/ Modify this rule and set the issuelength to something > 0: the due
date will be adjusted (same behavior as before this patch)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works ok, checked 1-4
All test pass
No koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 70ae3fd..81c6947 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1484,47 +1484,47 @@ sub GetLoanLength {
     my $loanlength = $sth->fetchrow_hashref;
 
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( $borrowertype, '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', $itemtype, $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', '*', $branchcode );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( $borrowertype, $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( $borrowertype, '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', $itemtype, '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
     $sth->execute( '*', '*', '*' );
     $loanlength = $sth->fetchrow_hashref;
     return $loanlength
-      if defined($loanlength) && $loanlength->{issuelength};
+      if defined($loanlength) && defined $loanlength->{issuelength};
 
-    # if no rule is set => 21 days (hardcoded)
+    # if no rule is set => 0 day (hardcoded)
     return {
-        issuelength => 21,
-        renewalperiod => 21,
+        issuelength => 0,
+        renewalperiod => 0,
         lengthunit => 'days',
     };
 

diff --git a/t/db_dependent/Circulation_Issuingrule.t b/t/db_dependent/Circulation_Issuingrule.t
index 89bdf8e..46efcf8 100644 (file)
--- a/t/db_dependent/Circulation_Issuingrule.t
+++ b/t/db_dependent/Circulation_Issuingrule.t
@@ -104,6 +104,12 @@ $dbh->do(
 
 #Begin Tests
 
+my $default = {
+    issuelength => 0,
+    renewalperiod => 0,
+    lengthunit => 'days'
+};
+
 #Test GetIssuingRule
 my $sampleissuingrule1 = {
     reservecharge      => '0.000000',
@@ -343,40 +349,25 @@ is_deeply(
     { issuelength => 5, lengthunit => 'days', renewalperiod => 5 },
     "GetLoanLength"
 );
+
 is_deeply(
     C4::Circulation::GetLoanLength(),
-    {
-        issuelength   => 21,
-        renewalperiod => 21,
-        lengthunit    => 'days',
-    },
+    $default,
     "Without parameters, GetLoanLength returns hardcoded values"
 );
 is_deeply(
     C4::Circulation::GetLoanLength( -1, -1 ),
-    {
-        issuelength   => 21,
-        renewalperiod => 21,
-        lengthunit    => 'days',
-    },
+    $default,
     "With wrong parameters, GetLoanLength returns hardcoded values"
 );
 is_deeply(
     C4::Circulation::GetLoanLength( $samplecat->{categorycode} ),
-    {
-        issuelength   => 21,
-        renewalperiod => 21,
-        lengthunit    => 'days',
-    },
+    $default,
     "With only one parameter, GetLoanLength returns hardcoded values"
 );    #NOTE : is that really what is expected?
 is_deeply(
     C4::Circulation::GetLoanLength( $samplecat->{categorycode}, 'BOOK' ),
-    {
-        issuelength   => 21,
-        renewalperiod => 21,
-        lengthunit    => 'days',
-    },
+    $default,
     "With only two parameters, GetLoanLength returns hardcoded values"
 );    #NOTE : is that really what is expected?
 is_deeply(

diff --git a/t/db_dependent/Circulation_issuingrules.t b/t/db_dependent/Circulation_issuingrules.t
index aff1f93..c2508c6 100644 (file)
--- a/t/db_dependent/Circulation_issuingrules.t
+++ b/t/db_dependent/Circulation_issuingrules.t
@@ -27,8 +27,8 @@ my $expected = {
 };
 
 my $default = {
-    issuelength => 21,
-    renewalperiod => 21,
+    issuelength => 0,
+    renewalperiod => 0,
     lengthunit => 'days'
 };