Bug 30639 - Patron search does not split search terms
Summary: Patron search does not split search terms
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 30576 30063
Blocks: 30645
  Show dependency treegraph
 
Reported: 2022-04-28 10:50 UTC by Nick Clemens
Modified: 2022-05-17 12:38 UTC (History)
2 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:
22.05.00


Attachments
Bug 30639: Split search terms for patron search (5.65 KB, patch)
2022-04-28 12:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30639: Split search terms for patron search (5.70 KB, patch)
2022-04-28 13:43 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30639: Split search terms for patron search (5.79 KB, patch)
2022-05-06 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 🦄