From 14be4400d84b28369d095b3b0bfa79c3396f44d4 Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Tue, 31 Mar 2009 15:08:10 -0500 Subject: [PATCH] Bug 3066 - Overhaul guided reports execute_query now refactored, returns reliable results, does zero presentation-layer crap. Arguments reduced, client scripts adapted to new API and performance improved. Text::CSV now used to generate CSV output, ensuring portability, encoding and accuracy. Replaced tools/runreport.pl with misc/cronjobs/runreport.pl: ~ security fixed ~ documentation improved ~ TODO: finish sendmail option. Bug 3077 also fixed. Signed-off-by: Galen Charlton --- C4/Reports/Guided.pm | 225 ++++++------------ .../prog/en/includes/guided-reports-view.inc | 4 +- .../modules/reports/guided_reports_start.tmpl | 166 ++++++------- misc/cronjobs/runreport.pl | 18 +- reports/guided_reports.pl | 216 ++++++++--------- tools/runreport.pl | 38 --- 6 files changed, 278 insertions(+), 389 deletions(-) delete mode 100755 tools/runreport.pl diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index 93a5aa06c3..669b3eb279 100644 --- a/C4/Reports/Guided.pm +++ b/C4/Reports/Guided.pm @@ -20,6 +20,7 @@ package C4::Reports::Guided; use strict; # use warnings; # FIXME: this module needs a lot of repair to run clean under warnings use CGI; +use Carp; use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); use C4::Context; @@ -41,6 +42,7 @@ BEGIN { save_report get_saved_reports execute_query get_saved_report create_compound run_compound get_column_type get_distinct_values save_dictionary get_from_dictionary delete_definition delete_report format_results get_sql + select_2_select_count_value ); } @@ -335,7 +337,7 @@ sub get_criteria { =over -($results, $total, $error) = execute_query($sql, $type, $offset, $limit, $format, $id) +($results, $total, $error) = execute_query($sql, $offset, $limit) =back @@ -355,133 +357,77 @@ sub get_criteria { =cut -# FIXME: This needs to be generalized to reports in general -# FIXME: This should NOT have ANY formatting responsibilities. -# Instead, is should just be returning a prepared sth. -# FIXME: $type is a TOTALLY UNUSED "required" argument? - -sub execute_query ($$$$;$$) { - my ( $sql, $type, $offset, $limit, $format, $id ) = @_; - $format or $format = ''; - $debug and print STDERR "execute_query($sql, $type, $offset, $limit, $format, $id)\n"; - my @params; - my $total = 0; - my ($useroffset, $userlimit); - my @errors = (); - my $error = {}; - my $sqlerr = 0; +# returns $sql, $offset, $limit +# $sql returned will be transformed to: +# ~ remove any LIMIT clause +# ~ repace SELECT clause w/ SELECT count(*) + +sub select_2_select_count_value ($) { + my $sql = shift or return; + my $countsql = select_2_select_count($sql); + $debug and warn "original query: $sql\ncount query: $countsql\n"; + my $sth1 = C4::Context->dbh->prepare($countsql); + $sth1->execute(); + my $total = $sth1->fetchrow(); + $debug and warn "total records for this query: $total\n"; + return $total; +} +sub select_2_select_count ($) { + # Modify the query passed in to create a count query... (I think this covers all cases -crn) + my ($sql) = strip_limit(shift) or return; + $sql =~ s/\bSELECT\W+(?:\w+\W+){1,}?FROM\b|\bSELECT\W\*\WFROM\b/SELECT count(*) FROM /ig; + return $sql; +} +sub strip_limit ($) { + my $sql = shift or return; + ($sql =~ /\bLIMIT\b/i) or return ($sql, 0, undef); + $sql =~ s/\bLIMIT\b\s*\d+(\,\s*\d+)?\s*/ /ig; + return ($sql, (defined $1 ? $1 : 0), $2); # offset can default to 0, LIMIT cannot! +} + +sub execute_query ($;$$$) { + + my ( $sql, $offset, $limit, $no_count ) = @_; + + # check parameters + unless ($sql) { + carp "execute_query() called without SQL argument"; + return; + } + $offset = 0 unless $offset; + $limit = 9999 unless $limit; + $debug and print STDERR "execute_query($sql, $offset, $limit)\n"; if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) { - $sqlerr = 1; - $error->{'sqlerr'} = $1; - push @errors, $error; - } elsif ($sql !~ /^(SELECT)/i) { - $sqlerr = 1; - $error->{'queryerr'} = 'Missing SELECT'; - push @errors, $error; + return (undef, { sqlerr => $1} ); + } elsif ($sql !~ /^\s*SELECT\b\s*/i) { + return (undef, { queryerr => 'Missing SELECT'} ); } - if ($sqlerr == 0) { - my $dbh = C4::Context->dbh(); - unless ($format eq 'text' || $format eq 'tab' || $format eq 'csv' || $format eq 'url'){ - # Grab offset/limit from user supplied LIMIT and drop the LIMIT so we can control pagination - if ($sql =~ /LIMIT/i) { - $sql =~ s/LIMIT\W?(\d+)?\,?\W+?(\d+)//ig; - $debug and warn "User has supplied LIMIT\n"; - $useroffset = $1; - $userlimit = $2; - $debug and warn "User supplied offset = $useroffset, limit = $userlimit\n"; - $offset += $useroffset if $useroffset; - # keep track of where we are if there is a user supplied LIMIT - if ( $offset + $limit > $userlimit ) { - $limit = $userlimit - $offset; - } - } - my $countsql = $sql; - $sql .= " LIMIT ?, ?"; - $debug and warn "Passing query with params offset = $offset, limit = $limit\n"; - @params = ($offset, $limit); - # Modify the query passed in to create a count query... (I think this covers all cases -crn) - $countsql =~ s/\bSELECT\W+(?:\w+\W+){1,}?FROM\b|\bSELECT\W\*\WFROM\b/SELECT count(*) FROM /ig; - $debug and warn "original query: $sql\n"; - $debug and warn "count query: $countsql\n"; - my $sth1 = $dbh->prepare($countsql); - $sth1->execute(); - $total = $sth1->fetchrow(); - $debug and warn "total records for this query: $total\n"; - $total = $userlimit if defined($userlimit) and $userlimit < $total; # we will never exceed a user defined LIMIT and... - $userlimit = $total if defined($userlimit) and $userlimit > $total; # we will never exceed the total number of records available to satisfy the query - } - my $sth = $dbh->prepare($sql); - $sth->execute(@params); - my $colnames=$sth->{'NAME'}; - my @results; - my $row; - my %temphash; - $row = join ('',@$colnames); - $row = "$row"; - $temphash{'row'} = $row; - push @results, \%temphash; - my $string; - if ($format eq 'tab') { - $string = join("\t",@$colnames); - } - elsif ($format eq 'csv') { - $string = join(",",@$colnames); - } - my @xmlarray; - while ( my @data = $sth->fetchrow_array() ) { - # if the field is a date field, it needs formatting - foreach my $data (@data) { - unless (defined $data) { - $data = ''; # suppress undef, so fields are joinable - next; - } - next unless $data =~ C4::Dates->regexp("iso"); - $data = C4::Dates->new($data, "iso")->output(); - } - # tabular - my %temphash; # FIXME: already declared %temphash in same scope. - my $row = join( '', @data ); - $row = "$row"; - $temphash{'row'} = $row; - if ($format eq 'text') { - $string .= "\n" . $row; - } - elsif ($format eq 'tab'){ - $row = join("\t",@data); - $string .="\n" . $row; - } - elsif ($format eq 'csv'){ - $row = join(",",@data); - $string .="\n" . $row; - } - elsif ($format eq 'url'){ - my $temphash; # FIXME: already declared %temphash in same scope. TWICE!! - @$temphash{@$colnames}=@data; - push @xmlarray,$temphash; - } - push @results, \%temphash; - } - if (defined($sth->errstr)) { - $error->{'queryerr'} = $sth->errstr; - push @errors, $error; - warn "Database returned: $sth->errstr"; - } - if ( $format eq 'text' || $format eq 'tab' || $format eq 'csv' ) { - return $string, $total, \@errors; - } - elsif ($format eq 'url') { - my $url = "/cgi-bin/koha/reports/guided_reports.pl?phase=retrieve%20results&id=$id"; - my $dump = new XML::Dumper; - my $xml = $dump->pl2xml( \@xmlarray ); - store_results($id,$xml); - return $url, $total, \@errors; - } - else { - return \@results, $total, \@errors; + + my ($useroffset, $userlimit); + + # Grab offset/limit from user supplied LIMIT and drop the LIMIT so we can control pagination + ($sql, $useroffset, $userlimit) = strip_limit($sql); + $debug and warn sprintf "User has supplied (OFFSET,) LIMIT = %s, %s", + $useroffset, + (defined($userlimit ) ? $userlimit : 'UNDEF'); + $offset += $useroffset; + my $total; + if (defined($userlimit)) { + if ($offset + $limit > $userlimit ) { + $limit = $userlimit - $offset; } - } else { - return undef, undef, \@errors; + $total = $userlimit if $userlimit < $total; # we will never exceed a user defined LIMIT and... + $userlimit = $total if $userlimit > $total; # we will never exceed the total number of records available to satisfy the query } + $sql .= " LIMIT ?, ?"; + + my $sth = C4::Context->dbh->prepare($sql); + $sth->execute($offset, $limit); + return ( $sth ); + # my @xmlarray = ... ; + # my $url = "/cgi-bin/koha/reports/guided_reports.pl?phase=retrieve%20results&id=$id"; + # my $xml = XML::Dumper->new()->pl2xml( \@xmlarray ); + # store_results($id,$xml); } =item save_report($sql,$name,$type,$notes) @@ -498,8 +444,6 @@ sub save_report { "INSERT INTO saved_sql (borrowernumber,date_created,last_modified,savedsql,report_name,type,notes) VALUES (?,now(),now(),?,?,?,?)"; my $sth = $dbh->prepare($query); $sth->execute( 0, $sql, $name, $type, $notes ); - $sth->finish(); - } sub store_results { @@ -512,15 +456,12 @@ sub store_results { my $query2 = "UPDATE saved_reports SET report=?,date_run=now() WHERE report_id=?"; my $sth2 = $dbh->prepare($query2); $sth2->execute($xml,$id); - $sth2->finish(); } else { my $query2 = "INSERT INTO saved_reports (report_id,report,date_run) VALUES (?,?,now())"; my $sth2 = $dbh->prepare($query2); $sth2->execute($id,$xml); - $sth2->finish(); } - $sth->finish(); } sub format_results { @@ -545,7 +486,6 @@ sub format_results { $sth = $dbh->prepare($query); $sth->execute($id); $data = $sth->fetchrow_hashref(); - $sth->finish(); return ($perl,$data->{'report_name'},$data->{'notes'}); } @@ -555,7 +495,6 @@ sub delete_report { my $query = "DELETE FROM saved_sql WHERE id = ?"; my $sth = $dbh->prepare($query); $sth->execute($id); - $sth->finish(); } sub get_saved_reports { @@ -565,12 +504,7 @@ sub get_saved_reports { ORDER by date_created"; my $sth = $dbh->prepare($query); $sth->execute(); - my @reports; - while ( my $data = $sth->fetchrow_hashref() ) { - push @reports, $data; - } - $sth->finish(); - return ( \@reports ); + return $sth->fetchall_arrayref({}); } sub get_saved_report { @@ -580,7 +514,6 @@ sub get_saved_report { my $sth = $dbh->prepare($query); $sth->execute($id); my $data = $sth->fetchrow_hashref(); - $sth->finish(); return ( $data->{'savedsql'}, $data->{'type'}, $data->{'report_name'}, $data->{'notes'} ); } @@ -635,7 +568,6 @@ sub get_column_type { return $info->{'TYPE_NAME'}; } } - $sth->finish(); } =item get_distinct_values($column) @@ -653,12 +585,7 @@ sub get_distinct_values { "SELECT distinct($column) as availablevalues FROM $table"; my $sth = $dbh->prepare($query); $sth->execute(); - my @values; - while ( my $row = $sth->fetchrow_hashref() ) { - push @values, $row; - } - $sth->finish(); - return \@values; + return $sth->fetchall_arrayref({}); } sub save_dictionary { @@ -668,7 +595,6 @@ sub save_dictionary { VALUES (?,?,?,?,now(),now())"; my $sth = $dbh->prepare($query); $sth->execute($name,$description,$sql,$area) || return 0; - $sth->finish(); return 1; } @@ -699,27 +625,24 @@ sub get_from_dictionary { push @loop,$data; } - $sth->finish(); return (\@loop); } sub delete_definition { - my ($id) = @_; + my ($id) = @_ or return; my $dbh = C4::Context->dbh(); my $query = "DELETE FROM reports_dictionary WHERE id = ?"; my $sth = $dbh->prepare($query); $sth->execute($id); - $sth->finish(); } sub get_sql { - my ($id) = @_; + my ($id) = @_ or return; my $dbh = C4::Context->dbh(); my $query = "SELECT * FROM saved_sql WHERE id = ?"; my $sth = $dbh->prepare($query); $sth->execute($id); my $data=$sth->fetchrow_hashref(); - $sth->finish(); return $data->{'savedsql'}; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/guided-reports-view.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/guided-reports-view.inc index bad5429826..f9ad7bf496 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/guided-reports-view.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/guided-reports-view.inc @@ -1,10 +1,10 @@
Build and Run Reports
Reports Dictionary
\ No newline at end of file + diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl index bc45328ad0..c0e5131753 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl @@ -1,20 +1,33 @@ -Koha › Reports <!-- TMPL_IF NAME="start" --> › Guided Reports Wizard <!-- /TMPL_IF --><!-- TMPL_IF NAME="saved1" --> › Guided Reports Wizard › Saved Reports<!-- /TMPL_IF --><!-- TMPL_IF NAME="create" --> › Guided Reports Wizard › Create from SQL<!-- /TMPL_IF --><!-- TMPL_IF NAME="showsql" --> - › Guided Reports Wizard › Saved Reports › SQL View<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="execute" --> › Guided Reports Wizard › Saved Reports › <!-- TMPL_VAR NAME="name" --> Report<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="build1" -->› Guided Reports Wizard › Build A Report, Step 1: Choose a Module<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="build2" -->› Guided Reports Wizard › Build A Report, Step 2: Pick a Report Type<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="build3" -->› Guided Reports Wizard › Build A Report, Step 3 of 6: Select Columns for Display<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="build4" --> › Guided Reports Wizard › Build A Report, Step 4 of 6: Select Criteria to Limit on<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="build5" --> › Guided Reports Wizard › Build A Report, Step 5 of 6: Pick which columns to total<!-- /TMPL_IF --> - <!-- TMPL_IF NAME="build6" -->› Guided Reports Wizard › Build A Report, Step 6 of 6: Select how you want the report ordered<!-- /TMPL_IF --> +<title>Koha › Reports › Guided Reports Wizard +<!-- TMPL_IF NAME="saved1" -->› Saved Reports +<!-- TMPL_ELSIF NAME="create" -->› Create from SQL +<!-- TMPL_ELSIF NAME="showsql" -->› Saved Reports › SQL View +<!-- TMPL_ELSIF NAME="execute" -->› Saved Reports › <!-- TMPL_VAR NAME="name" --> Report +<!-- TMPL_ELSIF NAME="buildx" -->› Build A Report, Step <!-- TMPL_VAR NAME="buildx" --> of 6: + <!-- TMPL_IF NAME="build1" -->Choose a Module + <!-- TMPL_ELSIF NAME="build2" -->Pick a Report Type + <!-- TMPL_ELSIF NAME="build3" -->Select Columns for Display + <!-- TMPL_ELSIF NAME="build4" -->Select Criteria to Limit on + <!-- TMPL_ELSIF NAME="build5" -->Pick which columns to total + <!-- TMPL_ELSIF NAME="build6" -->Select how you want the report ordered + <!-- /TMPL_IF --> +<!-- /TMPL_IF --> - + - - - - -
- -
-
+
+
- -
+

Guided Reports

@@ -71,22 +77,21 @@ $(document).ready(function(){ This feature aims to provide some middle ground between the built in canned reports and writing custom SQL reports.

-

Build And Run Reports

+

Build And Run Reports

- - + +

Reports Dictionary

-

Use the reports dictionary to define custom criteria to use in your -reports

+

Use the reports dictionary to define custom criteria to use in your reports

@@ -141,11 +146,13 @@ reports

" />
Step 2 of 6: Pick a Report Type -
+
  1. + +
@@ -275,7 +282,7 @@ TMPL_VAR NAME="id" -->" /> " value="" /> +NAME="name" -->">
- -
-
-The following error was encountered:
- -This report contains the SQL keyword .
Use of this keyword is not allowed in Koha reports due to security and data integrity risks.
Please return to the "Saved Reports" screen and delete this report. -The database returned the following error:

Please check the log for further details. - - -
-
-
-
- @@ -399,10 +396,16 @@ NAME="name" -->">
Create Report From SQL
    -
  1. value="" />
    2. -
    3. -
    4. -
    5. +
  5. value="" />
    6. +
    7. +
  7. + +
    8. +
@@ -442,10 +445,9 @@ Sub report:

Your report has been saved

The report you have created has now been saved. You can now

+ +
The following error was encountered:
-The SQL statement contains the keyword . Use of this keyword is not allowed in Koha reports due to security and data integrity risks. -The SQL statment is missing the SELECT keyword.
Please check the SQL statement syntax. - + This report contains the SQL keyword . +
Use of this keyword is not allowed in Koha reports due to security and data integrity risks. Only SELECT queries are allowed. +
Please return to the "Saved Reports" screen and delete this report or retry creating a new one. + The database returned the following error:

Please check the log for further details. + +
-" /> -" /> -" /> -" /> -
-
+
+
-
diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl index c0f67a298c..ebbaa6ef52 100755 --- a/misc/cronjobs/runreport.pl +++ b/misc/cronjobs/runreport.pl @@ -27,6 +27,7 @@ use Getopt::Long qw(:config auto_help auto_version); use Pod::Usage; use Mail::Sendmail; use Text::CSV_XS; +use CGI; use vars qw($VERSION); @@ -131,15 +132,22 @@ foreach my $report (@ARGV) { } $verbose and print "SQL: $sql\n\n"; # my $results = execute_query($sql, undef, 0, 99999, $format, $report); - my ($results) = execute_query($sql, undef, 0, 20, , ); + my ($sth) = execute_query($sql); # execute_query(sql, , 0, 20, , ) - my $count = scalar(@$results); + my $count = scalar($sth->rows); unless ($count) { print "NO OUTPUT: 0 results from execute_query\n"; next; } $verbose and print "$count results from execute_query\n"; - my $message = "\n" . join("\n", map {$_->{row}} @$results) . "\n
\n"; + + my $cgi = CGI->new(); + my @rows = (); + while (my $line = $sth->fetchrow_arrayref) { + foreach (@$line) { defined($_) or $_ = ''; } # catch undef values, replace w/ '' + push @rows, $cgi->TR( join('', $cgi->td($line)) ) . "\n"; + } + my $message = $cgi->table(join "", @rows); if ($email){ my %mail = ( @@ -152,4 +160,8 @@ foreach my $report (@ARGV) { } else { print $message; } + # my @xmlarray = ... ; + # my $url = "/cgi-bin/koha/reports/guided_reports.pl?phase=retrieve%20results&id=$id"; + # my $xml = XML::Dumper->new()->pl2xml( \@xmlarray ); + # store_results($id,$xml); } diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 234ffb9b3b..037029ce62 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -20,6 +20,7 @@ use strict; # use warnings; # FIXME use CGI; +use Text::CSV; use C4::Reports::Guided; use C4::Auth; use C4::Output; @@ -39,7 +40,6 @@ Script to control the guided report creation =cut my $input = new CGI; -my $referer = $input->referer(); my ( $template, $borrowernumber, $cookie ) = get_template_and_user( { @@ -52,43 +52,32 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); + my @errors = (); my $phase = $input->param('phase'); -my $no_html = 0; # this will be set if we dont want to print out an html::template if ( !$phase ) { $template->param( 'start' => 1 ); - # show welcome page } - elsif ( $phase eq 'Build new' ) { - # build a new report $template->param( 'build1' => 1 ); - - # get report areas - my $areas = get_report_areas(); - $template->param( 'areas' => $areas ); - + $template->param( 'areas' => get_report_areas() ); } - -elsif ( $phase eq 'Used saved' ) { - +elsif ( $phase eq 'Use saved' ) { # use a saved report # get list of reports and display them $template->param( 'saved1' => 1 ); - my $reports = get_saved_reports(); - $template->param( 'savedreports' => $reports ); + $template->param( 'savedreports' => get_saved_reports() ); } elsif ( $phase eq 'Delete Saved') { # delete a report from the saved reports list - $no_html = 1; my $id = $input->param('reports'); delete_report($id); - print $input->redirect("/cgi-bin/koha/reports/guided_reports.pl?phase=Used%20saved"); - + print $input->redirect("/cgi-bin/koha/reports/guided_reports.pl?phase=Use%20saved"); + exit; } elsif ( $phase eq 'Show SQL'){ @@ -98,7 +87,7 @@ elsif ( $phase eq 'Show SQL'){ $template->param( 'sql' => $sql, 'showsql' => 1, - ); + ); } elsif ($phase eq 'retrieve results') { @@ -110,23 +99,17 @@ elsif ($phase eq 'retrieve results') { 'results' => $results, 'name' => $name, 'notes' => $notes, - ); - + ); } elsif ( $phase eq 'Report on this Area' ) { # they have choosen a new report and the area to report on - # get area - my $area = $input->param('areas'); $template->param( 'build2' => 1, - 'area' => $area + 'area' => $input->param('areas'), + 'types' => get_report_types(), ); - - # get report types - my $types = get_report_types(); - $template->param( 'types' => $types ); } elsif ( $phase eq 'Choose this type' ) { @@ -139,11 +122,8 @@ elsif ( $phase eq 'Choose this type' ) { 'build3' => 1, 'area' => $area, 'type' => $type, + columns => get_columns($area,$input), ); - - # get columns - my $columns = get_columns($area,$input); - $template->param( 'columns' => $columns ); } elsif ( $phase eq 'Choose these columns' ) { @@ -154,16 +134,14 @@ elsif ( $phase eq 'Choose these columns' ) { my $type = $input->param('type'); my @columns = $input->param('columns'); my $column = join( ',', @columns ); - my $definitions = get_from_dictionary($area); $template->param( 'build4' => 1, 'area' => $area, 'type' => $type, 'column' => $column, + definitions => get_from_dictionary($area), + criteria => get_criteria($area,$input), ); - my $criteria = get_criteria($area,$input); - $template->param( 'criteria' => $criteria, - 'definitions' => $definitions); } elsif ( $phase eq 'Choose these criteria' ) { @@ -229,7 +207,7 @@ elsif ( $phase eq 'Choose These Operations' ) { 'column' => $column, 'criteriastring' => $criteria, 'totals' => $totals, - 'definition' => $definition, + 'definition' => $definition, ); # get columns @@ -301,19 +279,14 @@ elsif ( $phase eq 'Save Report' ) { my $name = $input->param('reportname'); my $type = $input->param('types'); my $notes = $input->param('notes'); - my @errors = (); - my $error = {}; if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) { - $error->{'sqlerr'} = $1; - push @errors, $error; + push @errors, {sqlerr => $1}; } elsif ($sql !~ /^(SELECT)/i) { - $error->{'queryerr'} = 1; - push @errors, $error; + push @errors, {queryerr => 1}; } if (@errors) { $template->param( - 'save_successful' => 1, 'errors' => \@errors, 'sql' => $sql, 'reportname'=> $name, @@ -329,44 +302,45 @@ elsif ( $phase eq 'Save Report' ) { } } -# This condition is not used currently -#elsif ( $phase eq 'Execute' ) { -# # run the sql, and output results in a template -# my $sql = $input->param('sql'); -# my $type = $input->param('type'); -# my ($results, $total, $errors) = execute_query($sql, $type); -# $template->param( -# 'results' => $results, -# 'sql' => $sql, -# 'execute' => 1, -# ); -#} - elsif ($phase eq 'Run this report'){ # execute a saved report - # FIXME The default limit should not be hardcoded... - my $limit = 20; - my $offset; + my $limit = 20; # page size. # TODO: move to DB or syspref? + my $offset = 0; my $report = $input->param('reports'); # offset algorithm if ($input->param('page')) { - $offset = ($input->param('page') - 1) * 20; - } - else { - $offset = 0; + $offset = ($input->param('page') - 1) * $limit; } my ($sql,$type,$name,$notes) = get_saved_report($report); - my ($results, $total, $errors) = execute_query($sql, $type, $offset, $limit); + unless ($sql) { + push @errors, {no_sql_for_id=>$report}; + } + my @rows = (); + my ($sth, $errors) = execute_query($sql, $offset, $limit); + my $total = select_2_select_count_value($sql) || 0; + unless ($sth) { + die "execute_query failed to return sth for report $report: $sql"; + } else { + my $headref = $sth->{NAME} || []; + my @headers = map { +{ cell => $_ } } @$headref; + $template->param(header_row => \@headers); + while (my $row = $sth->fetchrow_arrayref()) { + my @cells = map { +{ cell => $_ } } @$row; + push @rows, { cells => \@cells }; + } + } + my $totpages = int($total/$limit) + (($total % $limit) > 0 ? 1 : 0); my $url = "/cgi-bin/koha/reports/guided_reports.pl?reports=$report&phase=Run%20this%20report"; $template->param( - 'results' => $results, - 'sql' => $sql, - 'execute' => 1, - 'name' => $name, - 'notes' => $notes, - 'pagination_bar' => pagination_bar($url, $totpages, $input->param('page'), "page"), - 'errors' => $errors, + 'results' => \@rows, + 'sql' => $sql, + 'execute' => 1, + 'name' => $name, + 'notes' => $notes, + 'errors' => $errors, + 'pagination_bar' => pagination_bar($url, $totpages, $input->param('page')), + 'unlimited_total' => $total, ); } @@ -374,59 +348,62 @@ elsif ($phase eq 'Export'){ binmode STDOUT, ':utf8'; # export results to tab separated text - my $sql = $input->param('sql'); - my $format = $input->param('format'); - my ($results, $total, $errors) = execute_query($sql,1,0,0,$format); - if (!@$errors) { - $no_html=1; - print $input->header( -type => 'application/octet-stream', - -attachment=>'reportresults.csv' - ); - print $results; + my $sql = $input->param('sql'); # FIXME: use sql from saved report ID#, not new user-supplied SQL! + my $format = $input->param('format'); + my ($sth, $q_errors) = execute_query($sql); + unless ($q_errors and @$q_errors) { + print $input->header( -type => 'application/octet-stream', + -attachment=>'reportresults.csv' + ); + my $csv = Text::CSV->new({binary => 1}); + $csv or die "Text::CSV->new({binary => 1}) FAILED: " . Text::CSV->error_diag(); + if ($csv->combine(header_cell_values($sth))) { + print $csv->string(), "\n"; } else { - $template->param( - 'results' => $results, - 'sql' => $sql, - 'execute' => 1, - 'name' => 'Error exporting report!', - 'notes' => '', - 'pagination_bar' => '', - 'errors' => $errors, - ); + push @$q_errors, { combine => 'HEADER ROW: ' . $csv->error_diag() } ; } + while (my $row = $sth->fetchrow_arrayref()) { + if ($csv->combine(@$row)) { + print $csv->string(), "\n"; + } else { + push @$q_errors, { combine => $csv->error_diag() } ; + } + } + foreach my $err (@$q_errors, @errors) { + print "# ERROR: " . (map {$_ . ": " . $err->{$_}} keys %$err) . "\n"; + } # here we print all the non-fatal errors at the end. Not super smooth, but better than nothing. + exit; + } + $template->param( + 'sql' => $sql, + 'execute' => 1, + 'name' => 'Error exporting report!', + 'notes' => '', + 'errors' => $q_errors, + ); } elsif ($phase eq 'Create report from SQL') { # allow the user to paste in sql - if ($input->param('sql')) { - $template->param( - 'sql' => $input->param('sql'), - 'reportname' => $input->param('reportname'), - 'notes' => $input->param('notes'), - ); - } + if ($input->param('sql')) { + $template->param( + 'sql' => $input->param('sql'), + 'reportname' => $input->param('reportname'), + 'notes' => $input->param('notes'), + ); + } $template->param('create' => 1); - my $types = get_report_types(); - if (my $type = $input->param('type')) { - for my $i ( 0 .. $#{@$types}) { - @$types[$i]->{'selected'} = 1 if @$types[$i]->{'id'} eq $type; - } - } - $template->param( 'types' => $types ); } elsif ($phase eq 'Create Compound Report'){ - my $reports = get_saved_reports(); - $template->param( 'savedreports' => $reports, + $template->param( 'savedreports' => get_saved_reports(), 'compound' => 1, ); } elsif ($phase eq 'Save Compound'){ - my $master = $input->param('master'); + my $master = $input->param('master'); my $subreport = $input->param('subreport'); -# my $compound_report = create_compound($master,$subreport); -# my $results = run_compound($compound_report); my ($mastertables,$subtables) = create_compound($master,$subreport); $template->param( 'save_compound' => 1, master=>$mastertables, @@ -434,10 +411,23 @@ elsif ($phase eq 'Save Compound'){ ); } -$template->param( 'referer' => $referer, +# pass $sth, get back an array of names for the column headers +sub header_cell_values ($) { + my $sth = shift or return (); + return @{$sth->{NAME}}; +} + +# pass $sth, get back a TMPL_LOOP-able set of names for the column headers +sub header_cell_loop ($) { + my @headers = map { +{ cell => $_ } } header_cell_values (shift); + return \@headers; +} + +foreach (1..6) { + $template->param('build' . $_) and $template->param(buildx => $_) and last; +} +$template->param( 'referer' => $input->referer(), 'DHTMLcalendar_dateformat' => C4::Dates->DHTMLcalendar(), ); -if (!$no_html){ - output_html_with_http_headers $input, $cookie, $template->output; -} +output_html_with_http_headers $input, $cookie, $template->output; diff --git a/tools/runreport.pl b/tools/runreport.pl deleted file mode 100755 index 9801a4ae25..0000000000 --- a/tools/runreport.pl +++ /dev/null @@ -1,38 +0,0 @@ -#!/usr/bin/perl - -# fix this line -use C4::Reports::Guided; -use C4::Context; - -use Mail::Sendmail; - - -my ($report,$format,$email) = @ARGV; - -my ($sql,$type) = get_saved_report($report); -my $results = execute_query($sql,$type,$format,$report); -my $message; -if ($format eq 'text'){ - $message="$results
"; -} -if ($format eq 'url'){ - $message="$results"; -} - -if ($email){ - my $to = $email; - # should be koha admin email - my $from = C4::Context->preference('KohaAdminEmailAddress'); - my $subject = 'Automated job run'; - my %mail = ( - To => $to, - From => $from, - Subject => $subject, - Message => $message - ); - - - if (not(sendmail %mail)) { - warn "mail not sent"; - } - } -- 2.20.1