Bug 23626: Only fetch full chart data if requested

This patchset prevents the full return of report data unless explicitly requested by the user for charting purposes
Additionally the user is warned if requesting more than 1000 rows of data

To test:
 1 - Create a report that returns over 1000 rows of data
 2 - Run the report
 3 - Note you have two buttons now 'Chart data' and 'Fetch all data for chart'
 4 - Click chart data
 5 - Note the note that you are only charting visible data
 6 - Create the chart and confirm it works
 7 - Close the chart
 8 - Click 'Fetch all data'
 9 - Note the confirm window
10 - Click 'cancel', note there is no change
11 - Repeat and click ok
12 - Fetch all data button is gone
13 - Page to next data, note fetch all does not return
14 - Click 'Chart data'
15 - Note you now have checkbox option to use all data in report
16 - Click it
17 - Create chart
18 - Confirm it works as expected

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc
index cb1f6d3..eaec0db 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc
@@ -33,10 +33,16 @@
                             </select>
                         </li>
 
-                        <li>
-                            <label for="include-all">Include all rows (ignore pagination):</label>
-                            <input id="include-all" name="chart-include-all" type="checkbox" />
-                        </li>
+                        [% IF allresults.size %]
+                            <li>
+                                <label for="include-all">Include all rows (ignore pagination):</label>
+                                <input id="include-all" name="chart-include-all" type="checkbox" />
+                            </li>
+                        [% ELSE %]
+                            <li>
+                                <p>This chart will use only visible rows, click 'Fetch all data' to chart the full report</p>
+                            </li>
+                        [% END %]
 
                         <li>
                             <label for="exclude-last">Exclude last line (Rollup): </label>

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 cd787ca..8fd8c4d 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc
@@ -107,7 +107,13 @@
                 <a class="btn btn-default toggle_sql" id="toggle_sql_vis" href="#" style="display:none;"><i class="fa fa-eye-slash"></i> Hide SQL code</a>
             </div>
             <div class="btn-group">
-                <a class="btn btn-default" href="#" data-toggle="modal" data-target="#chartModal"><i class="fa fa-bar-chart"></i> Create chart</a>
+                [% IF allresults.size %]
+                    <a class="btn btn-default" href="#" data-toggle="modal" data-target="#chartModal"><i class="fa fa-bar-chart"></i> Create chart</a>
+                [% ELSE %]
+                    <a class="btn btn-default" href="#" data-toggle="modal" data-target="#chartModal"><i class="fa fa-bar-chart"></i> Create chart</a>
+                    <a class="btn btn-default fetch_chart_data" href="/cgi-bin/koha/reports/guided_reports.pl?reports=[% id | html %]&amp;phase=Run this report&amp;reportname=[% name |uri %][% PROCESS params %]&amp;want_full_chart=1"><i class="fa fa-bar-chart"></i> Fetch all data for chart</a>
+                [% END %]
+
             </div>
         [% END %]
 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
index 5ae50b6..3bd9794 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
@@ -1159,6 +1159,17 @@ canned reports and writing custom SQL reports.</p>
         }
 
         $(document).ready(function(){
+
+            $("body").on('click',".fetch_chart_data",function(){
+                if( [% unlimited_total %] > 1000 ){
+                    if( confirm("Fetching full chart data for reports with many rows can cause performance issues. Are you sure you with to chart this report?") ){
+                        return true;
+                    } else {
+                        return false;
+                    }
+                }
+            });
+
             var showsql;
             hide_bar_element();
 
@@ -1201,12 +1212,16 @@ canned reports and writing custom SQL reports.</p>
                     headers = [% header_row.json | $raw %];
 
                     var results;
-                    if ($('input[name="chart-include-all"]').prop('checked')) {
-                        results = [% allresults.json | $raw %]
-                    }
-                    else {
-                        results = [% results.json | $raw %]
-                    }
+                    [% IF allresults.size %]
+                        if ($('input[name="chart-include-all"]').prop('checked')) {
+                            results = [% allresults.json | $raw %]
+                        }
+                        else {
+                            results = [% results.json | $raw %]
+                        }
+                    [% ELSE %]
+                        results = [% results.json | $raw %];
+                    [% END %]
 
                     if ($('input[name="chart-exclude-last"]').prop('checked')) {
                         results.splice(-1, 1);

diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl
index 9204c5a..3cae7cd 100755 (executable)
--- a/reports/guided_reports.pl
+++ b/reports/guided_reports.pl
@@ -692,6 +692,7 @@ elsif ($phase eq 'Run this report'){
     my $report_id  = $input->param('reports');
     my @sql_params = $input->multi_param('sql_params');
     my @param_names = $input->multi_param('param_name');
+    my $want_full_chart = $input->param('want_full_chart') || 0;
 
     # offset algorithm
     if ($input->param('page')) {
@@ -829,7 +830,6 @@ elsif ($phase eq 'Run this report'){
             my ($sql,$header_types) = get_prepped_report( $sql, \@param_names, \@sql_params);
             $template->param(header_types => $header_types);
             my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id );
-            my ($sth2, $errors2) = execute_query($sql);
             my $total = nb_rows($sql) || 0;
             unless ($sth) {
                 die "execute_query failed to return sth for report $report_id: $sql";
@@ -840,14 +840,17 @@ elsif ($phase eq 'Run this report'){
                     my @cells = map { +{ cell => $_ } } @$row;
                     push @rows, { cells => \@cells };
                 }
-                while (my $row = $sth2->fetchrow_arrayref()) {
-                    my @cells = map { +{ cell => $_ } } @$row;
-                    push @allrows, { cells => \@cells };
+                if( $want_full_chart ){
+                    my ($sth2, $errors2) = execute_query($sql);
+                    while (my $row = $sth2->fetchrow_arrayref()) {
+                        my @cells = map { +{ cell => $_ } } @$row;
+                        push @allrows, { cells => \@cells };
+                    }
                 }
             }
 
             my $totpages = int($total/$limit) + (($total % $limit) > 0 ? 1 : 0);
-            my $url = "/cgi-bin/koha/reports/guided_reports.pl?reports=$report_id&phase=Run%20this%20report&limit=$limit";
+            my $url = "/cgi-bin/koha/reports/guided_reports.pl?reports=$report_id&phase=Run%20this%20report&limit=$limit&want_full_chart=$want_full_chart";
             if (@param_names) {
                 $url = join('&param_name=', $url, map { URI::Escape::uri_escape_utf8($_) } @param_names);
             }