[Koha-patches] [PATCH] Bug 4108 Catch cases when GetMember is generating errors

[Colin Campbell]

Fixed two calls where new interface was not used
Arguably new syntax allows more options than we require but it was not catching
error cases (when 0 or undef is passed). It also can now be called in ways
which would validly return multiple hits but explicitly does not
(nor does calling code expect this)
This patch should quieten the flood of errors
---
 C4/Members.pm       |   46 ++++++++++++++++++++++++++++++++++------------
 C4/SIP/ILS/Item.pm  |    2 +-
 acqui/acqui-home.pl |    8 +++++---
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index f9c2050..9007ece 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -529,22 +529,44 @@ the C<borrowers> table in the Koha database.
 #'
 sub GetMember {
     my ( %information ) = @_;
+    if (exists $information{borrowernumber} && !defined $information{borrowernumber}) {
+        #passing mysql's kohaadmin?? Makes no sense as a query
+        return;
+    }
     my $dbh = C4::Context->dbh;
-    my $sth;
-    my $select = "
-SELECT borrowers.*, categories.category_type, categories.description
-FROM borrowers 
-LEFT JOIN categories on borrowers.categorycode=categories.categorycode 
-";
-    $select.=" WHERE ".join(" AND ",map {"$_ = ?"}keys %information);
-    $select=~s/AND $//;
+    my $select =
+    q{SELECT borrowers.*, categories.category_type, categories.description
+    FROM borrowers 
+    LEFT JOIN categories on borrowers.categorycode=categories.categorycode WHERE };
+    my $more_p = 0;
+    my @values = ();
+    for (keys %information ) {
+        if ($more_p) {
+            $select .= ' AND ';
+        }
+        else {
+            $more_p++;
+        }
+
+        if (defined $information{$_}) {
+            $select .= "$_ = ?";
+            push @values, $information{$_};
+        }
+        else {
+            $select .= "$_ IS NULL";
+        }
+    }
     $debug && warn $select, " ",values %information;
-    $sth = $dbh->prepare("$select");
+    my $sth = $dbh->prepare("$select");
     $sth->execute(map{$information{$_}} keys %information);
     my $data = $sth->fetchall_arrayref({});
-    return undef if (scalar(@$data)==0);        
-    if (scalar(@$data)==1) {return $$data[0];}
-    ($data) and return $data;
+    #FIXME interface to this routine now allows generation of a result set
+    #so whole array should be returned but bowhere in the current code expects this
+    if (@{$data} ) {
+        return $data->[0];
+    }
+
+    return;
 }
 
 
diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm
index fa88699..4ddb344 100644
--- a/C4/SIP/ILS/Item.pm
+++ b/C4/SIP/ILS/Item.pm
@@ -188,7 +188,7 @@ sub hold_patron_name {
 sub hold_patron_bcode {
     my $self = shift or return;
     my $borrowernumber = (@_ ? shift: $self->hold_patron_id()) or return;
-    my $holder = GetMember($borrowernumber, 'borrowernumber');
+    my $holder = GetMember(borrowernumber => $borrowernumber);
     if ($holder) {
         if ($holder->{cardnumber}) {
             return $holder->{cardnumber};
diff --git a/acqui/acqui-home.pl b/acqui/acqui-home.pl
index 63e913a..f8541d1 100755
--- a/acqui/acqui-home.pl
+++ b/acqui/acqui-home.pl
@@ -67,6 +67,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 );
 
 # budget
+warn("LOGGED=$loggedinuser");
 my $borrower= GetMember('borrowernumber' => $loggedinuser);
 my ( $flags, $homebranch )= ($borrower->{'flags'},$borrower->{'branchcode'});
 
@@ -147,7 +148,7 @@ foreach my $result (@results) {
     my $r = GetBranchName( $result->{'budget_owner_id'} );
     $result->{'budget_branchname'} = GetBranchName( $result->{'budget_branchcode'} );
 
-    my $member      = GetMember( $result->{'budget_owner_id'} );
+    my $member      = GetMember( borrowernumber => $result->{budget_owner_id} );
     my $member_full = $member->{'firstname'} . ' ' . $member->{'surname'} if $member;
 
     $result->{'budget_owner'}   = $member_full;
@@ -168,8 +169,9 @@ foreach my $result (@results) {
     #        my $spent_percent = ( $result->{'budget_spent'} / $result->{'budget_amount'} ) * 100;
     #        $result->{'budget_spent_percent'} = sprintf( "%00d", $spent_percent );
 
-    my $borrower = &GetMember( $result->{budget_owner_id} );
-    $result->{budget_owner_name} = $borrower->{'firstname'} . ' ' . $borrower->{'surname'} if $borrower;
+    if ($member) {
+        $result->{budget_owner_name} = $member->{'firstname'} . ' ' . $member->{'surname'};
+    }
 
     push( @loop_budget, { %{$result}, toggle => $toggle++ % 2, } );
 }
-- 
1.6.6