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).

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm
index c55d54d..631f961 100644 (file)
--- 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 fb2f26c..4f5185d 100755 (executable)
--- 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;
@@ -470,14 +471,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;
@@ -517,7 +512,6 @@ while (my $row = $sth2->fetchrow_hashref) {
     }
     push @row_loop, $row;
 }
-$sth->finish;
 
 my @sorted_row_loop = sort by_category_and_itemtype @row_loop;
 
@@ -626,7 +620,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 3a9bdde..18fe7ea 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
@@ -277,8 +277,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>
@@ -549,8 +549,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 (file)
--- 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 (file)
--- 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 (executable)
--- 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 8e95556..c5ad623 100755 (executable)
--- a/reports/guided_reports.pl
+++ b/reports/guided_reports.pl
@@ -36,6 +36,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
 
@@ -698,14 +699,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 (executable)
--- 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 (executable)
--- 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, {