Bug 18497: Use report id to retrieve saved SQL instead of passing param
authorNick Clemens <nick@bywatersolutions.com>
Wed, 3 Jan 2018 13:13:40 +0000 (13:13 +0000)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Fri, 2 Feb 2018 13:20:43 +0000 (10:20 -0300)
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 <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc
reports/guided_reports.pl

index d7fa79d..00edf94 100644 (file)
         [% END %]
 
         [% IF ( execute ) %]
+            [% BLOCK params %]
+                [% FOREACH param IN sql_params %]&amp;sql_params=[% param %][% END %]
+            [% END %]
+
             <div class="btn-group">
                 <button class="btn btn-default btn-sm dropdown-toggle" data-toggle="dropdown" id="format"><i class="fa fa-upload"></i> Download <span class="caret"></span></button>
                 <ul class="dropdown-menu">
-                    <li><a id="csv" href="/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&amp;format=csv&amp;sql=[% sql |uri %]&amp;reportname=[% name |uri %]">Comma separated text</a></li>
-                    <li><a id="tab" href="/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&amp;format=tab&amp;sql=[% sql |uri %]&amp;reportname=[% name |uri %]">Tab separated text</a></li>
-                    <li><a id="ods" href="/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&amp;format=ods&amp;sql=[% sql |uri %]&amp;reportname=[% name |uri %]">Open Document Spreadsheet</a></li>
+                    <li><a id="csv" href="/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&amp;format=csv&amp;report_id=[% id %]&amp;reportname=[% name |uri %][% PROCESS params %]">Comma separated text</a></li>
+                    <li><a id="tab" href="/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&amp;format=tab&amp;report_id=[% id %]&amp;reportname=[% name |uri %][% PROCESS params %]">Tab separated text</a></li>
+                    <li><a id="ods" href="/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Export&amp;format=ods&amp;report_id=[% id %]&amp;reportname=[% name |uri %][% PROCESS params %]">Open Document Spreadsheet</a></li>
                 </ul>
             </div>
             <div class="btn-group">
index 106ddb5..3126988 100755 (executable)
@@ -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;
+}