From e321fc9184c86fe809376a3a97ff37698d12acda Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Thu, 4 Feb 2010 18:22:48 +0000
Subject: [PATCH] Bug 4108 Catch cases when GetMember is generating errors

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

Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
---
 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 f9c2050d55..9007ece8f7 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 fa88699ae1..4ddb344be1 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 63e913afb7..f8541d1d88 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, } );
 }
-- 
2.39.5