Bug 15252 - Patron search on start with does not work with several terms
Summary: Patron search on start with does not work with several terms
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-25 11:12 UTC by Fridolin Somers
Modified: 2016-12-05 21:25 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15252 - Patron search on start with does not work with several terms (3.63 KB, patch)
2015-11-25 12:12 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms - followup 1 (1.93 KB, patch)
2015-11-25 12:12 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - correct UT searchtype value is contain and not contains (3.93 KB, patch)
2015-11-25 13:02 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - UT (3.48 KB, patch)
2015-11-25 13:02 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms (3.64 KB, patch)
2015-12-02 07:47 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms - followup 1 (1.93 KB, patch)
2015-12-02 07:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - correct UT searchtype value is contain and not contains (3.93 KB, patch)
2015-12-02 07:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 15252 - UT (3.48 KB, patch)
2015-12-02 07:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (1.95 KB, patch)
2015-12-03 21:16 UTC, Alex
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (2.06 KB, patch)
2015-12-03 21:20 UTC, Alex
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (2.17 KB, patch)
2015-12-03 21:21 UTC, Alex
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (2.28 KB, patch)
2015-12-03 21:22 UTC, Alex
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (2.28 KB, patch)
2015-12-03 21:23 UTC, Alex
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (1.70 KB, patch)
2015-12-04 00:57 UTC, Alex
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms (3.69 KB, patch)
2015-12-04 01:46 UTC, Alex
Details | Diff | Splinter Review
Bug 15252 - correct UT searchtype value is contain and not contains (3.94 KB, patch)
2015-12-04 02:12 UTC, Alex
Details | Diff | Splinter Review
DBRev 3.23.00.000: 2016 Year of the Monkey (1.55 KB, patch)
2015-12-04 02:19 UTC, Alex
Details | Diff | Splinter Review
Bug 15252 - UT (3.69 KB, patch)
2015-12-04 03:06 UTC, Alex
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms (3.87 KB, patch)
2015-12-16 17:42 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms - followup 1 (2.11 KB, patch)
2015-12-16 17:43 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15252 - correct UT searchtype value is contain and not contains (4.12 KB, patch)
2015-12-16 17:44 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15252 - UT (3.87 KB, patch)
2015-12-16 17:46 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15252 - Patron search on start with does not work with several terms (8.93 KB, patch)
2015-12-18 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15252 - Add tests (3.73 KB, patch)
2015-12-18 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2015-11-25 11:12:07 UTC
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'.
Comment 1 Fridolin Somers 2015-11-25 12:12:04 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2015-11-25 12:12:17 UTC Comment hidden (obsolete)
Comment 3 Fridolin Somers 2015-11-25 13:02:33 UTC Comment hidden (obsolete)
Comment 4 Fridolin Somers 2015-11-25 13:02:45 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-11-25 15:24:03 UTC
-        $searchmember =~ s/\*/%/g; # * is replaced with % for sql
+
+        $term =~ s/\*/%/g; # * is replaced with % for sql

This changes has 2 consequences:
1/ Add a new feature: * is now a wildcard for the patron search
2/ Introduce a regression: * is not a wildcard anymore for the patron attribute search
Comment 6 Fridolin Somers 2015-12-02 07:47:58 UTC Comment hidden (obsolete)
Comment 7 Fridolin Somers 2015-12-02 07:48:09 UTC Comment hidden (obsolete)
Comment 8 Fridolin Somers 2015-12-02 07:48:20 UTC Comment hidden (obsolete)
Comment 9 Fridolin Somers 2015-12-02 07:48:31 UTC Comment hidden (obsolete)
Comment 10 Fridolin Somers 2015-12-02 07:50:01 UTC
(In reply to Jonathan Druart from comment #5)
> -        $searchmember =~ s/\*/%/g; # * is replaced with % for sql
> +
> +        $term =~ s/\*/%/g; # * is replaced with % for sql
> 
> This changes has 2 consequences:
> 1/ Add a new feature: * is now a wildcard for the patron search
> 2/ Introduce a regression: * is not a wildcard anymore for the patron
> attribute search

I corrected by keeping the original line but moved before creating terms.
Comment 11 Alex 2015-12-03 21:16:45 UTC Comment hidden (obsolete)
Comment 12 Alex 2015-12-03 21:20:25 UTC Comment hidden (obsolete)
Comment 13 Alex 2015-12-03 21:21:28 UTC Comment hidden (obsolete)
Comment 14 Alex 2015-12-03 21:22:20 UTC Comment hidden (obsolete)
Comment 15 Alex 2015-12-03 21:23:11 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2015-12-03 21:27:37 UTC
Hi Alex, something is not quite right here - your squashed patch included one commit too many - the year of the monkey one. Please check, best would be to reattach the 4 original patches with your signed off line.
Comment 17 Alex 2015-12-04 00:57:49 UTC Comment hidden (obsolete)
Comment 18 Alex 2015-12-04 01:46:09 UTC Comment hidden (obsolete)
Comment 19 Alex 2015-12-04 02:12:29 UTC Comment hidden (obsolete)
Comment 20 Alex 2015-12-04 02:19:58 UTC Comment hidden (obsolete)
Comment 21 Alex 2015-12-04 03:06:16 UTC Comment hidden (obsolete)
Comment 22 Owen Leonard 2015-12-16 14:06:31 UTC
What is the status of this bug? Some patches appear to be signed off, but the patch series doesn't apply when using git bz. Are they out of order? Need rebase?
Comment 23 Marc Véron 2015-12-16 17:30:49 UTC
(In reply to Owen Leonard from comment #22)
> What is the status of this bug? Some patches appear to be signed off, but
> the patch series doesn't apply when using git bz. Are they out of order?
> Need rebase?

Hi Owen,
I could apply by doing it interactive and changing the order as follows:
45403 - Bug 15252 - Patron search on start with does not work with several terms
45301 - Bug 15252 - Patron search on start with does not work with several terms - followup 1
45405 - Bug 15252 - correct UT searchtype value is contain and not contains
45407 - Bug 15252 - UT
Comment 24 Marc Véron 2015-12-16 17:42:01 UTC Comment hidden (obsolete)
Comment 25 Marc Véron 2015-12-16 17:43:22 UTC Comment hidden (obsolete)
Comment 26 Marc Véron 2015-12-16 17:44:38 UTC Comment hidden (obsolete)
Comment 27 Marc Véron 2015-12-16 17:46:58 UTC Comment hidden (obsolete)
Comment 28 Alex 2015-12-18 03:51:55 UTC
So what went wrong with my attaching the bug if it did not apply properly?(In reply to Marc Véron from comment #27)
> Created attachment 45738 [details] [review] [review]
> Bug 15252 - UT
> 
> Signed-off-by: Alex <alexklbuckley@gmail.com>
> 
> 	0004-Bug-15252-UT.patch
> 
> Signed-off-by: Alex <alexklbuckley@gmail.com>
> 
> 	0004-Bug-15252-UT.patch
> 
> Signed-off-by: Alex <alexklbuckley@gmail.com>
> 
> 	0004-Bug-15252-UT.patch
> 
> Tested 4 patches together, works as expected.
> Signed-off-by: Marc Véron <veron@veron.ch>
Comment 29 Marc Véron 2015-12-18 09:29:58 UTC
Hi Alex, the patches were in a wrong order, first was the follow up, then the other patches. That's why they could not apply.

What I did:
- Apply patches interactively one by one in the order of comment #23.
- Test to prepare signing off
- Reset and then one by one interactively apply patch, sing-off, attach.

I just tested again, all patches apply.
Comment 30 Jonathan Druart 2015-12-18 14:30:32 UTC
Created attachment 45830 [details] [review]
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>
Comment 31 Jonathan Druart 2015-12-18 14:30:38 UTC
Created attachment 45831 [details] [review]
Bug 15252 - Add tests

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>
Comment 32 Jonathan Druart 2015-12-18 14:41:23 UTC
I have squashed the 3 first patches for the readability of the git log
Comment 33 Alex 2015-12-19 01:08:15 UTC
(In reply to Marc Véron from comment #29)
> Hi Alex, the patches were in a wrong order, first was the follow up, then
> the other patches. That's why they could not apply.
> 
> What I did:
> - Apply patches interactively one by one in the order of comment #23.
> - Test to prepare signing off
> - Reset and then one by one interactively apply patch, sing-off, attach.
> 
> I just tested again, all patches apply.

Ok I see thanks Marc
Comment 34 Kyle M Hall 2015-12-30 13:08:21 UTC
Pushed to master, thanks Frido!
Comment 35 Julian Maurice 2015-12-31 09:42:33 UTC
Patches pushed to 3.22.x, will be in 3.22.2
Comment 36 Frédéric Demians 2015-12-31 18:31:13 UTC
This patch has been pushed to 3.20.x, will be in 3.20.8.