From be6021561f2cc5b998dd866a644e9511fef61147 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 11 Jul 2016 23:07:34 +0100 Subject: [PATCH] Bug 16909: Koha::Patrons - Remove checkuniquemember MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C4::Members::checkuniquemember was not really nicely written, was only used once and was not covered by tests. I think it does not make sense to keep such complexity and have this code in the subroutine/method. Looking at this patch it seems that what this subroutine did can be done easily in the pl script in few lines. Test plan: 1/ Create 2 organisations with the same "surname": you should get a warning. 2/ Create 2 patrons (non-organisation) with the same surname/firstname/date of birth, you should get a warning Signed-off-by: Marc VĂ©ron --- C4/Members.pm | 39 --------------------------------------- members/memberentry.pl | 22 +++++++++------------- 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index f9b3942..4c55050 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -1175,45 +1175,6 @@ sub GetBorNotifyAcctRecord { return ( $total, \@acctlines, $numlines ); } -=head2 checkuniquemember (OUEST-PROVENCE) - - ($result,$categorycode) = &checkuniquemember($collectivity,$surname,$firstname,$dateofbirth); - -Checks that a member exists or not in the database. - -C<&result> is nonzero (=exist) or 0 (=does not exist) -C<&categorycode> is from categorycode table -C<&collectivity> is 1 (= we add a collectivity) or 0 (= we add a physical member) -C<&surname> is the surname -C<&firstname> is the firstname (only if collectivity=0) -C<&dateofbirth> is the date of birth in ISO format (only if collectivity=0) - -=cut - -# FIXME: This function is not legitimate. Multiple patrons might have the same first/last name and birthdate. -# This is especially true since first name is not even a required field. - -sub checkuniquemember { - my ( $collectivity, $surname, $firstname, $dateofbirth ) = @_; - my $dbh = C4::Context->dbh; - my $request = ($collectivity) ? - "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? " : - ($dateofbirth) ? - "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=? and dateofbirth=?" : - "SELECT borrowernumber,categorycode FROM borrowers WHERE surname=? and firstname=?"; - my $sth = $dbh->prepare($request); - if ($collectivity) { - $sth->execute( uc($surname) ); - } elsif($dateofbirth){ - $sth->execute( uc($surname), ucfirst($firstname), $dateofbirth ); - }else{ - $sth->execute( uc($surname), ucfirst($firstname)); - } - my @data = $sth->fetchrow; - ( $data[0] ) and return $data[0], $data[1]; - return 0; -} - sub checkcardnumber { my ( $cardnumber, $borrowernumber ) = @_; diff --git a/members/memberentry.pl b/members/memberentry.pl index aea21c3..fc02647 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -220,22 +220,18 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' ) } } -#############test for member being unique ############# +# Test unicity of surname, firstname and dateofbirth if ( ( $op eq 'insert' ) and !$nodouble ) { - my $category_type_send; - if ( $category_type eq 'I' ) { - $category_type_send = $category_type; + my $conditions; + $conditions->{surname} = $newdata{surname} if $newdata{surname}; + if ( $category_type ne 'I' ) { + $conditions->{firstname} = $newdata{firstname} if $newdata{firstname}; + $conditions->{dateofbirth} = $newdata{dateofbirth} if $newdata{dateofbirth}; } - my $check_category; # recover the category code of the doublon suspect borrowers - # ($result,$categorycode) = checkuniquemember($collectivity,$surname,$firstname,$dateofbirth) - ( $check_member, $check_category ) = checkuniquemember( - $category_type_send, - ( $newdata{surname} ? $newdata{surname} : $data{surname} ), - ( $newdata{firstname} ? $newdata{firstname} : $data{firstname} ), - ( $newdata{dateofbirth} ? $newdata{dateofbirth} : $data{dateofbirth} ) - ); - if ( !$check_member ) { + my $patrons = Koha::Patrons->search($conditions); + if ( $patrons->count > 0) { $nodouble = 1; + $check_member = $patrons->next->borrowernumber; } } -- 1.7.10.4