From bdfc3baf012f6b96f18354b499c8669c24f19915 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Thu, 23 Apr 2020 15:16:17 -0400 Subject: [PATCH] Bug 25183: cleanup-database.pl --del-exp-selfreg fully deletes borrowers - should go into deleted-borrowers The cleanup database job for deleting expired self registrations fully deletes borrowers instead of transferring them to deleted borrowers. This is a problem when libraries think they are deleting email unverified patrons instead of ones that haven't moved out of the category, an easy thing to be confused about. Test Plan: 1) Apply this patch 2) Try running cleanup_database.pl with the new -del-exp-selfreg-type parameters 'soft' and 'hard' 3) Param value 'soft' should move patrons to deletedborrowers 4) Param value 'hard' should follow existing behavior 5) Leave our -del-exp-selfreg-type, you should get the existing behavior Signed-off-by: Martin Renvoize --- C4/Members.pm | 4 ++++ misc/cronjobs/cleanup_database.pl | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 05056345e3..a7b372ee95 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -622,12 +622,15 @@ sub IssueSlip { =head2 DeleteExpiredOpacRegistrations + $deleted_count = DeleteExpiredOpacRegistrations([$type == "hard|soft"]) + Delete accounts that haven't been upgraded from the 'temporary' category Returns the number of removed patrons =cut sub DeleteExpiredOpacRegistrations { + my ( $type ) = @_; my $delay = C4::Context->preference('PatronSelfRegistrationExpireTemporaryAccountsDelay'); my $category_code = C4::Context->preference('PatronSelfRegistrationDefaultCategory'); @@ -644,6 +647,7 @@ sub DeleteExpiredOpacRegistrations { my $cnt=0; while ( my $registration = $registrations_to_del->next() ) { next if $registration->checkouts->count || $registration->account->balance; + $registration->move_to_deleted if $type && $type eq 'soft'; $registration->delete; $cnt++; } diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl index 062efe6041..75db9af7b6 100755 --- a/misc/cronjobs/cleanup_database.pl +++ b/misc/cronjobs/cleanup_database.pl @@ -27,6 +27,7 @@ use constant DEFAULT_MESSAGES_PURGEDAYS => 365; use constant DEFAULT_SEARCHHISTORY_PURGEDAYS => 30; use constant DEFAULT_SHARE_INVITATION_EXPIRY_DAYS => 14; use constant DEFAULT_DEBARMENTS_PURGEDAYS => 30; +use constant DEFAULT_EXP_SELFREG_TYPE => 'hard'; BEGIN { # find Koha's Perl modules @@ -89,9 +90,11 @@ Usage: $0 [-h|--help] [--confirm] [--sessions] [--sessdays DAYS] [-v|--verbose] --restrictions DAYS purge patrons restrictions expired since more than DAYS days. Defaults to 30 days if no days specified. --all-restrictions purge all expired patrons restrictions. - --del-exp-selfreg Delete expired self registration accounts - --del-unv-selfreg DAYS Delete unverified self registrations older than DAYS - --unique-holidays DAYS Delete all unique holidays older than DAYS + --del-exp-selfreg Delete expired self registration accounts + --del-exp-selfreg-type TYPE Option 'hard' completely removes patrons from the database, + Option 'soft' moves patron to deleted patrons table. + --del-unv-selfreg DAYS Delete unverified self registrations older than DAYS + --unique-holidays DAYS Delete all unique holidays older than DAYS --temp-uploads Delete temporary uploads. --temp-uploads-days DAYS Override the corresponding preference value. --uploads-missing FLAG Delete upload records for missing files when FLAG is true, count them otherwise @@ -127,6 +130,7 @@ my $pListShareInvites; my $pDebarments; my $allDebarments; my $pExpSelfReg; +my $pExpSelfRegType = 'hard'; my $pUnvSelfReg; my $fees_days; my $special_holidays_days; @@ -162,6 +166,7 @@ GetOptions( 'restrictions:i' => \$pDebarments, 'all-restrictions' => \$allDebarments, 'del-exp-selfreg' => \$pExpSelfReg, + 'del-exp-selfreg-type' => \$pExpSelfRegType, 'del-unv-selfreg' => \$pUnvSelfReg, 'unique-holidays:i' => \$special_holidays_days, 'temp-uploads' => \$temp_uploads, @@ -222,6 +227,7 @@ unless ( $sessions || $pPseudoTransactionsFrom || $pPseudoTransactionsTo || $pMessages + || ( !defined $pExpSelfRegType || $pExpSelfRegType !~ m/^hard|soft$/ ) ) { print "You did not specify any cleanup work for the script to do.\n\n"; usage(1); @@ -429,7 +435,7 @@ if ($verbose) { # But non trivial changes to C4::Members need to be done before. if( $pExpSelfReg ) { if ( $confirm ) { - DeleteExpiredSelfRegs(); + DeleteExpiredSelfRegs( $pExpSelfRegType ); } elsif ( $verbose ) { say "self-registered borrowers may be deleted"; } @@ -689,7 +695,8 @@ sub PurgeDebarments { } sub DeleteExpiredSelfRegs { - my $cnt= C4::Members::DeleteExpiredOpacRegistrations(); + my ( $type ) = @_; + my $cnt = C4::Members::DeleteExpiredOpacRegistrations( $type ); print "Removed $cnt expired self-registered borrowers\n" if $verbose; } -- 2.20.1