From 118a1af8a3f53d52a0e3fbd3f6c4816abfee1242 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 a658646407..1a4b8ae2cb 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2063,14 +2063,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 892fb20cd5..ac45c9f698 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -187,36 +187,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 eab5e66838..d7d2a53fea 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'), ('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 ab68c75687..2e6d7c08cd 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 dc85eff568..9a36bfcff0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -449,12 +449,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 4faab3c218..3195325bfd 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 66; +use Test::More tests => 67; use Test::MockModule; use Test::Warn; @@ -1217,6 +1217,40 @@ subtest 'MergeHolds' => sub { is( $biblio_2->holds->count, 1, 'Hold has been transferred' ); }; +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