From 8b02d1f89f06847ec02d598da89549e10c9106c6 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 9 Dec 2019 04:33:42 +0000 Subject: [PATCH] Bug 24194: Add ReserveExpiration system preference to disable expiration date options for reserves To test: 1) Update database 2) Go to place a hold on any biblio in the staff intranet and confirm you can see the 'Hold expires on date' field. 3) In another tab, go to place a hold on any biblio in the OPAC and confirm you can see the 'Hold not needed after' field as an option. 4) In yet another tab, open the staff intranet and place a reserve for a user. Check it in and set the reserve as waiting. Notice that an expiration date has now been generated for this reserve. 5) Attempt to check out the item you reserved to some other borrower. Revert waiting status. Notice that the expiration date that was generated is removed. 6) Go to Administration -> system preferences and set ReserveExpiration system preference to 'Disable' 7) Refresh the hold request page in the intranet. Confirm the expiration date field disappears. 8) Refresh the hold request page in the OPAC. Confirm the expiration date field disappears. 9) Place another reserve. Check it in and set the reserve as waiting. Notice that no expiration date was generated for this reserve. 10) Attempt to check out the item you reserved to some other borrower. Revert waiting status. The expiration date should remain null. 11) Confirm tests pass t/db_dependent/Hold.t t/db_dependent/Reserves.t 12) Go to Admin -> System preferences and search for ReserveExpiration. Confirm related system preferences have a reference to ReserveExpiration and the links all work 13) Text the cancel_expired_holds.pl cronjob and confirm it works with ReserveExpiration enabled, and does nothing when disabled 14) Confirm all instances where hold expiration dates are set or referenced are shown or hidden correctly, depending on ReserveExpiration Pages in the staff interface: - 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 Pages in the OPAC: - Log in to the OPAC as a patron with holds - Your summary -> Holds tab - Your holds history (OPACHoldsHistory must be enabled). Sponsored-by: Horowhenua Library Trust Signed-off-by: David Nind Signed-off-by: Nicolas Giraud --- C4/ILSDI/Services.pm | 10 ++- C4/Reserves.pm | 2 +- Koha/Club/Hold.pm | 2 +- Koha/Hold.pm | 76 ++++++++++--------- Koha/REST/V1/Clubs/Holds.pm | 2 +- Koha/REST/V1/Holds.pm | 2 +- circ/waitingreserves.pl | 6 +- .../bug24194-add-ReserveExpiration-syspref.pl | 12 +++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../prog/en/includes/holds_table.inc | 12 ++- .../prog/en/includes/patron-detail-tabs.inc | 4 +- .../prog/en/includes/waiting_holds.inc | 8 +- .../admin/preferences/circulation.pref | 14 +++- .../prog/en/modules/circ/circulation.tt | 1 + .../prog/en/modules/members/holdshistory.tt | 16 +++- .../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 | 6 +- .../bootstrap/en/includes/holds-table.inc | 21 +++-- .../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 | 39 +++++++++- t/db_dependent/Reserves.t | 48 +++++++++++- 25 files changed, 239 insertions(+), 84 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug24194-add-ReserveExpiration-syspref.pl diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 112c5715f5c..c3bc7fa457f 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -814,7 +814,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, @@ -910,7 +913,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); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 7d6c106ceae..9ce0bd402f3 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -184,7 +184,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}; diff --git a/Koha/Club/Hold.pm b/Koha/Club/Hold.pm index a08b9788e20..6a2b23269f6 100644 --- a/Koha/Club/Hold.pm +++ b/Koha/Club/Hold.pm @@ -131,7 +131,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}, diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 3c4fcb8362f..f6eeaadaedc 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -289,53 +289,59 @@ sub set_waiting { desk_id => $desk_id, }; - my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); - my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); + my $new_expiration_date; - my $rule = Koha::CirculationRules->get_effective_rule( - { - categorycode => $self->borrower->categorycode, - itemtype => $self->item->effective_itemtype, - branchcode => $self->branchcode, - rule_name => 'holds_pickup_period', - } - ); - if ( defined($rule) and $rule->rule_value ne '' ) { - - # circulation rule overrides ReservesMaxPickUpDelay - $max_pickup_delay = $rule->rule_value; - } + if ( C4::Context->preference('ReserveExpiration') ) { - my $new_expiration_date = dt_from_string( $self->waitingdate )->clone->add( days => $max_pickup_delay ); + my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); + my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); - if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { - my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype; - my $daysmode = Koha::CirculationRules->get_effective_daysmode( + my $rule = Koha::CirculationRules->get_effective_rule( { categorycode => $self->borrower->categorycode, - itemtype => $itemtype, + itemtype => $self->item->effective_itemtype, branchcode => $self->branchcode, + rule_name => 'holds_pickup_period', } ); - my $calendar = Koha::Calendar->new( branchcode => $self->branchcode, days_mode => $daysmode ); + if ( defined($rule) and $rule->rule_value ne '' ) { - $new_expiration_date = $calendar->days_forward( dt_from_string( $self->waitingdate ), $max_pickup_delay ); - } + # circulation rule overrides ReservesMaxPickUpDelay + $max_pickup_delay = $rule->rule_value; + } - # 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 ); + $new_expiration_date = dt_from_string( $self->waitingdate )->clone->add( days => $max_pickup_delay ); - my $cmp = - $requested_expiration - ? DateTime->compare( $requested_expiration, $new_expiration_date ) - : 0; + if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { + my $itemtype = $self->item ? $self->item->effective_itemtype : $self->biblio->itemtype; + my $daysmode = Koha::CirculationRules->get_effective_daysmode( + { + categorycode => $self->borrower->categorycode, + itemtype => $itemtype, + branchcode => $self->branchcode, + } + ); + my $calendar = Koha::Calendar->new( branchcode => $self->branchcode, days_mode => $daysmode ); - $new_expiration_date = $cmp == -1 ? $requested_expiration : $new_expiration_date; - } + $new_expiration_date = $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. + 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; + } + + } # ReserveExpiration - $values->{expirationdate} = $new_expiration_date->ymd; + $values->{expirationdate} = $new_expiration_date ? $new_expiration_date->ymd : undef; $self->set($values)->store(); @@ -1010,7 +1016,7 @@ sub revert_found { priority => 1, found => undef, waitingdate => undef, - expirationdate => $self->patron_expiration_date, + expirationdate => undef, itemnumber => $self->item_level_hold ? $self->itemnumber : undef, ( $self->is_waiting ? ( desk_id => undef ) : () ), } diff --git a/Koha/REST/V1/Clubs/Holds.pm b/Koha/REST/V1/Clubs/Holds.pm index 64696702b09..3281df0f3bf 100644 --- a/Koha/REST/V1/Clubs/Holds.pm +++ b/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}; diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 4a9093f0fc8..7694a30cfc7 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -82,7 +82,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}; diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl index 7c5d58cc496..d93d7bb642a 100755 --- a/circ/waitingreserves.pl +++ b/circ/waitingreserves.pl @@ -104,10 +104,10 @@ my $today = Date_to_Days(&Today); 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 ( $expire_year, $expire_month, $expire_day ) = split (/-/, $hold->expirationdate); + my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day ) if $hold->expirationdate; - if ( $today > $calcDate ) { + if ($calcDate and $today > $calcDate) { if ( $op eq 'cud-cancelall' ) { my $res = cancel( $hold->item->itemnumber, $hold->borrowernumber, $hold->item->holdingbranch, diff --git a/installer/data/mysql/atomicupdate/bug24194-add-ReserveExpiration-syspref.pl b/installer/data/mysql/atomicupdate/bug24194-add-ReserveExpiration-syspref.pl new file mode 100644 index 00000000000..82668b44ae9 --- /dev/null +++ b/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") }); + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 7fb46c297ee..ca0e314a0fa 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -691,6 +691,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'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index 8e44790a453..073b7b3297f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -13,7 +13,9 @@ Patron Notes Date - Expiration + [% IF Koha.Preference('ReserveExpiration') %] + Expiration + [% END %] Pickup library Details [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] @@ -190,9 +192,11 @@ [% hold.date | $KohaDates %] [% END %] - - - + [% IF Koha.Preference('ReserveExpiration') %] + + + + [% END %] [%- IF ( hold.found ) -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc index 69b41b4dfb5..88b4e306ccd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-detail-tabs.inc @@ -155,7 +155,9 @@ Item type Barcode Pickup at - Expiration + [% IF Koha.Preference('ReserveExpiration') %] + Expiration + [% END %] Priority Notes Delete? diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc index 2ba57981f4f..f4c5bbdee43 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc @@ -10,7 +10,9 @@ [% IF table_name == 'holdscr' %] Date cancellation requested [% END %] - Expiration date + [% IF Koha.Preference('ReserveExpiration') %] + Expiration date + [% END %] Title Patron Home library @@ -39,7 +41,9 @@ [% END %] [% END %] - [% reserveloo.expirationdate | $KohaDates %] + [% IF Koha.Preference('ReserveExpiration') %] + [% reserveloo.expirationdate | $KohaDates %] + [% END %] [% INCLUDE 'biblio-title.inc' biblio=reserveloo.biblio link = 1 %] [% UNLESS ( item_level_itypes ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 82070552d6c..751140561c2 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -860,13 +860,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" @@ -881,7 +881,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.' - - The holds queue should prioritize filling a hold by matching the patron's home library with an item having a matching - pref: HoldsQueuePrioritizeBranch @@ -997,7 +997,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: @@ -1152,6 +1152,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. Fines policy: - - pref: finesCalendar diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index 7c78d55b6cf..69a76f094c8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -1109,6 +1109,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 %] [% IF Koha.Preference('ClaimReturnedLostValue') || Koha.Preference('BundleLostValue') %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt index 3fa944bdcf0..0a513383c0b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -50,6 +50,7 @@ [% ELSE %] [% SET show_itemtype_column = Koha.Preference('AllowHoldItemTypeSelection') %] + [% SET ReserveExpiration = Koha.Preference('ReserveExpiration') ? 1 : 0 %]

Current holds

@@ -90,7 +91,9 @@ Call number Library Hold date - Expiration date + [% IF ReserveExpiration == 1 %] + Expiration date + [% END %] Waiting date Cancellation date [% IF show_itemtype_column %] @@ -135,7 +138,9 @@ Call number Library Hold date - Expiration date + [% IF ReserveExpiration == 1 %] + Expiration date + [% END %] Waiting date Cancellation date [% IF show_itemtype_column %] @@ -159,12 +164,17 @@ [% Asset.js("js/members-menu.js") | $raw %] [% INCLUDE 'js-biblio-format.inc' %] [% Asset.js("js/pages/circulation.js") | $raw %] [% IF Koha.Preference('ClaimReturnedLostValue') || Koha.Preference('BundleLostValue') %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 6c6c27c8a01..90c32471cac 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -665,10 +665,12 @@ [% END %] -
  • - - -
  • + [% IF Koha.Preference('ReserveExpiration') %] +
  • + + +
  • + [% END %]
  • diff --git a/koha-tmpl/intranet-tmpl/prog/js/holds.js b/koha-tmpl/intranet-tmpl/prog/js/holds.js index 5fae127c4c2..50999755b50 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/holds.js +++ b/koha-tmpl/intranet-tmpl/prog/js/holds.js @@ -339,10 +339,8 @@ $(document).ready(function () { }, }, { - data: { - _: "expirationdate_formatted", - sort: "expirationdate", - }, + "visible": ReserveExpiration, + "data": { _: "expirationdate_formatted", "sort": "expirationdate" } }, { data: function (oObj) { diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc index 1bafd09778d..8d5fc04f1ce 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc @@ -15,7 +15,9 @@ [% ELSE %] Placed on [% END %] - Expires on + [% IF Koha.Preference('ReserveExpiration') %] + Expires on + [% END %] [% UNLESS( singleBranchMode) %] Pickup location [% END %] @@ -83,16 +85,19 @@ [% END %] [% HOLD.reservedate | $KohaDates %] - + [% IF Koha.Preference('ReserveExpiration') %] [% IF ! HOLD.found %] - [% IF ( HOLD.expirationdate ) %] - [% HOLD.expirationdate | $KohaDates %] - [% ELSE %] - Never expires - [% END %] + + [% IF ( HOLD.expirationdate ) %] + [% HOLD.expirationdate | $KohaDates %] + [% ELSE %] + Never expires + [% END %] [% ELSE %] - - + + - [% END %] + [% END %] [% UNLESS( singleBranchMode) %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt index 72ff3f544cf..e71fe11b575 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt @@ -102,7 +102,9 @@ Barcode Library Hold date - Expiration date + [% IF Koha.Preference('ReserveExpiration') %] + Expiration date + [% END %] Waiting date Cancellation date [% IF show_itemtype_column %] @@ -124,11 +126,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 %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt index 363702f9262..8d1f0aa80f7 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -290,6 +290,7 @@
  • [% END %] + [% IF Koha.Preference('ReserveExpiration') %]
  • [% INCLUDE 'date-format.inc' %]
  • + [% END %] [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
  • diff --git a/misc/cronjobs/holds/cancel_expired_holds.pl b/misc/cronjobs/holds/cancel_expired_holds.pl index 2daf92ecd3c..744c817fd55 100755 --- a/misc/cronjobs/holds/cancel_expired_holds.pl +++ b/misc/cronjobs/holds/cancel_expired_holds.pl @@ -74,6 +74,8 @@ GetOptions( ) or pod2usage(1); pod2usage(1) if $help; -C4::Reserves::CancelExpiredReserves($reason); +if( C4::Context->preference('ReserveExpiration') ) { + C4::Reserves::CancelExpiredReserves($reason); +} cronlogaction( { action => 'End', info => "COMPLETED" } ); diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index 281f32dc18a..663ec9eb3a7 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -30,7 +30,7 @@ use Koha::DateUtils qw( dt_from_string ); use t::lib::TestBuilder; use Test::NoWarnings; -use Test::More tests => 36; +use Test::More tests => 37; use Test::Exception; use Test::Warn; @@ -488,3 +488,40 @@ 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; +}; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 22778875a2b..2e052e0f995 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 70; +use Test::More tests => 71; use Test::NoWarnings; use Test::MockModule; use Test::Warn; @@ -1420,6 +1420,52 @@ 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 => { + found => 'W', + expirationdate => $expirationdate + } + } + ); + + # Disable expiration date for reserves + t::lib::Mocks::mock_preference( 'ReserveExpiration', 0 ); + + $hold->revert_found(); + + is( $hold->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 => { + found => 'W', + expirationdate => $expirationdate + } + } + ); + + $hold->set_waiting(); + $hold->revert_found(); + + is( + $hold->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{ -- 2.39.5