From 9ec5e61204f79a8bbfc6bd1b4618d737efd6a846 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 --- C4/Reserves.pm | 28 ++++++++---- Koha/Hold.pm | 51 ++++++++++++---------- ...24194-add-DisableReserveExpiration-syspref.perl | 7 +++ installer/data/mysql/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 8d7dcbc611..198dec34a0 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2027,14 +2027,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 360b43e7b5..2c8fea6624 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -173,36 +173,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/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 1df6a1fb53..8f049754e0 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/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 710b86cbe1..29b673827c 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 @@ -162,6 +162,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 984a71c415..0a316d4ac8 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 feee22d125..70a6e6ef32 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 dc588ded2b..f0f7adc0b8 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 => 29; +use Test::More tests => 30; use Test::Exception; use Test::Warn; @@ -270,3 +270,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 26f1f4863e..a435fdeb66 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 63; +use Test::More tests => 64; use Test::MockModule; use Test::Warn; @@ -1075,6 +1075,40 @@ subtest 'RevertWaitingStatus' => sub { ); }; +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