Bug 28457

Summary: cleanup_database.pl self-registration options are not configurable
Product: Koha Reporter: David Cook <dcook>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: Needs Signoff --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart+koha
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24968
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25429
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 28457: Add configuration to cleanup_database.pl for self-registration

Description David Cook 2021-05-26 01:05:44 UTC
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.
Comment 1 David Cook 2021-05-26 01:18:04 UTC
I notice that --del-unv-selfreg doesn't actually work correctly already either...
Comment 2 David Cook 2021-05-26 01:28:35 UTC
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
Comment 3 Jonathan Druart 2021-05-27 07:04:41 UTC
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).
Comment 4 David Cook 2021-05-27 23:35:47 UTC
(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...