Bug 32770 - Patron search field groups no longer exist
Summary: Patron search field groups no longer exist
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: Testopia
URL:
Keywords:
Depends on: 30063
Blocks:
  Show dependency treegraph
 
Reported: 2023-01-31 18:36 UTC by Nick Clemens
Modified: 2023-11-29 15:20 UTC (History)
7 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.04,22.05.11


Attachments
Bug 32770: Restore patron search on all emails and phone numbers (3.04 KB, patch)
2023-02-01 08:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32770: Restore patron search on all emails and phone numbers (3.12 KB, patch)
2023-02-01 10:44 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32770: [alternate] Restore patron search on all emails, phones, and addresses (7.43 KB, patch)
2023-02-01 12:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32770: [alternate] Restore patron search on all emails, phones, and addresses (7.52 KB, patch)
2023-02-07 13:49 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32770: Restore patron search on all emails, phones, and addresses (7.56 KB, patch)
2023-02-16 08:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32770: Restore phone (1.49 KB, patch)
2023-02-22 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32770: [22.05.x] Restore patron search on all emails, phones, and addresses (5.90 KB, patch)
2023-03-03 13:39 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 Nick Clemens 2023-01-31 18:36:08 UTC
Prior to bug 30063 we had a number of options in the patron dropdown that were grouped fields. I.e. choosing 'Address' would search accross multiple address fields
.

These have been lost, with no way for libraries to recreate them.

The mappings were:
         standard => C4::Context->preference('DefaultPatronSearchFields') || 'surname,firstname,othernames,cardnumber,userid',
         email => 'email,emailpro,B_email',
         borrowernumber => 'borrowernumber',
         phone => 'phone,phonepro,B_phone,altcontactphone,mobile',
         address => 'streetnumber,streettype,address,address2,city,state,zipcode,country',
Comment 1 Jonathan Druart 2023-02-01 08:35:25 UTC
Created attachment 145921 [details] [review]
Bug 32770: Restore patron search on all emails and phone numbers

Prior to bug 30063 we had "Email" and "Phone" in the search field list
that allowed a search on all email or phone attributes.

This patch restores that (in an a bit ugly way, but cannot do easier).

Test plan:
Confirm that "Email" and "Phone" are displayed in the search field list
when searching for patrons. Note that "Phone" was there already, but was
only searching on the first "phone" attribute.
Confirm that the search works as expected.

Note for QA: I was not expecting the REST API to handle search on
attributes with the DB column names, I think this is a bug.
Comment 2 Matt Blenkinsop 2023-02-01 10:44:33 UTC
Created attachment 145928 [details] [review]
Bug 32770: Restore patron search on all emails and phone numbers

Prior to bug 30063 we had "Email" and "Phone" in the search field list
that allowed a search on all email or phone attributes.

This patch restores that (in an a bit ugly way, but cannot do easier).

Test plan:
Confirm that "Email" and "Phone" are displayed in the search field list
when searching for patrons. Note that "Phone" was there already, but was
only searching on the first "phone" attribute.
Confirm that the search works as expected.

Note for QA: I was not expecting the REST API to handle search on
attributes with the DB column names, I think this is a bug.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 3 Nick Clemens 2023-02-01 12:33:38 UTC
Created attachment 145935 [details] [review]
Bug 32770: [alternate] Restore patron search on all emails, phones, and addresses

Prior to bug 30063 we had "Email", "Phone", and "Address" in the search field list
that allowed a search on all email, phone, or address attributes.

This patron restores these and makes sure the search box and the patron search filters use
the same block to keep them consistent

The fields are now labelled 'All emails', 'All phones', and 'Full address' to make it clear
that they search multiple - including any single field in the DefaultPatronSearchFields will
additionally add that to the dropdowns

To test:
 1 - define 'test@email.pro' as 'Secondary email' for a patron
 2 - Search from 'Search patrons' address bar (visible at top of moremember.pl)
 3 - Nothing is found using 'Standard' or 'Email fields'
 4 - Apply patch
 5 - Refresh page and repeat
 6 - Nothing is found for standard, Patron is found for 'All emails'
 7 - Define a unique value for patron in 'Address 2'
 8 - Confirm 'Full address' search returns the patron
 9 - Test with 'Secondary phone' and 'All phones' as well
10 - Repeat test from 'Patrons' module using the search on the left
Comment 4 Nick Clemens 2023-02-01 12:34:33 UTC
The original patch did not include 'address' and did not address the search bar - I attached an alternate to cover these and make the combination of fields more clear to users
Comment 5 Matt Blenkinsop 2023-02-07 13:49:33 UTC
Created attachment 146331 [details] [review]
Bug 32770: [alternate] Restore patron search on all emails, phones, and addresses

Prior to bug 30063 we had "Email", "Phone", and "Address" in the search field list
that allowed a search on all email, phone, or address attributes.

This patron restores these and makes sure the search box and the patron search filters use
the same block to keep them consistent

The fields are now labelled 'All emails', 'All phones', and 'Full address' to make it clear
that they search multiple - including any single field in the DefaultPatronSearchFields will
additionally add that to the dropdowns

To test:
 1 - define 'test@email.pro' as 'Secondary email' for a patron
 2 - Search from 'Search patrons' address bar (visible at top of moremember.pl)
 3 - Nothing is found using 'Standard' or 'Email fields'
 4 - Apply patch
 5 - Refresh page and repeat
 6 - Nothing is found for standard, Patron is found for 'All emails'
 7 - Define a unique value for patron in 'Address 2'
 8 - Confirm 'Full address' search returns the patron
 9 - Test with 'Secondary phone' and 'All phones' as well
10 - Repeat test from 'Patrons' module using the search on the left

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 6 Jonathan Druart 2023-02-16 08:02:42 UTC
Created attachment 146711 [details] [review]
Bug 32770: Restore patron search on all emails, phones, and addresses

Prior to bug 30063 we had "Email", "Phone", and "Address" in the search field list
that allowed a search on all email, phone, or address attributes.

This patron restores these and makes sure the search box and the patron search filters use
the same block to keep them consistent

The fields are now labelled 'All emails', 'All phones', and 'Full address' to make it clear
that they search multiple - including any single field in the DefaultPatronSearchFields will
additionally add that to the dropdowns

To test:
 1 - define 'test@email.pro' as 'Secondary email' for a patron
 2 - Search from 'Search patrons' address bar (visible at top of moremember.pl)
 3 - Nothing is found using 'Standard' or 'Email fields'
 4 - Apply patch
 5 - Refresh page and repeat
 6 - Nothing is found for standard, Patron is found for 'All emails'
 7 - Define a unique value for patron in 'Address 2'
 8 - Confirm 'Full address' search returns the patron
 9 - Test with 'Secondary phone' and 'All phones' as well
10 - Repeat test from 'Patrons' module using the search on the left

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Tomás Cohen Arazi 2023-02-16 15:02:02 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 8 Jonathan Druart 2023-02-22 13:44:01 UTC
Created attachment 147147 [details] [review]
Bug 32770: Restore phone

all_phone vs all_phones
That was caught by Selenium tests t/db_dependent/selenium/patrons_search.t
Comment 9 Tomás Cohen Arazi 2023-02-22 18:06:07 UTC
(In reply to Jonathan Druart from comment #8)
> Created attachment 147147 [details] [review] [review]
> Bug 32770: Restore phone
> 
> all_phone vs all_phones
> That was caught by Selenium tests t/db_dependent/selenium/patrons_search.t

Follow-up pushed, thanks.
Comment 10 Martin Renvoize 2023-02-24 07:52:50 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 11 Martin Renvoize 2023-02-27 13:38:41 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 12 Lucas Gass 2023-03-01 14:21:44 UTC
Doesn't apply cleanly in 22.05.x, no backport unless it can be rebased.
Comment 13 Nick Clemens 2023-03-03 13:39:51 UTC
Created attachment 147696 [details] [review]
Bug 32770: [22.05.x] Restore patron search on all emails, phones, and addresses

Prior to bug 30063 we had "Email", "Phone", and "Address" in the search field list
that allowed a search on all email, phone, or address attributes.

This patron restores these and makes sure the search box and the patron search filters use
the same block to keep them consistent

The fields are now labelled 'All emails', 'All phones', and 'Full address' to make it clear
that they search multiple - including any single field in the DefaultPatronSearchFields will
additionally add that to the dropdowns

To test:
 1 - define 'test@email.pro' as 'Secondary email' for a patron
 2 - Search from 'Search patrons' address bar (visible at top of moremember.pl)
 3 - Nothing is found using 'Standard' or 'Email fields'
 4 - Apply patch
 5 - Refresh page and repeat
 6 - Nothing is found for standard, Patron is found for 'All emails'
 7 - Define a unique value for patron in 'Address 2'
 8 - Confirm 'Full address' search returns the patron
 9 - Test with 'Secondary phone' and 'All phones' as well
10 - Repeat test from 'Patrons' module using the search on the left

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 32770: Restore phone

all_phone vs all_phones
That was caught by Selenium tests t/db_dependent/selenium/patrons_search.t
Comment 14 Lucas Gass 2023-03-08 13:17:35 UTC
Backported to 22.05.x for upcoming 22.05.11
Comment 15 Arthur Suzuki 2023-03-29 12:27:27 UTC
depends on 30063, can't backport to 21.11
Comment 16 Jonathan Druart 2023-11-29 10:16:11 UTC
(In reply to Lucas Gass from comment #14)
> Backported to 22.05.x for upcoming 22.05.11

selenium/patrons_search.t is failing, you should not ignore CI failures!

It can be recreated easily, if you empty DefaultPatronSearchFields and search for patrons, you get a 500

[2023/11/29 10:14:36] [ERROR] GET /api/v1/patrons: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'me.middle_name' in 'where clause' at /usr/share/perl5/Data/Page.pm line 49>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73)
Comment 17 Lucas Gass 2023-11-29 15:05:26 UTC
(In reply to Jonathan Druart from comment #16)
> (In reply to Lucas Gass from comment #14)
> > Backported to 22.05.x for upcoming 22.05.11
> 
> selenium/patrons_search.t is failing, you should not ignore CI failures!
> 
> It can be recreated easily, if you empty DefaultPatronSearchFields and
> search for patrons, you get a 500
> 
> [2023/11/29 10:14:36] [ERROR] GET /api/v1/patrons: unhandled exception
> (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI
> Exception: DBD::mysql::st execute failed: Unknown column 'me.middle_name' in
> 'where clause' at /usr/share/perl5/Data/Page.pm line 49>>
> Koha::REST::Plugin::Exceptions::__ANON__
> /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73)

Do we open a new bug for this?
Comment 18 Jonathan Druart 2023-11-29 15:20:31 UTC
what's better for you.