@@ -, +, @@ expiration date options for reserves - Check out to a patron and check the holds tab - View patron details and check the holds tab - Patron hold history - Patron details -> Print -> Print summary - Place a hold on a title which already has at least one hold on it, check the table of existing holds. - Circulation -> Holds awaiting pickup - Log in to the OPAC as a patron with holds - Your summary -> Holds tab - Your holds history (OPACHoldsHistory must be enabled). --- C4/ILSDI/Services.pm | 10 ++++- C4/Reserves.pm | 4 +- Koha/Club/Hold.pm | 2 +- Koha/Hold.pm | 40 ++++++++++++------- Koha/REST/V1/Clubs/Holds.pm | 2 +- Koha/REST/V1/Holds.pm | 2 +- circ/waitingreserves.pl | 4 +- .../bug24194-add-ReserveExpiration-syspref.pl | 12 ++++++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../prog/en/includes/holds_table.inc | 6 ++- .../prog/en/includes/waiting_holds.inc | 4 ++ .../admin/preferences/circulation.pref | 14 +++++-- .../prog/en/modules/circ/circulation.tt | 1 + .../prog/en/modules/members/holdshistory.tt | 21 +++++++--- .../en/modules/members/moremember-print.tt | 8 +++- .../prog/en/modules/members/moremember.tt | 1 + .../prog/en/modules/reserve/request.tt | 10 +++-- koha-tmpl/intranet-tmpl/prog/js/holds.js | 5 ++- .../bootstrap/en/includes/holds-table.inc | 32 ++++++++------- .../bootstrap/en/modules/opac-holdshistory.tt | 16 +++++--- .../bootstrap/en/modules/opac-reserve.tt | 2 + misc/cronjobs/holds/cancel_expired_holds.pl | 4 +- t/db_dependent/Hold.t | 37 ++++++++++++++++- t/db_dependent/Reserves.t | 36 ++++++++++++++++- 24 files changed, 210 insertions(+), 64 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug24194-add-ReserveExpiration-syspref.pl --- a/C4/ILSDI/Services.pm +++ a/C4/ILSDI/Services.pm @@ -777,7 +777,10 @@ sub HoldTitle { return { code => 'cannotBeTransferred' } unless $biblio->can_be_transferred({ to => $destination }); my $resdate = $cgi->param('start_date'); - my $expdate = $cgi->param('expiry_date'); + my $expdate; + if ( C4::Context->preference('ReserveExpiration') and $cgi->param('expiry_date') ) { + $expdate = $cgi->param('expiry_date'); + } # Add the reserve # $branch, $borrowernumber, $biblionumber, @@ -872,7 +875,10 @@ sub HoldItem { return { code => $canitembereserved } unless $canitembereserved eq 'OK'; my $resdate = $cgi->param('start_date'); - my $expdate = $cgi->param('expiry_date'); + my $expdate; + if ( C4::Context->preference('ReserveExpiration') and $cgi->param('expiry_date') ) { + $expdate = $cgi->param('expiry_date'); + } # Add the reserve my $priority = C4::Reserves::CalculatePriority($biblionumber); --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -186,7 +186,7 @@ sub AddReserve { my $biblionumber = $params->{biblionumber}; my $priority = $params->{priority}; my $resdate = $params->{reservation_date}; - my $patron_expiration_date = $params->{expiration_date}; + my $patron_expiration_date = C4::Context->preference('ReserveExpiration') ? $params->{expiration_date} : undef; my $notes = $params->{notes}; my $title = $params->{title}; my $checkitem = $params->{itemnumber}; @@ -2171,7 +2171,7 @@ sub RevertWaitingStatus { priority => 1, found => undef, waitingdate => undef, - expirationdate => $hold->patron_expiration_date, + expirationdate => undef, itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, } )->store(); --- a/Koha/Club/Hold.pm +++ a/Koha/Club/Hold.pm @@ -125,7 +125,7 @@ sub add { borrowernumber => $patron_id, biblionumber => $params->{biblio_id}, priority => $priority, - expiration_date => $params->{expiration_date}, + expiration_date => C4::Context->preference('ReserveExpiration') ? $params->{expiration_date} : undef, notes => $params->{notes}, title => $biblio->title, itemnumber => $params->{item_id}, --- a/Koha/Hold.pm +++ a/Koha/Hold.pm @@ -228,10 +228,14 @@ sub set_waiting { desk_id => $desk_id, }; + my $new_expiration_date; + + if ( C4::Context->preference('ReserveExpiration') ) { + my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); - my $new_expiration_date = $today->clone->add(days => $max_pickup_delay); + $new_expiration_date = $today->clone->add(days => $max_pickup_delay); if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype; @@ -243,25 +247,31 @@ sub set_waiting { } ); my $calendar = Koha::Calendar->new( branchcode => $self->branchcode, days_mode => $daysmode ); - $new_expiration_date = $calendar->days_forward( dt_from_string(), $max_pickup_delay ); } - # If patron's requested expiration date is prior to the - # calculated one, we keep the patron's one. - if ( $self->patron_expiration_date ) { - my $requested_expiration = dt_from_string( $self->patron_expiration_date ); - - my $cmp = - $requested_expiration - ? DateTime->compare( $requested_expiration, $new_expiration_date ) - : 0; - - $new_expiration_date = - $cmp == -1 ? $requested_expiration : $new_expiration_date; + if ( C4::Context->preference('ExpireReservesMaxPickUpDelay') ) { + # If patron's requested expiration date is prior to the + # calculated one, we keep the patron's one. + if ( $self->patron_expiration_date ) { + my $requested_expiration = dt_from_string( $self->patron_expiration_date ); + + my $cmp = + $requested_expiration + ? DateTime->compare( $requested_expiration, $new_expiration_date ) + : 0; + + $new_expiration_date = + $cmp == -1 ? $requested_expiration : $new_expiration_date; + } + } else { + # if holds don't need a waiting expiration date to be expired automatically, don't set one + $new_expiration_date = undef; } - $values->{expirationdate} = $new_expiration_date->ymd; + } # ReserveExpiration + + $values->{expirationdate} = $new_expiration_date ? $new_expiration_date->ymd : undef; $self->set($values)->store(); --- a/Koha/REST/V1/Clubs/Holds.pm +++ a/Koha/REST/V1/Clubs/Holds.pm @@ -53,7 +53,7 @@ sub add { my $pickup_library_id = $body->{pickup_library_id}; my $item_id = $body->{item_id}; my $item_type = $body->{item_type}; - my $expiration_date = $body->{expiration_date}; + my $expiration_date = C4::Context->preference('ReserveExpiration') ? $body->{expiration_date} : undef; my $notes = $body->{notes}; my $default_patron_home = $body->{default_patron_home}; --- a/Koha/REST/V1/Holds.pm +++ a/Koha/REST/V1/Holds.pm @@ -75,7 +75,7 @@ sub add { my $item_id = $body->{item_id}; my $patron_id = $body->{patron_id}; my $item_type = $body->{item_type}; - my $expiration_date = $body->{expiration_date}; + my $expiration_date = C4::Context->preference('ReserveExpiration') ? $body->{expiration_date} : undef; my $notes = $body->{notes}; my $hold_date = $body->{hold_date}; my $non_priority = $body->{non_priority}; --- a/circ/waitingreserves.pl +++ a/circ/waitingreserves.pl @@ -100,9 +100,9 @@ while ( my $hold = $holds->next ) { next unless $hold->waitingdate; my ( $expire_year, $expire_month, $expire_day ) = split (/-/, $hold->expirationdate); - my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day ); + my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day ) if $hold->expirationdate; - if ($today > $calcDate) { + if ($calcDate and $today > $calcDate) { if ($cancelall) { my $res = cancel( $hold->item->itemnumber, $hold->borrowernumber, $hold->item->holdingbranch, $hold->item->homebranch, !$transfer_when_cancel_all ); push @cancel_result, $res if $res; --- a/installer/data/mysql/atomicupdate/bug24194-add-ReserveExpiration-syspref.pl +++ a/installer/data/mysql/atomicupdate/bug24194-add-ReserveExpiration-syspref.pl @@ -0,0 +1,12 @@ +use Modern::Perl; + +return { + bug_number => "24194", + description => "Add new system preference ReserveExpiration", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + + $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ("ReserveExpiration", 1, NULL, "Enable the use of expiration date in holds module.", "YesNo") }); + }, +}; --- a/installer/data/mysql/mandatory/sysprefs.sql +++ a/installer/data/mysql/mandatory/sysprefs.sql @@ -606,6 +606,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('RequireChoosingExistingAuthority','0',NULL,'Require existing authority selection in controlled fields during cataloging.','YesNo'), ('RequirePaymentType','0','','Require staff to select a payment type when a payment is made','YesNo'), ('RequireStrongPassword','1','','Require a strong login password for staff and patrons','YesNo'), +('ReserveExpiration', 1, NULL, 'Enable the use of expiration date in holds module.', 'YesNo'), ('ReservesControlBranch','PatronLibrary','ItemHomeLibrary|PatronLibrary','Branch checked for members reservations rights','Choice'), ('ReservesMaxPickUpDelay','7','','Define the Maximum delay to pick up an item on hold','Integer'), ('ReservesNeedReturns','1','','If ON, a hold placed on an item available in this library must be checked-in, otherwise, a hold on a specific item, that is in the library & available is considered available','YesNo'), --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -12,7 +12,9 @@ Patron Notes Date - Expiration + [% IF Koha.Preference('ReserveExpiration') %] + Expiration + [% END %] Pickup library Details [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] @@ -124,9 +126,11 @@ [% hold.date | $KohaDates %] [% END %] + [% IF Koha.Preference('ReserveExpiration') %] + [% END %] [%- IF ( hold.found ) -%] --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc @@ -10,7 +10,9 @@ [% IF table_name == 'holdscr' %] Date cancellation requested [% END %] + [% IF Koha.Preference('ReserveExpiration') %] Expiration date + [% END %] Title Patron Home library @@ -35,7 +37,9 @@ [% END %] [% END %] + [% IF Koha.Preference('ReserveExpiration') %] [% reserveloo.expirationdate | $KohaDates %] + [% END %] [% INCLUDE 'biblio-title.inc' biblio=reserveloo.biblio link = 1 %] [% UNLESS ( item_level_itypes ) %] --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -766,13 +766,13 @@ Circulation: - Mark a hold as problematic if it has been waiting for more than - pref: ReservesMaxPickUpDelay class: integer - - days. + - 'days.
NOTE: If ReserveExpiration is disabled, holds marked as problematic must be dealt with manually as they will never automatically expire.' - - pref: ExpireReservesMaxPickUpDelay choices: 1: Allow 0: "Don't allow" - - 'holds to expire automatically if they have not been picked by within the time period specified in the ReservesMaxPickUpDelay system preference.
' + - 'holds to expire automatically if they have not been picked by within the time period specified in the ReservesMaxPickUpDelay system preference.
NOTE: This system preference requires the misc/cronjobs/holds/cancel_expired_holds.pl cronjob. Ask your system administrator to schedule it.
NOTE: This will not apply if ReserveExpiration is disabled.
' - pref: ExpireReservesAutoFill choices: 1: "Do" @@ -787,7 +787,7 @@ Circulation: - If using ExpireReservesMaxPickUpDelay, charge a patron who allows their waiting hold to expire a fee of - pref: ExpireReservesMaxPickUpDelayCharge class: currency - - "." + - '.
NOTE: This will not apply if ReserveExpiration is disabled.' - - Satisfy holds using items from the libraries - pref: StaticHoldsQueueWeight @@ -877,7 +877,7 @@ Circulation: choices: 1: allow 0: "don't allow" - - expired holds to be canceled on days the library is closed per the calendar. + - 'expired holds to be canceled on days the library is closed per the calendar.
NOTE: This will not apply if ReserveExpiration is disabled.' - - pref: ExcludeHolidaysFromMaxPickUpDelay choices: @@ -1006,6 +1006,12 @@ Circulation: 1: block 0: allow - renewing of items from the staff interface and via the misc/cronjobs/automatic_renewals.pl cronjob. + - + - pref: ReserveExpiration + choices: + 1: Enable + 0: "Disable" + - the use of expiration dates in holds. When enabled, patrons can specify a date when their hold is not needed, any hold awaiting pickup will automatically expire after it has been waiting for a problematic number of days, and expiration dates will show on both the OPAC and staff interface. Interlibrary loans: - - pref: ILLModule --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -1055,6 +1055,7 @@ relatives_borrowernumbers.push("[% b | html %]"); [% END %] var SuspendHoldsIntranet = [% ( Koha.Preference('SuspendHoldsIntranet') ) ? 1 : 0 | html %]; + var ReserveExpiration = [% ( Koha.Preference('ReserveExpiration') ) ? 1 : 0 | html %]; [% Asset.js("js/pages/circulation.js") | $raw %] [% Asset.js("js/checkouts.js") | $raw %] --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -139,12 +139,23 @@ [% INCLUDE 'str/members-menu.inc' %] [% Asset.js("js/members-menu.js") | $raw %] [% Asset.js("js/pages/circulation.js") | $raw %] [% Asset.js("js/checkouts.js") | $raw %] --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -591,10 +591,12 @@ [% END %] -
  • - - -
  • + [% IF Koha.Preference('ReserveExpiration') %] +
  • + + +
  • + [% END %] [% UNLESS ( multi_hold ) %]
  • --- a/koha-tmpl/intranet-tmpl/prog/js/holds.js +++ a/koha-tmpl/intranet-tmpl/prog/js/holds.js @@ -223,7 +223,10 @@ $(document).ready(function() { else { return oObj.branchcode.escapeHtml() || ""; } } }, - { "data": { _: "expirationdate_formatted", "sort": "expirationdate" } }, + { + "visible": ReserveExpiration, + "data": { _: "expirationdate_formatted", "sort": "expirationdate" } + }, { "mDataProp": function( oObj ) { if ( oObj.priority && parseInt( oObj.priority ) && parseInt( oObj.priority ) > 0 ) { --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc +++ a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc @@ -16,7 +16,9 @@ [% ELSE %] Placed on [% END %] - Expires on + [% IF Koha.Preference('ReserveExpiration') %] + Expires on + [% END %] [% UNLESS( singleBranchMode) %] Pickup location [% END %] @@ -68,20 +70,22 @@ [% HOLD.reservedate | $KohaDates %] - [% IF ! HOLD.found %] - - [% IF ( HOLD.expirationdate ) %] - Expiration: - [% HOLD.expirationdate | $KohaDates %] - [% ELSE %] - Expiration: - Never expires - [% END %] - [% ELSE %] - - - + [% IF Koha.Preference('ReserveExpiration') %] + [% IF ! HOLD.found %] + + [% IF ( HOLD.expirationdate ) %] + Expiration: + [% HOLD.expirationdate | $KohaDates %] + [% ELSE %] + Expiration: + Never expires + [% END %] + [% ELSE %] + + - + [% END %] + [% END %] - [% UNLESS( singleBranchMode) %] Pickup location: --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt @@ -106,7 +106,9 @@ Barcode Library Hold date - Expiration date + [% IF Koha.Preference('ReserveExpiration') %] + Expiration date + [% END %] Waiting date Cancellation date [% IF show_itemtype_column %] @@ -123,11 +125,13 @@ [% hold.item.barcode | html %] [% Branches.GetName( hold.branchcode ) | html %] [% hold.reservedate | $KohaDates %] - - [% IF hold.expirationdate %] - [% hold.expirationdate | $KohaDates %] - [% END %] - + [% IF Koha.Preference('ReserveExpiration') %] + + [% IF hold.expirationdate %] + [% hold.expirationdate | $KohaDates %] + [% END %] + + [% END %] [% IF hold.waitingdate %] [% hold.waitingdate | $KohaDates %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -296,11 +296,13 @@
  • [% END %] + [% IF Koha.Preference('ReserveExpiration') %]
  • [% INCLUDE 'date-format.inc' %]
  • + [% END %] [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
  • --- a/misc/cronjobs/holds/cancel_expired_holds.pl +++ a/misc/cronjobs/holds/cancel_expired_holds.pl @@ -75,6 +75,8 @@ pod2usage(1) if $help; cronlogaction({ info => $command_line_options }); -C4::Reserves::CancelExpiredReserves($reason); +if ( C4::Context->preference('ReserveExpiration') ) { + C4::Reserves::CancelExpiredReserves($reason); +} cronlogaction({ action => 'End', info => "COMPLETED" }); --- a/t/db_dependent/Hold.t +++ a/t/db_dependent/Hold.t @@ -29,7 +29,7 @@ use Koha::Item; use Koha::DateUtils qw( dt_from_string ); use t::lib::TestBuilder; -use Test::More tests => 35; +use Test::More tests => 36; use Test::Exception; use Test::Warn; @@ -386,3 +386,38 @@ subtest 'suspend() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'ReserveExpiration syspref tests' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 ); + + # Disable expiration date for reserves + t::lib::Mocks::mock_preference( 'ReserveExpiration', 0 ); + + my $expirationdate = dt_from_string->add( days => 1 ); + my $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { patron_expiration_date => $expirationdate } + } + ); + $hold->set_waiting; + + is( $hold->expirationdate, undef, "No expiration date should be set with reserve expiration disabled" ); + + # Re-enable expiration date for reserves + t::lib::Mocks::mock_preference( 'ReserveExpiration', 1 ); + + $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { patron_expiration_date => $expirationdate } + } + ); + $hold->set_waiting(); + + is( $hold->expirationdate, $expirationdate->ymd, "Expiration date is set as expected" ); + + $schema->storage->txn_rollback; +}; --- a/t/db_dependent/Reserves.t +++ a/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 77; +use Test::More tests => 78; use Test::MockModule; use Test::Warn; @@ -1406,6 +1406,40 @@ subtest 'ModReserveAffect logging' => sub { like( $log->info, qr{$expected}, 'Timestamp logged is the current one' ); }; +subtest 'ReserveExpiration syspref tests' => sub { + + plan tests => 2; + + t::lib::Mocks::mock_preference( 'ReserveExpiration', 1 ); + + my $expirationdate = dt_from_string->add( days => 1 ); + my $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { expirationdate => $expirationdate } + } + ); + + # Disable expiration date for reserves + t::lib::Mocks::mock_preference( 'ReserveExpiration', 0 ); + + my $reverted = C4::Reserves::RevertWaitingStatus({ itemnumber => $hold->itemnumber }); + + is( $reverted->expirationdate, undef, "Expiration date should be removed with reserve expiration disabled" ); + + # Re-enable expiration date for reserves + t::lib::Mocks::mock_preference( 'ReserveExpiration', 1 ); + + $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { expirationdate => $expirationdate } + } + ); + + $reverted = C4::Reserves::RevertWaitingStatus({ itemnumber => $hold->itemnumber }); + + is( $reverted->expirationdate, undef, "Expiration date is still removed because we can't reset to the original expiration date" ); +}; + sub count_hold_print_messages { my $message_count = $dbh->selectall_arrayref(q{ SELECT COUNT(*) --