From f35dbbf42eb7f5f9a1412d13a487e0e8eed65ef0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 Signed-off-by: Kyle M Hall Signed-off-by: Katrin Fischer All tests and QA script pass. Tests done: - Checked update works correctly for existing circulation rules. - Adding, deleting and overwriting circulation rules works. - Renewals work for different circulation rules and changes to the holiday calendar. Signed-off-by: Jared Camins-Esakov --- C4/Circulation.pm | 38 +++--- .../prog/en/modules/admin/smart-rules.tt | 2 +- t/db_dependent/Circulation_issuingrules.t | 123 ++++++++++++++++++ 3 files changed, 145 insertions(+), 18 deletions(-) create mode 100644 t/db_dependent/Circulation_issuingrules.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 425e16ca3e..185575ef3c 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1345,9 +1345,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}; @@ -2421,8 +2421,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. @@ -2432,7 +2430,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 @@ -2994,6 +2992,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 @@ -3011,22 +3010,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') { @@ -3035,11 +3041,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); @@ -3063,6 +3066,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 0326cf6a5e..586d310850 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 @@ -149,7 +149,7 @@ for="tobranch">Clone these rules to:   diff --git a/t/db_dependent/Circulation_issuingrules.t b/t/db_dependent/Circulation_issuingrules.t new file mode 100644 index 0000000000..4303af751b --- /dev/null +++ b/t/db_dependent/Circulation_issuingrules.t @@ -0,0 +1,123 @@ +#!/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 )"); -- 2.39.5