Bug 28943: Lower the risk of accidental patron deletion

If you do not use a temporary self registration patron category,
you should actually make the preference
PatronSelfRegistrationExpireTemporaryAccountsDelay empty.

As the comment in sysprefs.sql already said, we should not let
a zero value in the pref delete patrons too.

The module is changed now, the test adjusted and
the description of both related sysprefs modified.

Test plan:
Run t/db_dependent/Members.t
Check in Administration the two adjusted OPAC pref descriptions.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Marcel de Rooy 2021-09-07 12:22:05 +00:00 committed by Fridolin Somers
parent db07ca09cc
commit 54466c6bd7
3 changed files with 21 additions and 5 deletions

View file

@ -644,7 +644,11 @@ sub DeleteExpiredOpacRegistrations {
my $delay = C4::Context->preference('PatronSelfRegistrationExpireTemporaryAccountsDelay');
my $category_code = C4::Context->preference('PatronSelfRegistrationDefaultCategory');
return 0 if not $category_code or not defined $delay or $delay eq q||;
return 0 unless $category_code && $delay;
# DO NOT REMOVE test on delay here!
# Some libraries may not use a temporary category, but want to keep patrons.
# We should not delete patrons when the value is NULL, empty string or 0.
my $date_enrolled = dt_from_string();
$date_enrolled->subtract( days => $delay );

View file

@ -792,13 +792,14 @@ OPAC:
- pref: PatronSelfRegistrationDefaultCategory
choices: patron-categories
- "as the default patron category for patrons registered via the OPAC."
- "<br><strong>WARNING: Do not use a regular patron category for self registration.</strong> If the <code>misc/cronjobs/cleanup_database.pl</code> cronjob is setup to delete unverified and unfinished OPAC self registrations, it will permanently and unrecoverably delete all patrons that have registered more than <a href='/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=PatronSelfRegistrationExpireTemporaryAccountsDelay'>PatronSelfRegistrationExpireTemporaryAccountsDelay</a> days ago."
- "<br><strong>WARNING: Do not use a regular patron category for self registration.</strong><br>
If the <code>misc/cronjobs/cleanup_database.pl</code> cronjob is setup to delete unverified and unfinished OPAC self registrations, it will permanently and unrecoverably delete all patrons that have registered more than <a href='/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=PatronSelfRegistrationExpireTemporaryAccountsDelay'>PatronSelfRegistrationExpireTemporaryAccountsDelay</a> days ago (unless that delay is empty or zero)."
-
- Delete patrons still in the category indicated by <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=PatronSelfRegistrationDefaultCategory">PatronSelfRegistrationDefaultCategory</a>
- pref: PatronSelfRegistrationExpireTemporaryAccountsDelay
class: integer
- "days after account creation."
- "<br><strong>NOTE:</strong> This system preference requires the <code>misc/cronjobs/cleanup_database.pl</code> cronjob. Ask your system administrator to schedule it."
- "<br><strong>NOTE:</strong> This system preference requires the <code>misc/cronjobs/cleanup_database.pl</code> cronjob. Ask your system administrator to schedule it.<br>No patrons will be deleted if you set the pref to zero or make it empty."
-
- "The following <a href='http://schema.koha-community.org/__VERSION__/tables/borrowers.html' target='blank'>database columns</a> must be filled in on the patron modification screen:"
- pref: PatronSelfModificationMandatoryField

View file

@ -403,7 +403,7 @@ $borrower = Koha::Patrons->find( $borrowernumber )->unblessed;
ok( $borrower->{userid}, 'A userid should have been generated correctly' );
subtest 'purgeSelfRegistration' => sub {
plan tests => 5;
plan tests => 8;
#purge unverified
my $d=360;
@ -417,7 +417,6 @@ subtest 'purgeSelfRegistration' => sub {
my $c= 'XYZ';
$dbh->do("INSERT IGNORE INTO categories (categorycode) VALUES ('$c')");
t::lib::Mocks::mock_preference('PatronSelfRegistrationDefaultCategory', $c );
t::lib::Mocks::mock_preference('PatronSelfRegistrationExpireTemporaryAccountsDelay', 360);
C4::Members::DeleteExpiredOpacRegistrations();
my $self_reg = $builder->build_object({
class => 'Koha::Patrons',
@ -427,6 +426,18 @@ subtest 'purgeSelfRegistration' => sub {
}
});
# First test if empty PatronSelfRegistrationExpireTemporaryAccountsDelay returns zero
t::lib::Mocks::mock_preference('PatronSelfRegistrationExpireTemporaryAccountsDelay', q{} );
is( C4::Members::DeleteExpiredOpacRegistrations(), 0, "DeleteExpiredOpacRegistrations with empty delay" );
# Test zero too
t::lib::Mocks::mock_preference('PatronSelfRegistrationExpireTemporaryAccountsDelay', 0 );
is( C4::Members::DeleteExpiredOpacRegistrations(), 0, "DeleteExpiredOpacRegistrations with delay 0" );
# Also check empty category
t::lib::Mocks::mock_preference('PatronSelfRegistrationDefaultCategory', q{} );
t::lib::Mocks::mock_preference('PatronSelfRegistrationExpireTemporaryAccountsDelay', 360 );
is( C4::Members::DeleteExpiredOpacRegistrations(), 0, "DeleteExpiredOpacRegistrations with empty category" );
t::lib::Mocks::mock_preference('PatronSelfRegistrationDefaultCategory', $c );
my $checkout = $builder->build_object({
class=>'Koha::Checkouts',
value=>{