diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index 555e41d15e..cefecca892 100644 --- a/C4/Reports/Guided.pm +++ b/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)"); @@ -1050,7 +1051,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; diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl index 710de44f3d..701b52d727 100755 --- a/misc/cronjobs/runreport.pl +++ b/misc/cronjobs/runreport.pl @@ -269,7 +269,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"; diff --git a/opac/svc/report b/opac/svc/report index 5aad6fe3bf..5e6e50d94f 100755 --- a/opac/svc/report +++ b/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) { diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 8966dc4830..a3479a3023 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -821,7 +821,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"; @@ -834,7 +841,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 }; @@ -888,7 +895,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') { diff --git a/svc/report b/svc/report index a1fc818f78..209df383ca 100755 --- a/svc/report +++ b/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) { diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t index b38c722e98..3fd0b5eb3f 100755 --- a/t/db_dependent/Reports/Guided.t +++ b/t/db_dependent/Reports/Guided.t @@ -238,16 +238,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( @@ -258,11 +266,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)' @@ -287,14 +302,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' );