From 2c6e38b10d48901e9144742e79334e8f33ac336f Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 21 Dec 2015 15:04:45 +0000 Subject: [PATCH] Bug 15407: Koha::Patron::Categories - remove sql queries in some pl and pm 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). --- C4/Reports/Guided.pm | 4 +-- 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 | 13 ++++----- reports/issues_avg_stats.pl | 24 ++++------------- tools/overduerules.pl | 31 +++++++++++----------- 9 files changed, 54 insertions(+), 88 deletions(-) diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index daf810d..5b796ba 100644 --- a/C4/Reports/Guided.pm +++ b/C4/Reports/Guided.pm @@ -30,10 +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::Patron::Categories; + BEGIN { # set the version for version checking $VERSION = 3.07.00.049; diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 5be4c0a..95b90ed 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -31,6 +31,7 @@ use Koha::Database; use Koha::IssuingRule; use Koha::IssuingRules; use Koha::Libraries; +use Koha::Patron::Categories; my $input = CGI->new; my $dbh = C4::Context->dbh; @@ -430,14 +431,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; @@ -477,7 +472,6 @@ while (my $row = $sth2->fetchrow_hashref) { } push @row_loop, $row; } -$sth->finish; my @sorted_row_loop = sort by_category_and_itemtype @row_loop; @@ -585,7 +579,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 c7e27ad..4245043 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 @@ -261,8 +261,8 @@ for="tobranch">Clone these rules to: - [% FOREACH categoryloo IN categoryloop %] - + [% FOREACH patron_category IN patron_categories%] + [% END %] @@ -497,8 +497,8 @@ for="tobranch">Clone these rules to: - [% FOREACH categoryloo IN categoryloop %] - + [% FOREACH patron_category IN patron_categories%] + [% END %] 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 3c5397c..a3c8afa 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 @@ -78,13 +78,14 @@ Patron category - - + + + Patron status 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 @@ diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl index 83ef452..bafcd46 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 6a03e11..214cc5f 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -33,6 +33,7 @@ use C4::Koha qw/IsAuthorisedValueCategory GetFrameworksLoop/; use C4::Context; use C4::Log; use Koha::DateUtils qw/dt_from_string output_pref/; +use Koha::Patron::Categories; =head1 NAME @@ -695,16 +696,12 @@ 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 { + #---- "true" authorised value if ( IsAuthorisedValueCategory($authorised_value) ) { my $query = ' SELECT authorised_value,lib diff --git a/reports/issues_avg_stats.pl b/reports/issues_avg_stats.pl index f71ac48..e1590f7 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 369ac9c..02184b5 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, { -- 2.1.0