It doesn't make sense to rely on CLI options for cleanup_database.pl, especially not when it comes to self-registration options. Instead, cleanup_database.pl should consult system preferences.
I notice that --del-unv-selfreg doesn't actually work correctly already either...
Created attachment 121401 [details] [review] Bug 28457: Add configuration to cleanup_database.pl for self-registration This patch fixes the --del-unv-selfreg option so that it can actually accept a DAYS parameter. (Note that backwards compatibility is preserved so that not specifying DAYS will still work.) This patch also uses the following self registration system preferences to trigger the cleanup: PatronSelfRegistrationExpireTemporaryAccountsDelay PatronSelfRegistrationVerifyByEmail Note that --del-unv-selfreg will override an instance's system preference. This means that on a package install the cleanup will just work. Test plan: 0. Apply patch 1. perl ./misc/cronjobs/cleanup_database.pl -v You did not specify any cleanup work for the script to do. 2. perl ./misc/cronjobs/cleanup_database.pl -v --del-exp-selfreg Confirm flag not passed, running in dry-run mode... Start lock unsubscribed, anonymize and delete 0 patrons would have been locked 0 patrons would have been anonymized 0 patrons would have been deleted self-registered borrowers may be deleted 3. perl ./misc/cronjobs/cleanup_database.pl -v --del-unv-selfreg Confirm flag not passed, running in dry-run mode... Start lock unsubscribed, anonymize and delete 0 patrons would have been locked 0 patrons would have been anonymized 0 patrons would have been deleted unverified self-registrations may be deleted after 1 days 4. perl ./misc/cronjobs/cleanup_database.pl -v --del-unv-selfreg 3 Confirm flag not passed, running in dry-run mode... Start lock unsubscribed, anonymize and delete 0 patrons would have been locked 0 patrons would have been anonymized 0 patrons would have been deleted unverified self-registrations may be deleted after 3 days 5. Set "PatronSelfRegistrationExpireTemporaryAccountsDelay" to 1 6. Set "PatronSelfRegistrationVerifyByEmail" to "Require" 7. perl ./misc/cronjobs/cleanup_database.pl -v Confirm flag not passed, running in dry-run mode... Start lock unsubscribed, anonymize and delete 0 patrons would have been locked 0 patrons would have been anonymized 0 patrons would have been deleted self-registered borrowers may be deleted unverified self-registrations may be deleted after 1 days 8. perl ./misc/cronjobs/cleanup_database.pl -v --del-unv-selfreg 2 Confirm flag not passed, running in dry-run mode... Start lock unsubscribed, anonymize and delete 0 patrons would have been locked 0 patrons would have been anonymized 0 patrons would have been deleted self-registered borrowers may be deleted unverified self-registrations may be deleted after 2 days
The option vs syspref question for this script has been discussed recently on bug 25429. The last version of the patches there *should* reflect the consensus we found (I haven't had the time to review however).
(In reply to Jonathan Druart from comment #3) > The option vs syspref question for this script has been discussed recently > on bug 25429. > > The last version of the patches there *should* reflect the consensus we > found (I haven't had the time to review however). Hmm that last patch doesn't make sense to me...
Please don't forget to set assignee, thx!