Bug 10419: (follow-up) functional improvements

[1] Patron deletion now happens atomically; if one part
    of the process fails, the record isn't left in a
    partially deleted state.
[2] The routine for handling lists properly during patron
    deletion is now invoked.
[3] The script now prints an indication if it's run
    without --confirm; otherwise, one might think that
    patron records were actually being deleted.
[4] --verbose now actually does something.

Without --verbose, the script will print the dry-run
warning (if applicable), the number of patrons to be
deleted, and error messages.

With --verbose, the script will also print a line with
the borrowernumber of each patron to be deleted.

To test:

[1] Run the script with and without --verbose and compare
    the, well, verbosity.
[2] Run the script without --confirm and note that the script
    prints a message saying that it's running in dry-run mode.
[3] Use the script to try to delete one or more patrons that have
    loans.  Confirm that error messages are printed reporting
    foreign constraints preventing the deletion.  Also confirm that
    no new rows are added to deletedborrowers for those patrons that
    could not be completely deleted.
[4] Use the script to delete a patron that has a public list.  Verify
    that after the deletion, the public list still exists but now
    has a null owner.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/misc/cronjobs/delete_patrons.pl b/misc/cronjobs/delete_patrons.pl
index 8246146..119ac1c 100755 (executable)
--- a/misc/cronjobs/delete_patrons.pl
+++ b/misc/cronjobs/delete_patrons.pl
@@ -6,6 +6,7 @@ use Pod::Usage;
 use Getopt::Long;
 
 use C4::Members;
+use C4::VirtualShelves;
 use Koha::DateUtils;
 
 my ( $help, $verbose, $not_borrowed_since, $expired_before, $category_code,
@@ -44,28 +45,49 @@ my $members = GetBorrowersToExpunge(
     }
 );
 
+unless ($confirm) {
+    say "Doing a dry run; no patron records will actually be deleted.";
+    say "Run again with --confirm to delete the records.";
+}
+
 say scalar(@$members) . " patrons to delete";
 
 my $dbh = C4::Context->dbh;
 $dbh->{RaiseError} = 1;
 $dbh->{PrintError} = 0;
 
+@$members = ( { borrowernumber => 19 } );
+
+$dbh->{AutoCommit} = 0; # use transactions to avoid partial deletes
 for my $member (@$members) {
-    print "Trying to delete patron " . $member->{borrowernumber} . "... ";
+    print "Trying to delete patron $member->{borrowernumber}... "
+      if $verbose;
     eval {
         C4::Members::MoveMemberToDeleted( $member->{borrowernumber} )
           if $confirm;
     };
     if ($@) {
-        say "Failed, cannot move this patron ($@)";
+        say "Failed to delete patron $member->{borrowernumber}, cannot move it: ($@)";
+        $dbh->rollback;
+        next;
+    }
+    eval {
+        C4::VirtualShelves::HandleDelBorrower( $member->{borrowernumber} )
+          if $confirm;
+    };
+    if ($@) {
+        say "Failed to delete patron $member->{borrowernumber}, error handling its lists: ($@)";
+        $dbh->rollback;
         next;
     }
     eval { C4::Members::DelMember( $member->{borrowernumber} ) if $confirm; };
     if ($@) {
-        say "Failed, cannot delete this patron ($@)";
+        say "Failed to delete patron $member->{borrowernumber}: $@)";
+        $dbh->rollback;
         next;
     }
-    say "OK";
+    $dbh->commit;
+    say "OK" if $verbose;
 }
 
 =head1 NAME