Attachment 48881 Details for Bug 10612 – Bug 10612 - Add ability to delete patrons with batch patron deletion tool

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

Bug-10612---Add-ability-to-delete-patrons-with-bat.patch (text/plain), 15.49 KB, created by Nick Clemens (kidclamp) on 2016-03-09 15:48:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2016-03-09 15:48:34 UTC

Size: 15.49 KB

patch

obsolete

>From 6db0639f6811b7862b2abe55032d436cc1176405 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Wed, 11 Jun 2014 08:11:48 -0400
>Subject: [PATCH] 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
>---
> C4/Members.pm                                      | 31 ++++++----
> .../prog/en/modules/patron_lists/lists.tt          | 20 +++++++
> .../prog/en/modules/tools/cleanborrowers.tt        | 29 ++++++---
> tools/cleanborrowers.pl                            | 69 ++++++++++++++--------
> 4 files changed, 104 insertions(+), 45 deletions(-)
>
>diff --git a/C4/Members.pm b/C4/Members.pm
>index cc438f0..ac81195 100644
>--- 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 ($VERSION,@ISA,@EXPORT,@EXPORT_OK,$debug);
> 
>@@ -1851,6 +1852,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
>   );
> 
>@@ -1859,18 +1861,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|
>@@ -1886,11 +1889,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 = ? ";
>@@ -1904,20 +1909,24 @@ 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 ) ";
>         push @query_params,$filterdate;
>     }
>-    warn $query if $debug;
>+    warn $query;# if $debug;
> 
>     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 2f9b7e3..779058f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/lists.tt
>@@ -49,6 +49,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>
> 
>@@ -70,6 +76,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
>--- 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..c248929 100755
>--- a/tools/cleanborrowers.pl
>+++ b/tools/cleanborrowers.pl
>@@ -40,7 +40,9 @@ use C4::Output;
> use C4::Members;        # GetBorrowersWhoHavexxxBorrowed.
> use C4::Circulation;    # AnonymiseIssueHistory.
> use Koha::DateUtils qw( dt_from_string output_pref );
>+use Koha::Database;
> use Date::Calc qw/Today Add_Delta_YM/;
>+use Koha::List::Patron;
> 
> my $cgi = new CGI;
> 
>@@ -63,6 +65,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 +83,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 +116,23 @@ 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'}
>+              || $patrons_to_delete->[$i]->get_column('borrowernumber');
>+            $radio eq 'trash' && MoveMemberToDeleted($borrowernumber);
>+            C4::Members::HandleDelBorrower($borrowernumber);
>+            DelMember($borrowernumber);
>         }
>         $template->param(
>             do_delete => '1',
>@@ -145,6 +153,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(
>@@ -163,13 +179,13 @@ sub _skip_borrowers_with_nonzero_balance {
>     my $borrowers = shift;
>     my $balance;
>     @$borrowers = map {
>-        (undef, undef, $balance) = GetMemberIssuesAndFines( $_->{borrowernumber} );
>+        (undef, undef, $balance) = GetMemberIssuesAndFines( $_->{borrowernumber} || $_->get_column('borrowernumber') );
>         ($balance != 0) ? (): ($_);
>     } @$borrowers;
> }
> 
> 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 +199,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;
> };
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 10612: 19766 | 19767 | 21081 | 21444 | 21445 | 21446 | 22837 | 22838 | 28779 | 28780 | 36311 | 36313 | 36314 | 36315 | 41289 | 41363 | 41364 | 41365 | 46239 | 46279 | 46280 | 46834 | 47153 | 47224 | 47225 | 47226 | 47255 | 47259 | 47393 | 47394 | 47395 | 47396 | 47478 | 48833 | 48881 | 48888 | 48890 | 48891 | 48892 | 49596 | 49597 | 49607 | 49694 | 49740 | 49741 | 49766 | 49767 | 49768 | 49769 | 49770