Bug 28457 - cleanup_database.pl self-registration options are not configurable
Summary: cleanup_database.pl self-registration options are not configurable
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-26 01:05 UTC by David Cook
Modified: 2022-04-04 23:21 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28457: Add configuration to cleanup_database.pl for self-registration (4.25 KB, patch)
2021-05-26 01:28 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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...