Summary: | Patron autocomplete should not use contains on all fields | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Staff interface | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | Pushed to stable --- | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | bugzilla, egpetridis, gmcharlt, jonathan.druart, katrin.fischer, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
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
|
|
Bug Depends on: | 32520 | ||
Bug Blocks: | 33117, 34941 | ||
Attachments: |
Bug 32772: Make all autocomplete searches 'starts_with'
Bug 32772: [Alternate] Add support for '^' as "Starts with" in patron searching Bug 32772: Make all autocomplete searches 'starts_with' |
Description
Nick Clemens (kidclamp)
2023-01-31 19:58:48 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 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 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 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 I am raising to major, while it doesn't affect data, it has a huge impact on library workflows 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. OK.. we all agreed Option 1 is likely the behaviour we want.. 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> 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. In fact.. as this patch is trivial and fixes a major regression I'll go straight for PQA.. Thanks Nick Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x I don't understand this change, it's a regression, see bug 33117. I wonder if bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34580 is related to changes introduced here. |