From cf31604a0e4ba0f731f8fc8634007d1986e99f51 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 30 Sep 2013 14:14:15 +0200 Subject: [PATCH] Bug 10419: (follow-up) patrons with fines should not be deleted Test plan: - add a fine for a patron - verify the script does not delete this patron. Signed-off-by: Bernardo Gonzalez Kriegel Works, script do not delete a patron with fines. No koha-qa errors with all patches applied Test 1) Added a fine to a patron 2) run script 3) reports condition Trying to delete patron 5... Failed to delete patron 5: patron has 10.00 in fines 4) Patron is not deleted Signed-off-by: Katrin Fischer Passes all tests and QA script. Tested various combinations of options: ./delete_patrons.pl Gives a helpful message about the use of the script. ./delete_patrons.pl -h Outputs useful information about the use of the script and its various options. ./delete_patrons.pl --category_code ST --library CPL Gives the correct results in numbers and deletion was done properly. Also tested: --expired_before --not_borrowed_since -v Testing various conditions where a delete should not occur: - Patron has checkouts Patron is not in list of patrons to delete (x patrons to delete) - Patron has fines Patron is still in list of patrons to delete (x patrons to delete) Checked deleted patrons had been moved to deletedborrowers. Notes about possible enhancements: - only print the success message 'x patrons deleted' when confirm flag was set - patrons with current checkouts are silently excluded from the number of patrons to be deleted. Printing the number with current checkouts might be helpful. Changes made: Fixed a small error in the documentation: expired_date is expired_before. Signed-off-by: Galen Charlton --- misc/cronjobs/delete_patrons.pl | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/misc/cronjobs/delete_patrons.pl b/misc/cronjobs/delete_patrons.pl index cc47b7ac52..c1ee9efae1 100755 --- a/misc/cronjobs/delete_patrons.pl +++ b/misc/cronjobs/delete_patrons.pl @@ -57,37 +57,49 @@ $dbh->{RaiseError} = 1; $dbh->{PrintError} = 0; $dbh->{AutoCommit} = 0; # use transactions to avoid partial deletes +my $deleted = 0; for my $member (@$members) { print "Trying to delete patron $member->{borrowernumber}... " if $verbose; + + my $borrowernumber = $member->{borrowernumber}; + my $flags = C4::Members::patronflags( $member ); + if ( my $charges = $flags->{CHARGES}{amount} ) { + say "Failed to delete patron $borrowernumber: patron has $charges in fines"; + next; + } + eval { - C4::Members::MoveMemberToDeleted( $member->{borrowernumber} ) + C4::Members::MoveMemberToDeleted( $borrowernumber ) if $confirm; }; if ($@) { - say "Failed to delete patron $member->{borrowernumber}, cannot move it: ($@)"; + say "Failed to delete patron $borrowernumber, cannot move it: ($@)"; $dbh->rollback; next; } eval { - C4::VirtualShelves::HandleDelBorrower( $member->{borrowernumber} ) + C4::VirtualShelves::HandleDelBorrower( $borrowernumber ) if $confirm; }; if ($@) { - say "Failed to delete patron $member->{borrowernumber}, error handling its lists: ($@)"; + say "Failed to delete patron $borrowernumber, error handling its lists: ($@)"; $dbh->rollback; next; } - eval { C4::Members::DelMember( $member->{borrowernumber} ) if $confirm; }; + eval { C4::Members::DelMember( $borrowernumber ) if $confirm; }; if ($@) { - say "Failed to delete patron $member->{borrowernumber}: $@)"; + say "Failed to delete patron $borrowernumber: $@)"; $dbh->rollback; next; } $dbh->commit; + $deleted++; say "OK" if $verbose; } +say "$deleted patrons deleted"; + =head1 NAME delete_patrons - This script deletes patrons @@ -112,7 +124,7 @@ Print a brief help message Delete patrons who have not borrowed since this date. -=item B<--expired_date> +=item B<--expired_before> Delete patrons with an account expired before this date. -- 2.39.5