Bug 30576 - DefaultPatronSearchFields no longer takes effect
Summary: DefaultPatronSearchFields no longer takes effect
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 30063 30626
Blocks: 30639 21978
  Show dependency treegraph
 
Reported: 2022-04-21 11:00 UTC by Martin Renvoize
Modified: 2022-05-17 12:38 UTC (History)
5 users (show)

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


Attachments
Bug 30576: Use DefaultPatronSearchFields in patron search (5.16 KB, patch)
2022-04-21 12:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30576: Use DefaultPatronSearchFields in patron search (5.24 KB, patch)
2022-04-21 12:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30576: Use DefaultPatronSearchFields in patron search (5.24 KB, patch)
2022-04-22 14:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests (7.49 KB, patch)
2022-04-22 14:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30576: Use DefaultPatronSearchFields in patron search (5.31 KB, patch)
2022-04-27 11:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests (7.57 KB, patch)
2022-04-27 11:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30576: Use DefaultPatronSearchFields in patron search (5.31 KB, patch)
2022-04-27 15:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests (12.54 KB, patch)
2022-04-27 15:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30576: Use DefaultPatronSearchFields in patron search (5.36 KB, patch)
2022-04-28 10:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests (12.60 KB, patch)
2022-04-28 10:04 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 Martin Renvoize 2022-04-21 11:00:08 UTC
Searching from the header search for patrons in the staff client no longer takes notice of the DefaultPatronSearchFields preference... and 'Standard' search once on the member search page, also doesn't reflect the preference either.

To replicate:
1) Remove a field or two from the preference, but ensure at least some fields are present
2) Submit a patron search
3) Note that the search appears to user the 'Standard' option in the search
4) Inspect the network traffic and note that the fields searched upon do not reflect the preference.
Comment 1 Martin Renvoize 2022-04-21 12:08:05 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2022-04-21 12:53:49 UTC
Created attachment 133555 [details] [review]
Bug 30576: Use DefaultPatronSearchFields in patron search

This patch corrects the handling of the 'Standard' search field in
patron searches such that it accurately reflects the contents of the
DefaultPatronSearchFields system preference and falls back to a hard
coded list instead of always using the hard coded list.

To test:
1) Empty the contents of DefaultPatronSearchFields systempreference
2) Perform a patron search from the header search
3) Inspect the patrons API request and note that we search on the
   default fallback fields of firstname, surname, othernames,
   cardnumber and userid (and some extended attributes which are
   historically hard coded).
4) Navigate to the memebers-home by clicking the 'Patrons' button from
   the main page
5) Perform another search using the filters on the left with 'Standard'
   selected.
6) Note the same list of search fields used as above in the patrons API
   request.
7) Update the DefaultPatronSearchFields systempreference adding one or
   more borrower fields in a comma delimited list.
8) Repeat steps 2 -> 6 but this time upon inspecting the API request you
   should find that only the fields in your DefaultPatronSearchFields
   list are searched (along with the hard coded extended attributes
   list).
9) Signoff

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Jonathan Druart 2022-04-22 09:25:04 UTC
t/db_dependent/selenium/patrons_search.t should pass after this patch. We are fixing a bug but introducing a regression. Double check the syspref's description.
Comment 4 Jonathan Druart 2022-04-22 13:31:21 UTC
Behaviour on 21.11.x:

1. DefaultPatronSearchFields has the default value (surname,firstname,othernames,cardnumber,userid)
=> standard will search "standard" (retrieved from the pref in the pm)

2. DefaultPatronSearchFields=surname,initials,borrowernotes
=> standard will search "standard" (retrieved from the pref in the pm)
=> 2 new options are added to the dropdown, "Initials" and "Circulation note"
Comment 5 Martin Renvoize 2022-04-22 14:21:56 UTC
Created attachment 133684 [details] [review]
Bug 30576: Use DefaultPatronSearchFields in patron search

This patch corrects the handling of the 'Standard' search field in
patron searches such that it accurately reflects the contents of the
DefaultPatronSearchFields system preference and falls back to a hard
coded list instead of always using the hard coded list.

To test:
1) Empty the contents of DefaultPatronSearchFields systempreference
2) Perform a patron search from the header search
3) Inspect the patrons API request and note that we search on the
   default fallback fields of firstname, surname, othernames,
   cardnumber and userid (and some extended attributes which are
   historically hard coded).
4) Navigate to the memebers-home by clicking the 'Patrons' button from
   the main page
5) Perform another search using the filters on the left with 'Standard'
   selected.
6) Note the same list of search fields used as above in the patrons API
   request.
7) Update the DefaultPatronSearchFields systempreference adding one or
   more borrower fields in a comma delimited list.
8) Repeat steps 2 -> 6 but this time upon inspecting the API request you
   should find that only the fields in your DefaultPatronSearchFields
   list are searched (along with the hard coded extended attributes
   list).
9) Signoff

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Martin Renvoize 2022-04-22 14:22:00 UTC
Created attachment 133685 [details] [review]
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests

The unit tests highlighted my original patch didn't cover the full
preference description.

We now replace the 'standard' option with the fields from the preference
and we also add those fields as options to the field selection in
advanced searches.

This patch also adjusts the tests to test for that and reflects the
expected changes to the number of options displayed in the select boxes.
Comment 7 Martin Renvoize 2022-04-22 14:23:38 UTC
As discussed, there is still an issue should someone add a bad field into the system preference.. but we decided that was out of scope for here.

I'm also still stuck with one last test failure.. I have no idea how this patchset causes it.. hopefully it'll be something obvious for Jonathan to spot.
Comment 8 Martin Renvoize 2022-04-22 14:43:41 UTC
Ho.. that last failure is also on master.. I'll fix it on master
Comment 9 Jonathan Druart 2022-04-27 11:34:58 UTC
Created attachment 134059 [details] [review]
Bug 30576: Use DefaultPatronSearchFields in patron search

This patch corrects the handling of the 'Standard' search field in
patron searches such that it accurately reflects the contents of the
DefaultPatronSearchFields system preference and falls back to a hard
coded list instead of always using the hard coded list.

To test:
1) Empty the contents of DefaultPatronSearchFields systempreference
2) Perform a patron search from the header search
3) Inspect the patrons API request and note that we search on the
   default fallback fields of firstname, surname, othernames,
   cardnumber and userid (and some extended attributes which are
   historically hard coded).
4) Navigate to the memebers-home by clicking the 'Patrons' button from
   the main page
5) Perform another search using the filters on the left with 'Standard'
   selected.
6) Note the same list of search fields used as above in the patrons API
   request.
7) Update the DefaultPatronSearchFields systempreference adding one or
   more borrower fields in a comma delimited list.
8) Repeat steps 2 -> 6 but this time upon inspecting the API request you
   should find that only the fields in your DefaultPatronSearchFields
   list are searched (along with the hard coded extended attributes
   list).
9) Signoff

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2022-04-27 11:35:04 UTC
Created attachment 134060 [details] [review]
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests

The unit tests highlighted my original patch didn't cover the full
preference description.

We now replace the 'standard' option with the fields from the preference
and we also add those fields as options to the field selection in
advanced searches.

This patch also adjusts the tests to test for that and reflects the
expected changes to the number of options displayed in the select boxes.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2022-04-27 11:36:03 UTC
(In reply to Martin Renvoize from comment #8)
> Ho.. that last failure is also on master.. I'll fix it on master

Fixed on bug 30626.
Comment 12 Martin Renvoize 2022-04-27 15:46:04 UTC
Created attachment 134084 [details] [review]
Bug 30576: Use DefaultPatronSearchFields in patron search

This patch corrects the handling of the 'Standard' search field in
patron searches such that it accurately reflects the contents of the
DefaultPatronSearchFields system preference and falls back to a hard
coded list instead of always using the hard coded list.

To test:
1) Empty the contents of DefaultPatronSearchFields systempreference
2) Perform a patron search from the header search
3) Inspect the patrons API request and note that we search on the
   default fallback fields of firstname, surname, othernames,
   cardnumber and userid (and some extended attributes which are
   historically hard coded).
4) Navigate to the memebers-home by clicking the 'Patrons' button from
   the main page
5) Perform another search using the filters on the left with 'Standard'
   selected.
6) Note the same list of search fields used as above in the patrons API
   request.
7) Update the DefaultPatronSearchFields systempreference adding one or
   more borrower fields in a comma delimited list.
8) Repeat steps 2 -> 6 but this time upon inspecting the API request you
   should find that only the fields in your DefaultPatronSearchFields
   list are searched (along with the hard coded extended attributes
   list).
9) Signoff

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Martin Renvoize 2022-04-27 15:46:08 UTC
Created attachment 134085 [details] [review]
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests

The unit tests highlighted my original patch didn't cover the full
preference description.

We now replace the 'standard' option with the fields from the preference
and we also add those fields as options to the field selection in
advanced searches.

This patch also adjusts the tests to test for that and reflects the
expected changes to the number of options displayed in the select boxes.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Martin Renvoize 2022-04-27 15:46:41 UTC
Tiny rebase after setting dependencies to make sure these two are pushed in order :)
Comment 15 Nick Clemens 2022-04-28 10:04:24 UTC
Created attachment 134211 [details] [review]
Bug 30576: Use DefaultPatronSearchFields in patron search

This patch corrects the handling of the 'Standard' search field in
patron searches such that it accurately reflects the contents of the
DefaultPatronSearchFields system preference and falls back to a hard
coded list instead of always using the hard coded list.

To test:
1) Empty the contents of DefaultPatronSearchFields systempreference
2) Perform a patron search from the header search
3) Inspect the patrons API request and note that we search on the
   default fallback fields of firstname, surname, othernames,
   cardnumber and userid (and some extended attributes which are
   historically hard coded).
4) Navigate to the memebers-home by clicking the 'Patrons' button from
   the main page
5) Perform another search using the filters on the left with 'Standard'
   selected.
6) Note the same list of search fields used as above in the patrons API
   request.
7) Update the DefaultPatronSearchFields systempreference adding one or
   more borrower fields in a comma delimited list.
8) Repeat steps 2 -> 6 but this time upon inspecting the API request you
   should find that only the fields in your DefaultPatronSearchFields
   list are searched (along with the hard coded extended attributes
   list).
9) Signoff

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens 2022-04-28 10:04:29 UTC
Created attachment 134212 [details] [review]
Bug 30576: (follow-up) Corrections to behaviour to reflect unit tests

The unit tests highlighted my original patch didn't cover the full
preference description.

We now replace the 'standard' option with the fields from the preference
and we also add those fields as options to the field selection in
advanced searches.

This patch also adjusts the tests to test for that and reflects the
expected changes to the number of options displayed in the select boxes.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Fridolin Somers 2022-04-28 20:50:52 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄