From 66f1e396ea57c267468bef305682996c4b6b6b66 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 (cherry picked from commit 83a545d8318560eb2ee385289ff8c291f07f6716) Signed-off-by: Fridolin Somers --- .../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 58ff9e3acd..9b053f24bf 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 @@ -1058,6 +1058,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(); @@ -1100,12 +1111,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 0926419488..0eecafffc3 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')) { @@ -827,7 +828,6 @@ elsif ($phase eq 'Run this report'){ } else { my $sql = get_prepped_report( $sql, \@param_names, \@sql_params); 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"; @@ -838,14 +838,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