@@ -, +, @@ - when report is run publicly - when report is sent by email - when report is exported --- C4/Reports/Guided.pm | 14 +++++++----- misc/cronjobs/runreport.pl | 8 ++++++- opac/svc/report | 11 +++++++--- reports/guided_reports.pl | 13 ++++++++--- svc/report | 10 +++++++-- t/db_dependent/Reports/Guided.t | 39 +++++++++++++++++++++++---------- 6 files changed, 68 insertions(+), 27 deletions(-) --- a/C4/Reports/Guided.pm +++ a/C4/Reports/Guided.pm @@ -541,17 +541,18 @@ sub strip_limit { sub execute_query { - my ( $sql, $offset, $limit, $sql_params, $report_id ) = @_; - - $sql_params = [] unless defined $sql_params; + my $params = shift; + my $sql = $params->{sql}; + my $offset = $params->{offset} || 0; + my $limit = $params->{limit} || 999999; + my $sql_params = defined $params->{sql_params} ? $params->{sql_params} : []; + my $report_id = $params->{report_id}; # check parameters unless ($sql) { carp "execute_query() called without SQL argument"; return; } - $offset = 0 unless $offset; - $limit = 999999 unless $limit; Koha::Logger->get->debug("Report - execute_query($sql, $offset, $limit)"); @@ -1053,7 +1054,8 @@ sub EmailReport { my $sql = $report->savedsql; return ( { FATAL => "NO_REPORT" } ) unless $sql; - my ( $sth, $errors ) = execute_query( $sql ); #don't pass offset or limit, hardcoded limit of 999,999 will be used + #don't pass offset or limit, hardcoded limit of 999,999 will be used + my ( $sth, $errors ) = execute_query( { sql => $sql, report_id => $report_id } ); return ( undef, [{ FATAL => "REPORT_FAIL" }] ) if $errors; my $counter = 1; --- a/misc/cronjobs/runreport.pl +++ a/misc/cronjobs/runreport.pl @@ -262,7 +262,13 @@ foreach my $report_id (@ARGV) { my $params_needed = ( $sql =~ s/(<<[^>]+>>)/\?/g ); die("You supplied ". scalar @params . " parameter(s) and $params_needed are required by the report") if scalar @params != $params_needed; - my ($sth) = execute_query( $sql, undef, undef, \@params, $report_id ); + my ($sth) = execute_query( + { + sql => $sql, + sql_params => \@params, + report_id => $report_id, + } + ); my $count = scalar($sth->rows); unless ($count) { print "NO OUTPUT: 0 results from execute_query\n"; --- a/opac/svc/report +++ a/opac/svc/report @@ -58,13 +58,18 @@ if ($cache_active) { } unless ($json_text) { - my $offset = 0; my $limit = C4::Context->preference("SvcMaxReportRows") || 10; my ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params ); - my ( $sth, $errors ) = - execute_query( $sql, $offset, $limit, undef, $report_id ); + my ( $sth, $errors ) = execute_query( + { + sql => $sql, + offset => 0, + limit => $limit, + report_id => $report_id, + } + ); if ($sth) { my $lines; if ($report_annotation) { --- a/reports/guided_reports.pl +++ a/reports/guided_reports.pl @@ -845,7 +845,14 @@ elsif ($phase eq 'Run this report'){ } else { my ($sql,$header_types) = $report->prep_report( \@param_names, \@sql_params ); $template->param(header_types => $header_types); - my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id ); + my ( $sth, $errors ) = execute_query( + { + sql => $sql, + offset => $offset, + limit => $limit, + report_id => $report_id, + } + ); my $total; if (!$sth) { die "execute_query failed to return sth for report $report_id: $sql"; @@ -858,7 +865,7 @@ elsif ($phase eq 'Run this report'){ push @rows, { cells => \@cells }; } if( $want_full_chart ){ - my ($sth2, $errors2) = execute_query($sql); + my ( $sth2, $errors2 ) = execute_query( { sql => $sql, report_id => $report_id } ); while (my $row = $sth2->fetchrow_arrayref()) { my @cells = map { +{ cell => $_ } } @$row; push @allrows, { cells => \@cells }; @@ -912,7 +919,7 @@ elsif ($phase eq 'Export'){ my $reportfilename = $reportname ? "$reportname-reportresults.$format" : "reportresults.$format" ; ($sql, undef) = $report->prep_report( \@param_names, \@sql_params ); - my ($sth, $q_errors) = execute_query($sql); + my ( $sth, $q_errors ) = execute_query( { sql => $sql, report_id => $report_id } ); unless ($q_errors and @$q_errors) { my ( $type, $content ); if ($format eq 'tab') { --- a/svc/report +++ a/svc/report @@ -61,12 +61,18 @@ if ($cache_active) { } unless ($json_text) { - my $offset = 0; my $limit = C4::Context->preference("SvcMaxReportRows") || 10; my ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params ); - my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id ); + my ( $sth, $errors ) = execute_query( + { + sql => $sql, + offset => 0, + limit => $limit, + report_id => $report_id, + } + ); if ($sth) { my $lines; if ($report_annotation) { --- a/t/db_dependent/Reports/Guided.t +++ a/t/db_dependent/Reports/Guided.t @@ -241,16 +241,24 @@ subtest 'get_saved_reports' => sub { is( scalar @{ get_saved_reports() }, $count, "Report2 and report3 have been deleted" ); - my $sth = execute_query('SELECT COUNT(*) FROM systempreferences', 0, 10); + my $sth = execute_query( + { + sql => 'SELECT COUNT(*) FROM systempreferences', + offset => 0, + limit => 10, + } + ); my $results = $sth->fetchall_arrayref; is(scalar @$results, 1, 'running a query returned a result'); my $version = C4::Context->preference('Version'); $sth = execute_query( - 'SELECT value FROM systempreferences WHERE variable = ?', - 0, - 10, - [ 'Version' ], + { + sql => 'SELECT value FROM systempreferences WHERE variable = ?', + offset => 0, + limit => 10, + sql_params => ['Version'], + } ); $results = $sth->fetchall_arrayref; is_deeply( @@ -261,11 +269,18 @@ subtest 'get_saved_reports' => sub { # for next test, we want to let execute_query capture any SQL errors my $errors; - warning_like {local $dbh->{RaiseError} = 0; ($sth, $errors) = execute_query( - 'SELECT surname FRM borrowers', # error in the query is intentional - 0, 10 ) } - qr/DBD::mysql::st execute failed: You have an error in your SQL syntax;/, - "Wrong SQL syntax raises warning"; + warning_like { + local $dbh->{RaiseError} = 0; + ( $sth, $errors ) = execute_query( + { + sql => 'SELECT surname FRM borrowers', # error in the query is intentional + offset => 0, + limit => 10, + } + ) + } + qr/DBD::mysql::st execute failed: You have an error in your SQL syntax;/, + "Wrong SQL syntax raises warning"; ok( defined($errors) && exists($errors->{queryerr}), 'attempting to run a report with an SQL syntax error returns error message (Bug 12214)' @@ -290,14 +305,14 @@ subtest 'Ensure last_run is populated' => sub { is( $report->last_run, undef, 'Newly created report has null last_run ' ); - execute_query( $report->savedsql, undef, undef, undef, $report->id ); + execute_query( { sql => $report->savedsql, report_id => $report->id } ); $report->discard_changes(); isnt( $report->last_run, undef, 'First run of report populates last_run' ); my $previous_last_run = $report->last_run; sleep(1); # last_run is stored to the second, so we need to ensure at least one second has passed between runs - execute_query( $report->savedsql, undef, undef, undef, $report->id ); + execute_query( { sql => $report->savedsql, report_id => $report->id } ); $report->discard_changes(); isnt( $report->last_run, $previous_last_run, 'Second run of report updates last_run' ); --