From 148816bf51bb81cd6ed426f45686f7018b62b4ca Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
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
---
 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 6c10eeeb2c..1f191d1600 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -612,12 +612,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');
@@ -634,6 +637,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 a1a9cae442..9a9265338b 100755
--- a/misc/cronjobs/cleanup_database.pl
+++ b/misc/cronjobs/cleanup_database.pl
@@ -26,6 +26,7 @@ use constant DEFAULT_LOGS_PURGEDAYS               => 180;
 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
@@ -76,9 +77,11 @@ Usage: $0 [-h|--help] [--sessions] [--sessdays DAYS] [-v|--verbose] [--zebraqueu
    --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
@@ -109,6 +112,7 @@ my $pListShareInvites;
 my $pDebarments;
 my $allDebarments;
 my $pExpSelfReg;
+my $pExpSelfRegType = 'hard';
 my $pUnvSelfReg;
 my $fees_days;
 my $special_holidays_days;
@@ -140,6 +144,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,
@@ -192,6 +197,7 @@ unless ( $sessions
     || $pOldIssues
     || $pOldReserves
     || $pTransfers
+    || ( !defined $pExpSelfRegType || $pExpSelfRegType !~ m/^hard|soft$/ )
 ) {
     print "You did not specify any cleanup work for the script to do.\n\n";
     usage(1);
@@ -354,7 +360,7 @@ elsif ($verbose) {
 }
 
 if( $pExpSelfReg ) {
-    DeleteExpiredSelfRegs();
+    DeleteExpiredSelfRegs( $pExpSelfRegType );
 }
 if( $pUnvSelfReg ) {
     DeleteUnverifiedSelfRegs( $pUnvSelfReg );
@@ -569,7 +575,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.24.2 (Apple Git-127)