From 97b60660b3da8ca092f39e87902a112c7e0a6ffa 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 remains. 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 | 4 +- Koha/Club/Hold.pm | 2 +- Koha/Hold.pm | 33 +++++++++++------ 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 | 12 ++++-- .../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 | 26 +++++++------ .../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, 207 insertions(+), 60 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 62d4242f5dc..4e59da3732e 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -786,7 +786,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, @@ -881,7 +884,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 d86e9f46f94..2e1ebdd37a4 100644 --- a/C4/Reserves.pm +++ b/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}; @@ -2132,7 +2132,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(); diff --git a/Koha/Club/Hold.pm b/Koha/Club/Hold.pm index 18390559c75..93bc81519d0 100644 --- a/Koha/Club/Hold.pm +++ b/Koha/Club/Hold.pm @@ -126,7 +126,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 0224d019b76..3cf6382e918 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -228,6 +228,10 @@ 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'); @@ -261,21 +265,28 @@ sub set_waiting { $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 ); + 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; + my $cmp = + $requested_expiration + ? DateTime->compare( $requested_expiration, $new_expiration_date ) + : 0; - $new_expiration_date = - $cmp == -1 ? $requested_expiration : $new_expiration_date; + $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(); diff --git a/Koha/REST/V1/Clubs/Holds.pm b/Koha/REST/V1/Clubs/Holds.pm index d2111f9cee8..bbe643c7ee2 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 ff162cd9453..c2dea5068a9 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -74,7 +74,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 26924f27fd5..4f803db11e2 100755 --- a/circ/waitingreserves.pl +++ b/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; 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 044361b6ad0..cb8b28aa2e8 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -626,6 +626,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 6e70ced95eb..492bd38e8f0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/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 ) %] @@ -126,9 +128,11 @@ [% hold.date | $KohaDates %] [% END %] - - - + [% IF Koha.Preference('ReserveExpiration') %] + + + + [% END %] [%- IF ( hold.found ) -%] 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 484f1d609be..6e5d80ad891 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 %] + [% IF Koha.Preference('ReserveExpiration') %] Expiration date + [% END %] Title Patron Home library @@ -43,7 +45,9 @@ [% END %] [% END %] + [% 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 dd7494a73cf..39d72a804ad 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 @@ -773,13 +773,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" @@ -794,7 +794,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 @@ -891,7 +891,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: @@ -1030,6 +1030,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 649a3d42b56..d86192be6fd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -967,6 +967,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 %] 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 92a7e1d7584..04c513db78a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -140,12 +140,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 %] 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 a1e63a4fdbe..8cac07f761c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -582,10 +582,12 @@ [% END %] -
  • - - -
    • + [% IF Koha.Preference('ReserveExpiration') %] +
  • + + +
    • + [% END %] [% UNLESS ( multi_hold ) %]
  • diff --git a/koha-tmpl/intranet-tmpl/prog/js/holds.js b/koha-tmpl/intranet-tmpl/prog/js/holds.js index 50dce1ddd38..0839aafbdfa 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/holds.js +++ b/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 ) { 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 e74c2b13562..757a1e56e87 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc @@ -14,7 +14,9 @@ [% ELSE %] Placed on [% END %] - Expires on + [% IF Koha.Preference('ReserveExpiration') %] + Expires on + [% END %] [% UNLESS( singleBranchMode) %] Pickup location [% END %] @@ -68,16 +70,18 @@ [% HOLD.reservedate | $KohaDates %] - [% IF ! HOLD.found %] - - [% IF ( HOLD.expirationdate ) %] - [% HOLD.expirationdate | $KohaDates %] - [% ELSE %] - Never expires - [% END %] - [% ELSE %] - - - + [% IF Koha.Preference('ReserveExpiration') %] + [% IF ! HOLD.found %] + + [% 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 224c5441ead..3d0cd1283e9 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt @@ -101,7 +101,9 @@ Barcode Library Hold date - Expiration date + [% IF Koha.Preference('ReserveExpiration') %] + Expiration date + [% END %] Waiting date Cancellation date [% IF show_itemtype_column %] @@ -118,11 +120,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 7b1e9d3708a..f8a3b2038d0 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -293,12 +293,14 @@
    • [% 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 5d7de4cbeeb..fd4acb036e4 100755 --- a/misc/cronjobs/holds/cancel_expired_holds.pl +++ b/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" }); diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index 73007c18a08..69c9f62d1a0 100755 --- a/t/db_dependent/Hold.t +++ b/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; +}; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 0076dc2e15e..a41cade6dfd 100755 --- a/t/db_dependent/Reserves.t +++ b/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; @@ -1405,6 +1405,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(*) -- 2.30.2