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.m.hall, kyle |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
URL: | http://intranet/cgi-bin/koha/members/member.pl | ||
GIT URL: | Change sponsored?: | --- | |
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 |