From 1054d2da490f691e58824daa5837536ab3f5f0e4 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Tue, 6 Aug 2013 15:38:24 +0000 Subject: [PATCH] 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 --- misc/cronjobs/delete_patrons.pl | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/misc/cronjobs/delete_patrons.pl b/misc/cronjobs/delete_patrons.pl index 8246146..119ac1c 100755 --- 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 -- 1.7.10.4