Bugzilla – Attachment 4506 Details for
Bug 6513
Library and Category filters when searching for a borrower to add to a routing list don't work correctly.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 6513 - make the routing list search work properly, and faster too.
Bug-6513---make-the-routing-list-search-work-prope.patch (text/plain), 11.19 KB, created by
Robin Sheat
on 2011-06-20 06:06:00 UTC
(
hide
)
Description:
Bug 6513 - make the routing list search work properly, and faster too.
Filename:
MIME Type:
Creator:
Robin Sheat
Created:
2011-06-20 06:06:00 UTC
Size:
11.19 KB
patch
obsolete
>From d90445b4f4b58abe98c91a931d003a7989286ee7 Mon Sep 17 00:00:00 2001 >From: Robin Sheat <robin@catalyst.net.nz> >Date: Mon, 20 Jun 2011 11:15:01 +1200 >Subject: [PATCH] Bug 6513 - make the routing list search work properly, and faster too. > >--- > C4/Members.pm | 71 ++++++++++++- > .../prog/en/modules/serials/member-search.tt | 7 +- > serials/member-search.pl | 111 ++++++++----------- > 3 files changed, 119 insertions(+), 70 deletions(-) > >diff --git a/C4/Members.pm b/C4/Members.pm >index e03b40b..07a06b3 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -124,6 +124,10 @@ BEGIN { > &fixup_cardnumber > &checkcardnumber > ); >+ >+ push @EXPORT_OK, qw( >+ FindByPartialName >+ ); > } > > =head1 NAME >@@ -285,10 +289,14 @@ C<&search_on_fields> is an array ref to the fieldnames you want to limit search > > C<&searchtype> is a string telling the type of search you want todo : start_with, exact or contains are allowed > >+C<skipExtendedAttributes> is a boolean that tells us to avoid searching the >+extended attributes even if we otherwise would. >+ > =cut > > sub Search { >- my ( $filter, $orderby, $limit, $columns_out, $search_on_fields, $searchtype ) = @_; >+ my ( $filter, $orderby, $limit, $columns_out, $search_on_fields, >+ $searchtype, $skipExtendedAttributes ) = @_; > my @filters; > my %filtersmatching_record; > my @finalfilter; >@@ -297,7 +305,8 @@ sub Search { > } else { > push @filters, $filter; > } >- if ( C4::Context->preference('ExtendedPatronAttributes') ) { >+ if (!$skipExtendedAttributes && >+ C4::Context->preference('ExtendedPatronAttributes') ) { > my $matching_records = C4::Members::Attributes::SearchIdMatchingAttribute($filter); > if(scalar(@$matching_records)>0) { > foreach my $matching_record (@$matching_records) { >@@ -314,6 +323,64 @@ sub Search { > return ($data); > } > >+=head2 FindByPartialName >+ >+ @result = FindByPartialName('string', categorycode => 'C', branchcode => 'L' ); >+ >+This is a simplified version of the "Search" function above. It compares what >+has been entered against the surname, firstname, othernames, and email as >+a prefix, a literal match against the barcode, and allows filtering by the >+user's library and category. >+ >+=head3 RETURNS >+ >+An array of hashrefs that contain the values from the borrowers table of things >+that match. >+ >+=head3 NOTES >+ >+Supplying all fields as blank or C<undef> will return all users. Searches that >+have no result will end up in an empty list. If you get undef, there was >+an error of some sort. >+ >+=cut >+ >+sub FindByPartialName { >+ my ($prefix, %options) = @_; >+ >+ my $qry_start = 'SELECT * FROM borrowers'; >+ my ($qry_end, @params); >+ if ($prefix) { >+ $qry_end .= <<EOH; >+(surname LIKE ? OR >+firstname LIKE ? OR >+othernames LIKE ? OR >+email LIKE ? OR >+cardnumber=?) >+EOH >+ push @params, ("$prefix%","$prefix%","$prefix%","$prefix%", $prefix); >+ } >+ foreach my $field ("categorycode", "branchcode") { >+ next if !$options{$field}; >+ my $val = $options{$field}; >+ my $q = "($field=?)"; >+ push @params, $val; >+ $qry_end .= ($qry_end ? " AND $q" : $q) . "\n"; >+ } >+ my $qry_sort = " ORDER BY surname,firstname DESC"; >+ my $query = "$qry_start " . ($qry_end ? "WHERE $qry_end" : '') . " $qry_sort"; >+ >+ my $dbh = C4::Context->dbh; >+ my $sth = $dbh->prepare($query); >+ eval { $sth->execute(@params) }; >+ if ($@) { >+ warn $@; >+ return undef; >+ } >+ my $result = $sth->fetchall_arrayref( {} ); >+ return @$result; >+} >+ > =head2 GetMemberDetails > > ($borrower) = &GetMemberDetails($borrowernumber, $cardnumber); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/member-search.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/member-search.tt >index 91172be..67e3614 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/member-search.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/member-search.tt >@@ -24,10 +24,9 @@ function add_member(subscriptionid,borrowernumber){ > <form action="[% actionname %]" method="get"> > <input type="hidden" name="subscriptionid" id="subscriptionid" value="[% subscriptionid %]" /> > <fieldset class="rows"> >- <legend> Filter :</legend> >+ <legend> Filter:</legend> > <ol> >- <li><label for="member">Name:</label> <input type="hidden" name="surname" value="[% surname %]" /> >- <input type="text" name="member" id="member" value="[% member %]" /></li> >+ <li><label for="member">Name:</label><input type="text" name="member" id="member" value="[% member %]" /></li> > <li><label for="branchcode"> Library :</label><select name="branchcode" id="branchcode"> > <option value="">Any</option>[% FOREACH branchloo IN branchloop %] > [% IF ( branchloo.selected ) %] >@@ -46,7 +45,7 @@ function add_member(subscriptionid,borrowernumber){ > </form> > </div> > [% IF ( resultsloop ) %] >-<div id="searchheader" style="margin-top:.7em;"> <h3>Results [% from %] to [% to %] of [% numresults %] found for [% IF ( member ) %]'<span class="ex">[% member %]</span>'[% END %][% IF ( surname ) %]'<span class="ex">[% surname %]</span>'[% END %]</h3></div> >+<div id="searchheader" style="margin-top:.7em;"> <h3>Results [% from %] to [% to %] of [% numresults %] found [% IF ( member ) %]for '<span class="ex">[% member %]</span>'[% END %]</h3></div> > <div class="searchresults"> > > <table id="memberresultst"> >diff --git a/serials/member-search.pl b/serials/member-search.pl >index f027df5..60b19e3 100755 >--- a/serials/member-search.pl >+++ b/serials/member-search.pl >@@ -28,7 +28,7 @@ use warnings; > use CGI; > use C4::Auth; # get_template_and_user > use C4::Output; >-use C4::Members; # BornameSearch >+use C4::Members qw/ FindByPartialName /; > use C4::Branch; > use C4::Category; > use File::Basename; >@@ -38,19 +38,17 @@ my $theme = $cgi->param('theme') || "default"; > my $resultsperpage = $cgi->param('resultsperpage')||C4::Context->preference("PatronsPerPage")||20; > my $startfrom = $cgi->param('startfrom')||1; > >-my $patron = $cgi->Vars; >-foreach (keys %$patron){ >- delete $$patron{$_} unless($$patron{$_}); >-} >+my $member=$cgi->param('member'); >+my $branchcode = $cgi->param('branchcode'); >+my $categorycode = $cgi->param('categorycode'); > > my @categories=C4::Category->all; >-my $branches=(defined $$patron{branchcode}?GetBranchesLoop($$patron{branchcode}):GetBranchesLoop()); >-my $subscriptionid = $cgi->param('subscriptionid'); >-my $searchstring = $cgi->param('member'); >+my @branches=@{ GetBranchesLoop( >+ $branchcode || undef >+) }; > >-my %categories_dislay; >-my ($template, $loggedinuser, $cookie); >- ($template, $loggedinuser, $cookie) >+my %categories_display; >+my ($template, $loggedinuser, $cookie) > = get_template_and_user({template_name => "serials/member-search.tmpl", > query => $cgi, > type => "intranet", >@@ -63,84 +61,69 @@ foreach my $category (@categories){ > category_description=>$$category{description}, > category_type=>$$category{category_type} > }; >- $categories_dislay{$$category{categorycode}} = $hash; >+ $categories_display{$$category{categorycode}} = $hash; > } >-$template->param( >- "AddPatronLists_".C4::Context->preference("AddPatronLists")=> "1", >- ); > if (C4::Context->preference("AddPatronLists")=~/code/){ > $categories[0]->{'first'}=1; > } > >-my $member=$cgi->param('member'); >-my $orderby=$cgi->param('orderby'); >-$orderby = "surname,firstname" unless $orderby; >-if (defined $member) { >- $member =~ s/,//g; #remove any commas from search string >- $member =~ s/\*/%/g; >-} > >-my ($count,$results); >+my ($count,@results) = (0); >+ >+my $shouldsearch = $branchcode || $categorycode || $member; > > if (C4::Context->preference("IndependantBranches")){ > if (C4::Context->userenv && C4::Context->userenv->{flags} % 2 !=1 && C4::Context->userenv->{'branch'}){ >- $$patron{branchcode}=C4::Context->userenv->{'branch'} unless (C4::Context->userenv->{'branch'} eq "insecure"); >+ $branchcode = C4::Context->userenv->{'branch'} unless (C4::Context->userenv->{'branch'} eq "insecure"); > } > } >-$$patron{firstname}.="\%" if ($$patron{firstname}); >-$$patron{surname}.="\%" if ($$patron{surname}); > >-my @searchpatron; >-push @searchpatron, $member if ($member); >-push @searchpatron, $patron if ( keys %$patron ); >+ > my $from = ( $startfrom - 1 ) * $resultsperpage; > my $to = $from + $resultsperpage; >-if (@searchpatron) { >- ($results) = Search( >- \@searchpatron, >- [ { surname => 0 }, { firstname => 0 } ], >- undef, >- undef, >- [ "firstname", "surname", "email", "othernames", "cardnumber" ], >- "start_with" >+if ($shouldsearch) { >+ @results = FindByPartialName( >+ $member, >+ categorycode => $categorycode, >+ branchcode => $branchcode, > ); > } >-if ($results) { >- $count = scalar(@$results); >+if (@results) { >+ $count = @results; > } > my @resultsdata; > $to=($count>$to?$to:$count); > my $index=$from; >-foreach my $borrower(@$results[$from..$to-1]){ >+foreach my $borrower (@results[$from..$to-1]){ > # find out stats >- $borrower->{'dateexpiry'}= C4::Dates->new($borrower->{'dateexpiry'},'iso')->output('syspref'); >- if ($categories_dislay{$borrower->{'categorycode'}}){ >+ if ($categories_display{$borrower->{'categorycode'}}){ > my %row = ( >- count => $index++, >- %$borrower, >- %{$categories_dislay{$$borrower{categorycode}}}, >- ); >- push(@resultsdata, \%row); >- } >- else { >- warn $borrower->{'cardnumber'} ." has a bad category code of " . $borrower->{'categorycode'} ."\n"; >+ count => $index++, >+ %$borrower, >+ %{$categories_display{$$borrower{categorycode}}}, >+ ); >+ push(@resultsdata, \%row); >+ } else { >+ warn $borrower->{'cardnumber'} ." has a bad category code of " . $borrower->{'categorycode'} ."\n"; > } > } >-if ($$patron{branchcode}){ >- foreach my $branch (grep{$_->{value} eq $$patron{branchcode}}@$branches){ >+if ($branchcode){ >+ foreach my $branch (grep {$_->{value} eq $branchcode} @branches){ > $$branch{selected}=1; > } > } >-if ($$patron{categorycode}){ >- foreach my $category (grep{$_->{categorycode} eq $$patron{categorycode}}@categories){ >+if ($categorycode){ >+ foreach my $category (grep {$_->{categorycode} eq $categorycode} @categories){ > $$category{selected}=1; > } > } >-my %parameters= >-( %{$patron}, >- 'orderby' => $orderby, >- 'resultsperpage' => $resultsperpage, >- 'type'=> 'intranet'); >+my %parameters=( >+ member => $member, >+ categorycode => $categorycode, >+ branchcode => $branchcode, >+ resultsperpage => $resultsperpage, >+ type => 'intranet' >+); > my $base_url = > 'member-search.pl?&' > . join( >@@ -158,18 +141,18 @@ $template->param( > to => $to, > multipage => ($count != $to+1 || $startfrom!=1), > ); >+ > $template->param( >- branchloop=>$branches, >+ branchloop=>\@branches, > categoryloop=>\@categories, > ); > >- > $template->param( >- searching => "1", >+ searching => $shouldsearch, > actionname => basename($0), >- %$patron, >+ %parameters, > numresults => $count, > resultsloop => \@resultsdata, >- ); >+); > > output_html_with_http_headers $cgi, $cookie, $template->output; >-- >1.7.4.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=4506">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 6513
:
4506
|
4507
|
4870
|
4871