Bugzilla – Attachment 72523 Details for
Bug 20346
Patron filtering resulting in new search rather than filtering existing results
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20346 - Server side patron filtering implemented
Bug-20346---Server-side-patron-filtering-implement.patch (text/plain), 19.93 KB, created by
Alex Buckley
on 2018-03-07 23:08:59 UTC
(
hide
)
Description:
Bug 20346 - Server side patron filtering implemented
Filename:
MIME Type:
Creator:
Alex Buckley
Created:
2018-03-07 23:08:59 UTC
Size:
19.93 KB
patch
obsolete
>From e9e6adb764710cb7e47451a6e5467ac22ad63ce1 Mon Sep 17 00:00:00 2001 >From: Alex Buckley <alexbuckley@catalyst.net.nz> >Date: Wed, 7 Mar 2018 04:51:08 +0000 >Subject: [PATCH] Bug 20346 - Server side patron filtering implemented > >Server side (in the svc/members/search file) perl logic for filtering >results returned from patron searches checks all patron data fields for >a match with the filter value. > >This filtering works in the following steps: when filtering is initiated >the original search using the original search query is run again and >then the filter value is used to filter the resulting data, thereby >using both the search and filter values to easily find a patron. > >The submitted filter value is automatically checked against >every patron value in every patron record returned from the search and >so there is no need for a specific data value to be >selected with the select dropdown box id=searchfieldstype_filter and so >that select dropdown has been removed due to this redundancy. > >This filter works for both filtering based on finding a match containing >the filter value and, filtering based on finding a match starting with >the filter value. > >A tooltip is displayed on the searchmember_filter input box to inform >the user what format they should enter date formats. The tooltip tells >the user to follow the date format of the dateformat syspref, meaning if >the Koha instance has the 'metric' dataformat syspref value set then the >tooltip will tell the user to enter a date value in the format >dd/mm/yyyy > >Test plan: >1. Go to /members/members-home.pl and create 4 users. Make 3 of them have >the same first and last name (e.g. first name = 'Fred' and last name ='Jones') but vary the homebranch, email addresses (e.g. fred@gmail.com, fred@hotmail.com, fredj@gmail.com (make this user have a date of birth of 1st jan 1980)) and make the fourth user have the first and last name of "Fred" "Smith" > >2. Perform a search by writing in the 'Jones' value and you should get 3 results > >3. Filter by writing in the word "Fred", with the search field >="standard" and searchtype="Contains" and this will give you 4 results >because a new search for all users containing "Fred" is performed rather >than filtering the results of the "Jones" search for the term "Fred" > >4. Apply patch > >5. Restart plack, apache2 and memcached > >6. Refresh the patron page > >7. Notice the search field dropdown has been removed because the >filtering now automatically checks all patron data fields. Also notice >when you hover over the filter textbox a tooltip message is displayed >informing you the appropriate format for you to input date values in. >This date format recommended is based on the value of the dateformat >syspref > >8. Repeat step 2 and you should get 3 results > >9. Repeat step 3 and notice you get 3 results because the filtering is >happening on the results of search rather than performing a new search > >10. Perform another filter of the search by inputting the date 1st jan 1980 in the appropriate format described in >the tooltip and the patron page for the patron with the email address >fredj@gmail.com should be displayed > >Sponsored-By: Catalyst IT >--- > C4/Utils/DataTables/Members.pm | 80 +++++++------- > .../prog/en/modules/members/member.tt | 81 +++++--------- > svc/members/search | 117 ++++++++++++++++----- > 3 files changed, 152 insertions(+), 126 deletions(-) > >diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm >index 8912146..395d661 100644 >--- a/C4/Utils/DataTables/Members.pm >+++ b/C4/Utils/DataTables/Members.pm >@@ -9,12 +9,14 @@ use C4::Members::Attributes qw(SearchIdMatchingAttribute ); > sub search { > my ( $params ) = @_; > my $searchmember = $params->{searchmember}; >+ my $firstquery = $params->{firstquery}; > my $firstletter = $params->{firstletter}; > my $categorycode = $params->{categorycode}; > my $branchcode = $params->{branchcode}; > my $searchtype = $params->{searchtype} || 'contain'; > my $searchfieldstype = $params->{searchfieldstype} || 'standard'; > my $dt_params = $params->{dt_params}; >+ my $searchfiltering = $params->{search_filter}; > > unless ( $searchmember ) { > $searchmember = $dt_params->{sSearch} // ''; >@@ -55,46 +57,44 @@ sub search { > }; > } > } >- >- my $select = "SELECT >- borrowers.borrowernumber, borrowers.surname, borrowers.firstname, >- borrowers.streetnumber, borrowers.streettype, borrowers.address, >- borrowers.address2, borrowers.city, borrowers.state, borrowers.zipcode, >- borrowers.country, cardnumber, borrowers.dateexpiry, >- borrowers.borrowernotes, borrowers.branchcode, borrowers.email, >- borrowers.userid, borrowers.dateofbirth, borrowers.categorycode, >- categories.description AS category_description, categories.category_type, >- branches.branchname"; >- my $from = "FROM borrowers >- LEFT JOIN branches ON borrowers.branchcode = branches.branchcode >- LEFT JOIN categories ON borrowers.categorycode = categories.categorycode"; >- my @where_args; >- my @where_strs; >- if(defined $firstletter and $firstletter ne '') { >- push @where_strs, "borrowers.surname LIKE ?"; >- push @where_args, "$firstletter%"; >- } >- if(defined $categorycode and $categorycode ne '') { >- push @where_strs, "borrowers.categorycode = ?"; >- push @where_args, $categorycode; >- } >- if(@restricted_branchcodes ) { >- push @where_strs, "borrowers.branchcode IN (" . join( ',', ('?') x @restricted_branchcodes ) . ")"; >- push @where_args, @restricted_branchcodes; >- } >- >- my $searchfields = { >- standard => C4::Context->preference('DefaultPatronSearchFields') || 'surname,firstname,othernames,cardnumber,userid', >- surname => 'surname', >- email => 'email,emailpro,B_email', >- borrowernumber => 'borrowernumber', >- userid => 'userid', >- phone => 'phone,phonepro,B_phone,altcontactphone,mobile', >- address => 'streettype,address,address2,city,state,zipcode,country', >- dateofbirth => 'dateofbirth', >- sort1 => 'sort1', >- sort2 => 'sort2', >- }; >+ my $select = "SELECT >+ borrowers.borrowernumber, borrowers.surname, borrowers.firstname, >+ borrowers.streetnumber, borrowers.streettype, borrowers.address, >+ borrowers.address2, borrowers.city, borrowers.state, borrowers.zipcode, >+ borrowers.country, cardnumber, borrowers.dateexpiry, >+ borrowers.borrowernotes, borrowers.branchcode, borrowers.email, >+ borrowers.userid, borrowers.dateofbirth, borrowers.categorycode, >+ categories.description AS category_description, categories.category_type, >+ branches.branchname"; >+ my $from = "FROM borrowers >+ LEFT JOIN branches ON borrowers.branchcode = branches.branchcode >+ LEFT JOIN categories ON borrowers.categorycode = categories.categorycode"; >+ my @where_args; >+ my @where_strs; >+ if(defined $firstletter and $firstletter ne '') { >+ push @where_strs, "borrowers.surname LIKE ?"; >+ push @where_args, "$firstletter%"; >+ } >+ if(defined $categorycode and $categorycode ne '') { >+ push @where_strs, "borrowers.categorycode = ?"; >+ push @where_args, $categorycode; >+ } >+ if(@restricted_branchcodes ) { >+ push @where_strs, "borrowers.branchcode IN (" . join( ',', ('?') x @restricted_branchcodes ) . ")"; >+ push @where_args, @restricted_branchcodes; >+ } >+ my $searchfields = { >+ standard => C4::Context->preference('DefaultPatronSearchFields') || 'surname,firstname,othernames,cardnumber,userid', >+ surname => 'surname', >+ email => 'email,emailpro,B_email', >+ borrowernumber => 'borrowernumber', >+ userid => 'userid', >+ phone => 'phone,phonepro,B_phone,altcontactphone,mobile', >+ address => 'streettype,address,address2,city,state,zipcode,country', >+ dateofbirth => 'dateofbirth', >+ sort1 => 'sort1', >+ sort2 => 'sort2', >+ }; > > # * is replaced with % for sql > $searchmember =~ s/\*/%/g; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt >index b40bb03..d4b7168 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt >@@ -109,67 +109,14 @@ > <div class="yui-b"> > <form method="get" id="searchform"> > <input type="hidden" id="firstletter_filter" value="" /> >+ <input type="hidden" id="date_format" value="[% dateformat%]"/> > <fieldset class="brief"> > <h3>Filters</h3> >+ <p> Filter results by entering any value and all data values in the patron record(s) will be checked </p> > <ol> > <li> > <label for="searchmember_filter">Search:</label> >- <input type="text" id="searchmember_filter" value="[% searchmember %]"/> >- </li> >- <li> >- <label for="searchfieldstype_filter">Search fields:</label> >- <select name="searchfieldstype" id="searchfieldstype_filter"> >- [% IF searchfieldstype == "standard" %] >- <option selected="selected" value='standard'>Standard</option> >- [% ELSE %] >- <option value='standard'>Standard</option> >- [% END %] >- [% IF searchfieldstype == "surname" %] >- <option selected="selected" value='surname'>Surname</option> >- [% ELSE %] >- <option value='surname'>Surname</option> >- [% END %] >- [% IF searchfieldstype == "email" %] >- <option selected="selected" value='email'>Email</option> >- [% ELSE %] >- <option value='email'>Email</option> >- [% END %] >- [% IF searchfieldstype == "borrowernumber" %] >- <option selected="selected" value='borrowernumber'>Borrower number</option> >- [% ELSE %] >- <option value='borrowernumber'>Borrower number</option> >- [% END %] >- [% IF searchfieldstype == "userid" %] >- <option selected="selected" value='userid'>Username</option> >- [% ELSE %] >- <option value='userid'>Username</option> >- [% END %] >- [% IF searchfieldstype == "phone" %] >- <option selected="selected" value='phone'>Phone number</option> >- [% ELSE %] >- <option value='phone'>Phone number</option> >- [% END %] >- [% IF searchfieldstype == "address" %] >- <option selected="selected" value='address'>Street address</option> >- [% ELSE %] >- <option value='address'>Street address</option> >- [% END %] >- [% IF searchfieldstype == "dateofbirth" %] >- <option selected="selected" value='dateofbirth'>Date of birth</option> >- [% ELSE %] >- <option value='dateofbirth'>Date of birth</option> >- [% END %] >- [% IF searchfieldstype == "sort1" %] >- <option selected="selected" value='sort1'>Sort field 1</option> >- [% ELSE %] >- <option value='sort1'>Sort field 1</option> >- [% END %] >- [% IF searchfieldstype == "sort2" %] >- <option selected="selected" value='sort2'>Sort field 2</option> >- [% ELSE %] >- <option value='sort2'>Sort field 2</option> >- [% END %] >- </select> >+ <input type="text" id="searchmember_filter" onmouseover="tooltip_display()" value="[% searchmember %]"/> > </li> > <li> > <label for="searchtype_filter">Search type:</label> >@@ -324,12 +271,17 @@ > e.preventDefault(); > clearFilters(true); > }); >- $("#searchform").on("submit", filter); >+ $("#searchform").on("submit", function(e) { >+ e.preventDefault(); >+ search = 1; >+ dtMemberResults.fnDraw(); >+ }); > }); > > var dtMemberResults; > var search = 1; > $(document).ready(function() { >+ var first_query = document.getElementById("searchmember").value; > [% IF searchmember %] > $("#searchmember_filter").val("[% searchmember | html %]"); > [% END %] >@@ -390,6 +342,9 @@ > return; > } > aoData.push({ >+ 'name': 'first_query', >+ 'value': $("#searchmember").val() >+ },{ > 'name': 'searchmember', > 'value': $("#searchmember_filter").val() > },{ >@@ -475,6 +430,18 @@ > update_searched(); > }); > >+ function tooltip_display() { >+ var dateformat= document.getElementById("date_format").value; >+ if ( dateformat == 'us') { >+ $('#searchmember_filter').attr("title","Please enter date inputs in the format mm/dd/yyyy").tooltip('show'); >+ } else if ( dateformat == "metric") { >+ $('#searchmember_filter').attr("title","Please enter date inputs in the format dd/mm/yyyy").tooltip('show'); >+ } else if ( dateformat == "iso") { >+ $('#searchmember_filter').attr("title","Please enter date inputs in the format yyyy-mm-dd").tooltip('show'); >+ } else if ( dateformat == "dmydot") { >+ $('#searchmember_filter').attr("title","Please enter date inputs in the format dd.mm.yyyy").tooltip('show'); >+ } >+ } > // Update the string "Results found ..." > function update_searched(){ > var searched = $("#searchfieldstype_filter").find("option:selected").text(); >diff --git a/svc/members/search b/svc/members/search >index 86c7473..a1d2f72 100755 >--- a/svc/members/search >+++ b/svc/members/search >@@ -26,6 +26,10 @@ use C4::Utils::DataTables qw( dt_get_params ); > use C4::Utils::DataTables::Members qw( search ); > use Koha::DateUtils qw( output_pref dt_from_string ); > use Koha::Patrons; >+use Data::Dumper; >+use Time::Piece; >+use JSON::XS qw( ); >+use HTML::Entities qw( encode_entities ); > > my $input = new CGI; > >@@ -39,6 +43,7 @@ my ($template, $user, $cookie) = get_template_and_user({ > flagsrequired => { borrowers => 'edit_borrowers' } > }); > >+my $first_query = $input->param('first_query'); > my $searchmember = $input->param('searchmember'); > my $firstletter = $input->param('firstletter'); > my $categorycode = $input->param('categorycode'); >@@ -74,42 +79,96 @@ if ( $searchmember > > # Perform the patrons search > $results = C4::Utils::DataTables::Members::search( >- { >- searchmember => $searchmember, >- firstletter => $firstletter, >- categorycode => $categorycode, >- branchcode => $branchcode, >- searchtype => $searchtype, >- searchfieldstype => $searchfieldstype, >- dt_params => \%dt_params, >- } >+ { >+ searchmember => $first_query, >+ firstletter => $firstletter, >+ categorycode => $categorycode, >+ branchcode => $branchcode, >+ searchtype => $searchtype, >+ searchfieldstype => $searchfieldstype, >+ dt_params => \%dt_params, >+ } > ) unless $results; > >-# It is not recommanded to use the has_permission param if you use the pagination >-# The filter is done AFTER requested the data >-if ($has_permission) { >- my ( $permission, $subpermission ) = split /\./, $has_permission; >- my @patrons_with_permission; >- for my $patron ( @{ $results->{patrons} } ) { >- my $perms = haspermission( $patron->{userid} ); >- if ( $perms->{superlibrarian} == 1 >- or $perms->{$permission} == 1 ) >- { >- push @patrons_with_permission, $patron; >- next; >+ # It is not recommanded to use the has_permission param if you use the pagination >+ # The filter is done AFTER requested the data >+ if ($has_permission) { >+ my ( $permission, $subpermission ) = split /\./, $has_permission; >+ my @patrons_with_permission; >+ for my $patron ( @{ $results->{patrons} } ) { >+ my $perms = haspermission( $patron->{userid} ); >+ if ( $perms->{superlibrarian} == 1 >+ or $perms->{$permission} == 1 ) >+ { >+ push @patrons_with_permission, $patron; >+ next; >+ } >+ >+ if ($subpermission) { >+ my $subperms = get_user_subpermissions( $patron->{userid} ); >+ push @patrons_with_permission, $patron >+ if $subperms->{$permission}->{$subpermission}; >+ } > } >- >- if ($subpermission) { >- my $subperms = get_user_subpermissions( $patron->{userid} ); >- push @patrons_with_permission, $patron >- if $subperms->{$permission}->{$subpermission}; >+ $results->{patrons} = \@patrons_with_permission; >+ $results->{iTotalDisplayRecords} = scalar( @patrons_with_permission ); >+ } >+ my $date_format = C4::Context->preference("dateformat"); >+ >+ if ($searchmember) { >+ my @filtered_patron_list; >+ my $count; >+ for my $patron ( @{ $results->{patrons} } ) { >+ my $match; >+ for my $value (values $patron) { >+ warn $value; >+ my $lc_patron_value = lc $value; >+ my $lc_searchmember = lc $searchmember; >+ >+ #Reformat the date value >+ if ($lc_patron_value =~ /^\d{4}-\d\d-\d\d$/) { >+ if ($date_format eq "us" ) { >+ $lc_patron_value = Time::Piece->strptime($lc_patron_value, '%Y-%m-%d')->strftime('%m/%d/%Y'); >+ } elsif ($date_format eq "metric" ) { >+ $lc_patron_value = Time::Piece->strptime($lc_patron_value, '%Y-%m-%d')->strftime('%d/%m/%Y'); >+ } elsif ($date_format eq "iso" ) { >+ $lc_patron_value = Time::Piece->strptime($lc_patron_value, '%Y-%m-%d')->strftime('%Y-%m-%d'); >+ } elsif ($date_format eq "dmydot" ) { >+ $lc_patron_value = Time::Piece->strptime($lc_patron_value, '%Y-%m-%d')->strftime('%d.%m.%Y'); >+ >+ } >+ } >+ warn $lc_patron_value; >+ warn $lc_searchmember; >+ if ($searchtype eq "start_with") { >+ #Find a match starting with the start of a value in the patron record >+ if (index($lc_patron_value, $lc_searchmember) == 0) { >+ $match = 1; >+ warn $lc_patron_value; >+ warn $lc_searchmember; >+ } >+ } else { >+ if (($lc_patron_value) =~ /($lc_searchmember)/) { >+ $match = 1; >+ } >+ } >+ } >+ if ($match) { >+ push @filtered_patron_list, $patron; >+ next; >+ } > } >+ $results->{patrons} = \@filtered_patron_list; >+ $results->{iTotalDisplayRecords} = scalar( @filtered_patron_list ); > } >- $results->{patrons} = \@patrons_with_permission; >- $results->{iTotalDisplayRecords} = scalar( @patrons_with_permission ); >-} >+ >+ my $date_format = C4::Context->preference("dateformat"); >+ my $json = JSON::XS->new->allow_nonref; >+ my $date_format = $json->encode($date_format); >+ my $date_format = encode_entities($date_format); > > $template->param( >+ dateformat => $date_format, > sEcho => $sEcho, > iTotalRecords => $results->{iTotalRecords}, > iTotalDisplayRecords => $results->{iTotalDisplayRecords}, >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20346
:
72496
|
72520
|
72523
|
72623
|
72624
|
72815
|
73520