| Summary: | patrons search should match substrings | ||
|---|---|---|---|
| Product: | Koha | Reporter: | MJ Ray (software.coop) <mjr> |
| Component: | Patrons | Assignee: | MJ Ray (software.coop) <mjr> |
| Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | chris, gmcharlt, jonathan.druart, kyle |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | http://intranet/cgi-bin/koha/members/member.pl | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 8516 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 9070: patrons search should match substrings
Bug 9407: patrons search should match substrings Bug 9407: patrons search should match substrings [SIGNED-OFF] Bug 9407: patrons search should match substrings Bug 9407: patrons search should match substrings |
||
|
Description
MJ Ray (software.coop)
2013-01-16 19:14:00 UTC
Created attachment 14636 [details] [review] Bug 9070: patrons search should match substrings If a patron lives at 4345 Library Rd, a search on Street Address for Library should find it. However, it does not, but a search for 4345 Library does. This patch adds a "Search Type" drop-down, defaulting to the current behaviour. To test: 1) Search for a patron based on a non-leading part of a field. 2) Should return no results. 3) Search again with "Search Type" of "Contains". 4) Should return the patron. Created attachment 14637 [details] [review] Bug 9407: patrons search should match substrings If a patron lives at 4345 Library Rd, a search on Street Address for Library should find it. However, it does not, but a search for 4345 Library does. This patch adds a "Search Type" drop-down, defaulting to the current behaviour. To test: 1) Search for a patron based on a non-leading part of a field. 2) Should return no results. 3) Search again with "Search Type" of "Contains". 4) Should return the patron. Hi MJ, could you resubmit your patch please? Applying: Bug 9407: patrons search should match substrings fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 9407: patrons search should match substrings Created attachment 14659 [details] [review] Bug 9407: patrons search should match substrings If a patron lives at 4345 Library Rd, a search on Street Address for Library should find it. However, it does not, but a search for 4345 Library does. This patch adds a "Search Type" drop-down, defaulting to the current behaviour. To test: 1) Search for a patron based on a non-leading part of a field. 2) Should return no results. 3) Search again with "Search Type" of "Contains". 4) Should return the patron. MJ,
Thanks!
I will signoff your patch but I have 1 question:
Don't you think the following change gives the code easier to read?
-my $searchtype = $input->param('searchtype');
-my %searchtype_ok = ( 'contain' => 1 );
-if ( !defined($searchtype_ok{$searchtype}) ) {
- undef $searchtype;
-}
-
my $from = ( $startfrom - 1 ) * $resultsperpage;
my $to = $from + $resultsperpage;
my ($count,$results);
if ($member || keys %$patron) {
#($results)=Search($member || $patron,{surname=>1,firstname=>1},[$from,$to],undef,["firstname","surname","email","othernames"] );
- my $search_scope = $searchtype || ( $quicksearch ? "field_start_with" : "start_with" );
+ my $search_scope = ( $input->param('searchtype') eq 'contain' )
+ ? 'contain'
+ : ( $quicksearch ? "field_start_with" : "start_with" );
That change makes the code easier to read for now, but would have to be changed to something like the perlfaq4 idiom I used if C4::Patrons::Search (and C4::SQLHelper::SearchInTable) gains other possible values that we want to allow, or if someone chooses to add the currently-supported "exact" type. Should I add a comment to note that I'm using the perlfaq4 idiom to ease future expansion, or use the simpler form for now? Created attachment 14698 [details] [review] [SIGNED-OFF] Bug 9407: patrons search should match substrings If a patron lives at 4345 Library Rd, a search on Street Address for Library should find it. However, it does not, but a search for 4345 Library does. This patch adds a "Search Type" drop-down, defaulting to the current behaviour. To test: 1) Search for a patron based on a non-leading part of a field. 2) Should return no results. 3) Search again with "Search Type" of "Contains". 4) Should return the patron. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to comment #6) > That change makes the code easier to read for now, but would have to be > changed to something like the perlfaq4 idiom I used if C4::Patrons::Search > (and C4::SQLHelper::SearchInTable) gains other possible values that we want > to allow, or if someone chooses to add the currently-supported "exact" type. > Should I add a comment to note that I'm using the perlfaq4 idiom to ease > future expansion, or use the simpler form for now? MJ, I think it is more complicated to maintain a code in a philosophy of "think about the future if... and if ...". To me the code must be simple at a given moment. So I am in favour of not modifying the code until someone add another value. It was just a proposition and you presented arguments so the patch can pass QA :) Created attachment 14736 [details] [review] Bug 9407: patrons search should match substrings If a patron lives at 4345 Library Rd, a search on Street Address for Library should find it. However, it does not, but a search for 4345 Library does. This patch adds a "Search Type" drop-down, defaulting to the current behaviour. To test: 1) Search for a patron based on a non-leading part of a field. 2) Should return no results. 3) Search again with "Search Type" of "Contains". 4) Should return the patron. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> This patch has been pushed to master. Pushed to 3.10.x and 3.8.x will be in 3.10.3 ad 3.8.10 |