[Bug 43553] New: Convert misc/cronjobs/holds/holds_reminder.pl to the Getopt::Long::Descriptive convention (see bug 43546)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Bug ID: 43553 Summary: Convert misc/cronjobs/holds/holds_reminder.pl to the Getopt::Long::Descriptive convention (see bug 43546) Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: jake.deery@openfifth.co.uk, robin@catalyst.net.nz Depends on: 43546 Target Milestone: --- This is one of a set of bugs filed alongside bug 43546 (Standardize command-line scripts on Getopt::Long::Descriptive) to convert individual, commonly-used cron scripts to whatever declarative option-parsing convention that bug settles on. Depends on 43546. Current state: holds_reminder.pl uses plain Getopt::Long (GetOptions call around line 176). No option is required: --days defaults to 0, meaning all waiting holds, if omitted (around line 195); --library defaults to all branches if omitted; --date defaults to today and is only format-validated if actually supplied; --lettercode defaults to 'HOLD_REMINDER'. What this bug covers: convert the GetOptions call to the new convention purely for consistency and self-documenting --help output, matching bug 43546's convention. No behavior change is expected, since nothing here is actually a missing-required-option bug to fix. Test plan: - --help output covers all documented options and matches current behavior - Running with no options still behaves exactly as before (all branches, all waiting holds, today's date) Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43546 [Bug 43546] Standardize command-line scripts on Getopt::Long::Descriptive for safer, more consistent option handling -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 206185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206185&action=edit Bug 43553: Convert holds_reminder.pl to the Getopt::Long::Descriptive convention Convert the plain Getopt::Long option parsing to Koha::Script->describe_options, per the convention established by bug 43546. --lettercode's default moves into the option spec. Also adds a Params::Validate callback requiring --date to be a valid YYYY-MM-DD date (via Koha::DateUtils::dt_from_string), replacing the existing hand-rolled eval/die check with the same validation logic, now enforced consistently during option parsing with the usual usage text on failure, matching the pattern already used for --interval on bug 43560. Dropped $opt_out, a leftover variable that was declared but never wired to any option or used anywhere in the script -- dead code predating this conversion, not a behavior change. Updated the SYNOPSIS/OPTIONS POD to use the double-dash spelling for multi-letter options (--library, --days, etc.), matching what the new --help now displays; single-letter -v/-c/-t are unchanged. Left the "-n -csv" example in USAGE EXAMPLES alone -- it refers to options this script has never had (apparently copy-pasted from a different script's docs), predates this conversion, and isn't affected by it. Test plan: 1. holds_reminder.pl --help / --man 2. holds_reminder.pl --date not-a-date Rejected: --date must be a valid date in YYYY-MM-DD format. 3. holds_reminder.pl --date 2026-01-15 -v Runs to completion as before. 4. holds_reminder.pl -v (no options) Runs to completion exactly as before (today, all branches, all waiting holds). 5. holds_reminder.pl --library CPL --days 14 -v Runs to completion exactly as before. 6. koha-qa.pl passes. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Alex Carver [Acerock7] <alex@rcls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Alex Carver [Acerock7] <alex@rcls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #206185|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 --- Comment #2 from Alex Carver [Acerock7] <alex@rcls.org> --- Created attachment 207421 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207421&action=edit Bug 43553: Convert holds_reminder.pl to the Getopt::Long::Descriptive convention Convert the plain Getopt::Long option parsing to Koha::Script->describe_options, per the convention established by bug 43546. --lettercode's default moves into the option spec. Also adds a Params::Validate callback requiring --date to be a valid YYYY-MM-DD date (via Koha::DateUtils::dt_from_string), replacing the existing hand-rolled eval/die check with the same validation logic, now enforced consistently during option parsing with the usual usage text on failure, matching the pattern already used for --interval on bug 43560. Dropped $opt_out, a leftover variable that was declared but never wired to any option or used anywhere in the script -- dead code predating this conversion, not a behavior change. Updated the SYNOPSIS/OPTIONS POD to use the double-dash spelling for multi-letter options (--library, --days, etc.), matching what the new --help now displays; single-letter -v/-c/-t are unchanged. Left the "-n -csv" example in USAGE EXAMPLES alone -- it refers to options this script has never had (apparently copy-pasted from a different script's docs), predates this conversion, and isn't affected by it. Test plan: 1. holds_reminder.pl --help / --man 2. holds_reminder.pl --date not-a-date Rejected: --date must be a valid date in YYYY-MM-DD format. 3. holds_reminder.pl --date 2026-01-15 -v Runs to completion as before. 4. holds_reminder.pl -v (no options) Runs to completion exactly as before (today, all branches, all waiting holds). 5. holds_reminder.pl --library CPL --days 14 -v Runs to completion exactly as before. 6. koha-qa.pl passes. Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com Status|Signed Off |Failed QA --- Comment #3 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- The change to double-dash spelling will cause existing setups for this cron to fail. Is it possible to adjust this code so that either "--days" or "-days" will work? If that's not possible or is in some way prohibitively difficult then I'm prepared to PQA on this but we need to make the release notes very explicit about the need to update existing cronlines. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43857 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43857 [Bug 43857] Inaccurate usage example in holds_reminder.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #207421|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208587 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208587&action=edit Bug 43553: Convert holds_reminder.pl to the Getopt::Long::Descriptive convention Convert the plain Getopt::Long option parsing to Koha::Script->describe_options, per the convention established by bug 43546. --lettercode's default moves into the option spec. Also adds a Params::Validate callback requiring --date to be a valid YYYY-MM-DD date (via Koha::DateUtils::dt_from_string), replacing the existing hand-rolled eval/die check with the same validation logic, now enforced consistently during option parsing with the usual usage text on failure, matching the pattern already used for --interval on bug 43560. Dropped $opt_out, a leftover variable that was declared but never wired to any option or used anywhere in the script -- dead code predating this conversion, not a behavior change. Updated the SYNOPSIS/OPTIONS POD to use the double-dash spelling for multi-letter options (--library, --days, etc.), matching what the new --help now displays; single-letter -v/-c/-t are unchanged. Left the "-n -csv" example in USAGE EXAMPLES alone -- it refers to options this script has never had (apparently copy-pasted from a different script's docs), predates this conversion, and isn't affected by it. Test plan: 1. holds_reminder.pl --help / --man 2. holds_reminder.pl --date not-a-date Rejected: --date must be a valid date in YYYY-MM-DD format. 3. holds_reminder.pl --date 2026-01-15 -v Runs to completion as before. 4. holds_reminder.pl -v (no options) Runs to completion exactly as before (today, all branches, all waiting holds). 5. holds_reminder.pl --library CPL --days 14 -v Runs to completion exactly as before. 6. koha-qa.pl passes. Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |43557 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 [Bug 43557] Add declarative mutually-exclusive option support to Koha::Script->describe_options -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #208587|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208599 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208599&action=edit Bug 43553: Convert holds_reminder.pl to the Getopt::Long::Descriptive convention Convert the plain Getopt::Long option parsing to Koha::Script->describe_options, per the convention established by bug 43546. --lettercode's default moves into the option spec. Also adds a Params::Validate callback requiring --date to be a valid YYYY-MM-DD date (via Koha::DateUtils::dt_from_string), replacing the existing hand-rolled eval/die check with the same validation logic, now enforced consistently during option parsing with the usual usage text on failure, matching the pattern already used for --interval on bug 43560. Dropped $opt_out, a leftover variable that was declared but never wired to any option or used anywhere in the script -- dead code predating this conversion, not a behavior change. Updated the SYNOPSIS/OPTIONS POD to use the double-dash spelling for multi-letter options (--library, --days, etc.), matching what the new --help now displays; single-letter -v/-c/-t are unchanged. Left the "-n -csv" example in USAGE EXAMPLES alone -- it refers to options this script has never had (apparently copy-pasted from a different script's docs), predates this conversion, and isn't affected by it. Test plan: 1. holds_reminder.pl --help / --man 2. holds_reminder.pl --date not-a-date Rejected: --date must be a valid date in YYYY-MM-DD format. 3. holds_reminder.pl --date 2026-01-15 -v Runs to completion as before. 4. holds_reminder.pl -v (no options) Runs to completion exactly as before (today, all branches, all waiting holds). 5. holds_reminder.pl --library CPL --days 14 -v Runs to completion exactly as before. 6. koha-qa.pl passes. Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Back to Signed Off for QA. The failure was that the double-dash spelling would break existing cron lines such as "holds_reminder.pl -days 3 -lettercode HOLD_REMINDER", which is how the script's old usage text told people to write them. That is solved in the framework, by the new patch on bug 43557 (attachment 208591), which this bug now depends on: Koha::Script->describe_options accepts the legacy single-dash long options, rewrites them to the standard double-dash form and prints a deprecation warning to STDERR (cron mails it to the administrator). So existing setups keep working while they migrate, --days remains the documented form, and the shim is meant to be removed in a future release. The patch on this bug is unchanged (attachment 208599); it is only rebased on top of bug 43557. Checked with both applied: - holds_reminder.pl -days 3 -lettercode HOLD_REMINDER -v runs normally and prints one "WARNING: -days is deprecated, use --days instead" line per legacy option - holds_reminder.pl --days 3 --lettercode HOLD_REMINDER -v runs without any warning - holds_reminder.pl -help prints the usage Test plan: 1. Apply bug 43557 and this patch 2. perl misc/cronjobs/holds/holds_reminder.pl -days 3 -lettercode HOLD_REMINDER -v => it runs, with the deprecation warnings 3. perl misc/cronjobs/holds/holds_reminder.pl --days 3 --lettercode HOLD_REMINDER -v => it runs, without warnings Andrew, if you agree, a line in the release notes about the deprecation (single-dash long options will stop working in a future release) would be worth having, on bug 43557. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #208599|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 --- Comment #7 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 208604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208604&action=edit Bug 43553: Convert holds_reminder.pl to the Getopt::Long::Descriptive convention Convert the plain Getopt::Long option parsing to Koha::Script->describe_options, per the convention established by bug 43546. --lettercode's default moves into the option spec. Also adds a Params::Validate callback requiring --date to be a valid YYYY-MM-DD date (via Koha::DateUtils::dt_from_string), replacing the existing hand-rolled eval/die check with the same validation logic, now enforced consistently during option parsing with the usual usage text on failure, matching the pattern already used for --interval on bug 43560. Dropped $opt_out, a leftover variable that was declared but never wired to any option or used anywhere in the script -- dead code predating this conversion, not a behavior change. Updated the SYNOPSIS/OPTIONS POD to use the double-dash spelling for multi-letter options (--library, --days, etc.), matching what the new --help now displays; single-letter -v/-c/-t are unchanged. Left the "-n -csv" example in USAGE EXAMPLES alone -- it refers to options this script has never had (apparently copy-pasted from a different script's docs), predates this conversion, and isn't affected by it. Test plan: 1. holds_reminder.pl --help / --man 2. holds_reminder.pl --date not-a-date Rejected: --date must be a valid date in YYYY-MM-DD format. 3. holds_reminder.pl --date 2026-01-15 -v Runs to completion as before. 4. holds_reminder.pl -v (no options) Runs to completion exactly as before (today, all branches, all waiting holds). 5. holds_reminder.pl --library CPL --days 14 -v Runs to completion exactly as before. 6. koha-qa.pl passes. Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org