From 952f3f650f449abf292d37dbe455e9a63a72e07f Mon Sep 17 00:00:00 2001
From: Mark Tompsett <mtompset@hotmail.com>
Date: Thu, 23 Jun 2016 19:20:41 -0400
Subject: [PATCH] Bug 3311: Statistical reports should error if row and column
 aren't selected

There are three parameters to play with which do not always
generate friendly results. A couple of them seriously crash now.

TEST PLAN
---------
1) Staff -> Reports -> Catalog
   -- should be at 'Catalog statistics' area.
2) Run through combinations:
      no row, no column
      no row, column
      row, no column
      row, column
   After selecting a combination to test,
   click submit, see what happens.
   Pre-patch apply, nasty crashes sometimes.
   Step 1 may require repeating, since you
   can't select a radio button and then
   unselect it.
3) Apply the patch
4) Repeat the first two steps again, and
   this time, a pretty informational or
   warning message should be given.
5) Run koha qa test tools

Signed-off-by: Nicole C Engard <nengard@bywatersolutions.com>
---
 .../prog/en/modules/reports/catalogue_stats.tt     | 45 ++++++++++++++++-
 reports/catalogue_stats.pl                         | 59 ++++++++++++++++++----
 2 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt
index 7e04361..0ef130a 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/catalogue_stats.tt
@@ -4,6 +4,25 @@
 [% INCLUDE 'calendar.inc' %]
 <script type="text/javascript">
 //<![CDATA[
+    function validateLineColumn() {
+        var Line = $("input[name='Line']:checked").val();
+        var Column = $("input[name='Column']:checked").val();
+        var divParameterWarning = $("div[id='parameter_warning']");
+        if (Line && Column) {
+            return;
+        } else if (Line) {
+            <!-- Column is not defined -->
+            divParameterWarning.html('<p><strong><i class="fa fa-exclamation-triangle"> </i> ' + _("Warning:") + '</strong> ' + _("A column value must be selected.") + '</p>');
+        } else if (Column) {
+            <!-- Line is not defined -->
+            divParameterWarning.html('<p><strong><i class="fa fa-exclamation-triangle"> </i> ' + _("Warning:") + '</strong> ' + _("A row value must be selected.") + '</p>');
+        } else {
+            <!-- Neither are defined -->
+            divParameterWarning.html('<p><strong><i class="fa fa-exclamation-triangle"> </i> ' + _("Warning:") + '</strong> ' + _("Both row and column values must be selected.") + '</p>');
+        }
+        return false;
+    }
+
     function changeRemovedDateTrStatus() {
         var Cellvalue = $("input[name='Cellvalue']:checked").val();
         if(Cellvalue == "deleteditems") {
@@ -98,7 +117,31 @@
 	[% END %]
 [% ELSE %]
 
-	<form method="post" action="/cgi-bin/koha/reports/catalogue_stats.pl">
+        <div id="parameter_warning" class="dialog alert">
+        [% SWITCH invalid_parameters %]
+        [%     CASE 'line+column' %]
+            <p>
+<strong><i class="fa fa-exclamation-triangle"> </i> Warning:</strong>
+Both row and column values must be selected.
+            </p>
+        [%     CASE 'line' %]
+            <p>
+<strong><i class="fa fa-exclamation-triangle"> </i> Warning:</strong>
+A row value must be selected.
+            </p>
+        [%     CASE 'column' %]
+            <p>
+<strong><i class="fa fa-exclamation-triangle"> </i> Warning:</strong>
+A column value must be selected.
+            </p>
+        [%     CASE %]
+            <p>
+<strong><i class="fa fa-info-circle"> </i> Information:</strong>
+Both row and column values must be selected.
+            </p>
+        [% END %]
+        </div>
+        <form method="post" action="/cgi-bin/koha/reports/catalogue_stats.pl" onsubmit="return validateLineColumn();">
 	<fieldset class="rows">
 	<legend>Catalog statistics</legend>
 	<table>
diff --git a/reports/catalogue_stats.pl b/reports/catalogue_stats.pl
index c153947..c1d1fa1 100755
--- a/reports/catalogue_stats.pl
+++ b/reports/catalogue_stats.pl
@@ -19,10 +19,9 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use strict;
-#use warnings; FIXME - Bug 2505
+use warnings; #FIXME - Bug 2505
 use C4::Auth;
 use CGI qw ( -utf8 );
-use C4::Context;
 use C4::Branch; # GetBranches
 use C4::Output;
 use C4::Koha;
@@ -30,8 +29,11 @@ use C4::Reports;
 use C4::Circulation;
 use C4::Biblio qw/GetMarcSubfieldStructureFromKohaField/;
 
+use Koha::Database;
 use Koha::DateUtils;
 
+use List::MoreUtils qw/none/;
+
 =head1 NAME
 
 plugin that shows a stats on borrowers
@@ -41,7 +43,7 @@ plugin that shows a stats on borrowers
 =cut
 
 our $debug = 0;
-my $input = new CGI;
+my $input = CGI->new();
 my $fullreportname = "reports/catalogue_stats.tt";
 my $do_it       = $input->param('do_it');
 my $line        = $input->param("Line");
@@ -74,6 +76,47 @@ my ($template, $borrowernumber, $cookie)
 				flagsrequired => {reports => '*'},
 				debug => 1,
 				});
+
+my $schema             = Koha::Database->new()->schema();
+my @biblio_fields      = $schema->source('Biblio')->columns();
+my @biblioitems_fields = $schema->source('Biblioitem')->columns();
+my @items_fields       = $schema->source('Item')->columns();
+my @valid_fields;
+push @valid_fields, @biblio_fields;
+push @valid_fields, @biblioitems_fields;
+push @valid_fields, @items_fields;
+if (defined $line) {
+    my $chk_line   = $line;
+    $chk_line =~ s/^items[.]//gxsm;
+    if (none { $_ eq $chk_line } @valid_fields) {
+        $line = q{};
+    }
+}
+if (defined $column) {
+    my $chk_column   = $column;
+    $chk_column =~ s/^items[.]//gxsm;
+    if (none { $_ eq $chk_column } @valid_fields) {
+        $column = q{};
+    }
+}
+
+if (defined($line) && !$line &&
+    defined($column) && !$column) {
+    $template->param( invalid_parameters => 'line+column' );
+    $do_it = 0;
+}
+elsif (defined($line) && !$line) {
+    $template->param( invalid_parameters => 'line' );
+    $do_it = 0;
+}
+elsif (defined($column) && !$column) {
+    $template->param( invalid_parameters => 'column' );
+    $do_it = 0;
+}
+if (!defined $line || !defined $column) {
+    $do_it = 0;
+}
+
 $template->param(do_it => $do_it);
 if ($do_it) {
     my $results = calculate( $line, $column, $cellvalue, $cotedigits, \@filters );
@@ -113,12 +156,7 @@ if ($do_it) {
         exit;
     }
 } else {
-	my $dbh = C4::Context->dbh;
-	my @values;
-	my %labels;
-	my $count=0;
-	my $req;
-	my @select;
+    my $dbh = $schema->storage->dbh;
 
     my $itemtypes = GetItemTypes( style => 'array' );
 
@@ -184,7 +222,7 @@ sub calculate {
     my $not;
     my $itemstable = ($cellvalue eq 'deleteditems') ? 'deleteditems' : 'items';
 
-    my $dbh = C4::Context->dbh;
+    my $dbh = $schema->storage->dbh;
 
     # if barcodefilter is empty set as %
     if ($barcodefilter) {
@@ -407,7 +445,6 @@ sub calculate {
     }
 
     my $i = 0;
-    my @totalcol;
     my $hilighted = -1;
 
     #Initialization of cell values.....
-- 
2.1.4