From 6da8dfe71b9881a39396ad4937af9e833b4757a9 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Wed, 28 Oct 2015 18:37:26 +0200 Subject: [PATCH] Bug 10744 - ExpireReservesMaxPickUpDelay has minor workflow conflicts with hold(s) over report Initial commit. --- C4/Reserves.pm | 82 +++++++++++++++++++- circ/waitingreserves.pl | 4 +- ...Bug10744ExpireReservesConflictHoldOverReport.pl | 34 ++++++++ installer/data/mysql/kohastructure.sql | 4 + 4 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/Bug10744ExpireReservesConflictHoldOverReport.pl diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 7630292..5ac42d2 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -43,6 +43,7 @@ use Koha::Calendar; use DateTime; use List::MoreUtils qw( firstidx ); +use Scalar::Util qw(blessed); use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -791,6 +792,55 @@ sub GetReservesForBranch { return (@transreserv); } +=head GetExpiredReserves + + my $expiredReserves = C4::Reserves::GetExpiredReserves({branchcode => 'CPL', + from => DateTime->new(...), + to => DateTime->now(), + }); + +@RETURNS ARRAYRef of expired reserves from the given duration. Defaults to this day. +=cut + +sub GetExpiredReserves { + my ($params) = @_; + my $branchcode = $params->{branchcode}; + if ($params->{from}) { + unless (blessed($params->{from}) && $params->{from}->isa('DateTime')) { + Koha::Exception::BadParameter->throw(error => "GetExpiredReserves():> Parameter 'from' is not a DateTime-object or undef!"); + } + } + if ($params->{to}) { + unless (blessed($params->{from}) && $params->{from}->isa('DateTime')) { + Koha::Exception::BadParameter->throw(error => "GetExpiredReserves():> Parameter 'from' is not a DateTime-object or undef!"); + } + } + + my $fromDate = $params->{from} || DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1); + my $toDate = $params->{to} || DateTime->now(time_zone => C4::Context->tz()); + + my $dbh = C4::Context->dbh; + + my @params = ($fromDate->ymd(), $toDate->ymd()); + my $query = " + SELECT * + FROM old_reserves + WHERE priority='0' + AND pickupexpired BETWEEN ? AND ? + "; + if ( $branchcode ) { + push @params, $branchcode; + $query .= " AND branchcode=? "; + } + $query .= "ORDER BY waitingdate" ; + + my $sth = $dbh->prepare($query); + $sth->execute(@params); + + my $data = $sth->fetchall_arrayref({}); + return ($data); +} + =head2 GetReserveStatus $reservestatus = GetReserveStatus($itemnumber, $biblionumber); @@ -1021,7 +1071,9 @@ sub CancelExpiredReserves { if ( $charge ) { manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge); } - CancelReserve({ reserve_id => $res->{'reserve_id'} }); + CancelReserve({ reserve_id => $res->{'reserve_id'}, + pickupexpired => $expiration, + }); push @sb, printReserve($res,'tab',['reserve_id','borrowernumber','branchcode','waitingdate']).sprintf("% 14s",substr($expiration,0,10))."| past lastpickupdate.\n" if $verbose; } elsif($verbose > 1) { @@ -1064,7 +1116,12 @@ sub AutoUnsuspendReserves { =head2 CancelReserve - CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber ] }); + CancelReserve({ reserve_id => $reserve_id, + [ biblionumber => $biblionumber, + borrowernumber => $borrrowernumber, + itemnumber => $itemnumber ], + pickupexpired => DateTime->new(year => 2015, ...), #If the reserve was waiting for pickup, set the date the pickup wait period expired. + }); Cancels a reserve. @@ -1074,6 +1131,13 @@ sub CancelReserve { my ( $params ) = @_; my $reserve_id = $params->{'reserve_id'}; + my $pickupexpired = $params->{pickupexpired}; + if ($pickupexpired) { + unless (blessed($pickupexpired) && $pickupexpired->isa('DateTime')) { + Koha::Exception::BadParameter->throw(error => "CancelReserve():> Parameter 'pickupexpired' is not a DateTime-object or undef!"); + } + } + $reserve_id = GetReserveId( $params ) unless ( $reserve_id ); return unless ( $reserve_id ); @@ -1082,15 +1146,25 @@ sub CancelReserve { my $reserve = GetReserve( $reserve_id ); + my @params; my $query = " UPDATE reserves - SET cancellationdate = now(), + SET cancellationdate = DATE(NOW()), found = Null, + "; + if ($pickupexpired) { + push @params, $pickupexpired->ymd(); + $query .= " + pickupexpired = ?, + "; + } + push @params, $reserve_id; + $query .= " priority = 0 WHERE reserve_id = ? "; my $sth = $dbh->prepare($query); - $sth->execute( $reserve_id ); + $sth->execute( @params ); $query = " INSERT INTO old_reserves diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl index f4a93f0..6e6ded2 100755 --- a/circ/waitingreserves.pl +++ b/circ/waitingreserves.pl @@ -80,8 +80,10 @@ $template->param( all_branches => 1 ) if $all_branches; my (@reservloop, @overloop); my ($reservcount, $overcount); -my @getreserves = $all_branches ? GetReservesForBranch() : GetReservesForBranch($default); # get reserves for the branch we are logged into, or for all branches +my @getreserves = $all_branches ? GetReservesForBranch() : GetReservesForBranch($default); +my $expiredReserves = $all_branches ? C4::Reserves::GetExpiredReserves() : C4::Reserves::GetExpiredReserves({branchcode => $default}); +push @getreserves, @$expiredReserves; my $today = DateTime->now(); foreach my $num (@getreserves) { diff --git a/installer/data/mysql/atomicupdate/Bug10744ExpireReservesConflictHoldOverReport.pl b/installer/data/mysql/atomicupdate/Bug10744ExpireReservesConflictHoldOverReport.pl new file mode 100644 index 0000000..36f7e2b --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug10744ExpireReservesConflictHoldOverReport.pl @@ -0,0 +1,34 @@ +#!/usr/bin/perl + +# Copyright Open Source Freedom Fighters +# +# 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, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use C4::Context; +use Koha::AtomicUpdater; + +my $dbh = C4::Context->dbh(); +my $atomicUpdater = Koha::AtomicUpdater->new(); + +unless($atomicUpdater->find('Bug10744')) { + + $dbh->do("ALTER TABLE reserves ADD `pickupexpired` DATE DEFAULT NULL AFTER `expirationdate`"); + $dbh->do("ALTER TABLE reserves ADD KEY `reserves_pickupexpired` (`pickupexpired`)"); + $dbh->do("ALTER TABLE old_reserves ADD `pickupexpired` DATE DEFAULT NULL AFTER `expirationdate`"); + $dbh->do("ALTER TABLE old_reserves ADD KEY `old_reserves_pickupexpired` (`pickupexpired`)"); + + print "Upgrade done (Bug 10744 - ExpireReservesMaxPickUpDelay has minor workflow conflicts with hold(s) over report)\n"; +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 076473f..e9d836c 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1706,6 +1706,7 @@ CREATE TABLE `old_reserves` ( -- this table holds all holds/reserves that have b `itemnumber` int(11) default NULL, -- foreign key from the items table defining the specific item the patron has placed on hold or the item this hold was filled with `waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library `expirationdate` DATE DEFAULT NULL, -- the date the hold expires (usually the date entered by the patron to say they don't need the hold after a certain date) + `pickupexpired` DATE DEFAULT NULL, -- if hold has been waiting but it expired before it was picked up, the expiration date is set here `lowestPriority` tinyint(1) NOT NULL, -- has this hold been pinned to the lowest priority in the holds queue (1 for yes, 0 for no) `suspend` BOOLEAN NOT NULL DEFAULT 0, -- in this hold suspended (1 for yes, 0 for no) `suspend_until` DATETIME NULL DEFAULT NULL, -- the date this hold is suspended until (NULL for infinitely) @@ -1714,6 +1715,7 @@ CREATE TABLE `old_reserves` ( -- this table holds all holds/reserves that have b KEY `old_reserves_biblionumber` (`biblionumber`), KEY `old_reserves_itemnumber` (`itemnumber`), KEY `old_reserves_branchcode` (`branchcode`), + KEY `old_reserves_pickupexpired` (`pickupexpired`), CONSTRAINT `old_reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE SET NULL, CONSTRAINT `old_reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) @@ -1940,6 +1942,7 @@ CREATE TABLE `reserves` ( -- information related to holds/reserves in Koha `itemnumber` int(11) default NULL, -- foreign key from the items table defining the specific item the patron has placed on hold or the item this hold was filled with `waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library `expirationdate` DATE DEFAULT NULL, -- the date the hold expires (usually the date entered by the patron to say they don't need the hold after a certain date) + `pickupexpired` DATE DEFAULT NULL, -- if hold has been waiting but it expired before it was picked up, the expiration date is set here `lowestPriority` tinyint(1) NOT NULL, `suspend` BOOLEAN NOT NULL DEFAULT 0, `suspend_until` DATETIME NULL DEFAULT NULL, @@ -1949,6 +1952,7 @@ CREATE TABLE `reserves` ( -- information related to holds/reserves in Koha KEY `biblionumber` (`biblionumber`), KEY `itemnumber` (`itemnumber`), KEY `branchcode` (`branchcode`), + KEY `reserves_pickupexpired` (`pickupexpired`), CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `reserves_ibfk_3` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE, -- 1.7.9.5