From eb9d0b9ac5bbd7ff5d9da3b54f67ee5d0a12a001 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 9 Nov 2020 09:10:15 +1300 Subject: [PATCH] Bug 24914: (follow-up) Clarifying sysprefs, hiding columns, and more - change syspref description to say 'holds' instead of reserves - clarify descriptions of related sysprefs - hide expiration column on reserves page - move syspref to 'hold policy' section - fix cancel_expired_holds.pl cron to consider syspref - set expirationdate to undef always https://bugs.koha-community.org/show_bug.cgi?id=24194 Signed-off-by: David Nind Signed-off-by: David Nind --- C4/Reserves.pm | 29 +++++++--------------- ...24194-add-DisableReserveExpiration-syspref.perl | 2 +- installer/data/mysql/mandatory/sysprefs.sql | 2 +- .../en/modules/admin/preferences/circulation.pref | 20 +++++++-------- .../prog/en/modules/reserve/request.tt | 2 +- misc/cronjobs/holds/cancel_expired_holds.pl | 4 ++- t/db_dependent/Reserves.t | 2 +- 7 files changed, 26 insertions(+), 35 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 1a4b8ae2cb..6090090ce7 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2063,26 +2063,15 @@ sub RevertWaitingStatus { ->update({ priority => \'priority + 1' }, { no_triggers => 1 }); ## Fix up the currently waiting reserve - 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(); - } + $hold->set( + { + priority => 1, + found => undef, + waitingdate => undef, + expirationdate => undef, + itemnumber => $hold->item_level_hold ? $hold->itemnumber : undef, + } + )->store(); _FixPriority( { biblionumber => $hold->biblionumber } ); diff --git a/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl b/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl index a5fe773c18..bed084b3ce 100644 --- a/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl +++ b/installer/data/mysql/atomicupdate/bug24194-add-DisableReserveExpiration-syspref.perl @@ -1,6 +1,6 @@ $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" ) }); + $dbh->do(q{ INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES ( "DisableReserveExpiration", 0, NULL, "Disable the use of expiration date in holds 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 4851164e38..2802a8b2f4 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -164,7 +164,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'), +('DisableReserveExpiration', 0, NULL, 'Disable the use of expiration date in holds 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 2e6d7c08cd..8306a626fb 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,12 +168,6 @@ 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: - @@ -732,18 +726,18 @@ Circulation: - Mark a hold as problematic if it has been waiting for more than - pref: ReservesMaxPickUpDelay class: integer - - days. + - "days.
NOTE: If DisableReserveExpiration is enabled, holds marked as problematic must be dealt with manually as they will never automatically expire." - - pref: ExpireReservesMaxPickUpDelay choices: yes: Allow no: "Don't allow" - - '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.' + - '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 DisableReserveExpiration is enabled."' - - 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 DisableReserveExpiration is enabled." - - Satisfy holds using items from the libraries - pref: StaticHoldsQueueWeight @@ -825,7 +819,7 @@ Circulation: choices: yes: Allow no: "Don't allow" - - expired holds to be canceled on days the library is closed. + - expired holds to be canceled on days the library is closed.
NOTE: This will not apply if DisableReserveExpiration is enabled. - - pref: ExcludeHolidaysFromMaxPickUpDelay choices: @@ -900,6 +894,12 @@ Circulation: - pref: UpdateItemWhenLostFromHoldList type: textarea syntax: text/x-yaml + - + - pref: DisableReserveExpiration + choices: + yes: Disable + no: "Don't disable" + - the use of expiration dates in holds. This means patrons will not be able to specify a date when their hold is not needed, and any hold awaiting pickup will not automatically expire after it has been waiting for a problematic number of days. The expiration date will be hidden from both the OPAC and staff interface. Interlibrary loans: - - pref: ILLModule 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 9a36bfcff0..6dd456d5c3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -449,7 +449,7 @@ [% END %] - [% IF !Koha.Preference('DisableReserveExpiration') %] + [% UNLESS Koha.Preference('DisableReserveExpiration') %]
  • diff --git a/misc/cronjobs/holds/cancel_expired_holds.pl b/misc/cronjobs/holds/cancel_expired_holds.pl index 4932bbf346..587a6de1da 100755 --- a/misc/cronjobs/holds/cancel_expired_holds.pl +++ b/misc/cronjobs/holds/cancel_expired_holds.pl @@ -80,4 +80,6 @@ pod2usage(1) if $help; cronlogaction(); -CancelExpiredReserves($reason); +unless( C4::Context->preference('DisableReserveExpiration') ) { + CancelExpiredReserves($reason); +} diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 3195325bfd..bada784c6a 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -1248,7 +1248,7 @@ subtest 'DisableReserveExpiration syspref tests' => sub { $reverted = C4::Reserves::RevertWaitingStatus({ itemnumber => $hold->itemnumber }); - is( $reverted->expirationdate, $expirationdate->ymd, "Expiration date remains as expected" ); + is( $reverted->expirationdate, undef, "Expiration date is still removed because we can't reset to the original expiration date" ); }; sub count_hold_print_messages { -- 2.11.0