From d009ba33fe852011359ee99324067418cc30bb23 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 9 Dec 2019 04:33:42 +0000 Subject: [PATCH] Bug 24194: Add DisableReserveExpiration system preference to hide 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 DisableReserveExpiration 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 Sponsored-by: Horowhenua Library Trust Signed-off-by: David Nind Signed-off-by: David Nind Signed-off-by: David Nind https://bugs.koha-community.org/show_bug.cgi?id=24914 Signed-off-by: David Nind --- C4/Reserves.pm | 28 ++++++++---- Koha/Hold.pm | 51 ++++++++++++---------- ...24194-add-DisableReserveExpiration-syspref.perl | 7 +++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../en/modules/admin/preferences/circulation.pref | 6 +++ .../prog/en/modules/reserve/request.tt | 10 +++-- .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 12 ++--- t/db_dependent/Hold.t | 34 +++++++++++++++ t/db_dependent/Reserves.t | 36 ++++++++++++++- 9 files changed, 143 insertions(+), 42 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl diff --git a/C4/Reserves.pm b/C4/Reserves.pm index a42168bc72..8d178ed45a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2094,14 +2094,26 @@ sub RevertWaitingStatus { ->update({ priority => \'priority + 1' }, { no_triggers => 1 }); ## Fix up the currently waiting reserve - $hold->set( - { - priority => 1, - found => undef, - waitingdate => undef, - itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, - } - )->store(); + if ( C4::Context->preference('DisableReserveExpiration') ){ + $hold->set( + { + priority => 1, + found => undef, + waitingdate => undef, + expirationdate => undef, + itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, + } + )->store(); + } else { + $hold->set( + { + priority => 1, + found => undef, + waitingdate => undef, + itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, + } + )->store(); + } _FixPriority( { biblionumber => $hold->biblionumber } ); diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 9736dc527f..49c708868f 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -185,36 +185,39 @@ sub set_waiting { desk_id => $desk_id, }; - my $requested_expiration; - if ($self->expirationdate) { - $requested_expiration = dt_from_string($self->expirationdate); - } + if ( C4::Context->preference('DisableReserveExpiration') ){ + $values->{expirationdate} = undef; + } else { + my $requested_expiration; + if ($self->expirationdate) { + $requested_expiration = dt_from_string($self->expirationdate); + } - my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); - my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); + my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); + my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); - my $expirationdate = $today->clone; - $expirationdate->add(days => $max_pickup_delay); + my $expirationdate = $today->clone; + $expirationdate->add(days => $max_pickup_delay); - 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 ); + 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 ); + $expirationdate = $calendar->days_forward( dt_from_string(), $max_pickup_delay ); + } - $expirationdate = $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. + my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; + $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; } - # If patron's requested expiration date is prior to the - # calculated one, we keep the patron's one. - my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; - $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; - $self->set($values)->store(); return $self; diff --git a/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl b/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl new file mode 100644 index 0000000000..a5fe773c18 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl @@ -0,0 +1,7 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do(q{ INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES ( "DisableReserveExpiration", 0, NULL, "Disable the use of expiration date in reserves module.", "YesNo" ) }); + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 24194 - DisableReserveExpiration system preference)\n"; +} diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 68877bbbce..416c7d536f 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -175,6 +175,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('DefaultToLoggedInLibraryNoticesSlips', '0', NULL , 'If enabled,slips and notices editor will default to the logged in library''s rules, rather than the ''all libraries'' rules.', 'YesNo'), ('DefaultToLoggedInLibraryOverdueTriggers', '0', NULL , 'If enabled, overdue status triggers editor will default to the logged in library''s rules, rather than the ''default'' rules.', 'YesNo'), ('CSVDelimiter',';',';|tabulation|,|/|\\|#||','Define the default separator character for exporting reports','Choice'), +('DisableReserveExpiration', 0, NULL, 'Disable the use of expiration date in reserves module.', 'YesNo'), ('Display856uAsImage','OFF','OFF|Details|Results|Both','Display the URI in the 856u field as an image, the corresponding staff interface XSLT option must be on','Choice'), ('DisplayClearScreenButton','0','','If set to ON, a clear screen button will appear on the circulation page.','YesNo'), ('displayFacetCount','0',NULL,NULL,'YesNo'), 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 6c2a33bc6c..d72ed3db6f 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 @@ -168,6 +168,12 @@ Circulation: 1: "Use" 0: "Don't use" - circulation desks with circulation. + - + - pref: DisableReserveExpiration + choices: + yes: Disable + no: "Don't disable" + - the use of expiration dates in reserves. Checkout policy: - 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 b1ab990f13..1954d28ffd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -522,10 +522,12 @@ [% END %] -
  • - - -
    • + [% IF !Koha.Preference('DisableReserveExpiration') %] +
  • + + +
    • + [% END %] [% UNLESS ( multi_hold ) %]
  • 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 edc33ccf73..75d7798cac 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -286,11 +286,13 @@
    • [% END %] -
  • - - - [% INCLUDE 'date-format.inc' %] -
    • + [% IF !Koha.Preference('DisableReserveExpiration') %] +
  • + + + [% INCLUDE 'date-format.inc' %] +
    • + [% END %] [% IF Koha.Preference('AllowHoldItemTypeSelection') %] [% itemtypes = [] %] diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t index abeae5cc7a..5f74973ba4 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -367,3 +367,37 @@ subtest 'suspend() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'DisableReserveExpiration syspref tests' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + # Disable expiration date for reserves + t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 1 ); + + my $expirationdate = dt_from_string->add( days => 1 ); + my $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { expirationdate => $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( 'DisableReserveExpiration', 0 ); + + $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { expirationdate => $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 9b4122dbd4..c19b8f5d23 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 68; +use Test::More tests => 69; use Test::MockModule; use Test::Warn; @@ -1332,6 +1332,40 @@ subtest 'ModReserveAffect logging' => sub { like( $log->info, qr{$expected}, 'Timestamp logged is the current one' ); }; +subtest 'DisableReserveExpiration syspref tests' => sub { + + plan tests => 2; + + t::lib::Mocks::mock_preference( 'DisableReserveExpiration', 0 ); + + 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( 'DisableReserveExpiration', 1 ); + + 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( 'DisableReserveExpiration', 0 ); + + $hold = $builder->build_object( + { class => 'Koha::Holds', + value => { expirationdate => $expirationdate } + } + ); + + $reverted = C4::Reserves::RevertWaitingStatus({ itemnumber => $hold->itemnumber }); + + is( $reverted->expirationdate, $expirationdate->ymd, "Expiration date remains as expected" ); +}; + sub count_hold_print_messages { my $message_count = $dbh->selectall_arrayref(q{ SELECT COUNT(*) -- 2.11.0