From 769728015cea465bbb47d27f69077962a8f2429d Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 7 Apr 2016 09:45:58 +0100
Subject: [PATCH] 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>
---
 C4/Circulation.pm                         | 22 ++++++++--------
 t/db_dependent/Circulation_Issuingrule.t  | 31 ++++++++---------------
 t/db_dependent/Circulation_issuingrules.t |  4 +--
 3 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 70ae3fd473..81c694713a 100644
--- 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 89bdf8edaf..46efcf89e7 100644
--- 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 aff1f9342f..c2508c6732 100644
--- 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'
 };
 
-- 
2.39.5