Bug 34094 - Apply DefaultPatronSearchMethod to all patron search forms
Summary: Apply DefaultPatronSearchMethod to all patron search forms
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 28726 34083 34106
Blocks: 34253 34092
  Show dependency treegraph
 
Reported: 2023-06-22 11:44 UTC by Martin Renvoize
Modified: 2023-12-28 20:47 UTC (History)
12 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,23.05.02,22.11.08


Attachments
Bug 34094: Use DefaultPatronSearchMethod consistently (10.45 KB, patch)
2023-06-22 14:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34094: Use DefaultPatronSearchMethod consistently (9.77 KB, patch)
2023-06-27 11:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34094: Use DefaultPatronSearchMethod consistently (9.81 KB, patch)
2023-06-27 13:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34094: Use DefaultPatronSearchMethod consistently (9.81 KB, patch)
2023-06-28 12:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34094: Use DefaultPatronSearchMethod consistently (9.77 KB, patch)
2023-06-28 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34094: Use DefaultPatronSearchMethod consistently (9.81 KB, patch)
2023-06-28 15:47 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 34094: Use DefaultPatronSearchMethod consistently (9.91 KB, patch)
2023-06-30 06:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34094: (follow-up) Fix failing unit test (1.98 KB, patch)
2023-07-11 13:28 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2023-06-22 11:44:16 UTC
With Bug 33117 we introduced a new system preference to allow administrators the ability to define the default search method for patron searching (starts_with|contains).

However, on that bug, it was highlighted that the various search forms exposed are already out of sync with regards to this search method..  I tried to fix it inline on that bug with a follow-up, but it proved more involved that I first expected.. this bug serves to track progress on the syncing.
Comment 1 Martin Renvoize 2023-06-22 14:41:19 UTC
Created attachment 152567 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.
Comment 2 Martin Renvoize 2023-06-22 14:41:52 UTC
Pretty sure I've missed some spots here, so would love some thorough testing and feedback.
Comment 3 Martin Renvoize 2023-06-27 11:50:20 UTC
Created attachment 152726 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.
Comment 4 Martin Renvoize 2023-06-27 13:41:18 UTC
Created attachment 152742 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.
Comment 5 Martin Renvoize 2023-06-27 13:46:07 UTC
Rebased on 34106 as discussed with Pedro.. this patch included most of 34106... but it didn't clearly highlight it was doing so.. nice to keep them distinct.
Comment 6 Lucas Gass 2023-06-27 19:11:56 UTC
*** Bug 34106 has been marked as a duplicate of this bug. ***
Comment 7 Pedro Amorim 2023-06-28 08:26:22 UTC
Doesn't apply anymore now that bug 34106 was marked as duplicate.
Comment 8 Martin Renvoize 2023-06-28 12:31:18 UTC
Created attachment 152787 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.
Comment 9 Sam Lau 2023-06-28 15:06:44 UTC
Hi Martin, 
I'm running into one main issue when trying this patch. After applying, all the search forms have the same method as expected, however, the patron table is not loading for me. Thus, none of the searches are giving me back any results. 

I was able to fix this issue by removing this line in koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc:

    let filters = [];

Upon removing this line, everything seemed to be working fine for me
Comment 10 Martin Renvoize 2023-06-28 15:35:48 UTC
Ah, that looks like a rebase error.. I think that particular line was removed in a recently pushed patch and then re-introduced here accidentally as part of a rebase.

I will remove it.
Comment 11 Martin Renvoize 2023-06-28 15:37:39 UTC
Created attachment 152827 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.
Comment 12 Sam Lau 2023-06-28 15:47:56 UTC
Created attachment 152828 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 13 Marcel de Rooy 2023-06-30 06:26:34 UTC
Created attachment 152881 [details] [review]
Bug 34094: Use DefaultPatronSearchMethod consistently

This patch updates the patron search bar and pages to default search
method to that defined by DefaultPatronSearchMethod system preference.

Test plan
1) Prior to this patch confirm that regardless of what you set
   DefaultPatronSearchMethod to, the search in /members/member.pl,
   members/members-home.pl and the search from the patrons search top bar
   all default to 'contains'.
2) Apply the patch
3) Confirm that the system preference now affects the default option for
   match type upon page load.

Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Tomás Cohen Arazi 2023-07-03 17:51:13 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 15 Jonathan Druart 2023-07-04 12:54:31 UTC
This patch broke patrons_search.t

    #   Failed test at t/db_dependent/selenium/patrons_search.t line 247.
    #          got: 'No entries to show (filtered from 80 total entries)'
    #     expected: 'Showing 1 to 15 of 26 entries (filtered from 80 total entries)'
Comment 16 Martin Renvoize 2023-07-11 13:28:13 UTC
Created attachment 153324 [details] [review]
Bug 34094: (follow-up) Fix failing unit test

The unit tests were written to assume we always do a 'contains' search,
but the new system preference allows for 'starts_with' or 'contains'.

As such, for the tests to pass, we need to explicitly mock the
preference to 'contains'
Comment 17 Tomás Cohen Arazi 2023-07-12 17:32:02 UTC
Follow-up pushed, thanks Martin!
Comment 18 Martin Renvoize 2023-07-17 15:05:30 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 19 Matt Blenkinsop 2023-07-18 13:12:09 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x