Attachment 5632 Details for Bug 6253 – [Signed Off] bug_6253: Unified member Search()

[patch] [Signed Off] bug_6253: Unified member Search()

Signed-Off-bug6253-Unified-member-Search.patch (text/plain), 35.95 KB, created by Liz Rea on 2011-09-28 18:23:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Liz Rea

Created: 2011-09-28 18:23:28 UTC

Size: 35.95 KB

patch

obsolete

>From 9ca1642ff1fa2353251b7896a1dd7fa07e273728 Mon Sep 17 00:00:00 2001
>From: Srdjan Jankovic <srdjan@catalyst.net.nz>
>Date: Thu, 22 Sep 2011 09:43:12 -0500
>Subject: [PATCH] [Signed Off] bug_6253: Unified member Search()
>Content-Type: text/plain; charset="utf-8"
>
>Removed SearchMembers() and replaced with more generic Search()
>Amended Search() to try cardnumber first
>Replaced SearchMembers() calls with Search()
>Replaced SELECT with Search() where appropriate
>C4::SQLHelper:
>- added support for '' key for search filter.
>- when passing an array to filter, join with OR (rather than AND)
>- added support for key => [val1, val2] in filter
>- did not document - there was no input documentation to start with,
>  and SQLHelper should be replaced with something better anyway
>
>Signed-off-by: Liz Rea <lrea@nekls.org>
>(again - testing merge issue)
>The functionality of the patch seems to be maintained with Biblibre's changes.
>
>I tested the following:
>Extended attribute searching: works
>3 part name searching: works
>2 part name searching: works
>1 part name searching: works
>
>From:
>mainpage.pl
>members-home.pl
>	Patron search limited by branch: Works
>	Patron search limited by patron category: works
>	Ordering by cardnumber instead of surname: works
>The "Check Out" field in the masthead.
>
>Circ Autocomplete is not reliably functional at this time, but the problem appears to predate this patch.
>---
> C4/Members.pm                          |  256 ++++++++++++++------------------
> C4/Members/Attributes.pm               |   13 +-
> C4/SQLHelper.pm                        |   77 ++++++++--
> admin/aqbudget_owner_search.pl         |    2 +-
> circ/circulation.pl                    |    2 +-
> circ/ysearch.pl                        |   29 +---
> members/guarantor_search.pl            |   11 +-
> members/member.pl                      |   19 +--
> patroncards/members-search.pl          |   14 +--
> reserve/request.pl                     |   22 ++--
> t/db_dependent/Koha.t                  |   14 ++-
> t/db_dependent/Members.t               |  181 +++++++++++++++++------
> t/db_dependent/lib/KohaTest/Members.pm |    2 +-
> 13 files changed, 358 insertions(+), 284 deletions(-)
>
>diff --git a/C4/Members.pm b/C4/Members.pm
>index f009590..40a8e51 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -44,7 +44,6 @@ BEGIN {
> 	#Get data
> 	push @EXPORT, qw(
> 		&Search
>-		&SearchMember 
> 		&GetMemberDetails
>         &GetMemberRelatives
> 		&GetMember
>@@ -141,178 +140,139 @@ This module contains routines for adding, modifying and deleting members/patrons
> 
> =head1 FUNCTIONS
> 
>-=head2 SearchMember
>-
>-  ($count, $borrowers) = &SearchMember($searchstring, $type, 
>-                     $category_type, $filter, $showallbranches);
>+=head2 Search
> 
>-Looks up patrons (borrowers) by name.
>+  $borrowers_result_array_ref = &Search($filter,$orderby, $limit, 
>+                       $columns_out, $search_on_fields,$searchtype);
> 
>-BUGFIX 499: C<$type> is now used to determine type of search.
>-if $type is "simple", search is performed on the first letter of the
>-surname only.
>+Looks up patrons (borrowers) on filter. A wrapper for SearchInTable('borrowers').
> 
>-$category_type is used to get a specified type of user. 
>-(mainly adults when creating a child.)
>+For C<$filter>, C<$orderby>, C<$limit>, C<&columns_out>, C<&search_on_fields> and C<&searchtype>
>+refer to C4::SQLHelper:SearchInTable().
> 
>-C<$searchstring> is a space-separated list of search terms. Each term
>-must match the beginning a borrower's surname, first name, or other
>-name.
>+Special C<$filter> key '' is effectively expanded to search on surname firstname othernamescw
>+and cardnumber unless C<&search_on_fields> is defined
> 
>-C<$filter> is assumed to be a list of elements to filter results on
>+Examples:
> 
>-C<$showallbranches> is used in IndependantBranches Context to display all branches results.
>+  $borrowers = Search('abcd', 'cardnumber');
> 
>-C<&SearchMember> returns a two-element list. C<$borrowers> is a
>-reference-to-array; each element is a reference-to-hash, whose keys
>-are the fields of the C<borrowers> table in the Koha database.
>-C<$count> is the number of elements in C<$borrowers>.
>+  $borrowers = Search({''=>'abcd', category_type=>'I'}, 'surname');
> 
> =cut
> 
>-#'
>-#used by member enquiries from the intranet
>-sub SearchMember {
>-    my ($searchstring, $orderby, $type,$category_type,$filter,$showallbranches ) = @_;
>-    my $dbh   = C4::Context->dbh;
>-    my $query = "";
>-    my $count;
>-    my @data;
>-    my @bind = ();
>-    
>+sub _express_member_find {
>+    my ($filter) = @_;
>+
>     # this is used by circulation everytime a new borrowers cardnumber is scanned
>     # so we can check an exact match first, if that works return, otherwise do the rest
>-    $query = "SELECT * FROM borrowers
>-        LEFT JOIN categories ON borrowers.categorycode=categories.categorycode
>-        ";
>-    my $sth = $dbh->prepare("$query WHERE cardnumber = ?");
>-    $sth->execute($searchstring);
>-    my $data = $sth->fetchall_arrayref({});
>-    if (@$data){
>-        return ( scalar(@$data), $data );
>-    }
>-
>-    if ( $type eq "simple" )    # simple search for one letter only
>-    {
>-        $query .= ($category_type ? " AND category_type = ".$dbh->quote($category_type) : ""); 
>-        $query .= " WHERE (surname LIKE ? OR cardnumber like ?) ";
>-        if (C4::Context->preference("IndependantBranches") && !$showallbranches){
>-          if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){
>-            $query.=" AND borrowers.branchcode =".$dbh->quote(C4::Context->userenv->{'branch'}) unless (C4::Context->userenv->{'branch'} eq "insecure");
>-          }
>-        }
>-        $query.=" ORDER BY $orderby";
>-        @bind = ("$searchstring%","$searchstring");
>+    my $dbh   = C4::Context->dbh;
>+    my $query = "SELECT borrowernumber FROM borrowers WHERE cardnumber = ?";
>+    if ( my $borrowernumber = $dbh->selectrow_array($query, undef, $filter) ) {
>+        return( {"borrowernumber"=>$borrowernumber} );
>     }
>-    else    # advanced search looking in surname, firstname and othernames
>-    {
>-        @data  = split( ' ', $searchstring );
>-        $count = @data;
>-        $query .= " WHERE ";
>-        if (C4::Context->preference("IndependantBranches") && !$showallbranches){
>-          if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){
>-            $query.=" borrowers.branchcode =".$dbh->quote(C4::Context->userenv->{'branch'})." AND " unless (C4::Context->userenv->{'branch'} eq "insecure");
>-          }      
>-        }     
>-        $query.="((surname LIKE ? OR (surname LIKE ? AND surname REGEXP ?)
>-                OR firstname  LIKE ? OR (firstname LIKE ? AND firstname REGEXP ?)
>-                OR othernames LIKE ? OR (othernames LIKE ? AND othernames REGEXP ?))
>-        " .
>-        ($category_type?" AND category_type = ".$dbh->quote($category_type):"");
>-        my $regex = '[[:punct:][:space:]]'.$data[0];
>-        @bind = (
>-            "$data[0]%", "%$data[0]%", $regex, 
>-            "$data[0]%", "%$data[0]%", $regex, 
>-            "$data[0]%", "%$data[0]%", $regex 
>-        );
>-        for ( my $i = 1 ; $i < $count ; $i++ ) {
>-            $query = $query . " AND (" . " surname LIKE ? OR (surname LIKE ? AND surname REGEXP ?)
>-                OR firstname  LIKE ? OR (firstname LIKE ? AND firstname REGEXP ?)
>-                OR othernames LIKE ? OR (othernames LIKE ? AND othernames REGEXP ?))";
>-            $regex = '[[:punct:][:space:]]'.$data[$i];
>-            push( @bind,
>-              "$data[$i]%", "%$data[$i]%", $regex,
>-              "$data[$i]%", "%$data[$i]%", $regex,
>-              "$data[$i]%", "%$data[$i]%", $regex
>-            );
>-
>-
>-            # FIXME - .= <<EOT;
>-        }
>-        $query = $query . ") OR cardnumber LIKE ? ";
>-        push( @bind, $searchstring );
>-        $query .= "order by $orderby";
> 
>-        # FIXME - .= <<EOT;
>+    my ($search_on_fields, $searchtype);
>+    if ( length($filter) == 1 ) {
>+        $search_on_fields = [ qw(surname) ];
>+        $searchtype = 'start_with';
>+    } else {
>+        $search_on_fields = [ qw(surname firstname othernames cardnumber) ];
>+        $searchtype = 'contain';
>     }
> 
>-    $sth = $dbh->prepare($query);
>-
>-    $debug and print STDERR "Q $orderby : $query\n";
>-    $sth->execute(@bind);
>-    my @results;
>-    $data = $sth->fetchall_arrayref({});
>-
>-    return ( scalar(@$data), $data );
>+    return (undef, $search_on_fields, $searchtype);
> }
> 
>-=head2 Search
>-
>-  $borrowers_result_array_ref = &Search($filter,$orderby, $limit, 
>-                       $columns_out, $search_on_fields,$searchtype);
>-
>-Looks up patrons (borrowers) on filter.
>-
>-BUGFIX 499: C<$type> is now used to determine type of search.
>-if $type is "simple", search is performed on the first letter of the
>-surname only.
>-
>-$category_type is used to get a specified type of user. 
>-(mainly adults when creating a child.)
>-
>-C<$filter> can be
>-   - a space-separated list of search terms. Implicit AND is done on them
>-   - a hash ref containing fieldnames associated with queried value
>-   - an array ref combining the two previous elements Implicit OR is done between each array element
>-
>-
>-C<$orderby> is an arrayref of hashref. Contains the name of the field and 0 or 1 depending if order is ascending or descending
>-
>-C<$limit> is there to allow limiting number of results returned
>-
>-C<&columns_out> is an array ref to the fieldnames you want to see in the result list
>-
>-C<&search_on_fields> is an array ref to the fieldnames you want to limit search on when you are using string search
>-
>-C<&searchtype> is a string telling the type of search you want todo : start_with, exact or contains are allowed
>-
>-=cut
>-
> sub Search {
>     my ( $filter, $orderby, $limit, $columns_out, $search_on_fields, $searchtype ) = @_;
>-    my @filters;
>-    my %filtersmatching_record;
>-    my @finalfilter;
>-    if ( ref($filter) eq "ARRAY" ) {
>-        push @filters, @$filter;
>-    } else {
>-        push @filters, $filter;
>+
>+    my $search_string;
>+    my $found_borrower;
>+
>+    if ( my $fr = ref $filter ) {
>+        if ( $fr eq "HASH" ) {
>+            if ( my $search_string = $filter->{''} ) {
>+                my ($member_filter, $member_search_on_fields, $member_searchtype) = _express_member_find($search_string);
>+                if ($member_filter) {
>+                    $filter = $member_filter;
>+                    $found_borrower = 1;
>+                } else {
>+                    $search_on_fields ||= $member_search_on_fields;
>+                    $searchtype ||= $member_searchtype;
>+                }
>+            }
>+        }
>+        else {
>+            $search_string = $filter;
>+        }
>+    }
>+    else {
>+        $search_string = $filter;
>+        my ($member_filter, $member_search_on_fields, $member_searchtype) = _express_member_find($search_string);
>+        if ($member_filter) {
>+            $filter = $member_filter;
>+            $found_borrower = 1;
>+        } else {
>+            $search_on_fields ||= $member_search_on_fields;
>+            $searchtype ||= $member_searchtype;
>+        }
>     }
>-    if ( C4::Context->preference('ExtendedPatronAttributes') ) {
>-        my $matching_records = C4::Members::Attributes::SearchIdMatchingAttribute($filter);
>+
>+    if ( !$found_borrower && C4::Context->preference('ExtendedPatronAttributes') && $search_string ) {
>+        my $matching_records = C4::Members::Attributes::SearchIdMatchingAttribute($search_string);
>         if(scalar(@$matching_records)>0) {
>-			foreach my $matching_record (@$matching_records) {
>-				$filtersmatching_record{$$matching_record[0]}=1;
>-			}
>-			foreach my $k (keys(%filtersmatching_record)) {
>-				push @filters, {"borrowernumber"=>$k};
>-			}
>+            if ( my $fr = ref $filter ) {
>+                if ( $fr eq "HASH" ) {
>+                    my %f = %$filter;
>+                    $filter = [ $filter ];
>+                    delete $f{''};
>+                    push @$filter, { %f, "borrowernumber"=>$$matching_records };
>+                }
>+                else {
>+                    push @$filter, {"borrowernumber"=>$matching_records};
>+                }
>+            }
>+            else {
>+                $filter = [ $filter ];
>+                push @$filter, {"borrowernumber"=>$matching_records};
>+            }
> 		}
>     }
>+
>+    # $showallbranches was not used at the time SearchMember() was mainstreamed into Search().
>+    # Mentioning for the reference
>+
>+    if ( C4::Context->preference("IndependantBranches") ) { # && !$showallbranches){
>+        if ( my $userenv = C4::Context->userenv ) {
>+            my $branch =  $userenv->{'branch'};
>+            if ( ($userenv->{flags} % 2 !=1) &&
>+                 $branch && $branch ne "insecure" ){
>+
>+                if (my $fr = ref $filter) {
>+                    if ( $fr eq "HASH" ) {
>+                        $filter->{branchcode} = $branch;
>+                    }
>+                    else {
>+                        foreach (@$filter) {
>+                            $_ = { '' => $_ } unless ref $_;
>+                            $_->{branchcode} = $branch;
>+                        }
>+                    }
>+                }
>+                else {
>+                    $filter = { '' => $filter, branchcode => $branch };
>+                }
>+            }      
>+        }
>+    }
>+
>+    if ($found_borrower) {
>+        $searchtype = "exact";
>+    }
>     $searchtype ||= "start_with";
>-	push @finalfilter, \@filters;
>-	my $data = SearchInTable( "borrowers", \@finalfilter, $orderby, $limit, $columns_out, $search_on_fields, $searchtype );
>-    return ($data);
>+
>+	return SearchInTable( "borrowers", $filter, $orderby, $limit, $columns_out, $search_on_fields, $searchtype );
> }
> 
> =head2 GetMemberDetails
>diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm
>index 29a31e6..4ae5600 100644
>--- a/C4/Members/Attributes.pm
>+++ b/C4/Members/Attributes.pm
>@@ -119,23 +119,24 @@ sub GetBorrowerAttributeValue {
> 
> =head2 SearchIdMatchingAttribute
> 
>-  my $matching_records = C4::Members::Attributes::SearchIdMatchingAttribute($filter);
>+  my $matching_borrowernumbers = C4::Members::Attributes::SearchIdMatchingAttribute($filter);
> 
> =cut
> 
> sub SearchIdMatchingAttribute{
>     my $filter = shift;
>-    my $finalfilter=$filter->[0];
>+    $filter = [$filter] unless ref $filter;
>+
>     my $dbh   = C4::Context->dbh();
>     my $query = qq{
>-SELECT borrowernumber
>+SELECT DISTINCT borrowernumber
> FROM borrower_attributes
> JOIN borrower_attribute_types USING (code)
> WHERE staff_searchable = 1
>-AND attribute like ?};
>+AND (} . join (" OR ", map "attribute like ?", @$filter) .qq{)};
>     my $sth = $dbh->prepare_cached($query);
>-    $sth->execute("%$finalfilter%");
>-    return $sth->fetchall_arrayref;
>+    $sth->execute(map "%$_%", @$filter);
>+    return [map $_->[0], @{ $sth->fetchall_arrayref }];
> }
> 
> =head2 CheckUniqueness
>diff --git a/C4/SQLHelper.pm b/C4/SQLHelper.pm
>index a15beb8..720dc99 100644
>--- a/C4/SQLHelper.pm
>+++ b/C4/SQLHelper.pm
>@@ -85,6 +85,21 @@ $filtercolums is an array ref on field names : is used to limit expansion of res
> 
> $searchtype is string Can be "start_with" or "exact" 
> 
>+This query builder is very limited, it should be replaced with DBIx::Class
>+or similar  very soon
>+Meanwhile adding support for special key '' in case of a data_hashref to
>+support filters of type
>+
>+  ( f1 = a OR f2 = a ) AND fx = b AND fy = c
>+
>+Call for the query above is:
>+
>+  SearchInTable($tablename, {'' => a, fx => b, fy => c}, $orderby, $limit,
>+                $columns_out, [f1, f2], 'exact');
>+
>+NOTE: Current implementation may remove parts of the iinput hashrefs. If that is a problem
>+a copy needs to be created in _filter_fields() below
>+
> =cut
> 
> sub SearchInTable{
>@@ -101,7 +116,7 @@ sub SearchInTable{
> 	if ($keys){
> 		my @criteria=grep{defined($_) && $_ !~/^\W$/ }@$keys;
> 		if (@criteria) { 
>-			$sql.= do { local $"=') AND ('; 
>+			$sql.= do { local $"=') OR ('; 
> 					qq{ WHERE (@criteria) } 
> 				   }; 
> 		} 
>@@ -109,8 +124,12 @@ sub SearchInTable{
>     if ($orderby){ 
> 		#Order by desc by default
> 		my @orders;
>-		foreach my $order (@$orderby){
>-			push @orders,map{ "$_".($order->{$_}? " DESC " : "") } keys %$order; 
>+		foreach my $order ( ref($orderby) ? @$orderby : $orderby ){
>+            if (ref $order) {
>+			    push @orders,map{ "$_".($order->{$_}? " DESC " : "") } keys %$order; 
>+            } else {
>+			    push @orders,$order; 
>+            }
> 		}
> 		$sql.= do { local $"=', '; 
> 				qq{ ORDER BY @orders} 
>@@ -287,13 +306,21 @@ sub _filter_fields{
>     my @keys; 
> 	my @values;
> 	if (ref($filter_input) eq "HASH"){
>-		my ($keys, $values) = _filter_hash($tablename,$filter_input, $searchtype);
>+		my ($keys, $values);
>+        if (my $special = delete $filter_input->{''}) { # XXX destroyes '' key
>+		    ($keys, $values) = _filter_fields($tablename,$special, $searchtype,$filtercolumns);
>+        }
>+		my ($hkeys, $hvalues) = _filter_hash($tablename,$filter_input, $searchtype);
>+		if ($hkeys){
>+            push @$keys, @$hkeys;
>+            push @$values, @$hvalues;
>+        }
> 		if ($keys){
>-		my $stringkey="(".join (") AND (",@$keys).")";
>-		return [$stringkey],$values;
>+		    my $stringkey="(".join (") AND (",@$keys).")";
>+		    return [$stringkey],$values;
> 		}
> 		else {
>-		return ();
>+		    return ();
> 		}
> 	} elsif (ref($filter_input) eq "ARRAY"){
> 		foreach my $element_data (@$filter_input){
>@@ -334,7 +361,9 @@ sub _filter_hash{
>     my $elements=join "|",@columns_filtered;
> 	foreach my $field (grep {/\b($elements)\b/} keys %$filter_input){
> 		## supposed to be a hash of simple values, hashes of arrays could be implemented
>-		$filter_input->{$field}=format_date_in_iso($filter_input->{$field}) if ($columns->{$field}{Type}=~/date/ && $filter_input->{$field} !~C4::Dates->regexp("iso"));
>+		$filter_input->{$field}=format_date_in_iso($filter_input->{$field})
>+          if $columns->{$field}{Type}=~/date/ &&
>+             $filter_input->{$field} && $filter_input->{$field} !~C4::Dates->regexp("iso");
> 		my ($tmpkeys, $localvalues)=_Process_Operands($filter_input->{$field},"$tablename.$field",$searchtype,$columns);
> 		if (@$tmpkeys){
> 			push @values, @$localvalues;
>@@ -352,7 +381,11 @@ sub _filter_hash{
> sub _filter_string{
> 	my ($tablename,$filter_input, $searchtype,$filtercolumns)=@_;
> 	return () unless($filter_input);
>-	my @operands=split / /,$filter_input;
>+	my @operands=split /\s+/,$filter_input;
>+
>+    # An act of desperation
>+    $searchtype = 'contain' if @operands > 1 && $searchtype =~ /start_with/o;
>+
> 	my @columns_filtered= _filter_columns($tablename,$searchtype,$filtercolumns);
> 	my $columns= _get_columns($tablename);
> 	my (@values,@keys);
>@@ -381,8 +414,12 @@ sub _Process_Operands{
> 	my @values;
> 	my @tmpkeys;
> 	my @localkeys;
>-	push @tmpkeys, " $field = ? ";
>-	push @values, $operand;
>+
>+    $operand = [$operand] unless ref $operand;
>+    foreach (@$operand) {
>+	    push @tmpkeys, " $field = ? ";
>+	    push @values, $_;
>+    }
> 	#By default, exact search
> 	if (!$searchtype ||$searchtype eq "exact"){
> 		return \@tmpkeys,\@values;
>@@ -394,16 +431,22 @@ sub _Process_Operands{
> 	if ($columns->{$col_field}->{Type}=~/varchar|text/i){
> 		my @localvaluesextended;
> 		if ($searchtype eq "contain"){
>-			push @tmpkeys,(" $field LIKE ? ");
>-			push @localvaluesextended,("\%$operand\%") ;
>+            foreach (@$operand) {
>+			    push @tmpkeys,(" $field LIKE ? ");
>+			    push @localvaluesextended,("\%$_\%") ;
>+            }
> 		}
> 		if ($searchtype eq "field_start_with"){
>-			push @tmpkeys,("$field LIKE ?");
>-			push @localvaluesextended, ("$operand\%") ;
>+            foreach (@$operand) {
>+			    push @tmpkeys,("$field LIKE ?");
>+			    push @localvaluesextended, ("$_\%") ;
>+            }
> 		}
> 		if ($searchtype eq "start_with"){
>-			push @tmpkeys,("$field LIKE ?","$field LIKE ?");
>-			push @localvaluesextended, ("$operand\%", " $operand\%") ;
>+            foreach (@$operand) {
>+			    push @tmpkeys,("$field LIKE ?","$field LIKE ?");
>+			    push @localvaluesextended, ("$_\%", " $_\%") ;
>+            }
> 		}
> 		push @values,@localvaluesextended;
> 	}
>diff --git a/admin/aqbudget_owner_search.pl b/admin/aqbudget_owner_search.pl
>index 69e465f..5c73e9e 100755
>--- a/admin/aqbudget_owner_search.pl
>+++ b/admin/aqbudget_owner_search.pl
>@@ -64,7 +64,7 @@ my @resultsdata;
> my $toggle = 0;
> 
> if ( $member ) {
>-	my $results= SearchMember($member,"surname",undef,undef,undef);
>+	my $results= Search($member,"surname");
> 
>     foreach my $res (@$results) {
> 
>diff --git a/circ/circulation.pl b/circ/circulation.pl
>index 98fee97..bbcccd5 100755
>--- a/circ/circulation.pl
>+++ b/circ/circulation.pl
>@@ -187,7 +187,7 @@ if ( $print eq 'yes' && $borrowernumber ne '' ) {
> my $borrowerslist;
> my $message;
> if ($findborrower) {
>-    my ($count, $borrowers) = SearchMember($findborrower, 'cardnumber', 'web');
>+    my $borrowers = Search($findborrower, 'cardnumber');
>     my @borrowers = @$borrowers;
>     if (C4::Context->preference("AddPatronLists")) {
>         $template->param(
>diff --git a/circ/ysearch.pl b/circ/ysearch.pl
>index ce890f5..beab6a1 100755
>--- a/circ/ysearch.pl
>+++ b/circ/ysearch.pl
>@@ -28,6 +28,7 @@ use strict;
> #use warnings; FIXME - Bug 2505
> use CGI;
> use C4::Context;
>+use C4::Members;
> use C4::Auth qw/check_cookie_auth/;
> 
> my $input   = new CGI;
>@@ -41,23 +42,11 @@ if ($auth_status ne "ok") {
>     exit 0;
> }
> 
>-my $dbh = C4::Context->dbh;
>-my $sql = qq(SELECT surname, firstname, cardnumber, address, city, zipcode, country 
>-             FROM borrowers 
>-             WHERE surname LIKE ?
>-             OR firstname LIKE ?
>-             OR cardnumber LIKE ?
>-             ORDER BY surname, firstname
>-             LIMIT 10);
>-my $sth = $dbh->prepare( $sql );
>-$sth->execute("$query%", "$query%", "$query%");
>-
>-while ( my $rec = $sth->fetchrow_hashref ) {
>-    print $rec->{surname} . ", " . $rec->{firstname} . "\t" .
>-          $rec->{cardnumber} . "\t" .
>-          $rec->{address} . "\t" .
>-          $rec->{city} . "\t" .
>-          $rec->{zip} . "\t" .
>-          $rec->{country} .
>-          "\n";
>-}
>+print map $_->{surname} . ", " . $_->{firstname} . "\t" .
>+          $_->{cardnumber} . "\t" .
>+          $_->{address} . "\t" .
>+          $_->{city} . "\t" .
>+          $_->{zipcode} . "\t" .
>+          $_->{country} .
>+          "\n",
>+          @{ Search($query, [qw(surname firstname cardnumber)], [10], [qw(surname firstname cardnumber address city zipcode country)]) };
>diff --git a/members/guarantor_search.pl b/members/guarantor_search.pl
>index 9eb60de..59ea5b1 100755
>--- a/members/guarantor_search.pl
>+++ b/members/guarantor_search.pl
>@@ -66,14 +66,9 @@ my @resultsdata;
> my $background = 0;
> 
> if ($member ne ''){
>-	if(length($member) == 1)
>-	{
>-		($count,$results)=SearchMember($member,$orderby,"simple",$search_category);
>-	}
>-	else
>-	{
>-		($count,$results)=SearchMember($member,$orderby,"advanced",$search_category);
>-	}
>+    $results = Search({''=>$member, category_type=>$search_category},$orderby);
>+    $count = $results ? @$results : 0;
>+
> 	for (my $i=0; $i < $count; $i++){
> 	#find out stats
> 	my ($od,$issue,$fines)=GetMemberIssuesAndFines($results->[$i]{'borrowerid'});
>diff --git a/members/member.pl b/members/member.pl
>index 724c996..e2a6ef5 100755
>--- a/members/member.pl
>+++ b/members/member.pl
>@@ -75,10 +75,11 @@ foreach my $category (@categories){
> 			 };
> 	$categories_dislay{$$category{categorycode}} = $hash;
> }
>+my $AddPatronLists = C4::Context->preference("AddPatronLists") || '';
> $template->param( 
>-        "AddPatronLists_".C4::Context->preference("AddPatronLists")=> "1",
>+        "AddPatronLists_$AddPatronLists" => "1",
>             );
>-if (C4::Context->preference("AddPatronLists")=~/code/){
>+if ($AddPatronLists=~/code/){
>     $categories[0]->{'first'}=1;
> }  
> 
>@@ -96,17 +97,15 @@ else {
> $member =~ s/,//g;   #remove any commas from search string
> $member =~ s/\*/%/g;
> 
>-my ($count,$results);
>-
>-my @searchpatron;
>-push @searchpatron, $member if ($member);
>-push @searchpatron, $patron if ( keys %$patron );
> my $from = ( $startfrom - 1 ) * $resultsperpage;
> my $to   = $from + $resultsperpage;
> 
>-#($results)=Search(\@searchpatron,{surname=>1,firstname=>1},[$from,$to],undef,["firstname","surname","email","othernames"]  ) if (@searchpatron);
>-my $search_scope = ( $quicksearch ? "field_start_with" : "start_with" );
>-($results) = Search( \@searchpatron, \@orderby, undef, undef, [ "firstname", "surname", "othernames", "cardnumber", "userid" ], $search_scope ) if (@searchpatron);
>+my ($count,$results);
>+if ($member || keys %$patron) {
>+    #($results)=Search($member || $patron,{surname=>1,firstname=>1},[$from,$to],undef,["firstname","surname","email","othernames"]  );
>+    my $search_scope = ( $quicksearch ? "field_start_with" : "start_with" );
>+    ($results) = Search( $member || $patron, \@orderby, undef, undef, [ "firstname", "surname", "othernames", "cardnumber", "userid" ], $search_scope );
>+}
> 
> if ($results) {
> 	for my $field ('categorycode','branchcode'){
>diff --git a/patroncards/members-search.pl b/patroncards/members-search.pl
>index c2abe07..dfa0462 100755
>--- a/patroncards/members-search.pl
>+++ b/patroncards/members-search.pl
>@@ -49,17 +49,9 @@ $member =~ s/,//g;   #remove any commas from search string
> $member =~ s/\*/%/g;
> 
> if ($member || $category) {
>-    my ($count,$results) = 0,0;
>-
>-    if(length($member) == 1)
>-    {
>-        ($count,$results) = SearchMember($member,$orderby,"simple");
>-    }
>-    else
>-    {
>-        ($count,$results) = SearchMember($member,$orderby,"advanced",$category);
>-    }
>-
>+    my $results = $category ? Search({''=>$member, category_type=>$category}, $orderby)
>+                            : Search($member, $orderby);
>+    my $count = $results ? @$results : 0;
> 
>     my @resultsdata = ();
>     my $to = ($count>($startfrom * $resultsperpage)?$startfrom * $resultsperpage:$count);
>diff --git a/reserve/request.pl b/reserve/request.pl
>index b345845..d2ba55a 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -112,20 +112,18 @@ if ( $action eq 'move' ) {
> }
> 
> if ($findborrower) {
>-    my ( $count, $borrowers ) =
>-      SearchMember($findborrower, 'cardnumber', 'web' );
>+    my $borrowers = Search($findborrower, 'cardnumber');
> 
>-    my @borrowers = @$borrowers;
>-
>-    if ( !@borrowers ) {
>+    if ($borrowers && @$borrowers) {
>+        if ( @$borrowers == 1 ) {
>+            $borrowernumber_hold = $borrowers->[0]->{'borrowernumber'};
>+        }
>+        else {
>+            $borrowerslist = $borrowers;
>+        }
>+    } else {
>         $messageborrower = "'$findborrower'";
>     }
>-    elsif ( @borrowers == 1 ) {
>-        $borrowernumber_hold = $borrowers[0]->{'borrowernumber'};
>-    }
>-    else {
>-        $borrowerslist = \@borrowers;
>-    }
> }
> 
> # If we have the borrowernumber because we've performed an action, then we
>diff --git a/t/db_dependent/Koha.t b/t/db_dependent/Koha.t
>index 016525a..378a0c1 100644
>--- a/t/db_dependent/Koha.t
>+++ b/t/db_dependent/Koha.t
>@@ -7,10 +7,11 @@ use strict;
> use warnings;
> use C4::Context;
> 
>-use Test::More tests => 4;
>+use Test::More tests => 8;
> 
> BEGIN {
>     use_ok('C4::Koha');
>+    use_ok('C4::Members');
> }
> 
> my $data = {
>@@ -32,10 +33,19 @@ ok($insert_success, "Insert data in database");
> 
> # Tests
> SKIP: {
>-    skip "INSERT failed", 2 unless $insert_success;
>+    skip "INSERT failed", 5 unless $insert_success;
>     
>     is ( GetAuthorisedValueByCode($data->{category}, $data->{authorised_value}), $data->{lib}, "GetAuthorisedValueByCode" );
>     is ( GetKohaImageurlFromAuthorisedValues($data->{category}, $data->{lib}), $data->{imageurl}, "GetKohaImageurlFromAuthorisedValues" );
>+
>+    my $sortdet=C4::Members::GetSortDetails("lost", "3");
>+    is ($sortdet, "Lost and Paid For", "lost and paid works");
>+
>+    my $sortdet2=C4::Members::GetSortDetails("loc", "child");
>+    is ($sortdet2, "Children's Area", "Child area works");
>+
>+    my $sortdet3=C4::Members::GetSortDetails("withdrawn", "1");
>+    is ($sortdet3, "Withdrawn", "Withdrawn works");
> }
> 
> # Clean up
>diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
>index e28fbae..b09d887 100755
>--- a/t/db_dependent/Members.t
>+++ b/t/db_dependent/Members.t
>@@ -6,62 +6,146 @@
> use strict;
> use warnings;
> 
>-use Test::More tests => 15;
>+use Test::More tests => 20;
>+use Data::Dumper;
> 
> BEGIN {
>         use_ok('C4::Members');
> }
> 
> 
>-# Make a borrower for testing
>-my $data = { cardnumber => 'TESTCARD01',
>-    firstname => 'Marie',
>-    surname => 'Mcknight',
>-    categorycode => 'S',
>-    branchcode => 's'
>-    };
>+my $CARDNUMBER   = 'TESTCARD01';
>+my $FIRSTNAME    = 'Marie';
>+my $SURNAME      = 'Mcknight';
>+my $CATEGORYCODE = 'S';
>+my $BRANCHCODE   = 's';
> 
>-my $addmem=AddMember(%$data);
>+my $CHANGED_FIRSTNAME = "Marry Ann";
>+my $EMAIL             = "Marie\@email.com";
>+my $ETHNICITY         = "German";
>+my $PHONE             = "555-12123";
> 
>+# XXX should be randomised and checked against the database
>+my $IMPOSSIBLE_CARDNUMBER = "XYZZZ999";
> 
>-my $member=GetMemberDetails("","TESTCARD01");
>-is ($member->{firstname}, "Marie", "Got member");
>+my $INDEPENDENT_BRANCHES_PREF = 'IndependantBranches';
> 
>-$member->{firstname}="Claire";
>-ModMember(%$member);
>-my $changedmember=GetMemberDetails("","TESTCARD01");
>-is ($changedmember->{firstname}, "Claire", "Member Changed");
>+# XXX make a non-commit transaction and rollback rather than insert/delete
> 
>-$member->{firstname}="Marie";
>-ModMember(%$member);
>-$changedmember=GetMemberDetails("","TESTCARD01");
>-is ($changedmember->{firstname}, "Marie", "Member Returned");
>+#my ($usernum, $userid, $usercnum, $userfirstname, $usersurname, $userbranch, $branchname, $userflags, $emailaddress, $branchprinter)= @_;
>+my @USERENV = (
>+    1,
>+    'test',
>+    'MASTERTEST',
>+    'Test',
>+    'Test',
>+    't',
>+    'Test',
>+    0,
>+);
>+my $BRANCH_IDX = 5;
> 
>-$member->{email}="Marie\@email.com";
>-ModMember(%$member);
>-$changedmember=GetMemberDetails("","TESTCARD01");
>-is ($changedmember->{email}, "Marie\@email.com", "Email Set works");
>+C4::Context->_new_userenv ('DUMMY_SESSION_ID');
>+C4::Context->set_userenv ( @USERENV );
> 
>-$member->{ethnicity}="German";
>-ModMember(%$member);
>-$changedmember=GetMemberDetails("","TESTCARD01");
>-is ($changedmember->{ethnicity}, "German", "Ethnicity Works");
>-
>-my @searchstring=("Mcknight");
>-my ($results) = Search(\@searchstring,undef,undef,undef,["surname"]);
>-is ($results->[0]->{surname}, "Mcknight", "Surname Search works");
>+my $userenv = C4::Context->userenv
>+  or BAIL_OUT("No userenv");
> 
>-$member->{phone}="555-12123";
>+# Make a borrower for testing
>+my %data = (
>+    cardnumber => $CARDNUMBER,
>+    firstname =>  $FIRSTNAME,
>+    surname => $SURNAME,
>+    categorycode => $CATEGORYCODE,
>+    branchcode => $BRANCHCODE,
>+);
>+
>+my $addmem=AddMember(%data);
>+ok($addmem, "AddMember()");
>+
>+my $member=GetMemberDetails("",$CARDNUMBER)
>+  or BAIL_OUT("Cannot read member with card $CARDNUMBER");
>+
>+ok ( $member->{firstname}    eq $FIRSTNAME    &&
>+     $member->{surname}      eq $SURNAME      &&
>+     $member->{categorycode} eq $CATEGORYCODE &&
>+     $member->{branchcode}   eq $BRANCHCODE
>+     , "Got member")
>+  or diag("Mismatching member details: ".Dumper(\%data, $member));
>+
>+$member->{firstname} = $CHANGED_FIRSTNAME;
>+$member->{email}     = $EMAIL;
>+$member->{ethnicity} = $ETHNICITY;
>+$member->{phone}     = $PHONE;
> ModMember(%$member);
>-
>-@searchstring=("555-12123");
>-($results) = Search(\@searchstring,undef,undef,undef,["phone"]);
>-is ($results->[0]->{phone}, "555-12123", "phone Search works");
>-
>-my $checkcardnum=C4::Members::checkcardnumber("TESTCARD01", "");
>+my $changedmember=GetMemberDetails("",$CARDNUMBER);
>+ok ( $changedmember->{firstname} eq $CHANGED_FIRSTNAME &&
>+     $changedmember->{email}     eq $EMAIL             &&
>+     $changedmember->{ethnicity} eq $ETHNICITY         &&
>+     $changedmember->{phone}     eq $PHONE
>+     , "Member Changed")
>+  or diag("Mismatching member details: ".Dumper($member, $changedmember));
>+
>+C4::Context->set_preference( $INDEPENDENT_BRANCHES_PREF, '0' );
>+C4::Context->clear_syspref_cache();
>+
>+my $results = Search($CARDNUMBER);
>+ok (@$results == 1, "Search cardnumber returned only one result")
>+  or diag("Multiple members with Card $CARDNUMBER: ".Dumper($results));
>+ok (_find_member($results), "Search cardnumber")
>+  or diag("Card $CARDNUMBER not found in the resultset: ".Dumper($results));
>+
>+my @searchstring=($SURNAME);
>+$results = Search(\@searchstring);
>+ok (_find_member($results), "Search (arrayref)")
>+  or diag("Card $CARDNUMBER not found in the resultset: ".Dumper($results));
>+
>+$results = Search(\@searchstring,undef,undef,undef,["surname"]);
>+ok (_find_member($results), "Surname Search (arrayref)")
>+  or diag("Card $CARDNUMBER not found in the resultset: ".Dumper($results));
>+
>+$results = Search("$CHANGED_FIRSTNAME $SURNAME", "surname");
>+ok (_find_member($results), "Full name  Search (string)")
>+  or diag("Card $CARDNUMBER not found in the resultset: ".Dumper($results));
>+
>+@searchstring=($PHONE);
>+$results = Search(\@searchstring,undef,undef,undef,["phone"]);
>+ok (_find_member($results), "Phone Search (arrayref)")
>+  or diag("Card $CARDNUMBER not found in the resultset: ".Dumper($results));
>+
>+$results = Search($PHONE,undef,undef,undef,["phone"]);
>+ok (_find_member($results), "Phone Search (string)")
>+  or diag("Card $CARDNUMBER not found in the resultset: ".Dumper($results));
>+
>+C4::Context->set_preference( $INDEPENDENT_BRANCHES_PREF, '1' );
>+C4::Context->clear_syspref_cache();
>+
>+$results = Search("$CHANGED_FIRSTNAME $SURNAME", "surname");
>+ok (!_find_member($results), "Full name  Search (string) for independent branches, different branch")
>+  or diag("Card $CARDNUMBER found in the resultset for independent branches: ".Dumper(C4::Context->preference($INDEPENDENT_BRANCHES_PREF), $results));
>+
>+@searchstring=($SURNAME);
>+$results = Search(\@searchstring);
>+ok (!_find_member($results), "Search (arrayref) for independent branches, different branch")
>+  or diag("Card $CARDNUMBER found in the resultset for independent branches: ".Dumper(C4::Context->preference($INDEPENDENT_BRANCHES_PREF), $results));
>+
>+$USERENV[$BRANCH_IDX] = $BRANCHCODE;
>+C4::Context->set_userenv ( @USERENV );
>+
>+$results = Search("$CHANGED_FIRSTNAME $SURNAME", "surname");
>+ok (_find_member($results), "Full name  Search (string) for independent branches, same branch")
>+  or diag("Card $CARDNUMBER not found in the resultset for independent branches: ".Dumper(C4::Context->preference($INDEPENDENT_BRANCHES_PREF), $results));
>+
>+@searchstring=($SURNAME);
>+$results = Search(\@searchstring);
>+ok (_find_member($results), "Search (arrayref) for independent branches, same branch")
>+  or diag("Card $CARDNUMBER not found in the resultset for independent branches: ".Dumper(C4::Context->preference($INDEPENDENT_BRANCHES_PREF), $results));
>+
>+
>+my $checkcardnum=C4::Members::checkcardnumber($CARDNUMBER, "");
> is ($checkcardnum, "1", "Card No. in use");
> 
>-$checkcardnum=C4::Members::checkcardnumber("67", "");
>+$checkcardnum=C4::Members::checkcardnumber($IMPOSSIBLE_CARDNUMBER, "");
> is ($checkcardnum, "0", "Card No. not used");
> 
> my $age=GetAge("1992-08-14", "2011-01-19");
>@@ -70,14 +154,17 @@ is ($age, "18", "Age correct");
> $age=GetAge("2011-01-19", "1992-01-19");
> is ($age, "-19", "Birthday In the Future");
> 
>-my $sortdet=C4::Members::GetSortDetails("lost", "3");
>-is ($sortdet, "Lost and Paid For", "lost and paid works");
>+# clean up 
>+DelMember($member->{borrowernumber});
>+$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));
> 
>-my $sortdet2=C4::Members::GetSortDetails("loc", "child");
>-is ($sortdet2, "Children's Area", "Child area works");
> 
>-my $sortdet3=C4::Members::GetSortDetails("withdrawn", "1");
>-is ($sortdet3, "Withdrawn", "Withdrawn works");
>+exit;
> 
>-# clean up 
>-DelMember($member->{borrowernumber});
>+sub _find_member {
>+    my ($resultset) = @_;
>+    my $found = $resultset && grep( { $_->{cardnumber} && $_->{cardnumber} eq $CARDNUMBER } @$resultset );
>+    return $found;
>+}
>diff --git a/t/db_dependent/lib/KohaTest/Members.pm b/t/db_dependent/lib/KohaTest/Members.pm
>index ff18869..5646be1 100644
>--- a/t/db_dependent/lib/KohaTest/Members.pm
>+++ b/t/db_dependent/lib/KohaTest/Members.pm
>@@ -12,7 +12,7 @@ sub testing_class { 'C4::Members' };
> 
> sub methods : Test( 1 ) {
>     my $self = shift;
>-    my @methods = qw( SearchMember 
>+    my @methods = qw( Search
>                       GetMemberDetails 
>                       patronflags 
>                       GetMember 
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 6253: 5025 | 5031 | 5490 | 5529 | 5540 | 5624 | 5632 | 5986