From 5cb0da7a56353988182cebfa0d16b22a4daebbfa Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 3 Jan 2018 13:13:40 +0000 Subject: [PATCH] Bug 18497: Use report id to retrieve saved SQL instead of passing param This patch takes some of the code when executing report and moves it to a sub to be reused when downloading To test: 1 - Run some very long report (see comment #1) 2 - Try to download, erk! 3 - Apply patch 4 - Run report, results hould not have changed 5 - Try to download, success! 6 - Ensure reports work as before Signed-off-by: Mark Tompsett Signed-off-by: Julian Maurice Signed-off-by: Jonathan Druart Signed-off-by: Nick Clemens --- .../prog/en/includes/reports-toolbar.inc | 10 +++-- reports/guided_reports.pl | 44 +++++++++++-------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc index a425919820..3d834329d7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc @@ -47,12 +47,16 @@ [% END %] [% IF ( execute ) %] + [% BLOCK params %] + [% FOREACH param IN sql_params %]&sql_params=[% param %][% END %] + [% END %] +
diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 106ddb528b..31269880f1 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -785,21 +785,7 @@ elsif ($phase eq 'Run this report'){ 'reports' => $report_id, ); } else { - # OK, we have parameters, or there are none, we run the report - # if there were parameters, replace before running - # split on ??. Each odd (2,4,6,...) entry should be a parameter to fill - my @split = split /<<|>>/,$sql; - my @tmpl_parameters; - for(my $i=0;$i<$#split/2;$i++) { - my $quoted = $sql_params[$i]; - # if there are special regexp chars, we must \ them - $split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; - if ($split[$i*2+1] =~ /\|\s*date\s*$/) { - $quoted = output_pref({ dt => dt_from_string($quoted), dateformat => 'iso', dateonly => 1 }) if $quoted; - } - $quoted = C4::Context->dbh->quote($quoted); - $sql =~ s/<<$split[$i*2+1]>>/$quoted/; - } + my $sql = get_prepped_report( $sql, @sql_params ); my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id ); my $total = nb_rows($sql) || 0; unless ($sth) { @@ -841,10 +827,15 @@ elsif ($phase eq 'Run this report'){ elsif ($phase eq 'Export'){ # export results to tab separated text or CSV - my $sql = $input->param('sql'); # FIXME: use sql from saved report ID#, not new user-supplied SQL! - my $format = $input->param('format'); - my $reportname = $input->param('reportname'); + my $report_id = $input->param('report_id'); + my $report = get_saved_report($report_id); + my $sql = $report->{savedsql}; + my @sql_params = $input->multi_param('sql_params'); + my $format = $input->param('format'); + my $reportname = $input->param('reportname'); my $reportfilename = $reportname ? "$reportname-reportresults.$format" : "reportresults.$format" ; + + $sql = get_prepped_report( $sql, @sql_params ); my ($sth, $q_errors) = execute_query($sql); unless ($q_errors and @$q_errors) { my ( $type, $content ); @@ -1053,3 +1044,20 @@ sub create_non_existing_group_and_subgroup { } } } + +# pass $sth and sql_params, get back an executable query +sub get_prepped_report { + my ($sql, @sql_params ) = @_; + my @split = split /<<|>>/,$sql; + for(my $i=0;$i<$#split/2;$i++) { + my $quoted = $sql_params[$i]; + # if there are special regexp chars, we must \ them + $split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; + if ($split[$i*2+1] =~ /\|\s*date\s*$/) { + $quoted = output_pref({ dt => dt_from_string($quoted), dateformat => 'iso', dateonly => 1 }) if $quoted; + } + $quoted = C4::Context->dbh->quote($quoted); + $sql =~ s/<<$split[$i*2+1]>>/$quoted/; + } + return $sql; +} -- 2.39.5