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'.
Created attachment 45131 [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"
Created attachment 45132 [details] [review] 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.
Created attachment 45133 [details] [review] Bug 15252 - correct UT searchtype value is contain and not contains
Created attachment 45134 [details] [review] Bug 15252 - UT
- $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
Created attachment 45300 [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"
Created attachment 45301 [details] [review] 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.
Created attachment 45302 [details] [review] Bug 15252 - correct UT searchtype value is contain and not contains
Created attachment 45303 [details] [review] Bug 15252 - UT
(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.
Created attachment 45390 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) http://bugs.koha-community.org/show_bug.cgi?id=15252 Signed-off-by: Alex <alexklbuckley@gmail.com> http://bugs.koha-community.org/show_bug.cgi?id=6473 Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed)
Created attachment 45391 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) http://bugs.koha-community.org/show_bug.cgi?id=15252 Signed-off-by: Alex <alexklbuckley@gmail.com> http://bugs.koha-community.org/show_bug.cgi?id=6473 Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) Signed-off-by: Alex <alexklbuckley@gmail.com> 0002-Bug-15252-Patron-search-on-start-with-does-not-work-.patch
Created attachment 45392 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) http://bugs.koha-community.org/show_bug.cgi?id=15252 Signed-off-by: Alex <alexklbuckley@gmail.com> http://bugs.koha-community.org/show_bug.cgi?id=6473 Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) Signed-off-by: Alex <alexklbuckley@gmail.com> 0002-Bug-15252-Patron-search-on-start-with-does-not-work-.patch Signed-off-by: Alex <alexklbuckley@gmail.com> 0003-Bug-15252-correct-UT-searchtype-value-is-contain-and.patch
Created attachment 45393 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) http://bugs.koha-community.org/show_bug.cgi?id=15252 Signed-off-by: Alex <alexklbuckley@gmail.com> http://bugs.koha-community.org/show_bug.cgi?id=6473 Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) Signed-off-by: Alex <alexklbuckley@gmail.com> 0002-Bug-15252-Patron-search-on-start-with-does-not-work-.patch Signed-off-by: Alex <alexklbuckley@gmail.com> 0003-Bug-15252-correct-UT-searchtype-value-is-contain-and.patch Signed-off-by: Alex <alexklbuckley@gmail.com> 0001-Bug-15252-Patron-search-on-start-with-does-not-work-.patch
Created attachment 45395 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) http://bugs.koha-community.org/show_bug.cgi?id=15252 Signed-off-by: Alex <alexklbuckley@gmail.com> http://bugs.koha-community.org/show_bug.cgi?id=6473 Signed-off-by: Alex <alexklbuckley@gmail.com> (use "git reset HEAD^1 <file>..." to unstage) (use "git add <file>..." to include in what will be committed) Signed-off-by: Alex <alexklbuckley@gmail.com> 0002-Bug-15252-Patron-search-on-start-with-does-not-work-.patch Signed-off-by: Alex <alexklbuckley@gmail.com> 0003-Bug-15252-correct-UT-searchtype-value-is-contain-and.patch Signed-off-by: Alex <alexklbuckley@gmail.com> 0001-Bug-15252-Patron-search-on-start-with-does-not-work-.patch
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.
Created attachment 45401 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> 0001-Bug-15252-Patron-search-on-start-with-does-not-work-.patch Signed-off-by: Alex <alexklbuckley@gmail.com> 0001-Bug-15252-Patron-search-on-start-with-does-not-work-.patch
Created attachment 45403 [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>
Created attachment 45405 [details] [review] Bug 15252 - correct UT searchtype value is contain and not contains
Created attachment 45406 [details] [review] DBRev 3.23.00.000: 2016 Year of the Monkey Signed-off-by: Alex <alexklbuckley@gmail.com> 0004-Bug-15252-UT.patch
Created attachment 45407 [details] [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
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?
(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
Created attachment 45735 [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>
Created attachment 45736 [details] [review] 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>
Created attachment 45737 [details] [review] 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>
Created attachment 45738 [details] [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>
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>
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.
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>
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>
I have squashed the 3 first patches for the readability of the git log
(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
Pushed to master, thanks Frido!
Patches pushed to 3.22.x, will be in 3.22.2
This patch has been pushed to 3.20.x, will be in 3.20.8.