Bugzilla – Attachment 115166 Details for
Bug 26669
Last Run column not updated when report is run publicly (via CoverFlow or elsewhere)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26669: Last Run if report not always updated
Bug-26669-Last-Run-if-report-not-always-updated.patch (text/plain), 9.80 KB, created by
Fridolin Somers
on 2021-01-14 15:50:46 UTC
(
hide
)
Description:
Bug 26669: Last Run if report not always updated
Filename:
MIME Type:
Creator:
Fridolin Somers
Created:
2021-01-14 15:50:46 UTC
Size:
9.80 KB
patch
obsolete
>From d869c8297a3535c565d53c388686b999d963a93f Mon Sep 17 00:00:00 2001 >From: Fridolin Somers <fridolin.somers@biblibre.com> >Date: Thu, 14 Jan 2021 16:46:25 +0100 >Subject: [PATCH] Bug 26669: Last Run if report not always updated > >The last run of a report is updated only if method execute_query() is >called with report_id. >This whas missing for : >- when report is run publicly >- when report is sent by email >- when report is exported > >Patch changes the method signature to use a hash of params, in order to >easily avoid some params. > >Test plan is coming... >--- > C4/Reports/Guided.pm | 14 +++++++----- > misc/cronjobs/runreport.pl | 8 ++++++- > opac/svc/report | 12 +++++++--- > reports/guided_reports.pl | 13 ++++++++--- > svc/report | 11 ++++++++-- > t/db_dependent/Reports/Guided.t | 39 +++++++++++++++++++++++---------- > 6 files changed, 70 insertions(+), 27 deletions(-) > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index b625443a7a..30366a960a 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -539,17 +539,18 @@ sub strip_limit { > > sub execute_query { > >- my ( $sql, $offset, $limit, $sql_params, $report_id ) = @_; >- >- $sql_params = [] unless defined $sql_params; >+ my $params = shift; >+ my $sql = $params->{sql}; >+ my $offset = $params->{offset} || 0; >+ my $limit = $params->{limit} || 999999; >+ my $sql_params = defined $params->{sql_params} ? $params->{sql_params} : []; >+ my $report_id = $params->{report_id}; > > # check parameters > unless ($sql) { > carp "execute_query() called without SQL argument"; > return; > } >- $offset = 0 unless $offset; >- $limit = 999999 unless $limit; > $debug and print STDERR "execute_query($sql, $offset, $limit)\n"; > if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) { > return (undef, { sqlerr => $1} ); >@@ -1033,7 +1034,8 @@ sub EmailReport { > my $sql = $report->savedsql; > return ( { FATAL => "NO_REPORT" } ) unless $sql; > >- my ( $sth, $errors ) = execute_query( $sql ); #don't pass offset or limit, hardcoded limit of 999,999 will be used >+ #don't pass offset or limit, hardcoded limit of 999,999 will be used >+ my ( $sth, $errors ) = execute_query( { sql => $sql, report_id => $report_id } ); > return ( undef, [{ FATAL => "REPORT_FAIL" }] ) if $errors; > > my $counter = 1; >diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl >index 3e81e08a97..6a1b0b4024 100755 >--- a/misc/cronjobs/runreport.pl >+++ b/misc/cronjobs/runreport.pl >@@ -269,7 +269,13 @@ foreach my $report_id (@ARGV) { > my $params_needed = ( $sql =~ s/(<<[^>]+>>)/\?/g ); > die("You supplied ". scalar @params . " parameter(s) and $params_needed are required by the report") if scalar @params != $params_needed; > >- my ($sth) = execute_query( $sql, undef, undef, \@params, $report_id ); >+ my ($sth) = execute_query( >+ { >+ sql => $sql, >+ sql_params => \@params, >+ report_id => $report_id, >+ } >+ ); > my $count = scalar($sth->rows); > unless ($count) { > print "NO OUTPUT: 0 results from execute_query\n"; >diff --git a/opac/svc/report b/opac/svc/report >index ef07334573..6bae8aac2e 100755 >--- a/opac/svc/report >+++ b/opac/svc/report >@@ -56,7 +56,6 @@ if ($cache_active) { > } > > unless ($json_text) { >- my $offset = 0; > my $limit = C4::Context->preference("SvcMaxReportRows") || 10; > my $sql = $report_rec->savedsql; > >@@ -64,8 +63,15 @@ unless ($json_text) { > $sql =~ s/(<<.*?>>)/\?/g; > $sql =~ s/\[\[(.*?)\|(.*?)\]\]/$1 AS $2/g; > >- my ( $sth, $errors ) = >- execute_query( $sql, $offset, $limit, \@sql_params, $report_id ); >+ my ( $sth, $errors ) = execute_query( >+ { >+ sql => $sql, >+ offset => 0, >+ limit => $limit, >+ sql_params => \@sql_params, >+ report_id => $report_id, >+ } >+ ); > if ($sth) { > my $lines; > if ($report_annotation) { >diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl >index aab868ed19..6062242515 100755 >--- a/reports/guided_reports.pl >+++ b/reports/guided_reports.pl >@@ -829,7 +829,14 @@ elsif ($phase eq 'Run this report'){ > } else { > 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 ( $sth, $errors ) = execute_query( >+ { >+ sql => $sql, >+ offset => $offset, >+ limit => $limit, >+ report_id => $report_id, >+ } >+ ); > my $total = nb_rows($sql) || 0; > unless ($sth) { > die "execute_query failed to return sth for report $report_id: $sql"; >@@ -841,7 +848,7 @@ elsif ($phase eq 'Run this report'){ > push @rows, { cells => \@cells }; > } > if( $want_full_chart ){ >- my ($sth2, $errors2) = execute_query($sql); >+ my ( $sth2, $errors2 ) = execute_query( { sql => $sql, report_id => $report_id } ); > while (my $row = $sth2->fetchrow_arrayref()) { > my @cells = map { +{ cell => $_ } } @$row; > push @allrows, { cells => \@cells }; >@@ -893,7 +900,7 @@ elsif ($phase eq 'Export'){ > my $reportfilename = $reportname ? "$reportname-reportresults.$format" : "reportresults.$format" ; > > ($sql, undef) = get_prepped_report( $sql, \@param_names, \@sql_params ); >- my ($sth, $q_errors) = execute_query($sql); >+ my ( $sth, $q_errors ) = execute_query( { sql => $sql, report_id => $report_id } ); > unless ($q_errors and @$q_errors) { > my ( $type, $content ); > if ($format eq 'tab') { >diff --git a/svc/report b/svc/report >index 0ab8fb5dcb..8df4c9c23a 100755 >--- a/svc/report >+++ b/svc/report >@@ -59,7 +59,6 @@ if ($cache_active) { > } > > unless ($json_text) { >- my $offset = 0; > my $limit = C4::Context->preference("SvcMaxReportRows") || 10; > my $sql = $report_rec->savedsql; > >@@ -67,7 +66,15 @@ unless ($json_text) { > $sql =~ s/(<<.*?>>)/\?/g; > $sql =~ s/\[\[(.*?)\|(.*?)\]\]/$1 AS $2/g; > >- my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, \@sql_params, $report_id ); >+ my ( $sth, $errors ) = execute_query( >+ { >+ sql => $sql, >+ offset => 0, >+ limit => $limit, >+ sql_params => \@sql_params, >+ report_id => $report_id, >+ } >+ ); > if ($sth) { > my $lines; > if ($report_annotation) { >diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t >index 4352d48568..2fe1a982c8 100755 >--- a/t/db_dependent/Reports/Guided.t >+++ b/t/db_dependent/Reports/Guided.t >@@ -234,16 +234,24 @@ subtest 'get_saved_reports' => sub { > is( scalar @{ get_saved_reports() }, > $count, "Report2 and report3 have been deleted" ); > >- my $sth = execute_query('SELECT COUNT(*) FROM systempreferences', 0, 10); >+ my $sth = execute_query( >+ { >+ sql => 'SELECT COUNT(*) FROM systempreferences', >+ offset => 0, >+ limit => 10, >+ } >+ ); > my $results = $sth->fetchall_arrayref; > is(scalar @$results, 1, 'running a query returned a result'); > > my $version = C4::Context->preference('Version'); > $sth = execute_query( >- 'SELECT value FROM systempreferences WHERE variable = ?', >- 0, >- 10, >- [ 'Version' ], >+ { >+ sql => 'SELECT value FROM systempreferences WHERE variable = ?', >+ offset => 0, >+ limit => 10, >+ sql_params => ['Version'], >+ } > ); > $results = $sth->fetchall_arrayref; > is_deeply( >@@ -254,11 +262,18 @@ subtest 'get_saved_reports' => sub { > > # for next test, we want to let execute_query capture any SQL errors > my $errors; >- warning_like {local $dbh->{RaiseError} = 0; ($sth, $errors) = execute_query( >- 'SELECT surname FRM borrowers', # error in the query is intentional >- 0, 10 ) } >- qr/DBD::mysql::st execute failed: You have an error in your SQL syntax;/, >- "Wrong SQL syntax raises warning"; >+ warning_like { >+ local $dbh->{RaiseError} = 0; >+ ( $sth, $errors ) = execute_query( >+ { >+ sql => 'SELECT surname FRM borrowers', # error in the query is intentional >+ offset => 0, >+ limit => 10, >+ } >+ ) >+ } >+ qr/DBD::mysql::st execute failed: You have an error in your SQL syntax;/, >+ "Wrong SQL syntax raises warning"; > ok( > defined($errors) && exists($errors->{queryerr}), > 'attempting to run a report with an SQL syntax error returns error message (Bug 12214)' >@@ -283,14 +298,14 @@ subtest 'Ensure last_run is populated' => sub { > > is( $report->last_run, undef, 'Newly created report has null last_run ' ); > >- execute_query( $report->savedsql, undef, undef, undef, $report->id ); >+ execute_query( { sql => $report->savedsql, report_id => $report->id } ); > $report->discard_changes(); > > isnt( $report->last_run, undef, 'First run of report populates last_run' ); > > my $previous_last_run = $report->last_run; > sleep(1); # last_run is stored to the second, so we need to ensure at least one second has passed between runs >- execute_query( $report->savedsql, undef, undef, undef, $report->id ); >+ execute_query( { sql => $report->savedsql, report_id => $report->id } ); > $report->discard_changes(); > > isnt( $report->last_run, $previous_last_run, 'Second run of report updates last_run' ); >-- >2.30.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26669
:
115165
|
115166
|
115414
|
131539
|
132547
|
132548
|
132549
|
132587
|
132588
|
132589
|
132590