[Koha-patches] [PATCH] bug_6253: Unified member Search()
Srdjan Jankovic
srdjan at catalyst.net.nz
Thu Sep 22 06:37:40 CEST 2011
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
---
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 | 28 +---
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(+), 283 deletions(-)
diff --git a/C4/Members.pm b/C4/Members.pm
index 8d4a6c5..0b56e6f 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 35d6702..b61d7f1 100644
--- a/C4/Members/Attributes.pm
+++ b/C4/Members/Attributes.pm
@@ -118,23 +118,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 "|", at 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, at 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, at 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 fff32a2..d2bd78c 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -186,7 +186,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 f8fc52a..7e458aa 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,22 +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);
-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)], [20], [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);
-
- at 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));
+
+ at 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));
+
+ at 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));
+
+ at 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.6.5
More information about the Koha-patches
mailing list