Attachment 50014 Details for Bug 15757 – Bug 15757: Make GetLoanLength defaults to 0 instead of 21

[patch] Bug 15757: Make GetLoanLength defaults to 0 instead of 21

Bug-15757-Make-GetLoanLength-defaults-to-0-instead.patch (text/plain), 5.87 KB, created by Jonathan Druart on 2016-04-07 09:42:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-04-07 09:42:34 UTC

Size: 5.87 KB

patch

obsolete

>From 8d83bf3f0a45a9c7b6d2923e1d5b6590a9b58963 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)
>---
> C4/Circulation.pm                         | 22 +++++++++---------
> t/db_dependent/Circulation_Issuingrule.t  | 38 +++++++++++++++----------------
> t/db_dependent/Circulation_issuingrules.t |  4 ++--
> 3 files changed, 31 insertions(+), 33 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 70ae3fd..81c6947 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 89bdf8e..1bf5226 100644
>--- a/t/db_dependent/Circulation_Issuingrule.t
>+++ b/t/db_dependent/Circulation_Issuingrule.t
>@@ -104,6 +104,19 @@ $dbh->do(
> 
> #Begin Tests
> 
>+my $default = {
>+    issuelength => 0,
>+    renewalperiod => 0,
>+    lengthunit => 'days'
>+};
>+
>+is_deeply(
>+    C4::Circulation::GetLoanLength(),
>+    $default,
>+    "Without parameters, GetLoanLength returns hardcoded values"
>+);
>+
>+
> #Test GetIssuingRule
> my $sampleissuingrule1 = {
>     reservecharge      => '0.000000',
>@@ -343,40 +356,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
>--- 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.7.0

Actions: View | Diff | Splinter Review

Attachments on bug 15757: 50013 | 50014 | 50015 | 50029 | 50131 | 50132 | 50133 | 50513 | 50514 | 50515 | 50516