From 21eb28eac448e1a85f696d1b7c2c39336a1fe2f2 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 25 Sep 2013 10:14:49 -0400 Subject: [PATCH] Bug 10612 - Add ability to delete patrons with batch patron modification tool - QA Followup - Move member deletion related calls to DelMember --- C4/Members.pm | 21 ++++++++++++++----- C4/VirtualShelves.pm | 4 +- members/deletemem.pl | 4 +-- misc/cronjobs/delete_expired_opac_registrations.pl | 2 +- .../delete_unverified_opac_registrations.pl | 1 - t/db_dependent/Members.t | 2 +- tools/cleanborrowers.pl | 7 +---- tools/modborrowers.pl | 3 +- 8 files changed, 23 insertions(+), 21 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 7ac69df..a76c311 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -36,6 +36,7 @@ use C4::Letters; use C4::SQLHelper qw(InsertInTable UpdateInTable SearchInTable); use C4::Members::Attributes qw(SearchIdMatchingAttribute); use C4::NewsChannels; #get slip news +use C4::VirtualShelves qw(HandleDelBorrower); use DateTime; use DateTime::Format::DateParse; use Koha::DateUtils; @@ -1819,19 +1820,27 @@ sub MoveMemberToDeleted { =head2 DelMember - DelMember($borrowernumber); + DelMember({ borrowernumber => $borrowernumber [, permanently => 1 ]); -This function remove directly a borrower whitout writing it on deleteborrower. -+ Deletes reserves for the borrower + * Deletes reserves for the borrower + * Calls C4::VirtualShelfs::HandleDelBorrower + * Calls MoveMemberToDelete unless the parameter 'permanently' is true =cut sub DelMember { - my $dbh = C4::Context->dbh; - my $borrowernumber = shift; - #warn "in delmember with $borrowernumber"; + my ( $params ) = @_; + + my $borrowernumber = $params->{borrowernumber}; + my $permanently = $params->{permanently}; + + my $dbh = C4::Context->dbh; + return unless $borrowernumber; # borrowernumber is mandatory. + MoveMemberToDeleted( $borrowernumber ) unless $permanently; + C4::VirtualShelves::HandleDelBorrower( $borrowernumber ); + my $query = qq|DELETE FROM reserves WHERE borrowernumber=?|; diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm index 38fa0e8..3407f06 100644 --- a/C4/VirtualShelves.pm +++ b/C4/VirtualShelves.pm @@ -45,7 +45,7 @@ BEGIN { &GetBibliosShelves ); @EXPORT_OK = qw( - &GetAllShelves &ShelvesMax + &GetAllShelves &ShelvesMax &HandleDelBorrower ); } @@ -601,7 +601,7 @@ sub ShelvesMax { HandleDelBorrower($borrower); -When a member is deleted (DelMember in Members.pm), you should call me first. +When a member is deleted, this subroutine is called. This routine deletes/moves lists and entries for the deleted member/borrower. You could just delete everything (and lose more than you want), but instead we now try to save all public/shared stuff and keep others happy. diff --git a/members/deletemem.pl b/members/deletemem.pl index ef6ba01..44ea8e3 100755 --- a/members/deletemem.pl +++ b/members/deletemem.pl @@ -118,9 +118,7 @@ if ($countissues > 0 or $flags->{'CHARGES'} or $data->{'borrowernumber'}){ output_html_with_http_headers $input, $cookie, $template->output; } else { - MoveMemberToDeleted($member); - C4::VirtualShelves::HandleDelBorrower($member); - DelMember($member); + DelMember({ borrowernumber => $member }); print $input->redirect("/cgi-bin/koha/members/members-home.pl"); } diff --git a/misc/cronjobs/delete_expired_opac_registrations.pl b/misc/cronjobs/delete_expired_opac_registrations.pl index c7c3375..55787da 100755 --- a/misc/cronjobs/delete_expired_opac_registrations.pl +++ b/misc/cronjobs/delete_expired_opac_registrations.pl @@ -76,5 +76,5 @@ my $sth = $dbh->prepare($query); $sth->execute( $category_code, $delay ); while ( my ($borrowernumber) = $sth->fetchrow_array() ) { - DelMember($borrowernumber); + DelMember({ borrowernumber => $borrowernumber, permanently => 1 }); } diff --git a/misc/cronjobs/delete_unverified_opac_registrations.pl b/misc/cronjobs/delete_unverified_opac_registrations.pl index 6909c70..f5d87e3 100755 --- a/misc/cronjobs/delete_unverified_opac_registrations.pl +++ b/misc/cronjobs/delete_unverified_opac_registrations.pl @@ -29,7 +29,6 @@ BEGIN { } use C4::Context; -use C4::Members qw/ DelMember /; my $help; my $confirm; diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 634ac0c..122a77e 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -171,7 +171,7 @@ is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when # clean up -DelMember($member->{borrowernumber}); +DelMember({ borrowernumber => $member->{borrowernumber}, permanently => 1 }); $results = Search($CARDNUMBER,undef,undef,undef,["cardnumber"]); ok (!_find_member($results), "Delete member") or diag("Card $CARDNUMBER found for the deleted member in the resultset: ".Dumper($results)); diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl index 0d48b99..84f4367 100755 --- a/tools/cleanborrowers.pl +++ b/tools/cleanborrowers.pl @@ -126,15 +126,12 @@ if ( $params->{'step3'} ) { if ( $radio eq 'trash' ) { my $i; for ( $i = 0 ; $i < $totalDel ; $i++ ) { - MoveMemberToDeleted( $membersToDelete->[$i]->{'borrowernumber'} ); - C4::VirtualShelves::HandleDelBorrower( $membersToDelete->[$i]->{'borrowernumber'} ); - DelMember( $membersToDelete->[$i]->{'borrowernumber'} ); + DelMember({ borrowernumber => $membersToDelete->[$i]->{'borrowernumber'} }); } } else { # delete completly. my $i; for ( $i = 0 ; $i < $totalDel ; $i++ ) { - C4::VirtualShelves::HandleDelBorrower( $membersToDelete->[$i]->{'borrowernumber'} ); - DelMember( $membersToDelete->[$i]->{'borrowernumber'} ); + DelMember({ borrowernumber => $membersToDelete->[$i]->{'borrowernumber'}, permanently => 1 }); } } $template->param( diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl index 8717d8e..664d625 100755 --- a/tools/modborrowers.pl +++ b/tools/modborrowers.pl @@ -259,8 +259,7 @@ if ( $op eq 'do' ) { { error => "fees_owed", borrower => $borrower } ); } else { - MoveMemberToDeleted($borrowernumber); - DelMember($borrowernumber); + DelMember({ borrowernumber => $borrowernumber }); push( @deleted_borrowers, $borrower ); } } -- 1.7.2.5