https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43559 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 206176 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206176&action=edit Bug 43559: Convert membership_expiry.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. -c ("confirm") is declared required => 1: the script already refused to do anything without it (pod2usage + exit), which is exactly what the required constraint gives for free, so the hand-rolled check is gone. --active/--inactive are declared exclusive (bug 43557), and each gets a Params::Validate callback requiring a positive number of months, replacing the equivalent hand-rolled "defined but zero" check. --letter/--letter_renew's defaults move into the option spec, replacing the "if !$letter_expiry" post-parse assignment. The TrackLastPatronActivityTriggers syspref gate on --active/--inactive stays a post-parse check, same as the DB-existence checks audited on the sibling bugs: it's a business-state check, not a shape one, and Koha::Script->describe_options has no declarative home for it. One behavior note: previously, when multiple problems existed at once (e.g. TrackLastPatronActivityTriggers unset AND --active 0), the syspref message always won because of the if/elsif chain's ordering. Now the positive-months callback runs during option parsing, before the script body's syspref check, so that message can win instead when both are wrong simultaneously. Either way the script still exits non-zero with a clear message; only which message is shown when two things are wrong at once can change. Also found, not touched: -n ("no mail") is parsed but was already unused by the rest of the script before this conversion -- the "prints to standard out instead" behavior the POD describes isn't implemented, and -p/-n aren't actually enforced as mutually exclusive in code despite the POD claiming so. Both predate this bug; worth their own bug rather than silently changing behavior here. Test plan: 1. membership_expiry.pl --help / --man 2. membership_expiry.pl (no options) Rejected: mandatory parameter 'c' missing. 3. membership_expiry.pl -c --active 0 Rejected: needs a positive number of months. 4. membership_expiry.pl -c --active 3 --inactive 2 Rejected: mutually exclusive. 5. membership_expiry.pl -c --active 3 Rejected: needs TrackLastPatronActivityTriggers (same as before). 6. membership_expiry.pl -c -v Runs to completion (exits early on MembershipExpiryDaysNotice not being set in the test DB, same as before). 7. koha-qa.pl passes. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.