Attachment #44294 for bug #10744

View | Details | Raw Unified | Return to bug 10744
Collapse All | Expand All




use DateTime;

use List::MoreUtils qw( firstidx );
use Scalar::Util qw(blessed);

use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);


    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 $pickupExpiredHoldsOverReportDuration = C4::Context->preference('PickupExpiredHoldsOverReportDuration');
    return [] unless $pickupExpiredHoldsOverReportDuration;

    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 => $pickupExpiredHoldsOverReportDuration);
    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) ? $data : [];
}

=head2 GetReserveStatus

  $reservestatus = GetReserveStatus($itemnumber, $biblionumber);

                if ( $charge ) {
                    manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge);
                }
                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) {


=head2 CancelReserve

  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.


    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 );


    my $reserve = GetReserve( $reserve_id );

    my @params;
    my $query = "
        UPDATE reserves
        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( @params );

    $query = "
        INSERT INTO old_reserves

Lines 80-87 $template->param( all_branches => 1 ) if $all_branches; Link Here

(-)a/circ/waitingreserves.pl (-1 / +3 lines)
80		80
81	my (@reservloop, @overloop);	81	my (@reservloop, @overloop);
82	my ($reservcount, $overcount);	82	my ($reservcount, $overcount);
83	my @getreserves = $all_branches ? GetReservesForBranch() : GetReservesForBranch($default);
84	# get reserves for the branch we are logged into, or for all branches	83	# get reserves for the branch we are logged into, or for all branches
		84	my @getreserves = $all_branches ? GetReservesForBranch() : GetReservesForBranch($default);
		85	my $expiredReserves = $all_branches ? C4::Reserves::GetExpiredReserves() : C4::Reserves::GetExpiredReserves({branchcode => $default});
		86	push @getreserves, @$expiredReserves;
85		87
86	my $today = DateTime->now();	88	my $today = DateTime->now();
87	foreach my $num (@getreserves) {	89	foreach my $num (@getreserves) {




#!/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`)");

    $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('PickupExpiredHoldsOverReportDuration','1',NULL,\"For how many days holds expired by the 'ExpireReservesMaxPickUpDelay'-syspref are visible in the 'Hold Over'-tab in /circ/waitingreserves.pl ?\",'Integer')");

    print "Upgrade done (Bug 10744 - ExpireReservesMaxPickUpDelay has minor workflow conflicts with hold(s) over report)\n";
}




  `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)

  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`)

  `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,

  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,

Lines 118-123 INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` Link Here

(-)a/installer/data/mysql/sysprefs.sql (+1 lines)
118	('EnhancedMessagingPreferences','0','','If ON, allows patrons to select to receive additional messages about items due or nearly due.','YesNo'),	118	('EnhancedMessagingPreferences','0','','If ON, allows patrons to select to receive additional messages about items due or nearly due.','YesNo'),
119	('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'),	119	('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'),
120	('ExpireReservesMaxPickUpDelay','0','','Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay','YesNo'),	120	('ExpireReservesMaxPickUpDelay','0','','Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay','YesNo'),
		121	('PickupExpiredHoldsOverReportDuration','1',NULL,"For how many days holds expired by the 'ExpireReservesMaxPickUpDelay'-syspref are visible in the 'Hold Over'-tab in /circ/waitingreserves.pl ?",'Integer'),
121	('ExpireReservesMaxPickUpDelayCharge','0',NULL,'If ExpireReservesMaxPickUpDelay is enabled, and this field has a non-zero value, than a borrower whose waiting hold has expired will be charged this amount.','free'),	122	('ExpireReservesMaxPickUpDelayCharge','0',NULL,'If ExpireReservesMaxPickUpDelay is enabled, and this field has a non-zero value, than a borrower whose waiting hold has expired will be charged this amount.','free'),
122	('ExtendedPatronAttributes','0',NULL,'Use extended patron IDs and attributes','YesNo'),	123	('ExtendedPatronAttributes','0',NULL,'Use extended patron IDs and attributes','YesNo'),
123	('FacetLabelTruncationLength','20',NULL,'Specify the facet max length in OPAC','Integer'),	124	('FacetLabelTruncationLength','20',NULL,'Specify the facet max length in OPAC','Integer'),

Lines 416-421 Circulation: Link Here

(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref (+4 lines)
416	- pref: ExpireReservesMaxPickUpDelayCharge	416	- pref: ExpireReservesMaxPickUpDelayCharge
417	class: currency	417	class: currency
418	-	418	-
		419	- pref: PickupExpiredHoldsOverReportDuration
		420	class: integer
		421	- "For how many days holds expired by the 'ExpireReservesMaxPickUpDelay'-syspref are visible in the 'Hold Over'-tab in /circ/waitingreserves.pl ?"
		422	-
419	- Satisfy holds from the libraries	423	- Satisfy holds from the libraries
420	- pref: StaticHoldsQueueWeight	424	- pref: StaticHoldsQueueWeight
421	class: multi	425	class: multi




#!/usr/bin/perl

# Copyright 2015 KohaSuomi
#
# 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;
use Try::Tiny;
use Scalar::Util qw(blessed);

use t::lib::TestObjects::ObjectFactory;
use t::lib::TestObjects::HoldFactory;
use t::lib::TestObjects::SystemPreferenceFactory;

##Setting up the test context
my $testContext = {};

t::lib::TestObjects::SystemPreferenceFactory->createTestGroup([{preference => 'PickupExpiredHoldsOverReportDuration',
                                                                value => 2,
                                                               },
                                                               {preference => 'ExpireReservesMaxPickUpDelay',
                                                                value => 1,
                                                               },
                                                               {preference => 'ReservesMaxPickUpDelay',
                                                                value => 6,
                                                               },
                                                              ], undef, $testContext);

my $holds = t::lib::TestObjects::HoldFactory->createTestGroup([
            {cardnumber  => '1A01',
             isbn        => '987Kivi',
             barcode     => '1N01',
             branchcode  => 'CPL',
             waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 9)->iso8601(),
             reservenotes => 'expire3daysAgo',
            },
            {cardnumber  => '1A01',
             isbn        => '987Kivi',
             barcode     => '1N02',
             branchcode  => 'CPL',
             waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 8)->iso8601(),
             reservenotes => 'expire2daysAgo',
            },
            {cardnumber  => '1A02',
             isbn        => '987Kivi',
             barcode     => '1N03',
             branchcode  => 'CPL',
             waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 7)->iso8601(),
             reservenotes => 'expire1dayAgo1',
            },
            {cardnumber  => '1A03',
             isbn        => '987Kivi',
             barcode     => '1N04',
             branchcode  => 'CPL',
             waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 7)->iso8601(),
             reservenotes => 'expire1dayAgo2',
            },
            {cardnumber  => '1A04',
             isbn        => '987Kivi',
             barcode     => '1N05',
             branchcode  => 'CPL',
             waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 6)->iso8601(),
             reservenotes => 'expiresToday',
            },
            {cardnumber  => '1A05',
             isbn        => '987Kivi',
             barcode     => '1N06',
             branchcode  => 'CPL',
             waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 5)->iso8601(),
             reservenotes => 'expiresTomorrow',
            },
        ], undef, $testContext);



##Test context set, starting testing:
subtest "Expiring holds and getting old_reserves" => \&expiringHoldsAndOld_reserves;
sub expiringHoldsAndOld_reserves {
    eval { #run in a eval-block so we don't die without tearing down the test context
        C4::Reserves::CancelExpiredReserves();
        my $expiredReserves = C4::Reserves::GetExpiredReserves({});
        ok($expiredReserves->[0]->{reserve_id} == $holds->{'expire2daysAgo'}->{reserve_id} &&
           $expiredReserves->[0]->{pickupexpired} eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 2)->ymd()
           , "Hold for Item 1N02 expired yesterday");
        ok($expiredReserves->[1]->{reserve_id} == $holds->{'expire1dayAgo1'}->{reserve_id} &&
           $expiredReserves->[1]->{pickupexpired} eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->ymd()
           , "Hold for Item 1N03 expired today");
        ok($expiredReserves->[2]->{reserve_id} == $holds->{'expire1dayAgo2'}->{reserve_id} &&
           $expiredReserves->[2]->{pickupexpired} eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->ymd()
           , "Hold for Item 1N04 expired today");
        is($expiredReserves->[3], undef,
           "Holds for Items 1N05 and 1N06 not expired.");
    };
    if ($@) { #Catch all leaking errors and gracefully terminate.
        warn $@;
        tearDown();
        exit 1;
    }
}

##All tests done, tear down test context
tearDown();
done_testing;

sub tearDown {
    t::lib::TestObjects::ObjectFactory->tearDownTestContext($testContext);
}

Return to bug 10744

Lines 43-48 use Koha::Calendar; Link Here

(-)a/C4/Reserves.pm (-4 / +82 lines)
43	use DateTime;	43	use DateTime;
44		44
45	use List::MoreUtils qw( firstidx );	45	use List::MoreUtils qw( firstidx );
		46	use Scalar::Util qw(blessed);
46		47
47	use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);	48	use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
48		49
Lines 791-796 sub GetReservesForBranch { Link Here
791	return (@transreserv);	792	return (@transreserv);
792	}	793	}
793		794
		795	=head GetExpiredReserves
		796
		797	my $expiredReserves = C4::Reserves::GetExpiredReserves({branchcode => 'CPL',
		798	from => DateTime->new(...),
		799	to => DateTime->now(),
		800	});
		801
		802	@RETURNS ARRAYRef of expired reserves from the given duration. Defaults to this day.
		803	=cut
		804
		805	sub GetExpiredReserves {
		806	my ($params) = @_;
		807
		808	my $pickupExpiredHoldsOverReportDuration = C4::Context->preference('PickupExpiredHoldsOverReportDuration');
		809	return [] unless $pickupExpiredHoldsOverReportDuration;
		810
		811	my $branchcode = $params->{branchcode};
		812	if ($params->{from}) {
		813	unless (blessed($params->{from}) && $params->{from}->isa('DateTime')) {
		814	Koha::Exception::BadParameter->throw(error => "GetExpiredReserves():> Parameter 'from' is not a DateTime-object or undef!");
		815	}
		816	}
		817	if ($params->{to}) {
		818	unless (blessed($params->{from}) && $params->{from}->isa('DateTime')) {
		819	Koha::Exception::BadParameter->throw(error => "GetExpiredReserves():> Parameter 'from' is not a DateTime-object or undef!");
		820	}
		821	}
		822
		823	my $fromDate = $params->{from} \|\| DateTime->now(time_zone => C4::Context->tz())->subtract(days => $pickupExpiredHoldsOverReportDuration);
		824	my $toDate = $params->{to} \|\| DateTime->now(time_zone => C4::Context->tz());
		825
		826	my $dbh = C4::Context->dbh;
		827
		828	my @params = ($fromDate->ymd(), $toDate->ymd());
		829	my $query = "
		830	SELECT *
		831	FROM old_reserves
		832	WHERE priority='0'
		833	AND pickupexpired BETWEEN ? AND ?
		834	";
		835	if ( $branchcode ) {
		836	push @params, $branchcode;
		837	$query .= " AND branchcode=? ";
		838	}
		839	$query .= "ORDER BY waitingdate" ;
		840
		841	my $sth = $dbh->prepare($query);
		842	$sth->execute(@params);
		843
		844	my $data = $sth->fetchall_arrayref({});
		845	return ($data) ? $data : [];
		846	}
		847
794	=head2 GetReserveStatus	848	=head2 GetReserveStatus
795		849
796	$reservestatus = GetReserveStatus($itemnumber, $biblionumber);	850	$reservestatus = GetReserveStatus($itemnumber, $biblionumber);
Lines 1021-1027 sub CancelExpiredReserves { Link Here
1021	if ( $charge ) {	1075	if ( $charge ) {
1022	manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge);	1076	manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge);
1023	}	1077	}
1024	CancelReserve({ reserve_id => $res->{'reserve_id'} });	1078	CancelReserve({ reserve_id => $res->{'reserve_id'},
		1079	pickupexpired => $expiration,
		1080	});
1025	push @sb, printReserve($res,'tab',['reserve_id','borrowernumber','branchcode','waitingdate']).sprintf("% 14s",substr($expiration,0,10))."\| past lastpickupdate.\n" if $verbose;	1081	push @sb, printReserve($res,'tab',['reserve_id','borrowernumber','branchcode','waitingdate']).sprintf("% 14s",substr($expiration,0,10))."\| past lastpickupdate.\n" if $verbose;
1026	}	1082	}
1027	elsif($verbose > 1) {	1083	elsif($verbose > 1) {
Lines 1064-1070 sub AutoUnsuspendReserves { Link Here
1064		1120
1065	=head2 CancelReserve	1121	=head2 CancelReserve
1066		1122
1067	CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber ] });	1123	CancelReserve({ reserve_id => $reserve_id,
		1124	[ biblionumber => $biblionumber,
		1125	borrowernumber => $borrrowernumber,
		1126	itemnumber => $itemnumber ],
		1127	pickupexpired => DateTime->new(year => 2015, ...), #If the reserve was waiting for pickup, set the date the pickup wait period expired.
		1128	});
1068		1129
1069	Cancels a reserve.	1130	Cancels a reserve.
1070		1131
Lines 1074-1079 sub CancelReserve { Link Here
1074	my ( $params ) = @_;	1135	my ( $params ) = @_;
1075		1136
1076	my $reserve_id = $params->{'reserve_id'};	1137	my $reserve_id = $params->{'reserve_id'};
		1138	my $pickupexpired = $params->{pickupexpired};
		1139	if ($pickupexpired) {
		1140	unless (blessed($pickupexpired) && $pickupexpired->isa('DateTime')) {
		1141	Koha::Exception::BadParameter->throw(error => "CancelReserve():> Parameter 'pickupexpired' is not a DateTime-object or undef!");
		1142	}
		1143	}
		1144
1077	$reserve_id = GetReserveId( $params ) unless ( $reserve_id );	1145	$reserve_id = GetReserveId( $params ) unless ( $reserve_id );
1078		1146
1079	return unless ( $reserve_id );	1147	return unless ( $reserve_id );
Lines 1082-1096 sub CancelReserve { Link Here
1082		1150
1083	my $reserve = GetReserve( $reserve_id );	1151	my $reserve = GetReserve( $reserve_id );
1084		1152
		1153	my @params;
1085	my $query = "	1154	my $query = "
1086	UPDATE reserves	1155	UPDATE reserves
1087	SET cancellationdate = now(),	1156	SET cancellationdate = DATE(NOW()),
1088	found = Null,	1157	found = Null,
		1158	";
		1159	if ($pickupexpired) {
		1160	push @params, $pickupexpired->ymd();
		1161	$query .= "
		1162	pickupexpired = ?,
		1163	";
		1164	}
		1165	push @params, $reserve_id;
		1166	$query .= "
1089	priority = 0	1167	priority = 0
1090	WHERE reserve_id = ?	1168	WHERE reserve_id = ?
1091	";	1169	";
1092	my $sth = $dbh->prepare($query);	1170	my $sth = $dbh->prepare($query);
1093	$sth->execute( $reserve_id );	1171	$sth->execute( @params );
1094		1172
1095	$query = "	1173	$query = "
1096	INSERT INTO old_reserves	1174	INSERT INTO old_reserves

Line 0 Link Here

(-)a/installer/data/mysql/atomicupdate/Bug10744ExpireReservesConflictHoldOverReport.pl (+36 lines)
	1	#!/usr/bin/perl
	2
	3	# Copyright Open Source Freedom Fighters
	4	#
	5	# This file is part of Koha.
	6	#
	7	# Koha is free software; you can redistribute it and/or modify it under the
	8	# terms of the GNU General Public License as published by the Free Software
	9	# Foundation; either version 3 of the License, or (at your option) any later
	10	# version.
	11	#
	12	# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
	13	# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
	14	# A PARTICULAR PURPOSE. See the GNU General Public License for more details.
	15	#
	16	# You should have received a copy of the GNU General Public License along
	17	# with Koha; if not, write to the Free Software Foundation, Inc.,
	18	# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
	19
	20	use C4::Context;
	21	use Koha::AtomicUpdater;
	22
	23	my $dbh = C4::Context->dbh();
	24	my $atomicUpdater = Koha::AtomicUpdater->new();
	25
	26	unless($atomicUpdater->find('Bug10744')) {
	27
	28	$dbh->do("ALTER TABLE reserves ADD `pickupexpired` DATE DEFAULT NULL AFTER `expirationdate`");
	29	$dbh->do("ALTER TABLE reserves ADD KEY `reserves_pickupexpired` (`pickupexpired`)");
	30	$dbh->do("ALTER TABLE old_reserves ADD `pickupexpired` DATE DEFAULT NULL AFTER `expirationdate`");
	31	$dbh->do("ALTER TABLE old_reserves ADD KEY `old_reserves_pickupexpired` (`pickupexpired`)");
	32
	33	$dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('PickupExpiredHoldsOverReportDuration','1',NULL,\"For how many days holds expired by the 'ExpireReservesMaxPickUpDelay'-syspref are visible in the 'Hold Over'-tab in /circ/waitingreserves.pl ?\",'Integer')");
	34
	35	print "Upgrade done (Bug 10744 - ExpireReservesMaxPickUpDelay has minor workflow conflicts with hold(s) over report)\n";
	36	}

Lines 1706-1711 CREATE TABLE `old_reserves` ( -- this table holds all holds/reserves that have b Link Here

(-)a/installer/data/mysql/kohastructure.sql (+4 lines)
1706	`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	1706	`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
1707	`waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library	1707	`waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library
1708	`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)	1708	`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)
		1709	`pickupexpired` DATE DEFAULT NULL, -- if hold has been waiting but it expired before it was picked up, the expiration date is set here
1709	`lowestPriority` tinyint(1) NOT NULL, -- has this hold been pinned to the lowest priority in the holds queue (1 for yes, 0 for no)	1710	`lowestPriority` tinyint(1) NOT NULL, -- has this hold been pinned to the lowest priority in the holds queue (1 for yes, 0 for no)
1710	`suspend` BOOLEAN NOT NULL DEFAULT 0, -- in this hold suspended (1 for yes, 0 for no)	1711	`suspend` BOOLEAN NOT NULL DEFAULT 0, -- in this hold suspended (1 for yes, 0 for no)
1711	`suspend_until` DATETIME NULL DEFAULT NULL, -- the date this hold is suspended until (NULL for infinitely)	1712	`suspend_until` DATETIME NULL DEFAULT NULL, -- the date this hold is suspended until (NULL for infinitely)
Lines 1714-1719 CREATE TABLE `old_reserves` ( -- this table holds all holds/reserves that have b Link Here
1714	KEY `old_reserves_biblionumber` (`biblionumber`),	1715	KEY `old_reserves_biblionumber` (`biblionumber`),
1715	KEY `old_reserves_itemnumber` (`itemnumber`),	1716	KEY `old_reserves_itemnumber` (`itemnumber`),
1716	KEY `old_reserves_branchcode` (`branchcode`),	1717	KEY `old_reserves_branchcode` (`branchcode`),
		1718	KEY `old_reserves_pickupexpired` (`pickupexpired`),
1717	CONSTRAINT `old_reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`)	1719	CONSTRAINT `old_reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`)
1718	ON DELETE SET NULL ON UPDATE SET NULL,	1720	ON DELETE SET NULL ON UPDATE SET NULL,
1719	CONSTRAINT `old_reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`)	1721	CONSTRAINT `old_reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`)
Lines 1940-1945 CREATE TABLE `reserves` ( -- information related to holds/reserves in Koha Link Here
1940	`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	1942	`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
1941	`waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library	1943	`waitingdate` date default NULL, -- the date the item was marked as waiting for the patron at the library
1942	`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)	1944	`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)
		1945	`pickupexpired` DATE DEFAULT NULL, -- if hold has been waiting but it expired before it was picked up, the expiration date is set here
1943	`lowestPriority` tinyint(1) NOT NULL,	1946	`lowestPriority` tinyint(1) NOT NULL,
1944	`suspend` BOOLEAN NOT NULL DEFAULT 0,	1947	`suspend` BOOLEAN NOT NULL DEFAULT 0,
1945	`suspend_until` DATETIME NULL DEFAULT NULL,	1948	`suspend_until` DATETIME NULL DEFAULT NULL,
Lines 1949-1954 CREATE TABLE `reserves` ( -- information related to holds/reserves in Koha Link Here
1949	KEY `biblionumber` (`biblionumber`),	1952	KEY `biblionumber` (`biblionumber`),
1950	KEY `itemnumber` (`itemnumber`),	1953	KEY `itemnumber` (`itemnumber`),
1951	KEY `branchcode` (`branchcode`),	1954	KEY `branchcode` (`branchcode`),
		1955	KEY `reserves_pickupexpired` (`pickupexpired`),
1952	CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,	1956	CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
1953	CONSTRAINT `reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE,	1957	CONSTRAINT `reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE CASCADE,
1954	CONSTRAINT `reserves_ibfk_3` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE,	1958	CONSTRAINT `reserves_ibfk_3` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE,

Line 0 Link Here

(-)a/t/db_dependent/Reserves/pickupExpiredHoldsOverReportDuration.t (-1 / +121 lines)
0	-	1	#!/usr/bin/perl
		2
		3	# Copyright 2015 KohaSuomi
		4	#
		5	# This file is part of Koha.
		6	#
		7	# Koha is free software; you can redistribute it and/or modify it
		8	# under the terms of the GNU General Public License as published by
		9	# the Free Software Foundation; either version 3 of the License, or
		10	# (at your option) any later version.
		11	#
		12	# Koha is distributed in the hope that it will be useful, but
		13	# WITHOUT ANY WARRANTY; without even the implied warranty of
		14	# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
		15	# GNU General Public License for more details.
		16	#
		17	# You should have received a copy of the GNU General Public License
		18	# along with Koha; if not, see <http://www.gnu.org/licenses>.
		19
		20	use Modern::Perl;
		21	use Test::More;
		22	use Try::Tiny;
		23	use Scalar::Util qw(blessed);
		24
		25	use t::lib::TestObjects::ObjectFactory;
		26	use t::lib::TestObjects::HoldFactory;
		27	use t::lib::TestObjects::SystemPreferenceFactory;
		28
		29	##Setting up the test context
		30	my $testContext = {};
		31
		32	t::lib::TestObjects::SystemPreferenceFactory->createTestGroup([{preference => 'PickupExpiredHoldsOverReportDuration',
		33	value => 2,
		34	},
		35	{preference => 'ExpireReservesMaxPickUpDelay',
		36	value => 1,
		37	},
		38	{preference => 'ReservesMaxPickUpDelay',
		39	value => 6,
		40	},
		41	], undef, $testContext);
		42
		43	my $holds = t::lib::TestObjects::HoldFactory->createTestGroup([
		44	{cardnumber => '1A01',
		45	isbn => '987Kivi',
		46	barcode => '1N01',
		47	branchcode => 'CPL',
		48	waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 9)->iso8601(),
		49	reservenotes => 'expire3daysAgo',
		50	},
		51	{cardnumber => '1A01',
		52	isbn => '987Kivi',
		53	barcode => '1N02',
		54	branchcode => 'CPL',
		55	waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 8)->iso8601(),
		56	reservenotes => 'expire2daysAgo',
		57	},
		58	{cardnumber => '1A02',
		59	isbn => '987Kivi',
		60	barcode => '1N03',
		61	branchcode => 'CPL',
		62	waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 7)->iso8601(),
		63	reservenotes => 'expire1dayAgo1',
		64	},
		65	{cardnumber => '1A03',
66	isbn => '987Kivi',
67	barcode => '1N04',
68	branchcode => 'CPL',
69	waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 7)->iso8601(),
70	reservenotes => 'expire1dayAgo2',
71	},
72	{cardnumber => '1A04',
73	isbn => '987Kivi',
74	barcode => '1N05',
75	branchcode => 'CPL',
76	waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 6)->iso8601(),
77	reservenotes => 'expiresToday',
78	},
79	{cardnumber => '1A05',
80	isbn => '987Kivi',
81	barcode => '1N06',
82	branchcode => 'CPL',
83	waitingdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 5)->iso8601(),
84	reservenotes => 'expiresTomorrow',
85	},
86	], undef, $testContext);
87
88
89
90	##Test context set, starting testing:
91	subtest "Expiring holds and getting old_reserves" => \&expiringHoldsAndOld_reserves;
92	sub expiringHoldsAndOld_reserves {
93	eval { #run in a eval-block so we don't die without tearing down the test context
94	C4::Reserves::CancelExpiredReserves();
95	my $expiredReserves = C4::Reserves::GetExpiredReserves({});
96	ok($expiredReserves->[0]->{reserve_id} == $holds->{'expire2daysAgo'}->{reserve_id} &&
97	$expiredReserves->[0]->{pickupexpired} eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 2)->ymd()
98	, "Hold for Item 1N02 expired yesterday");
99	ok($expiredReserves->[1]->{reserve_id} == $holds->{'expire1dayAgo1'}->{reserve_id} &&
100	$expiredReserves->[1]->{pickupexpired} eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->ymd()
101	, "Hold for Item 1N03 expired today");
102	ok($expiredReserves->[2]->{reserve_id} == $holds->{'expire1dayAgo2'}->{reserve_id} &&
103	$expiredReserves->[2]->{pickupexpired} eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->ymd()
104	, "Hold for Item 1N04 expired today");
105	is($expiredReserves->[3], undef,
106	"Holds for Items 1N05 and 1N06 not expired.");
107	};
108	if ($@) { #Catch all leaking errors and gracefully terminate.
109	warn $@;
110	tearDown();
111	exit 1;
112	}
113	}
114
115	##All tests done, tear down test context
116	tearDown();
117	done_testing;
118
119	sub tearDown {
120	t::lib::TestObjects::ObjectFactory->tearDownTestContext($testContext);
121	}