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!
Hey David, looks like this one needs a simple rebase.
Created attachment 158297 [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
(In reply to Lucas Gass from comment #6) > Hey David, looks like this one needs a simple rebase. Thanks, Lucas. I rebased but haven't re-tested myself.
In step 3 I don't have the message : 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 But I have : You did not specify any cleanup work for the script to do. The step 3 works only after I did the next steps