Bug 22052: Refactor DeleteExpiredOpacReigstrations

This patch changes the sub to use Koha::Objects and updates the tests.
Previously the sub would die on borrowers with checkouts and would
delete borrowers if they had fines

To test:
 1 - prove -v t/db_dependent/
 2 - Set your selfreg preferences:
    PatronSelfRegistration: Allow
    PatronSelfRegistrationDefaultCategory: SELFREG (or of your choice)
    PatronSelfRegistrationExpireTemporaryAccountsDelay: 30
 3 - Register a patron into SELFREG or how you set above
 4 - Set their date enrolled to two months ago
 5 - Checkout an item to the patron
 6 - Issue a fine to that patron
 7 - perl misc/cronjobs/cleanup_database.pl --del-exp-selfreg -v
 8 - The job should die with an error
 9 - Check in the item
10 - run the corn again - patron is deleted, oops
11 - Apply patch
12 - Create another patron in the same way
13 - Checkout and fine the patron
14 - run the cron
15 - they are not deleted, and no error
16 - checkin the item
17 - run the cron
18 - they are not deleted and no error
19 - clear the fine
20 - run the cron
21 - patron is deleted, huzzah

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit b32756a5d7819e13290c2a145f3f0009280785c4)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit 8102f6dc70623786c3247e008a0601cfd234d1e6)

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index 58b0cb2..2b7c775 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -99,7 +99,7 @@ use C4::Members;
 
 =head1 DESCRIPTION
 
-This module contains routines for adding, modifying and deleting members/patrons/borrowers 
+This module contains routines for adding, modifying and deleting members/patrons/borrowers
 
 =head1 FUNCTIONS
 
@@ -141,7 +141,7 @@ The following will be set where applicable:
  $flags->{WAITING}->{message}       Message -- deprecated
  $flags->{WAITING}->{itemlist}      ref-to-array: list of available items
 
-=over 
+=over
 
 =item C<$flags-E<gt>{ODUES}-E<gt>{itemlist}> is a reference-to-array listing the
 overdue items. Its elements are references-to-hash, each describing an
@@ -157,7 +157,7 @@ fields from the reserves table of the Koha database.
 
 =back
 
-All the "message" fields that include language generated in this function are deprecated, 
+All the "message" fields that include language generated in this function are deprecated,
 because such strings belong properly in the display layer.
 
 The "message" field that comes from the DB is OK.
@@ -537,14 +537,14 @@ sub GetAllIssues {
     my $dbh = C4::Context->dbh;
     my $query =
 'SELECT *, issues.timestamp as issuestimestamp, issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
-  FROM issues 
+  FROM issues
   LEFT JOIN items on items.itemnumber=issues.itemnumber
   LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
   LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
-  WHERE borrowernumber=? 
+  WHERE borrowernumber=?
   UNION ALL
-  SELECT *, old_issues.timestamp as issuestimestamp, old_issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp 
-  FROM old_issues 
+  SELECT *, old_issues.timestamp as issuestimestamp, old_issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
+  FROM old_issues
   LEFT JOIN items on items.itemnumber=old_issues.itemnumber
   LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
   LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
@@ -641,11 +641,11 @@ sub GetBorrowersToExpunge {
     my $filtercategory   = $params->{'category_code'};
     my $filterbranch     = $params->{'branchcode'} ||
                         ((C4::Context->preference('IndependentBranches')
-                             && C4::Context->userenv 
+                             && C4::Context->userenv
                              && !C4::Context->IsSuperLibrarian()
                              && C4::Context->userenv->{branch})
                          ? C4::Context->userenv->{branch}
-                         : "");  
+                         : "");
     my $filterpatronlist = $params->{'patron_list_id'};
 
     my $dbh   = C4::Context->dbh;
@@ -703,13 +703,13 @@ sub GetBorrowersToExpunge {
     warn $query if $debug;
 
     my $sth = $dbh->prepare($query);
-    if (scalar(@query_params)>0){  
+    if (scalar(@query_params)>0){
         $sth->execute(@query_params);
     }
     else {
         $sth->execute;
     }
-    
+
     my @results;
     while ( my $data = $sth->fetchrow_hashref ) {
         push @results, $data;
@@ -920,18 +920,18 @@ sub DeleteExpiredOpacRegistrations {
     my $category_code = C4::Context->preference('PatronSelfRegistrationDefaultCategory');
 
     return 0 if not $category_code or not defined $delay or $delay eq q||;
+    my $date_enrolled = dt_from_string();
+    $date_enrolled->subtract( days => $delay );
 
-    my $query = qq|
-SELECT borrowernumber
-FROM borrowers
-WHERE categorycode = ? AND DATEDIFF( NOW(), dateenrolled ) > ? |;
+    my $registrations_to_del = Koha::Patrons->search({
+        dateenrolled => {'<=' => $date_enrolled->ymd},
+        categorycode => $category_code,
+    });
 
-    my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare($query);
-    $sth->execute( $category_code, $delay );
     my $cnt=0;
-    while ( my ($borrowernumber) = $sth->fetchrow_array() ) {
-        Koha::Patrons->find($borrowernumber)->delete;
+    while ( my $registration = $registrations_to_del->next() ) {
+        next if $registration->checkouts->count || $registration->account->balance;
+        $registration->delete;
         $cnt++;
     }
     return $cnt;