Bug 33428 - Should only search in searchable patron attributes if searching in standard fields
Summary: Should only search in searchable patron attributes if searching in standard f...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 29822
Blocks: 34519
  Show dependency treegraph
 
Reported: 2023-04-06 07:10 UTC by Fridolin Somers
Modified: 2023-11-11 01:31 UTC (History)
4 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:
23.11.00


Attachments
Bug 33428: Parse search fields in buildPatronSearchQuery (5.34 KB, patch)
2023-08-10 19:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33428: Parse search fields in buildPatronSearchQuery (5.26 KB, patch)
2023-08-10 20:35 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 33428: Parse search fields in buildPatronSearchQuery (5.26 KB, patch)
2023-08-11 12:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33428: Adjust Unit Tests (4.51 KB, patch)
2023-08-11 12:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33428: Parse search fields in buildPatronSearchQuery (5.33 KB, patch)
2023-08-17 10:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33428: Adjust Unit Tests (4.57 KB, patch)
2023-08-17 10:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33428: (QA follow-up) Fix escaping in a split call (1.23 KB, patch)
2023-08-17 10:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33428 (follow-up): Default to searching 'standard' fields (1.60 KB, patch)
2023-08-18 11:19 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2023-04-06 07:10:47 UTC
Before patrons search using REST API, the search on patron attributes was only in standard fields.

See Bug 18094 : Only search in searchable patron attributes if searching in standard fields
Comment 1 Fridolin Somers 2023-04-06 07:11:51 UTC
This creates strange behaviors for example when using search field surname and 
searching starts with one letter. Some results are matches on patron attributes.
Comment 2 Fridolin Somers 2023-04-06 07:30:45 UTC
Ich

How can we tell the searching is standard fields ?

https://git.koha-community.org/Koha-community/Koha/src/commit/f74a353097710cf33b3b75b9912b2b00d2aa1a30/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc#L307

Using search_fields == 'firstname,middle_name,surname,othernames,cardnumber,userid' ?
Looks not very strong to me :(
Comment 3 Nick Clemens 2023-08-10 19:02:48 UTC
Created attachment 154352 [details] [review]
Bug 33428: Parse search fields in buildPatronSearchQuery

This patch moves the parsing of standard search_field into the buildPatronQuery subroutine
and adds a check for 'standard' field before adding attributes to the search

To test:
1 - Add a new attribute type and make it searchable
2 - Add a value to a patron
3 - Search for this value using 'Standard' fields, confirm you get the patron
4 - Search for the value using 'Cardnumber' field, confirm you get the patron - BAD!
5 - Apply patch
6 - Repeat cardnumebr search, confirm patron not found - Yay!
7 - Search standard, confirm patron is found
8 - Add a new field to 'DefaultPatronSearchFields
9 - Confirm it appears in patron search dropdown
10 - Confirm a search of this field with the attribute value does not return the patron
Comment 4 Nick Clemens 2023-08-10 19:03:32 UTC
Frido, I stole this, apologies, feel free to provide an alternate
Comment 5 Fridolin Somers 2023-08-10 20:24:54 UTC
(In reply to Nick Clemens from comment #4)
> Frido, I stole this, apologies, feel free to provide an alternate

Your welcome :D
Comment 6 Fridolin Somers 2023-08-10 20:35:11 UTC
Created attachment 154355 [details] [review]
Bug 33428: Parse search fields in buildPatronSearchQuery

This patch moves the parsing of standard search_field into the buildPatronQuery subroutine
and adds a check for 'standard' field before adding attributes to the search

To test:
1 - Add a new attribute type and make it searchable
2 - Add a value to a patron
3 - Search for this value using 'Standard' fields, confirm you get the patron
4 - Search for the value using 'Cardnumber' field, confirm you get the patron - BAD!
5 - Apply patch
6 - Repeat cardnumebr search, confirm patron not found - Yay!
7 - Search standard, confirm patron is found
8 - Add a new field to 'DefaultPatronSearchFields
9 - Confirm it appears in patron search dropdown
10 - Confirm a search of this field with the attribute value does not return the patron

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 7 Fridolin Somers 2023-08-10 20:36:27 UTC
Works great.
I signed and removed unnecessary empty line addition in staff-global.js
Comment 8 Nick Clemens 2023-08-11 12:32:00 UTC
Created attachment 154367 [details] [review]
Bug 33428: Parse search fields in buildPatronSearchQuery

This patch moves the parsing of standard search_field into the buildPatronQuery subroutine
and adds a check for 'standard' field before adding attributes to the search

To test:
1 - Add a new attribute type and make it searchable
2 - Add a value to a patron
3 - Search for this value using 'Standard' fields, confirm you get the patron
4 - Search for the value using 'Cardnumber' field, confirm you get the patron - BAD!
5 - Apply patch
6 - Repeat cardnumebr search, confirm patron not found - Yay!
7 - Search standard, confirm patron is found
8 - Add a new field to 'DefaultPatronSearchFields
9 - Confirm it appears in patron search dropdown
10 - Confirm a search of this field with the attribute value does not return the patron

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 9 Nick Clemens 2023-08-11 12:32:02 UTC
Created attachment 154368 [details] [review]
Bug 33428: Adjust Unit Tests
Comment 10 Martin Renvoize 2023-08-17 10:55:09 UTC
Created attachment 154528 [details] [review]
Bug 33428: Parse search fields in buildPatronSearchQuery

This patch moves the parsing of standard search_field into the buildPatronQuery subroutine
and adds a check for 'standard' field before adding attributes to the search

To test:
1 - Add a new attribute type and make it searchable
2 - Add a value to a patron
3 - Search for this value using 'Standard' fields, confirm you get the patron
4 - Search for the value using 'Cardnumber' field, confirm you get the patron - BAD!
5 - Apply patch
6 - Repeat cardnumebr search, confirm patron not found - Yay!
7 - Search standard, confirm patron is found
8 - Add a new field to 'DefaultPatronSearchFields
9 - Confirm it appears in patron search dropdown
10 - Confirm a search of this field with the attribute value does not return the patron

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2023-08-17 10:55:11 UTC
Created attachment 154529 [details] [review]
Bug 33428: Adjust Unit Tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2023-08-17 10:55:14 UTC
Created attachment 154530 [details] [review]
Bug 33428: (QA follow-up) Fix escaping in a split call

We'd missed an escape case in one of the calls to .split for the pipe
delimited split operations.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2023-08-17 10:55:56 UTC
Nice work, Passing QA
Comment 14 Nick Clemens 2023-08-18 11:19:24 UTC
Created attachment 154638 [details] [review]
Bug 33428 (follow-up): Default to searching 'standard' fields

Before this patch set, a search for patrons from article requests or
reserves would search the attributes by default. They were not considered
'standard' searches, however.

This patch simply defaults to searching the 'standard' fields unless a value is passed

To test:
1 - Apply other patches
2 - Have patrons with searchable attributes
3 - From a title, click the holds or artickle requests tab
4 - Search for the attribute values and confirm patrons are not returned
5 - Apply this patch
6 - Search again, patrons are now returned
Comment 15 Tomás Cohen Arazi 2023-09-25 13:49:27 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 16 Fridolin Somers 2023-09-25 19:24:27 UTC
Depends on Bug 29822 not in 23.05.x