Bug 10612 - Add ability to delete patrons with batch patron deletion tool

This patch adds the ability to select a patron list for deletetion
when using the Batch patron deletion/anonymization tool. It also adds
buttons to the the patron lists table to access both the batch deletion
and batch modification directly from the lists view.

This is a squash of previous patches but now adds a patron_list_id
parameter to C4::Members::GetBorrowersToExpunge and uses that routine to
fetch patrons from a list.

Test Plan:
1) Apply this patch
2) Create a list of patrons with the new Patron Lists feature
3) Try using the batch edit link form the lists table
4) Try using the batch delete link from the lists table
5) Verify previous functionality has not changed

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Patron batches are correctly passed to the edit and delete pages.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index 2b8f913..ad42335 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -40,6 +40,7 @@ use Koha::Patron::Debarments qw(IsDebarred);
 use Text::Unaccent qw( unac_string );
 use Koha::AuthUtils qw(hash_password);
 use Koha::Database;
+use Koha::List::Patron;
 
 our (@ISA,@EXPORT,@EXPORT_OK,$debug);
 
@@ -1740,6 +1741,7 @@ sub GetHideLostItemsPreference {
       not_borrowered_since => $not_borrowered_since,
       expired_before       => $expired_before,
       category_code        => $category_code,
+      patron_list_id       => $patron_list_id,
       branchcode           => $branchcode
   );
 
@@ -1748,18 +1750,19 @@ sub GetHideLostItemsPreference {
 =cut
 
 sub GetBorrowersToExpunge {
-    my $params = shift;
 
-    my $filterdate     = $params->{'not_borrowered_since'};
-    my $filterexpiry   = $params->{'expired_before'};
-    my $filtercategory = $params->{'category_code'};
-    my $filterbranch   = $params->{'branchcode'} ||
+    my $params = shift;
+    my $filterdate       = $params->{'not_borrowered_since'};
+    my $filterexpiry     = $params->{'expired_before'};
+    my $filtercategory   = $params->{'category_code'};
+    my $filterbranch     = $params->{'branchcode'} ||
                         ((C4::Context->preference('IndependentBranches')
                              && 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;
     my $query = q|
@@ -1775,11 +1778,13 @@ sub GetBorrowersToExpunge {
                 AND guarantorid <> 0
         ) as tmp ON borrowers.borrowernumber=tmp.guarantorid
         LEFT JOIN old_issues USING (borrowernumber)
-        LEFT JOIN issues USING (borrowernumber) 
-        WHERE  category_type <> 'S'
+        LEFT JOIN issues USING (borrowernumber)|;
+    if ( $filterpatronlist  ){
+        $query .= q| LEFT JOIN patron_list_patrons USING (borrowernumber)|;
+    }
+    $query .= q| WHERE  category_type <> 'S'
         AND tmp.guarantorid IS NULL
    |;
-
     my @query_params;
     if ( $filterbranch && $filterbranch ne "" ) {
         $query.= " AND borrowers.branchcode = ? ";
@@ -1793,6 +1798,10 @@ sub GetBorrowersToExpunge {
         $query .= " AND categorycode = ? ";
         push( @query_params, $filtercategory );
     }
+    if ( $filterpatronlist ){
+        $query.=" AND patron_list_id = ? ";
+        push( @query_params, $filterpatronlist );
+    }
     $query.=" GROUP BY borrowers.borrowernumber HAVING currentissue IS NULL ";
     if ( $filterdate ) {
         $query.=" AND ( latestissue < ? OR latestissue IS NULL ) ";
@@ -1803,10 +1812,10 @@ sub GetBorrowersToExpunge {
     my $sth = $dbh->prepare($query);
     if (scalar(@query_params)>0){  
         $sth->execute(@query_params);
-    } 
+    }
     else {
         $sth->execute;
-    }      
+    }
     
     my @results;
     while ( my $data = $sth->fetchrow_hashref ) {

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
index 8d90493..14ce99a 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
@@ -51,6 +51,12 @@
                     <th>Name</th>
                     <th>Patrons in list</th>
                     <th>&nbsp;</th>
+                    [% IF CAN_user_tools_edit_patrons %]
+                        <th>&nbsp;</th>
+                    [% END %]
+                    [% IF CAN_user_tools_delete_anonymize_patrons %]
+                        <th>&nbsp;</th>
+                    [% END %]
                 </tr>
             </thead>
 
@@ -72,6 +78,20 @@
                                 </ul>
                             </div>
                         </td>
+                        [% IF CAN_user_tools_edit_patrons %]
+                            <td>
+                                <a class="btn btn-mini" href="/cgi-bin/koha/tools/modborrowers.pl?patron_list_id=[% l.patron_list_id %]&op=show">
+                                    <i class="fa fa-edit"></i> Edit patrons</i>
+                                </a>
+                            </td>
+                        [% END %]
+                        [% IF CAN_user_tools_delete_anonymize_patrons %]
+                            <td>
+                                <a class="btn btn-mini" href="/cgi-bin/koha/tools/cleanborrowers.pl?step=2&patron_list_id=[% l.patron_list_id %]&checkbox=borrower">
+                                    <i class="fa fa-trash"></i> Delete patrons</i>
+                                </a>
+                            </td>
+                        [% END %]
                     </tr>
                 [% END %]
             </tbody>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt
index 388da10..9e68141 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt
@@ -12,7 +12,7 @@
          */
           function checkForm(form) {
               if((form.checkbox[0].checked)){
-                  if ( (!form.date1.value) && (!form.borrower_dateexpiry.value) && (!form.borrower_categorycode.value) ){
+                  if ( (!form.date1.value) && (!form.borrower_dateexpiry.value) && (!form.borrower_categorycode.value) && (!form.patron_list_id.value)){
                     alert(_("Please enter at least one criterion for deletion!"));
                     return false;
                   }
@@ -88,8 +88,20 @@
                         [% END %]
                     </select>
                 </li>
+                [% IF patron_lists %]
+                <li>
+                    <label for="patron_list_id">who are in patron list: </label>
+                    <select id="patron_list_id" name="patron_list_id">
+                        <option value=""></option>
+                        [% FOREACH pl IN patron_lists %]
+                            <option value="[% pl.patron_list_id %]">[% pl.name %]</option>
+                        [% END %]
+                    </select>
+                </li>
+                [% END %]
             </ul>
         </fieldset>
+
         <fieldset>
         <legend>Anonymize checkout history</legend>
         [% UNLESS Koha.Preference('AnonymousPatron') %]
@@ -121,12 +133,12 @@
     <fieldset>
         <legend>Warnings</legend>
         <ul>
-            <li>[% IF ( totalToDelete ) %][% totalToDelete %] [% ELSE %]0 [% END %] patrons will be deleted</li>
-            <li>[% IF ( totalToAnonymize ) %][% totalToAnonymize %] [% ELSE %]0 [% END %] checkout history will be anonymized</li>
+            <li>[% patrons_to_delete.size || 0 %] patrons will be deleted</li>
+            <li>[% patrons_to_anonymize.size || 0 %] patron's checkout histories will be anonymized</li>
         </ul>
 
         <br />
-            [% IF ( totalToDelete ) %]
+            [% IF patrons_to_delete.size %]
                 <fieldset><legend>What do you want to do for deleted patrons?</legend>
                 <input id="delete" type="radio" name="radio" value="delete" />
                 <label for="delete">Permanently delete these patrons</label>
@@ -136,12 +148,12 @@
 
                 <br /><input id="testrun" type="radio" name="radio" value="testrun" checked="checked" />
                 <label for="testrun">Do not remove any patrons (test run)</label>
-                <input type="hidden" name="do_delete" value="[% totalToDelete %]" /></fieldset>
+                <input type="hidden" name="do_delete" value="[% patrons_to_delete.size %]" /></fieldset>
 
             [% END %]
-            [% IF ( totalToAnonymize ) %]
-                Checkout history for [% totalToAnonymize %] patrons will be anonymized
-                <input type="hidden" name="do_anonym" value="[% totalToAnonymize %]" />
+            [% IF patrons_to_anonymize.size %]
+                Checkout history for [% patrons_to_anonymize.size %] patrons will be anonymized
+                <input type="hidden" name="do_anonym" value="[% patrons_to_anonymize.size %]" />
             [% END %]
 
             <input type="hidden" name="step" value="3" />
@@ -149,6 +161,7 @@
             <input type="hidden" name="last_issue_date" value="[% last_issue_date | $KohaDates %]" />
             <input type="hidden" name="borrower_dateexpiry" value="[% borrower_dateexpiry | $KohaDates %]" />
             <input type="hidden" name="borrower_categorycode" value="[% borrower_categorycode %]" />
+            <input type="hidden" name="patron_list_id" value="[% patron_list_id %]" />
     </fieldset>
     <fieldset class="action"><input type="submit" value="Finish" /> <a class="cancel" href="/cgi-bin/koha/tools/cleanborrowers.pl">Cancel</a></fieldset>
         </form>

diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl
index 7888b99..0214729 100755 (executable)
--- a/tools/cleanborrowers.pl
+++ b/tools/cleanborrowers.pl
@@ -41,6 +41,7 @@ use C4::Members;        # GetBorrowersWhoHavexxxBorrowed.
 use C4::Circulation;    # AnonymiseIssueHistory.
 use Koha::DateUtils qw( dt_from_string output_pref );
 use Date::Calc qw/Today Add_Delta_YM/;
+use Koha::List::Patron;
 
 my $cgi = new CGI;
 
@@ -63,6 +64,7 @@ my $borrower_dateexpiry =
   $params->{borrower_dateexpiry}
   ? dt_from_string $params->{borrower_dateexpiry}
   : undef;
+my $patron_list_id = $params->{patron_list_id};
 
 my $borrower_categorycode = $params->{'borrower_categorycode'} || q{};
 
@@ -80,28 +82,28 @@ if ( $step == 2 ) {
 
     my %checkboxes = map { $_ => 1 } split /\0/, $params->{'checkbox'};
 
-    my $totalDel;
-    my $membersToDelete;
+    my $patrons_to_delete;
     if ( $checkboxes{borrower} ) {
-        $membersToDelete = GetBorrowersToExpunge(
-            _get_selection_params($not_borrowered_since, $borrower_dateexpiry, $borrower_categorycode)
+        $patrons_to_delete = GetBorrowersToExpunge(
+             _get_selection_params(
+                  $not_borrowered_since,
+                  $borrower_dateexpiry,
+                  $borrower_categorycode,
+                  $patron_list_id,
+             )
         );
-        _skip_borrowers_with_nonzero_balance( $membersToDelete );
-        $totalDel = scalar @$membersToDelete;
-
     }
-    my $totalAno;
-    my $membersToAnonymize;
+    _skip_borrowers_with_nonzero_balance($patrons_to_delete);
+
+    my $members_to_anonymize;
     if ( $checkboxes{issue} ) {
-        $membersToAnonymize = GetBorrowersWithIssuesHistoryOlderThan($last_issue_date);
-        $totalAno           = scalar @$membersToAnonymize;
+        $members_to_anonymize = GetBorrowersWithIssuesHistoryOlderThan($last_issue_date);
     }
 
     $template->param(
-        totalToDelete           => $totalDel,
-        totalToAnonymize        => $totalAno,
-        memberstodelete_list    => $membersToDelete,
-        memberstoanonymize_list => $membersToAnonymize,
+        patrons_to_delete    => $patrons_to_delete,
+        patrons_to_anonymize => $members_to_anonymize,
+        patron_list_id          => $patron_list_id,
     );
 }
 
@@ -113,18 +115,22 @@ elsif ( $step == 3 ) {
 
     # delete members
     if ($do_delete) {
-        my $membersToDelete = GetBorrowersToExpunge(
-            _get_selection_params($not_borrowered_since, $borrower_dateexpiry, $borrower_categorycode)
-        );
-        _skip_borrowers_with_nonzero_balance( $membersToDelete );
-        $totalDel = scalar(@$membersToDelete);
+        my $patrons_to_delete = GetBorrowersToExpunge(
+                _get_selection_params(
+                    $not_borrowered_since, $borrower_dateexpiry,
+                    $borrower_categorycode, $patron_list_id
+                )
+            );
+        _skip_borrowers_with_nonzero_balance($patrons_to_delete);
+
+        $totalDel = scalar(@$patrons_to_delete);
         $radio    = $params->{'radio'};
         for ( my $i = 0 ; $i < $totalDel ; $i++ ) {
             $radio eq 'testrun' && last;
-            my $borrowernumber = $membersToDelete->[$i]->{'borrowernumber'};
-            $radio eq 'trash' && MoveMemberToDeleted( $borrowernumber );
-            C4::Members::HandleDelBorrower( $borrowernumber );
-            DelMember( $borrowernumber );
+            my $borrowernumber = $patrons_to_delete->[$i]->{'borrowernumber'};
+            $radio eq 'trash' && MoveMemberToDeleted($borrowernumber);
+            C4::Members::HandleDelBorrower($borrowernumber);
+            DelMember($borrowernumber);
         }
         $template->param(
             do_delete => '1',
@@ -145,6 +151,14 @@ elsif ( $step == 3 ) {
         trash => ( $radio eq "trash" ) ? (1) : (0),
         testrun => ( $radio eq "testrun" ) ? 1: 0,
     );
+} else { # $step == 1
+    my @all_lists = GetPatronLists();
+    my @non_empty_lists;
+    foreach my $list (@all_lists){
+    my @patrons = $list->patron_list_patrons();
+        if( scalar @patrons ) { push(@non_empty_lists,$list) }
+    }
+    $template->param( patron_lists => [ @non_empty_lists ] );
 }
 
 $template->param(
@@ -169,7 +183,7 @@ sub _skip_borrowers_with_nonzero_balance {
 }
 
 sub _get_selection_params {
-    my ($not_borrowered_since, $borrower_dateexpiry, $borrower_categorycode) = @_;
+    my ($not_borrowered_since, $borrower_dateexpiry, $borrower_categorycode, $patron_list_id) = @_;
 
     my $params = {};
     $params->{not_borrowered_since} = output_pref({
@@ -183,6 +197,7 @@ sub _get_selection_params {
         dateonly   => 1
     }) if $borrower_dateexpiry;
     $params->{category_code} = $borrower_categorycode if $borrower_categorycode;
+    $params->{patron_list_id} = $patron_list_id if $patron_list_id;
 
     return $params;
 };