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