From 76c321eeb76de085a82cabe78dfb15225f3681bb 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 --- 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 | 14 +++--- .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 12 ++--- t/db_dependent/Hold.t | 36 ++++++++++++++- t/db_dependent/Reserves.t | 36 ++++++++++++++- 9 files changed, 146 insertions(+), 45 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f379cec8ae..5e2aaf7ea7 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2048,14 +2048,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 921906a07b..ccd69525a7 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -185,36 +185,39 @@ sub set_waiting { waitingdate => $today->ymd, }; - 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 55533edbb4..08b97088a3 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -163,6 +163,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'), ('delimiter',';',';|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 753534cecb..f5e4f7cee7 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: yes: "Use" no: "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 95861f6be2..b51669e52d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -445,12 +445,14 @@ [% END %] -
  • - - - - Clear date -
    • + [% IF !Koha.Preference('DisableReserveExpiration') %] +
  • + + + + Clear date +
    • + [% 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 ec4aaf5b03..987a720993 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -284,11 +284,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 5b71b64292..883f1f08c6 100755 --- a/t/db_dependent/Hold.t +++ b/t/db_dependent/Hold.t @@ -29,7 +29,7 @@ use Koha::Item; use Koha::DateUtils; use t::lib::TestBuilder; -use Test::More tests => 33; +use Test::More tests => 34; use Test::Exception; use Test::Warn; @@ -286,3 +286,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 6ddb3f4f56..67819e17d9 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 65; +use Test::More tests => 66; use Test::MockModule; use Test::Warn; @@ -1190,6 +1190,40 @@ subtest 'AllowHoldOnPatronPossession test' => sub { 'Patron can place hold on an item loaned to itself'); }; +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