Bug 32772 - Patron autocomplete should not use contains on all fields
Summary: Patron autocomplete should not use contains on all fields
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on: 32520
Blocks: 34941 33117
  Show dependency treegraph
 
Reported: 2023-01-31 19:58 UTC by Nick Clemens (kidclamp)
Modified: 2024-07-25 11:11 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.03
Circulation function:


Attachments
Bug 32772: Make all autocomplete searches 'starts_with' (1.40 KB, patch)
2023-01-31 20:17 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 32772: [Alternate] Add support for '^' as "Starts with" in patron searching (3.46 KB, patch)
2023-01-31 20:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 32772: Make all autocomplete searches 'starts_with' (1.46 KB, patch)
2023-02-02 17:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2023-01-31 19:58:48 UTC
Bug 31782 added all default patron search fields to the autocomplete, and made them all use 'contains'

This makes searching in large systems difficult

Previous behaviour:
Every library has a unique starting cardnumber sequence, but it can repeat in later numbers
Library A types "3A" to find their patrons only, but autocomplete now returns all of library B patrons with cards ending in 3A


Other issue:
Library has a large number of patrons with surnames 'Williams' and 'Fitzwilliams'
Type 'Williams' into the search, only see Fitzwilliams

I think we should default to 'Starts with' which allows the use of '%' at the start to make it a contains, rather than force contains which we don't have a way to limit.

Otherwise, we need to support '^' for starts with
Comment 1 Nick Clemens (kidclamp) 2023-01-31 20:01:38 UTC
For master this bug includes all fields in DefaultPatronSearchFields - depending on the backport of 31782 we still need a solution for surname search in stable versions
Comment 2 Nick Clemens (kidclamp) 2023-01-31 20:17:22 UTC
Created attachment 145905 [details] [review]
Bug 32772: Make all autocomplete searches 'starts_with'

To test:
1 - Add patrons with surname Fitzwilliams and Williams
2 - Enable 'PatronAutocomplete' syspref
3 - In checkout box type 'Will'
4 - You see fitwilliams first
5 - Apply patch
6 - Type 'Will'
7 - Autocomplete does not suggest Fitzwilliam, only Williams
8 - Type '%will'
9 - Fitzwilliams is suggested
Comment 3 Nick Clemens (kidclamp) 2023-01-31 20:28:37 UTC
Created attachment 145914 [details] [review]
Bug 32772: [Alternate] Add support for '^' as "Starts with" in patron searching

This patch parses each term we send to patron search - if it finds it starts with '^' then the
term is searched as a starts with - other terms will still be 'contains'

To test:
1 - Make a bunch of williams and fitzwiliams
    UPDATE borrowers SET surname = 'Fitzwilliams' IF borrowernumber < 25;
    UPDATE borrowers SET surname = 'Williams' IF borrowernumber > 25;
2 - Enable PatronAutrocomplete syspref
3 - Search for 'Will' in patron search
4 - You get Fitzwilliams (you see them in the automplete too)
5 - Apply patch
6 - Search for '^Will'
7 - Confirm that autocomplete and full search results only have williams, not fitzwilliams
8 - Search for '^Will an'
9 - Confirm you get 'Frances Williams' and 'Annie Williams'  - as the 'an' is searched as contains
Comment 4 Nick Clemens (kidclamp) 2023-01-31 20:29:48 UTC
The first patch makes al the searches 'Starts with'
The second goes on top and undoes that and allows for '^' to mean starts with in a term
Comment 5 Nick Clemens (kidclamp) 2023-01-31 20:33:02 UTC
I am raising to major, while it doesn't affect data, it has a huge impact on library workflows
Comment 6 Martin Renvoize (ashimema) 2023-02-01 08:01:09 UTC
Updateing to NSO.. both patches work as described so I'd be happy to sign off.. However, I'm not entirely sure which we should adopt.. the addition of `^` to turn a search into 'starts_with' or the addition of `%` to turn a search into 'contains'.  My gut says, users were comfortable with default being `starts_with` and so documenting the addition of `%` switching it to `contains` may be the least friction approach.

Asking colleagues to weight in.
Comment 7 Martin Renvoize (ashimema) 2023-02-01 10:46:29 UTC
OK.. we all agreed Option 1 is likely the behaviour we want..
Comment 8 Martin Renvoize (ashimema) 2023-02-02 17:04:40 UTC
Created attachment 146021 [details] [review]
Bug 32772: Make all autocomplete searches 'starts_with'

To test:
1 - Add patrons with surname Fitzwilliams and Williams
2 - Enable 'PatronAutocomplete' syspref
3 - In checkout box type 'Will'
4 - You see fitwilliams first
5 - Apply patch
6 - Type 'Will'
7 - Autocomplete does not suggest Fitzwilliam, only Williams
8 - Type '%will'
9 - Fitzwilliams is suggested

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize (ashimema) 2023-02-02 17:06:04 UTC
Whilst the nerd/coder side of me prefers the alternative.. I think in real life people will all prefer the functionality of the first patch.

As such, I tested that one and am signing off confident in the approach as agreed by the staff and a few customers here.
Comment 10 Martin Renvoize (ashimema) 2023-02-02 17:07:18 UTC
In fact.. as this patch is trivial and fixes a major regression I'll go straight for PQA.. 

Thanks Nick
Comment 11 Tomás Cohen Arazi (tcohen) 2023-02-06 21:36:28 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 12 Matt Blenkinsop 2023-02-07 15:24:03 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 13 Jonathan Druart 2023-03-07 09:51:06 UTC
I don't understand this change, it's a regression, see bug 33117.
Comment 14 Manos PETRIDIS 2023-08-22 10:24:07 UTC
I wonder if bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34580 is related to changes introduced here.