From db50ac5b8c6c72d15f4c5e2a96544abde432b0f6 Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Wed, 25 Nov 2015 12:34:18 +0100 Subject: [PATCH] Bug 15252 - Patron search on start with does not work with several terms MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Tested 4 patches together, works as expected Signed-off-by: Marc Véron 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 Bug 15252 - correct UT searchtype value is contain and not contains Tested 4 patches together, works as expected Signed-off-by: Marc Véron Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall (cherry picked from commit a4f5564c855e31f6872fb5e3ef378381473f837c) Signed-off-by: Julian Maurice --- C4/Utils/DataTables/Members.pm | 30 ++++++++++++++++------- t/DataTables/Members.t | 2 +- t/db_dependent/Utils/Datatables_Members.t | 16 ++++++------ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm index bfa73e01e1..1ebecd7e59 100644 --- 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 942f93f6eb..c9dcc7f8a8 100644 --- 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 57b42f7aba..9f8cda8d54 100644 --- 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 }); -- 2.39.5