Bugzilla – Attachment 54361 Details for
Bug 15407
Move the patron categories related code to Koha::Patron::Categories - part 2
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15407: Koha::Patron::Categories - remove sql queries in some pl and pm
Bug-15407-KohaPatronCategories---remove-sql-querie.patch (text/plain), 17.65 KB, created by
Marcel de Rooy
on 2016-08-12 11:45:38 UTC
(
hide
)
Description:
Bug 15407: Koha::Patron::Categories - remove sql queries in some pl and pm
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2016-08-12 11:45:38 UTC
Size:
17.65 KB
patch
obsolete
>From eb8a9df894f21657a34f27e51135673904dd61d4 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 21 Dec 2015 15:04:45 +0000 >Subject: [PATCH] Bug 15407: Koha::Patron::Categories - remove sql queries in > some pl and pm >Content-Type: text/plain; charset=utf-8 > >This patch replaces sql queries done in some pl script and in >C4::Reports::Guided. >Since we have now a Koha::Patron::Categories module, we should use it >where it is possible. > >Test plan: >- Prerequisite: Be sure you have several patron categories created, with > different option enabled, and limit some to certain libraries. >- On the 'Circulation and fine rules' admin page (admin/smart-rules.pl), > all the patron categories should be displayed (even the ones limited to > another library), ordered by description. Try to add/update existing rules. >- On the overdue rules page (tools/overduerules.pl), all the patron > categories with overduenoticerequired set should be displayed. > Try to add/update existing rules. >- On the following reports: > reports/borrowers_stats.pl > reports/issues_avg_stats.pl >The patron categories should be displayed. Note that there is an >inconsistency with these 2 reports: the patron categories limited to >other libraries are displayed on them, when they are not on the other >reports. This should certainly be fixed (on another bug report). > >Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Reports/Guided.pm | 3 +- > admin/smart-rules.pl | 13 +++----- > .../prog/en/modules/admin/smart-rules.tt | 8 ++--- > .../prog/en/modules/reports/borrowers_stats.tt | 15 +++++----- > .../prog/en/modules/reports/issues_avg_stats.tt | 4 +-- > reports/borrowers_stats.pl | 30 ++++++------------- > reports/guided_reports.pl | 12 +++----- > reports/issues_avg_stats.pl | 24 ++++----------- > tools/overduerules.pl | 31 ++++++++++---------- > 9 files changed, 52 insertions(+), 88 deletions(-) > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index c55d54d..631f961 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -30,11 +30,10 @@ use C4::Output; > use XML::Simple; > use XML::Dumper; > use C4::Debug; >-# use Smart::Comments; >-# use Data::Dumper; > use C4::Log; > > use Koha::AuthorisedValues; >+use Koha::Patron::Categories; > > BEGIN { > require Exporter; >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index 9d2d0fd..ecdecee 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -34,6 +34,7 @@ use Koha::Logger; > use Koha::RefundLostItemFeeRule; > use Koha::RefundLostItemFeeRules; > use Koha::Libraries; >+use Koha::Patron::Categories; > > my $input = CGI->new; > my $dbh = C4::Context->dbh; >@@ -468,14 +469,8 @@ for my $thisbranch (sort { $branches->{$a}->{branchname} cmp $branches->{$b}->{b > }; > } > >-my $sth=$dbh->prepare("SELECT description,categorycode FROM categories ORDER BY description"); >-$sth->execute; >-my @category_loop; >-while (my $data=$sth->fetchrow_hashref){ >- push @category_loop,$data; >-} >+my $patron_categories = Koha::Patron::Categories->search({}, { order_by => ['description'] }); > >-$sth->finish; > my @row_loop; > my @itemtypes = @{ GetItemTypes( style => 'array' ) }; > @itemtypes = sort { lc $a->{translated_description} cmp lc $b->{translated_description} } @itemtypes; >@@ -515,7 +510,6 @@ while (my $row = $sth2->fetchrow_hashref) { > } > push @row_loop, $row; > } >-$sth->finish; > > my @sorted_row_loop = sort by_category_and_itemtype @row_loop; > >@@ -624,7 +618,8 @@ if ($defaults) { > > $template->param(default_rules => ($defaults ? 1 : 0)); > >-$template->param(categoryloop => \@category_loop, >+$template->param( >+ patron_categories => $patron_categories, > itemtypeloop => \@itemtypes, > rules => \@sorted_row_loop, > branchloop => \@branchloop, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >index 8d197af..2c32e88 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >@@ -275,8 +275,8 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <td> > <select name="categorycode" id="categorycode"> > <option value="*">All</option> >- [% FOREACH categoryloo IN categoryloop %] >- <option value="[% categoryloo.categorycode %]">[% categoryloo.description %]</option> >+ [% FOREACH patron_category IN patron_categories%] >+ <option value="[% patron_category.categorycode %]">[% patron_category.description %]</option> > [% END %] > </select> > </td> >@@ -545,8 +545,8 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <tr> > <td> > <select name="categorycode"> >- [% FOREACH categoryloo IN categoryloop %] >- <option value="[% categoryloo.categorycode %]">[% categoryloo.description %]</option> >+ [% FOREACH patron_category IN patron_categories%] >+ <option value="[% patron_category.categorycode %]">[% patron_category.description %]</option> > [% END %] > </select> > </td> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt >index bc93aa4..10882da 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt >@@ -77,13 +77,14 @@ > <td>Patron category</td> > <td><input type="radio" name="Line" value="categorycode" /></td> > <td><input type="radio" name="Column" value="categorycode" /></td> >- <td><select name="Filter" size="1" id="catcode"> >- <option value=""></option> >- [% FOREACH CAT_LOO IN CAT_LOOP %] >- <option value="[% CAT_LOO.categorycode %]">[% CAT_LOO.description %]</option> >- [% END %] >- </select> >- </td> >+ <td> >+ <select name="Filter" size="1" id="catcode"> >+ <option value=""></option> >+ [% FOREACH patron_category IN patron_categories %] >+ <option value="[% patron_category.categorycode %]">[% patron_category.description %]</option> >+ [% END %] >+ </select> >+ </td> > </tr> > <tr> > <td>Patron status</td> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/issues_avg_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/issues_avg_stats.tt >index e6cb22f..7b38c76 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/issues_avg_stats.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/issues_avg_stats.tt >@@ -155,8 +155,8 @@ > <td> > <select name="Filter" size="1" id="borcat"> > <option value=""></option> >- [% FOREACH value IN BorCat.values %] >- <option value="[%- value -%]">[%- BorCat.labels.$value -%]</option> >+ [% FOREACH patron_category IN patron_categories %] >+ <option value="[%- patron_category.categorycode -%]">[%- patron_category.description-%]</option> > [% END %] > </select> > </td> >diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl >index 607f9d1..342dad4 100755 >--- a/reports/borrowers_stats.pl >+++ b/reports/borrowers_stats.pl >@@ -31,6 +31,9 @@ use C4::Output; > use C4::Reports; > use C4::Circulation; > use C4::Members::AttributeTypes; >+ >+use Koha::Patron::Categories; >+ > use Date::Calc qw( > Today > Add_Delta_YM >@@ -116,7 +119,8 @@ if ($do_it) { > } else { > my $dbh = C4::Context->dbh; > my $req; >- $template->param( CAT_LOOP => &catcode_aref); >+ my $patron_categories = Koha::Patron::Categories->search({}, {order_by => ['description']}); >+ $template->param( patron_categories => $patron_categories ); > my @branchloop; > foreach (sort {$branches->{$a}->{branchname} cmp $branches->{$b}->{branchname}} keys %$branches) { > my $line = {branchcode => $_, branchname => $branches->{$_}->{branchname} || 'UNKNOWN'}; >@@ -148,20 +152,6 @@ if ($do_it) { > } > output_html_with_http_headers $input, $cookie, $template->output; > >-sub catcode_aref { >- my $req = C4::Context->dbh->prepare("SELECT categorycode, description FROM categories ORDER BY description"); >- $req->execute; >- return $req->fetchall_arrayref({}); >-} >-sub catcodes_hash { >- my %cathash; >- my $catcodes = &catcode_aref; >- foreach (@$catcodes) { >- $cathash{$_->{categorycode}} = ($_->{description} || 'NO_DESCRIPTION') . " ($_->{categorycode})"; >- } >- return %cathash; >-} >- > sub calculate { > my ($line, $column, $digits, $status, $activity, $filters, $attr_filters) = @_; > >@@ -269,9 +259,8 @@ sub calculate { > } else { > $linefield = $line; > } >- >- my %cathash = ($line eq 'categorycode' or $column eq 'categorycode') ? &catcodes_hash : (); >- push @loopfilter, {debug=>1, crit=>"\%cathash", filter=>join(", ", map {$cathash{$_}} sort keys %cathash)}; >+ my $patron_categories = Koha::Patron::Categories->search({}, {order_by => ['categorycode']}); >+ push @loopfilter, {debug=>1, crit=>"\%cathash", filter=>join(", ", map { $_->categorycode . ' (' . ( $_->description || 'NO_DESCRIPTION' ) . ')'} $patron_categories->as_list )}; > > my $strsth; > my @strparams; # bind parameters for the query >@@ -299,8 +288,7 @@ sub calculate { > my %cell; > if ($celvalue) { > $cell{rowtitle} = $celvalue; >- # $cell{rowtitle_display} = ($linefield eq 'branchcode') ? $branches->{$celvalue}->{branchname} : $celvalue; >- $cell{rowtitle_display} = ($cathash{$celvalue} || "$celvalue\*") if ($line eq 'categorycode'); >+ $cell{rowtitle_display} = ($patron_categories->find($celvalue)->description || "$celvalue\*") if ($line eq 'categorycode'); > } > $cell{totalrow} = 0; > push @loopline, \%cell; >@@ -348,7 +336,7 @@ sub calculate { > if (defined $celvalue) { > $cell{coltitle} = $celvalue; > # $cell{coltitle_display} = ($colfield eq 'branchcode') ? $branches->{$celvalue}->{branchname} : $celvalue; >- $cell{coltitle_display} = $cathash{$celvalue} if ($column eq 'categorycode'); >+ $cell{coltitle_display} = $patron_categories->find($celvalue)->description if ($column eq 'categorycode'); > } > push @loopcol, \%cell; > } >diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl >index a825707..b0c28d1 100755 >--- a/reports/guided_reports.pl >+++ b/reports/guided_reports.pl >@@ -35,6 +35,7 @@ use C4::Log; > use Koha::DateUtils qw/dt_from_string output_pref/; > use Koha::AuthorisedValue; > use Koha::AuthorisedValues; >+use Koha::Patron::Categories; > > =head1 NAME > >@@ -697,14 +698,9 @@ elsif ($phase eq 'Run this report'){ > } > } > elsif ( $authorised_value eq "categorycode" ) { >- my $sth = $dbh->prepare("SELECT categorycode, description FROM categories ORDER BY description"); >- $sth->execute; >- while ( my ( $categorycode, $description ) = $sth->fetchrow_array ) { >- push @authorised_values, $categorycode; >- $authorised_lib{$categorycode} = $description; >- } >- >- #---- "true" authorised value >+ my @patron_categories = Koha::Patron::Categories->search({}, { order_by => ['description']}); >+ %authorised_lib = map { $_->categorycode => $_->description } @patron_categories; >+ push @authorised_values, $_->categorycode for @patron_categories; > } > else { > if ( Koha::AuthorisedValues->search({ category => $authorised_value })->count ) { >diff --git a/reports/issues_avg_stats.pl b/reports/issues_avg_stats.pl >index b0a1d4f..797d5f4 100755 >--- a/reports/issues_avg_stats.pl >+++ b/reports/issues_avg_stats.pl >@@ -29,6 +29,7 @@ use C4::Koha; > use C4::Circulation; > use C4::Reports; > use Koha::DateUtils; >+use Koha::Patron::Categories; > use Date::Calc qw(Delta_Days); > > =head1 NAME >@@ -37,8 +38,6 @@ plugin that shows a stats on borrowers > > =head1 DESCRIPTION > >-=over 2 >- > =cut > > my $input = new CGI; >@@ -119,25 +118,12 @@ if ($do_it) { > } > # Displaying choices > } else { >- my $dbh = C4::Context->dbh; >- my @values; >- my $req; >- $req = $dbh->prepare("select distinctrow categorycode,description from categories order by description"); >- $req->execute; >- my %labelsc; >- my @selectc; >- while (my ($value, $desc) =$req->fetchrow) { >- push @selectc, $value; >- $labelsc{$value} = $desc; >- } >- my $BorCat = { >- values => \@selectc, >- labels => \%labelsc, >- }; >+ my $patron_categories = Koha::Patron::Categories->search({}, {order_by => ['description']}); > > my $itemtypes = GetItemTypes( style => 'array' ); > >- $req = $dbh->prepare("select distinctrow sort1 from borrowers where sort1 is not null order by sort1"); >+ my $dbh = C4::Context->dbh; >+ my $req = $dbh->prepare("select distinctrow sort1 from borrowers where sort1 is not null order by sort1"); > $req->execute; > my @selects1; > my $hassort1; >@@ -166,7 +152,7 @@ if ($do_it) { > my $CGIsepChoice=GetDelimiterChoices; > > $template->param( >- BorCat => $BorCat, >+ patron_categories => $patron_categories, > itemtypes => $itemtypes, > branchloop => GetBranchesLoop(), > hassort1 => $hassort1, >diff --git a/tools/overduerules.pl b/tools/overduerules.pl >index 1478c3a..d77a6ad 100755 >--- a/tools/overduerules.pl >+++ b/tools/overduerules.pl >@@ -30,14 +30,13 @@ use C4::Members; > use C4::Overdues; > use Koha::Libraries; > >+use Koha::Patron::Categories; >+ > our $input = new CGI; > my $dbh = C4::Context->dbh; > >-my @categories = @{$dbh->selectall_arrayref( >- 'SELECT description, categorycode FROM categories WHERE overduenoticerequired > 0', >- { Slice => {} } >-)}; >-my @category_codes = map { $_->{categorycode} } @categories; >+my @patron_categories = Koha::Patron::Categories->search( { overduenoticerequired => { '>' => 0 } } ); >+my @category_codes = map { $_->categorycode } @patron_categories; > our @rule_params = qw(delay letter debarred); > > # blank_row($category_code) - return true if the entire row is blank. >@@ -224,7 +223,7 @@ my @line_loop; > > my $message_transport_types = C4::Letters::GetMessageTransportTypes(); > my ( @first, @second, @third ); >-for my $data (@categories) { >+for my $patron_category (@patron_categories) { > if (%temphash and not $input_saved){ > # if we managed to save the form submission, don't > # reuse %temphash, but take the values from the >@@ -232,13 +231,13 @@ for my $data (@categories) { > # bugs where the form submission was not correctly saved > for my $i ( 1..3 ){ > my %row = ( >- overduename => $data->{'categorycode'}, >- line => $data->{'description'} >+ overduename => $patron_category->categorycode, >+ line => $patron_category->description, > ); >- $row{delay}=$temphash{$data->{'categorycode'}}->{"delay$i"}; >- $row{debarred}=$temphash{$data->{'categorycode'}}->{"debarred$i"}; >- $row{selected_lettercode} = $temphash{ $data->{categorycode} }->{"letter$i"}; >- my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $data->{'categorycode'}, $i) }; >+ $row{delay}=$temphash{$patron_category->categorycode}->{"delay$i"}; >+ $row{debarred}=$temphash{$patron_category->categorycode}->{"debarred$i"}; >+ $row{selected_lettercode} = $temphash{ $patron_category->categorycode }->{"letter$i"}; >+ my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $patron_category->categorycode, $i) }; > my @mtts; > for my $mtt ( @$message_transport_types ) { > push @mtts, { >@@ -258,19 +257,19 @@ for my $data (@categories) { > } else { > #getting values from table > my $sth2=$dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=?"); >- $sth2->execute($branch,$data->{'categorycode'}); >+ $sth2->execute($branch,$patron_category->categorycode); > my $dat=$sth2->fetchrow_hashref; > for my $i ( 1..3 ){ > my %row = ( >- overduename => $data->{'categorycode'}, >- line => $data->{'description'} >+ overduename => $patron_category->categorycode, >+ line => $patron_category->description, > ); > > $row{selected_lettercode} = $dat->{"letter$i"}; > > if ($dat->{"delay$i"}){$row{delay}=$dat->{"delay$i"};} > if ($dat->{"debarred$i"}){$row{debarred}=$dat->{"debarred$i"};} >- my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $data->{'categorycode'}, $i) }; >+ my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $patron_category->categorycode, $i) }; > my @mtts; > for my $mtt ( @$message_transport_types ) { > push @mtts, { >-- >1.7.10.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=54361">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 15407
:
45906
|
45907
|
45908
|
45909
|
45910
|
45913
|
45914
|
47098
|
47099
|
47100
|
47101
|
47102
|
47646
|
47647
|
47648
|
47649
|
47650
|
47754
|
47755
|
47756
|
47757
|
47758
|
50170
|
50171
|
50172
|
50173
|
50174
|
52013
|
52014
|
52015
|
52032
|
52033
|
52034
|
52035
|
52036
|
53419
|
53420
|
53421
|
53422
|
53423
|
53892
|
54342
|
54343
|
54344
|
54345
|
54346
|
54352
|
54353
|
54355
| 54361 |
54362
|
54363
|
54364
|
54365
|
54366
|
54367
|
54368
|
54369
|
54371
|
55517