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 <bgkriegel@gmail.com>
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 <Katrin.Fischer.83@web.de>
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 <gmc@esilibrary.com>

diff --git a/misc/cronjobs/delete_patrons.pl b/misc/cronjobs/delete_patrons.pl
index cc47b7a..c1ee9ef 100755 (executable)
--- 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.