From 6db0639f6811b7862b2abe55032d436cc1176405 Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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 @@ Name Patrons in list   + [% IF CAN_user_tools_edit_patrons %] +   + [% END %] + [% IF CAN_user_tools_delete_anonymize_patrons %] +   + [% END %] @@ -70,6 +76,20 @@ + [% IF CAN_user_tools_edit_patrons %] + + + Edit patrons + + + [% END %] + [% IF CAN_user_tools_delete_anonymize_patrons %] + + + Delete patrons + + + [% END %] [% END %] 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 %] + [% IF patron_lists %] +
  • + + +
  • + [% END %] +
    Anonymize checkout history [% UNLESS Koha.Preference('AnonymousPatron') %] @@ -121,12 +133,12 @@
    Warnings
      -
    • [% IF ( totalToDelete ) %][% totalToDelete %] [% ELSE %]0 [% END %] patrons will be deleted
    • -
    • [% IF ( totalToAnonymize ) %][% totalToAnonymize %] [% ELSE %]0 [% END %] checkout history will be anonymized
    • +
    • [% patrons_to_delete.size || 0 %] patrons will be deleted
    • +
    • [% patrons_to_anonymize.size || 0 %] patron's checkout histories will be anonymized

    - [% IF ( totalToDelete ) %] + [% IF patrons_to_delete.size %]
    What do you want to do for deleted patrons? @@ -136,12 +148,12 @@
    -
    +
    [% END %] - [% IF ( totalToAnonymize ) %] - Checkout history for [% totalToAnonymize %] patrons will be anonymized - + [% IF patrons_to_anonymize.size %] + Checkout history for [% patrons_to_anonymize.size %] patrons will be anonymized + [% END %] @@ -149,6 +161,7 @@ +
    Cancel
    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