Attachment 15303 Details for Bug 8365 – Bug 8365: Add unit tests and fix QA issues

[patch] Bug 8365: Add unit tests and fix QA issues

Bug-8365-Add-unit-tests-and-fix-QA-issues.patch (text/plain), 9.08 KB, created by Jonathan Druart on 2013-02-13 14:57:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2013-02-13 14:57:56 UTC

Size: 9.08 KB

patch

obsolete

>From dce0dac5df8fbb605e1c79b7132ed825984acd53 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>Date: Wed, 13 Feb 2013 15:08:41 +0100
>Subject: [PATCH] Bug 8365: Add unit tests and fix QA issues
>
>This patch adds some unit tests for CalcDateDue and GetLoanLength
>---
> C4/Circulation.pm                                  |   38 +++---
> .../prog/en/modules/admin/smart-rules.tt           |    2 +-
> t/db_dependent/Circulation_issuingrules.t          |  125 ++++++++++++++++++++
> 3 files changed, 147 insertions(+), 18 deletions(-)
> create mode 100644 t/db_dependent/Circulation_issuingrules.t
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index c40aa11..897a0ac 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1335,9 +1335,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};
> 
>@@ -2411,8 +2411,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.
> 
>@@ -2422,7 +2420,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
>@@ -2984,6 +2982,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
> 
>@@ -3001,22 +3000,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} );
>-            $dt->set_hour(23);
>-            $dt->set_minute(59);
>+            $datedue->add( days => $loanlength->{$length_key} );
>+            $datedue->set_hour(23);
>+            $datedue->set_minute(59);
>         }
>-        # break
>-        return $dt;
>     } else {
>         my $dur;
>         if ($loanlength->{lengthunit} eq 'hours') {
>@@ -3025,11 +3031,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);
>@@ -3053,6 +3056,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
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>index d33a243..c69376f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -75,7 +75,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>&nbsp;</th>
>diff --git a/t/db_dependent/Circulation_issuingrules.t b/t/db_dependent/Circulation_issuingrules.t
>new file mode 100644
>index 0000000..8051a66
>--- /dev/null
>+++ b/t/db_dependent/Circulation_issuingrules.t
>@@ -0,0 +1,125 @@
>+#!/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 )");
>+
>+
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 8365: 12054 | 12374 | 13972 | 14142 | 15300 | 15301 | 15302 | 15303 | 15360 | 15361 | 16120 | 16121 | 16213 | 16214 | 16382 | 16386 | 16396 | 16433 | 16434 | 16435 | 16436