From a103f95881ce90368d176009f7f2f27abac60da3 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 | 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 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..46efcf8 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 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