From 3b531792d54c13e1cffdb3ab575547c7dc8b6354 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 8d178ed45a..ab4a3093e7 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2094,26 +2094,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 416c7d536f..f6cee4d560 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -175,7 +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'), +('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 d72ed3db6f..ae583ff669 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: 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: - @@ -733,18 +727,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: 1: Allow 0: "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 @@ -827,7 +821,7 @@ Circulation: choices: 1: allow 0: "don't allow" - - expired holds to be canceled on days the library is closed per the calendar. + - expired holds to be canceled on days the library is closed per the calendar.
NOTE: This will not apply if DisableReserveExpiration is enabled. - - pref: ExcludeHolidaysFromMaxPickUpDelay choices: @@ -921,6 +915,12 @@ Circulation: months: months years: years - from reserve date. + - + - 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 1954d28ffd..93f57436e3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -522,7 +522,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 3cc9152d17..2c72d4b83c 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(); -C4::Reserves::CancelExpiredReserves($reason); +unless( C4::Context->preference('DisableReserveExpiration') ) { + C4::Reserves::CancelExpiredReserves($reason); +} diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index c19b8f5d23..bbac530212 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -1363,7 +1363,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