Bug 15252 - Patron search on start with does not work with several terms

When searching a patron, search type can be 'start with' and 'contain'.
If the search text contains a space (or a coma), this text is splitted into several terms.

Actually, the search on 'start with' with several terms never returns a result.

It is because the search composes an "AND" SQL query on terms.
For example (I display only the surname part) :
search type = contain :
  'jean paul' => surname like '%jean% AND %paul%'
search type = start with :
  'jean paul' => surname like 'jean% AND paul%'
The query for 'start with' is impossible.

I propose, for search with start with, to not split terms :
  jean paul => surname like 'jean paul%'

One can always use '*' to add more truncation :
  jea* pau* => surname like 'jea% pau%'

This bug affects a lot surnames with several terms like 'LE GUELEC' or 'MAC BETH'.

Note that the patch moves :
  $searchmember =~ s/,/ /g;
It removes the test "if $searchmember" because $searchmember is tested and set to empty string previously :
    unless ( $searchmember ) {
        $searchmember = $dt_params->{sSearch} // '';
    }

Test plan :
==========
- Create two patrons with firstname "Jean Paul"
- Go to Patrons module
- Choose "Starts with" in "Search type" filter
- Perform a search on "Jean Paul"
=> without patch : you get no result
=> with this patch : you get the two results
- Check you get the two results for search on "Jean Pau"
- Check you get the two results for search on "Jea* Pau*"
- Check you do not get results for search on "Jea Paul"
- Choose "Contains" in "Search type" filter
- Check you get the two results for search on "Jean Paul"
- Check you get the two results for search on "Jean Pau"
- Check you get the two results for search on "Jea* Pau*"
- Check you get the two results for search on "Jea Paul"
- Check you get the two results for search on "Paul Jean"

Signed-off-by: Alex <alexklbuckley@gmail.com>
Tested 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Bug 15252 - Patron search on start with does not work with several terms - followup 1

'start_with' is the default value of $searchtype, it can be explicit.

Tested 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Bug 15252 - correct UT searchtype value is contain and not contains

Tested 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm
index bfa73e0..1ebecd7 100644 (file)
--- a/C4/Utils/DataTables/Members.pm
+++ b/C4/Utils/DataTables/Members.pm
@@ -13,7 +13,7 @@ sub search {
     my $firstletter = $params->{firstletter};
     my $categorycode = $params->{categorycode};
     my $branchcode = $params->{branchcode};
-    my $searchtype = $params->{searchtype};
+    my $searchtype = $params->{searchtype} || 'start_with';
     my $searchfieldstype = $params->{searchfieldstype} || 'standard';
     my $dt_params = $params->{dt_params};
 
@@ -59,8 +59,6 @@ sub search {
         push @where_args, $branchcode;
     }
 
-    # split on coma
-    $searchmember =~ s/,/ /g if $searchmember;
     my $searchfields = {
         standard => 'surname,firstname,othernames,cardnumber,userid',
         email => 'email,emailpro,B_email',
@@ -72,14 +70,28 @@ sub search {
         sort1 => 'sort1',
         sort2 => 'sort2',
     };
-    foreach my $term ( split / /, $searchmember) {
+
+    # * is replaced with % for sql
+    $searchmember =~ s/\*/%/g;
+
+    # split into search terms
+    my @terms;
+    # consider coma as space
+    $searchmember =~ s/,/ /g;
+    if ( $searchtype eq 'contain' ) {
+       @terms = split / /, $searchmember;
+    } else {
+       @terms = ($searchmember);
+    }
+
+    foreach my $term (@terms) {
         next unless $term;
-        $searchmember =~ s/\*/%/g; # * is replaced with % for sql
+
         $term .= '%' # end with anything
             if $term !~ /%$/;
         $term = "%$term" # begin with anythin unless start_with
-            if (defined $searchtype) && $searchtype eq "contain"
-                && $term !~ /^%/;
+            if $searchtype eq 'contain' && $term !~ /^%/;
+
         my @where_strs_or;
         for my $searchfield ( split /,/, $searchfields->{$searchfieldstype} ) {
             push @where_strs_or, "borrowers." . $dbh->quote_identifier($searchfield) . " LIKE ?";
@@ -197,11 +209,11 @@ $params is a hashref with some keys:
 
 =item searchtype
 
-  Can be 'contain' or 'start_with'. Used for the searchmember parameter.
+  Can be 'contain' or 'start_with' (default value). Used for the searchmember parameter.
 
 =item searchfieldstype
 
-  Can be 'standard', 'email', 'borrowernumber', 'phone', 'address' or 'dateofbirth', 'sort1', 'sort2'
+  Can be 'standard' (default value), 'email', 'borrowernumber', 'phone', 'address' or 'dateofbirth', 'sort1', 'sort2'
 
 =item dt_params
 

diff --git a/t/DataTables/Members.t b/t/DataTables/Members.t
index 942f93f..c9dcc7f 100644 (file)
--- a/t/DataTables/Members.t
+++ b/t/DataTables/Members.t
@@ -6,7 +6,7 @@ use_ok( "C4::Utils::DataTables::Members" );
 my $patrons = C4::Utils::DataTables::Members::search({
     searchmember => "Doe",
     searchfieldstype => 'standard',
-    searchtype => 'contains'
+    searchtype => 'contain'
 });
 
 isnt( $patrons->{iTotalDisplayRecords}, undef, "The iTotalDisplayRecords key is defined");

diff --git a/t/db_dependent/Utils/Datatables_Members.t b/t/db_dependent/Utils/Datatables_Members.t
index 57b42f7..9f8cda8 100644 (file)
--- a/t/db_dependent/Utils/Datatables_Members.t
+++ b/t/db_dependent/Utils/Datatables_Members.t
@@ -104,7 +104,7 @@ my %dt_params = (
 my $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "John Doe",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -120,7 +120,7 @@ ok( $search_results->{ patrons }[0]->{ cardnumber } eq $john_doe{ cardnumber }
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "Jane Doe",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -136,7 +136,7 @@ is( $search_results->{ patrons }[0]->{ cardnumber },
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "John",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -156,7 +156,7 @@ is( $search_results->{ patrons }[1]->{ cardnumber },
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "Doe",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -176,7 +176,7 @@ is( $search_results->{ patrons }[1]->{ cardnumber },
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "john.doe",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -187,7 +187,7 @@ is( $search_results->{ iTotalDisplayRecords }, 1,
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "john.doe",
     searchfieldstype => 'userid',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -211,7 +211,7 @@ t::lib::Mocks::mock_preference('ExtendedPatronAttributes', 1);
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "common user",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });
@@ -222,7 +222,7 @@ t::lib::Mocks::mock_preference('ExtendedPatronAttributes', 0);
 $search_results = C4::Utils::DataTables::Members::search({
     searchmember     => "common user",
     searchfieldstype => 'standard',
-    searchtype       => 'contains',
+    searchtype       => 'contain',
     branchcode       => $branchcode,
     dt_params        => \%dt_params
 });