Bugzilla – Attachment 62005 Details for
Bug 12063
Change date calculation for reserve expiration to skip all holidays
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12063 - Fix QA failures
Bug-12063---Fix-QA-failures.patch (text/plain), 11.44 KB, created by
Alex Arnaud
on 2017-04-10 13:41:41 UTC
(
hide
)
Description:
Bug 12063 - Fix QA failures
Filename:
MIME Type:
Creator:
Alex Arnaud
Created:
2017-04-10 13:41:41 UTC
Size:
11.44 KB
patch
obsolete
>From bdc79309649d9e645366e722adec1cfa99f40e54 Mon Sep 17 00:00:00 2001 >From: Alex Arnaud <alex.arnaud@biblibre.com> >Date: Thu, 30 Mar 2017 09:53:10 +0000 >Subject: [PATCH] Bug 12063 - Fix QA failures > > - Remove expiration date calculation in C4::Letter since it's done > when setting the reserve waiting, > - remove expiration date calculation in circ/waitingreserves.pl. Use > the one in DB, > - add a new atomic update that calculate expiration date for > waiting reserves, > - add tests for days_foward function and fix the infinite loop. >--- > C4/Letters.pm | 10 ---- > C4/Reserves.pm | 4 +- > Koha/Calendar.pm | 2 + > Koha/Hold.pm | 30 +++++----- > circ/waitingreserves.pl | 9 +-- > ...efine_expirationdate_for_waitting_reserves.perl | 30 ++++++++++ > t/db_dependent/Calendar.t | 69 ++++++++++++++++++++++ > t/db_dependent/Holds/CancelReserves.t | 9 ++- > 8 files changed, 127 insertions(+), 36 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_12063-define_expirationdate_for_waitting_reserves.perl > create mode 100644 t/db_dependent/Calendar.t > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 74263f0..66a1a3b 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -871,17 +871,7 @@ sub _parseletter { > } > > if ( $table eq 'reserves' && $values->{'waitingdate'} ) { >- my @waitingdate = split /-/, $values->{'waitingdate'}; >- >- $values->{'expirationdate'} = ''; >- if ( C4::Context->preference('ReservesMaxPickUpDelay') ) { >- my $dt = dt_from_string(); >- $dt->add( days => C4::Context->preference('ReservesMaxPickUpDelay') ); >- $values->{'expirationdate'} = output_pref( { dt => $dt, dateonly => 1 } ); >- } >- > $values->{'waitingdate'} = output_pref({ dt => dt_from_string( $values->{'waitingdate'} ), dateonly => 1 }); >- > } > > if ($letter->{content} && $letter->{content} =~ /<<today>>/) { >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 4615d97..4a34305 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -721,7 +721,7 @@ sub GetReservesForBranch { > my $dbh = C4::Context->dbh; > > my $query = " >- SELECT reserve_id,borrowernumber,reservedate,itemnumber,waitingdate >+ SELECT reserve_id,borrowernumber,reservedate,itemnumber,waitingdate, expirationdate > FROM reserves > WHERE priority='0' > AND found='W' >@@ -940,6 +940,8 @@ Cancels all reserves with an expiration date from before today. > =cut > > sub CancelExpiredReserves { >+ return unless C4::Context->preference("ExpireReservesMaxPickUpDelay"); >+ > my $today = dt_from_string(); > my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); > >diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm >index 3c04fbe..67d2a6b 100644 >--- a/Koha/Calendar.pm >+++ b/Koha/Calendar.pm >@@ -301,6 +301,8 @@ sub days_forward { > my $start_dt = shift; > my $num_days = shift; > >+ return $start_dt unless $num_days > 0; >+ > my $base_dt = $start_dt->clone(); > > while ($num_days--) { >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 1e2e730..030e90a 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -132,24 +132,22 @@ sub set_waiting { > $requested_expiration = dt_from_string($self->expirationdate); > } > >- if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) { >- my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); >- my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); >- my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); >- >- my $expirationdate = $today->clone; >- $expirationdate->add(days => $max_pickup_delay); >- >- if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { >- $expirationdate = $calendar->days_forward( dt_from_string($self->waitingdate), $max_pickup_delay ); >- } >- >- # If patron's requested expiration date is prior to the >- # calculated one, we keep the patron's one. >- my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; >- $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; >+ my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); >+ my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); >+ my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); >+ >+ my $expirationdate = $today->clone; >+ $expirationdate->add(days => $max_pickup_delay); >+ >+ if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { >+ $expirationdate = $calendar->days_forward( dt_from_string($self->waitingdate), $max_pickup_delay ); > } > >+ # If patron's requested expiration date is prior to the >+ # calculated one, we keep the patron's one. >+ my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; >+ $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; >+ > $self->set($values)->store(); > > return $self; >diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl >index d0b3942..af41394 100755 >--- a/circ/waitingreserves.pl >+++ b/circ/waitingreserves.pl >@@ -86,7 +86,6 @@ my @getreserves = $all_branches ? GetReservesForBranch() : GetReservesForBranch( > > my $today = Date_to_Days(&Today); > my $max_pickup_delay = C4::Context->preference('ReservesMaxPickUpDelay'); >-$max_pickup_delay-- if C4::Context->preference('ExpireReservesMaxPickUpDelay'); > > foreach my $num (@getreserves) { > next unless ($num->{'waitingdate'} && $num->{'waitingdate'} ne '0000-00-00'); >@@ -107,12 +106,8 @@ foreach my $num (@getreserves) { > my $getborrower = GetMember(borrowernumber => $num->{'borrowernumber'}); > my $itemtypeinfo = getitemtypeinfo( $gettitle->{'itemtype'} ); # using the fixed up itype/itemtype > $getreserv{'waitingdate'} = $num->{'waitingdate'}; >- my ( $waiting_year, $waiting_month, $waiting_day ) = split (/-/, $num->{'waitingdate'}); >- >- ( $waiting_year, $waiting_month, $waiting_day ) = >- Add_Delta_Days( $waiting_year, $waiting_month, $waiting_day, >- $max_pickup_delay); >- my $calcDate = Date_to_Days( $waiting_year, $waiting_month, $waiting_day ); >+ my ( $expire_year, $expire_month, $expire_day ) = split (/-/, $num->{'expirationdate'}); >+ my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day ); > > $getreserv{'itemtype'} = $itemtypeinfo->{'description'}; > $getreserv{'title'} = $gettitle->{'title'}; >diff --git a/installer/data/mysql/atomicupdate/bug_12063-define_expirationdate_for_waitting_reserves.perl b/installer/data/mysql/atomicupdate/bug_12063-define_expirationdate_for_waitting_reserves.perl >new file mode 100644 >index 0000000..3a5d232 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_12063-define_expirationdate_for_waitting_reserves.perl >@@ -0,0 +1,30 @@ >+use C4::Context; >+ >+use Koha::Holds; >+use Koha::DateUtils; >+use Koha::Calendar; >+ >+my $waiting_holds = Koha::Holds->search({ found => 'W', priority => 0 }); >+while ( my $hold = $waiting_holds->next ) { >+ >+ my $requested_expiration; >+ if ($hold->expirationdate) { >+ $requested_expiration = dt_from_string($hold->expirationdate); >+ } >+ >+ if ( my $waitingdate = dt_from_string($hold->waitingdate) ) { >+ my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); >+ my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); >+ my $calendar = Koha::Calendar->new( branchcode => $hold->branchcode ); >+ >+ my $expirationdate = $waitingdate->clone; >+ $expirationdate->add(days => $max_pickup_delay); >+ >+ if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { >+ $expirationdate = $calendar->days_forward( dt_from_string($hold->waitingdate), $max_pickup_delay ); >+ } >+ >+ my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; >+ $hold->expirationdate($cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd)->store; >+ } >+} >\ No newline at end of file >diff --git a/t/db_dependent/Calendar.t b/t/db_dependent/Calendar.t >new file mode 100644 >index 0000000..d443f11 >--- /dev/null >+++ b/t/db_dependent/Calendar.t >@@ -0,0 +1,69 @@ >+#!/usr/bin/perl >+ >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <http://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+ >+use Test::More tests => 5; >+use t::lib::TestBuilder; >+ >+use DateTime; >+use Koha::Caches; >+use Koha::DateUtils; >+ >+use_ok('Koha::Calendar'); >+ >+my $schema = Koha::Database->new->schema; >+$schema->storage->txn_begin; >+ >+my $today = dt_from_string(); >+my $holiday_dt = $today->clone; >+$holiday_dt->add(days => 15); >+ >+Koha::Caches->get_instance()->flush_all(); >+ >+my $builder = t::lib::TestBuilder->new(); >+my $holiday = $builder->build({ >+ source => 'SpecialHoliday', >+ value => { >+ branchcode => 'LIB1', >+ day => $holiday_dt->day, >+ month => $holiday_dt->month, >+ year => $holiday_dt->year, >+ title => 'My holiday', >+ isexception => 0 >+ }, >+}); >+ >+my $calendar = Koha::Calendar->new( branchcode => 'LIB1'); >+my $forwarded_dt = $calendar->days_forward($today, 10); >+ >+my $expected = $today->clone; >+$expected->add(days => 10); >+is($forwarded_dt->ymd, $expected->ymd, 'With no holiday on the perioddays_forward should add 10 days'); >+ >+$forwarded_dt = $calendar->days_forward($today, 20); >+ >+$expected->add(days => 11); >+is($forwarded_dt->ymd, $expected->ymd, 'With holiday on the perioddays_forward should add 20 days + 1 day for holiday'); >+ >+$forwarded_dt = $calendar->days_forward($today, 0); >+is($forwarded_dt->ymd, $today->ymd, '0 day should return start dt'); >+ >+$forwarded_dt = $calendar->days_forward($today, -2); >+is($forwarded_dt->ymd, $today->ymd, 'negative day should return start dt'); >+ >+$schema->storage->txn_rollback(); >diff --git a/t/db_dependent/Holds/CancelReserves.t b/t/db_dependent/Holds/CancelReserves.t >index b6d216f..3330a1c 100644 >--- a/t/db_dependent/Holds/CancelReserves.t >+++ b/t/db_dependent/Holds/CancelReserves.t >@@ -8,7 +8,7 @@ use Koha::DateUtils; > use t::lib::Mocks; > use t::lib::TestBuilder; > >-use Test::More tests => 5; >+use Test::More tests => 6; > > use_ok('C4::Reserves'); > >@@ -63,7 +63,12 @@ my $reserve2 = $builder->build({ > > CancelExpiredReserves(); > my $r2 = Koha::Holds->find($reserve2->{reserve_id}); >-is($r2, undef,'Reserve 2 should be canceled.'); >+ok($r2, 'Without ExpireReservesMaxPickUpDelay, reserve 2 should not be canceled.'); >+ >+t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelay', 1); >+CancelExpiredReserves(); >+$r2 = Koha::Holds->find($reserve2->{reserve_id}); >+is($r2, undef,'With ExpireReservesMaxPickUpDelay, reserve 2 should be canceled.'); > > # Reserve expired on holiday > my $reserve3 = $builder->build({ >-- >2.7.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=62005">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12063
:
26966
|
52558
|
52949
|
52950
|
53100
|
53101
|
59161
|
59162
|
59163
|
59165
|
61401
|
61440
|
61461
|
61735
|
62001
|
62002
|
62003
|
62004
|
62005
|
62474
|
63057
|
63091
|
63092
|
63093
|
63094
|
63095
|
63096
|
63097
|
63098
|
63099