Bug 30639

Summary: Patron search does not split search terms
Product: Koha Reporter: Nick Clemens <nick>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: jonathan.druart, m.de.rooy
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00
Bug Depends on: 30063, 30576    
Bug Blocks: 30645    
Attachments: Bug 30639: Split search terms for patron search
Bug 30639: Split search terms for patron search
Bug 30639: Split search terms for patron search

Description Nick Clemens 2022-04-28 10:50:16 UTC
Search for: "edna acosta" OR "henry acevedo" OR you favorite default patron
No results!

The query is search for both terms in each field:
i.e. firstname LIKE 'edna acosta'
Comment 1 Jonathan Druart 2022-04-28 12:44:24 UTC
Created attachment 134251 [details] [review]
Bug 30639: Split search terms for patron search

If several terms are passed we should split them.

Test plan:
Search for "edna acosta" (without quotes) in the filters from the left
side of the main patron search
Do some regression tests
Comment 2 Jonathan Druart 2022-04-28 12:45:27 UTC
This is still not perfect for patron attributes.
If you have:
  attribute 1 = xxx
  attribute 2 = yyy

and you search for "xxx yyy", the patron won't be returned.
Comment 3 Jonathan Druart 2022-04-28 13:14:20 UTC
(In reply to Jonathan Druart from comment #2)
> This is still not perfect for patron attributes.
> If you have:
>   attribute 1 = xxx
>   attribute 2 = yyy
> 
> and you search for "xxx yyy", the patron won't be returned.

Reported on bug 30645.
Comment 4 Nick Clemens 2022-04-28 13:43:40 UTC
Created attachment 134263 [details] [review]
Bug 30639: Split search terms for patron search

If several terms are passed we should split them.

Test plan:
Search for "edna acosta" (without quotes) in the filters from the left
side of the main patron search
Do some regression tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Marcel de Rooy 2022-05-06 08:07:25 UTC
Looking here
Comment 6 Marcel de Rooy 2022-05-06 08:28:45 UTC
    # Pattern terms must be split
    $s->fill_form( { search_patron_filter => 'test patron' } );
    $s->submit_form;

    $s->wait_for_ajax;
    is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text, sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)', $PatronsPerPage, 26, $total_number_of_patrons) );

Hmm I cant run this test. But Nick tested it.
Just dont understand the 26 here. Reading the test I expect 25 ?
Comment 7 Jonathan Druart 2022-05-06 08:42:39 UTC
(In reply to Marcel de Rooy from comment #6)
>     # Pattern terms must be split
>     $s->fill_form( { search_patron_filter => 'test patron' } );
>     $s->submit_form;
> 
>     $s->wait_for_ajax;
>     is( $driver->find_element('//div[@id="'.$table_id.'_info"]')->get_text,
> sprintf('Showing 1 to %s of %s entries (filtered from %s total entries)',
> $PatronsPerPage, 26, $total_number_of_patrons) );
> 
> Hmm I cant run this test. But Nick tested it.
> Just dont understand the 26 here. Reading the test I expect 25 ?

For most of the test we are filtering by library and so we get the 25 from

 77     for my $i ( 1 .. 25 ) {
 78         push @patrons,
 79           $builder->build_object(

But there is a 26th patron matching test_patron_*

 97     push @patrons,
 98       $builder->build_object(
 99         {
100             class => 'Koha::Patrons',
101             value => {
102                 surname       => "test_patron_26",

Which is from a different library.
Comment 8 Marcel de Rooy 2022-05-06 08:47:42 UTC
Created attachment 134709 [details] [review]
Bug 30639: Split search terms for patron search

If several terms are passed we should split them.

Test plan:
Search for "edna acosta" (without quotes) in the filters from the left
side of the main patron search
Do some regression tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Fridolin Somers 2022-05-06 20:35:11 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄