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

(-)a/C4/Reserves.pm (+3 lines)
Lines 948-953 sub CancelExpiredReserves { Link Here
948
        if ( defined($hold->found) && $hold->found eq 'W' ) {
948
        if ( defined($hold->found) && $hold->found eq 'W' ) {
949
            $cancel_params->{charge_cancel_fee} = 1;
949
            $cancel_params->{charge_cancel_fee} = 1;
950
        }
950
        }
951
952
        $cancel_params->{pickupexpired} = dt_from_string($hold->expirationdate);
953
951
        $hold->cancel( $cancel_params );
954
        $hold->cancel( $cancel_params );
952
    }
955
    }
953
}
956
}
(-)a/Koha/Hold.pm (+11 lines)
Lines 38-43 use Koha::Old::Holds; Link Here
38
use Koha::Calendar;
38
use Koha::Calendar;
39
39
40
use Koha::Exceptions::Hold;
40
use Koha::Exceptions::Hold;
41
use Koha::Exceptions::BadParameter;
41
42
42
use base qw(Koha::Object);
43
use base qw(Koha::Object);
43
44
Lines 499-504 sub cancel { Link Here
499
    my ( $self, $params ) = @_;
500
    my ( $self, $params ) = @_;
500
    $self->_result->result_source->schema->txn_do(
501
    $self->_result->result_source->schema->txn_do(
501
        sub {
502
        sub {
503
504
            my $pickupexpired = $params->{pickupexpired};
505
            if ($pickupexpired) {
506
                unless ($pickupexpired && $pickupexpired->isa('DateTime')) {
507
                    Koha::Exceptions::BadParameter->throw(error => "cancel():> Parameter 'pickupexpired' is not a DateTime-object or undef!");
508
                } else {
509
                    $self->pickupexpired($pickupexpired->ymd());
510
                }
511
            }
512
502
            $self->cancellationdate( dt_from_string->strftime( '%Y-%m-%d %H:%M:%S' ) );
513
            $self->cancellationdate( dt_from_string->strftime( '%Y-%m-%d %H:%M:%S' ) );
503
            $self->priority(0);
514
            $self->priority(0);
504
            $self->cancellation_reason( $params->{cancellation_reason} );
515
            $self->cancellation_reason( $params->{cancellation_reason} );
(-)a/Koha/Old/Holds.pm (+59 lines)
Lines 20-27 package Koha::Old::Holds; Link Here
20
use Modern::Perl;
20
use Modern::Perl;
21
21
22
use Carp;
22
use Carp;
23
use Scalar::Util qw(blessed);
23
24
24
use Koha::Database;
25
use Koha::Database;
26
use Koha::Exceptions::BadParameter;
25
27
26
use Koha::Old::Hold;
28
use Koha::Old::Hold;
27
29
Lines 39-44 This object represents a set of holds that have been filled or canceled Link Here
39
41
40
=cut
42
=cut
41
43
44
=head3
45
46
my $expired_holds = Koha::Old::Holds->expired();
47
48
Returns all holds that have already expired and moved to old_reserves during period
49
of time given in 'PickupExpiredHoldsOverReportDuration'.
50
51
If branchcode is given returns expired holds for that branch and take into account
52
its holidays.
53
54
=cut
55
56
sub expired {
57
    my ($self, $params) = @_;
58
59
    my $pickupExpiredHoldsOverReportDuration = C4::Context->preference('PickupExpiredHoldsOverReportDuration');
60
    return undef unless $pickupExpiredHoldsOverReportDuration;
61
62
    my $branchcode = $params->{branchcode};
63
    if ($params->{from}) {
64
        unless (blessed($params->{from}) && $params->{from}->isa('DateTime')) {
65
            Koha::Exceptions::BadParameter->throw(error => "expired():> Parameter 'from' is not a DateTime-object or undef!");
66
        }
67
    }
68
    if ($params->{to}) {
69
        unless (blessed($params->{from}) && $params->{from}->isa('DateTime')) {
70
            Koha::Exceptions::BadParameter->throw(error => "expired():> Parameter 'from' is not a DateTime-object or undef!");
71
        }
72
    }
73
74
    #Calculate the days for which we get the expired reserves.
75
    my $fromDate   = $params->{from};
76
    my $toDate     = $params->{to}   || DateTime->now(time_zone => C4::Context->tz());
77
78
    unless ($fromDate) {
79
        $fromDate = DateTime->now( time_zone => C4::Context->tz() );
80
        #Look for previous open days
81
        if ($branchcode) {
82
            my $calendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => 'Default');
83
            foreach my $i (1..$pickupExpiredHoldsOverReportDuration) {
84
                $fromDate = $calendar->prev_open_days($fromDate, 1);
85
            }
86
        }
87
        #If no branch has been specified we cannot use a calendar, so simply just go back in time.
88
        else {
89
            $fromDate = DateTime->now(time_zone => C4::Context->tz())->subtract(days => $pickupExpiredHoldsOverReportDuration);
90
        }
91
    }
92
93
    my $search_params;
94
    $search_params->{priority} = 0;
95
    $search_params->{pickupexpired} = { -between => [ $fromDate->ymd(), $toDate->ymd() ] };
96
    $search_params->{branchcode} = $branchcode if defined $branchcode;
97
98
    return $self->search( $search_params, { order_by => 'waitingdate' } );
99
}
100
42
=head3 type
101
=head3 type
43
102
44
=cut
103
=cut
(-)a/circ/waitingreserves.pl (-1 / +8 lines)
Lines 39-44 use Koha::BiblioFrameworks; Link Here
39
use Koha::Items;
39
use Koha::Items;
40
use Koha::ItemTypes;
40
use Koha::ItemTypes;
41
use Koha::Patrons;
41
use Koha::Patrons;
42
use Koha::Old::Holds;
42
43
43
my $input = CGI->new;
44
my $input = CGI->new;
44
45
Lines 83-89 $template->param( all_branches => 1 ) if $all_branches; Link Here
83
my (@reserve_loop, @over_loop);
84
my (@reserve_loop, @over_loop);
84
# FIXME - Is priority => 0 useful? If yes it must be moved to waiting, otherwise we need to remove it from here.
85
# FIXME - Is priority => 0 useful? If yes it must be moved to waiting, otherwise we need to remove it from here.
85
my $holds = Koha::Holds->waiting->search({ priority => 0, ( $all_branches ? () : ( branchcode => $default ) ) }, { order_by => ['waitingdate'] });
86
my $holds = Koha::Holds->waiting->search({ priority => 0, ( $all_branches ? () : ( branchcode => $default ) ) }, { order_by => ['waitingdate'] });
86
87
my $expired_holds = $all_branches ? Koha::Old::Holds->expired() : Koha::Old::Holds->expired({branchcode => $default});
87
# get reserves for the branch we are logged into, or for all branches
88
# get reserves for the branch we are logged into, or for all branches
88
89
89
my $today = Date_to_Days(&Today);
90
my $today = Date_to_Days(&Today);
Lines 108-113 while ( my $hold = $holds->next ) { Link Here
108
    
109
    
109
}
110
}
110
111
112
if($expired_holds){
113
    while (my $expired_hold = $expired_holds->next){
114
        push @over_loop, $expired_hold;
115
    }
116
}
117
111
$template->param(cancel_result => \@cancel_result) if @cancel_result;
118
$template->param(cancel_result => \@cancel_result) if @cancel_result;
112
119
113
$template->param(
120
$template->param(
(-)a/installer/data/mysql/atomicupdate/bug_10744-expire_reserves_conflict_hold_over_report.perl (+16 lines)
Line 0 Link Here
1
$DBversion = 'XXX'; # will be replaced by the RM
2
if( CheckVersion( $DBversion ) ) {
3
    # you can use $dbh here like:
4
    # $dbh->do( "ALTER TABLE biblio ADD COLUMN badtaste int" );
5
6
    $dbh->do("ALTER TABLE reserves ADD `pickupexpired` DATE DEFAULT NULL AFTER `expirationdate`");
7
    $dbh->do("ALTER TABLE reserves ADD KEY `reserves_pickupexpired` (`pickupexpired`)");
8
    $dbh->do("ALTER TABLE old_reserves ADD `pickupexpired` DATE DEFAULT NULL AFTER `expirationdate`");
9
    $dbh->do("ALTER TABLE old_reserves ADD KEY `old_reserves_pickupexpired` (`pickupexpired`)");
10
11
    $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')");
12
13
    # Always end with this (adjust the bug info)
14
    SetVersion( $DBversion );
15
    print "Upgrade to $DBversion done (Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report)\n";
16
}
(-)a/installer/data/mysql/kohastructure.sql (+4 lines)
Lines 3872-3877 CREATE TABLE `old_reserves` ( Link Here
3872
  `itemnumber` int(11) DEFAULT NULL COMMENT 'foreign key from the items table defining the specific item the patron has placed on hold or the item this hold was filled with',
3872
  `itemnumber` int(11) DEFAULT NULL COMMENT 'foreign key from the items table defining the specific item the patron has placed on hold or the item this hold was filled with',
3873
  `waitingdate` date DEFAULT NULL COMMENT 'the date the item was marked as waiting for the patron at the library',
3873
  `waitingdate` date DEFAULT NULL COMMENT 'the date the item was marked as waiting for the patron at the library',
3874
  `expirationdate` date DEFAULT NULL COMMENT 'the date the hold expires (usually the date entered by the patron to say they don''t need the hold after a certain date)',
3874
  `expirationdate` date DEFAULT NULL COMMENT 'the date the hold expires (usually the date entered by the patron to say they don''t need the hold after a certain date)',
3875
  `pickupexpired` DATE DEFAULT NULL COMMENT 'if hold has been waiting but it expired before it was picked up, the expiration date is set here',
3875
  `lowestPriority` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'has this hold been pinned to the lowest priority in the holds queue (1 for yes, 0 for no)',
3876
  `lowestPriority` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'has this hold been pinned to the lowest priority in the holds queue (1 for yes, 0 for no)',
3876
  `suspend` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'in this hold suspended (1 for yes, 0 for no)',
3877
  `suspend` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'in this hold suspended (1 for yes, 0 for no)',
3877
  `suspend_until` datetime DEFAULT NULL COMMENT 'the date this hold is suspended until (NULL for infinitely)',
3878
  `suspend_until` datetime DEFAULT NULL COMMENT 'the date this hold is suspended until (NULL for infinitely)',
Lines 3883-3888 CREATE TABLE `old_reserves` ( Link Here
3883
  KEY `old_reserves_biblionumber` (`biblionumber`),
3884
  KEY `old_reserves_biblionumber` (`biblionumber`),
3884
  KEY `old_reserves_itemnumber` (`itemnumber`),
3885
  KEY `old_reserves_itemnumber` (`itemnumber`),
3885
  KEY `old_reserves_branchcode` (`branchcode`),
3886
  KEY `old_reserves_branchcode` (`branchcode`),
3887
  KEY `old_reserves_pickupexpired` (`pickupexpired`),
3886
  KEY `old_reserves_itemtype` (`itemtype`),
3888
  KEY `old_reserves_itemtype` (`itemtype`),
3887
  CONSTRAINT `old_reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE SET NULL,
3889
  CONSTRAINT `old_reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE SET NULL,
3888
  CONSTRAINT `old_reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE SET NULL ON UPDATE SET NULL,
3890
  CONSTRAINT `old_reserves_ibfk_2` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE SET NULL ON UPDATE SET NULL,
Lines 4311-4316 CREATE TABLE `reserves` ( Link Here
4311
  `itemnumber` int(11) DEFAULT NULL COMMENT 'foreign key from the items table defining the specific item the patron has placed on hold or the item this hold was filled with',
4313
  `itemnumber` int(11) DEFAULT NULL COMMENT 'foreign key from the items table defining the specific item the patron has placed on hold or the item this hold was filled with',
4312
  `waitingdate` date DEFAULT NULL COMMENT 'the date the item was marked as waiting for the patron at the library',
4314
  `waitingdate` date DEFAULT NULL COMMENT 'the date the item was marked as waiting for the patron at the library',
4313
  `expirationdate` date DEFAULT NULL COMMENT 'the date the hold expires (usually the date entered by the patron to say they don''t need the hold after a certain date)',
4315
  `expirationdate` date DEFAULT NULL COMMENT 'the date the hold expires (usually the date entered by the patron to say they don''t need the hold after a certain date)',
4316
  `pickupexpired` DATE DEFAULT NULL COMMENT 'if hold has been waiting but it expired before it was picked up, the expiration date is set here',
4314
  `lowestPriority` tinyint(1) NOT NULL DEFAULT 0,
4317
  `lowestPriority` tinyint(1) NOT NULL DEFAULT 0,
4315
  `suspend` tinyint(1) NOT NULL DEFAULT 0,
4318
  `suspend` tinyint(1) NOT NULL DEFAULT 0,
4316
  `suspend_until` datetime DEFAULT NULL,
4319
  `suspend_until` datetime DEFAULT NULL,
Lines 4323-4328 CREATE TABLE `reserves` ( Link Here
4323
  KEY `biblionumber` (`biblionumber`),
4326
  KEY `biblionumber` (`biblionumber`),
4324
  KEY `itemnumber` (`itemnumber`),
4327
  KEY `itemnumber` (`itemnumber`),
4325
  KEY `branchcode` (`branchcode`),
4328
  KEY `branchcode` (`branchcode`),
4329
  KEY `reserves_pickupexpired` (`pickupexpired`),
4326
  KEY `desk_id` (`desk_id`),
4330
  KEY `desk_id` (`desk_id`),
4327
  KEY `itemtype` (`itemtype`),
4331
  KEY `itemtype` (`itemtype`),
4328
  CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
4332
  CONSTRAINT `reserves_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
(-)a/installer/data/mysql/mandatory/sysprefs.sql (+1 lines)
Lines 195-200 INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` Link Here
195
('EnhancedMessagingPreferencesOPAC', '1', NULL, 'If ON, show patrons messaging setting on the OPAC.', 'YesNo'),
195
('EnhancedMessagingPreferencesOPAC', '1', NULL, 'If ON, show patrons messaging setting on the OPAC.', 'YesNo'),
196
('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'),
196
('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'),
197
('ExpireReservesMaxPickUpDelay','0','','Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay','YesNo'),
197
('ExpireReservesMaxPickUpDelay','0','','Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay','YesNo'),
198
('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'),
198
('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'),
199
('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'),
199
('ExpireReservesOnHolidays', '1', NULL, 'If false, reserves at a library will not be canceled on days the library is not open.', 'YesNo'),
200
('ExpireReservesOnHolidays', '1', NULL, 'If false, reserves at a library will not be canceled on days the library is not open.', 'YesNo'),
200
('ExcludeHolidaysFromMaxPickUpDelay', '0', NULL, 'If ON, reserves max pickup delay takes into accountthe closed days.', 'YesNo'),
201
('ExcludeHolidaysFromMaxPickUpDelay', '0', NULL, 'If ON, reserves max pickup delay takes into accountthe closed days.', 'YesNo'),
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref (+4 lines)
Lines 738-743 Circulation: Link Here
738
            - pref: ExpireReservesMaxPickUpDelayCharge
738
            - pref: ExpireReservesMaxPickUpDelayCharge
739
              class: currency
739
              class: currency
740
            - "."
740
            - "."
741
        -
742
            - pref: PickupExpiredHoldsOverReportDuration
743
              class: integer
744
            - "For how many days holds expired by the 'ExpireReservesMaxPickUpDelay'-syspref are visible in the 'Hold Over'-tab in /circ/waitingreserves.pl ?"
741
        -
745
        -
742
            - Satisfy holds using items from the libraries
746
            - Satisfy holds using items from the libraries
743
            - pref: StaticHoldsQueueWeight
747
            - pref: StaticHoldsQueueWeight
(-)a/t/db_dependent/Koha/Old.t (-2 / +102 lines)
Lines 19-35 Link Here
19
19
20
use Modern::Perl;
20
use Modern::Perl;
21
21
22
use Test::More tests => 3;
22
use Test::More tests => 4;
23
23
24
use DateTime;
25
use DateTime::TimeZone;
26
27
use C4::Reserves;
24
use Koha::Database;
28
use Koha::Database;
25
use Koha::Old::Patrons;
29
use Koha::Old::Patrons;
26
use Koha::Old::Biblios;
30
use Koha::Old::Biblios;
27
use Koha::Old::Checkouts;
31
use Koha::Old::Checkouts;
28
use Koha::Old::Items;
32
use Koha::Old::Items;
33
use Koha::Old::Holds;
34
use Koha::Calendar;
29
35
30
use t::lib::TestBuilder;
36
use t::lib::TestBuilder;
37
use t::lib::Mocks;
31
38
32
my $schema  = Koha::Database->new->schema;
39
my $schema  = Koha::Database->new->schema;
40
41
#since we use calendar here, flush caches
42
Koha::Caches->get_instance()->flush_all();
43
33
my $builder = t::lib::TestBuilder->new;
44
my $builder = t::lib::TestBuilder->new;
34
45
35
subtest 'Koha::Old::Patrons' => sub {
46
subtest 'Koha::Old::Patrons' => sub {
Lines 85-87 subtest 'Koha::Old::Checkout->library() tests' => sub { Link Here
85
96
86
    $schema->storage->txn_rollback;
97
    $schema->storage->txn_rollback;
87
};
98
};
88
- 
99
100
subtest 'Koha::Old::Holds' => sub {
101
102
    $schema->storage->txn_begin;
103
104
    plan tests => 5;
105
106
    my $branch = $builder->build_object({ class => 'Koha::Libraries' });
107
    my $calendar = Koha::Calendar->new( branchcode => $branch->branchcode );
108
    my $holiday_date = DateTime->now(time_zone => C4::Context->tz())->subtract(days => 2);
109
    my $holiday = $builder->build({
110
        source => 'SpecialHoliday',
111
        value  => {
112
            branchcode  => $branch->branchcode,
113
            day         => $holiday_date->day,
114
            month       => $holiday_date->month,
115
            year        => $holiday_date->year,
116
        },
117
    });
118
119
    t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelay', 1);
120
121
    t::lib::Mocks::mock_preference('ReservesMaxPickUpDelay', 6);
122
    t::lib::Mocks::mock_preference('PickupExpiredHoldsOverReportDuration', 2);
123
124
    my $hold_1 = $builder->build_object({
125
        class => 'Koha::Holds',
126
        value => {
127
            branchcode => $branch->branchcode,
128
            expirationdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 3)->iso8601(),
129
        }
130
    });
131
    my $hold_2 = $builder->build_object({
132
        class => 'Koha::Holds',
133
        value => {
134
            branchcode => $branch->branchcode,
135
            expirationdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 2)->iso8601(),
136
        }
137
    });
138
    my $hold_3 = $builder->build_object({
139
        class => 'Koha::Holds',
140
        value => {
141
            branchcode => $branch->branchcode,
142
            expirationdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->iso8601(),
143
        }
144
    });
145
    my $hold_4 = $builder->build_object({
146
        class => 'Koha::Holds',
147
        value => {
148
            branchcode => $branch->branchcode,
149
            expirationdate => DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->iso8601(),
150
        }
151
    });
152
    my $hold_5 = $builder->build_object({
153
        class => 'Koha::Holds',
154
        value => {
155
            branchcode => $branch->branchcode,
156
            expirationdate => DateTime->now(time_zone => C4::Context->tz()),
157
        }
158
    });
159
    my $hold_6 = $builder->build_object({
160
        class => 'Koha::Holds',
161
        value => {
162
            branchcode => $branch->branchcode,
163
            expirationdate => DateTime->now(time_zone => C4::Context->tz())->add(days => 1)->iso8601(),
164
        }
165
    });
166
167
    C4::Reserves::CancelExpiredReserves();
168
169
    my $expired_holds = Koha::Old::Holds->expired({ branchcode => $branch->branchcode });
170
    is($expired_holds->count(), 3, 'expired() returns 3 holds');
171
172
    my $expired_hold_1 = $expired_holds->find($hold_1->id);
173
    my $expired_hold_2 = $expired_holds->find($hold_2->id);
174
    my $expired_hold_3 = $expired_holds->find($hold_3->id);
175
    my $expired_hold_4 = $expired_holds->find($hold_4->id);
176
    my $expired_hold_5 = $expired_holds->find($hold_5->id);
177
    my $expired_hold_6 = $expired_holds->find($hold_6->id);
178
179
    is($expired_hold_1, undef, "Hold 1 not found with expired().");
180
    ok($expired_hold_2->pickupexpired eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 2)->ymd()
181
    , "Pick up for hold 2 expired 2 days ago.");
182
    ok($expired_hold_3->pickupexpired && $expired_hold_4->pickupexpired eq DateTime->now(time_zone => C4::Context->tz())->subtract(days => 1)->ymd()
183
    , "Pick up for holds 3 and 4 expired yesterday.");
184
    is($expired_hold_5 && $expired_hold_6, undef, "Holds 5 and 6 not expired.");
185
186
    $schema->storage->txn_rollback;
187
188
};

Return to bug 10744