From 83a545d8318560eb2ee385289ff8c291f07f6716 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 2 Oct 2019 09:58:59 +0000 Subject: [PATCH] Bug 23626: Only fetch full chart data if requested MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize --- .../intranet-tmpl/prog/en/includes/chart.inc | 14 +++++++--- .../prog/en/includes/reports-toolbar.inc | 8 +++++- .../modules/reports/guided_reports_start.tt | 27 ++++++++++++++----- reports/guided_reports.pl | 13 +++++---- 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc index cb1f6d3b55..eaec0db29e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc @@ -33,10 +33,16 @@ -
  • - - -
    • + [% IF allresults.size %] +
  • + + +
    • + [% ELSE %] +
  • +

    This chart will use only visible rows, click 'Fetch all data' to chart the full report

    +
    • + [% END %]
  • 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 cd787cadfb..8fd8c4dfd8 100644 --- 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 @@
    - Create chart + [% IF allresults.size %] + Create chart + [% ELSE %] + Create chart + Fetch all data for chart + [% END %] +
    [% 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 5ae50b6504..3bd9794a36 100644 --- 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.

    } $(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.

    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 9204c5af23..3cae7cd546 100755 --- 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); } -- 2.39.5