Bugzilla – Attachment 131539 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), 10.36 KB, created by
Fridolin Somers
on 2022-03-10 06:52:34 UTC
(
hide
)
Description:
Bug 26669: Last Run if report not always updated
Filename:
MIME Type:
Creator:
Fridolin Somers
Created:
2022-03-10 06:52:34 UTC
Size:
10.36 KB
patch
obsolete
>From e5d65483d6941f42fddc5c0cbd81356e36dde532 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 : > >1) Create a report. >2) Run report. >3) Check the report listing. Confirm that the last run info on the report is updated. >4) Make report public. >5) Run report via public url. >6) Check the report listing. Confirm that the last run info on the report IS NOT updated. >7) Schedule the report to run at a given time and e-mailed to an address. >8) After the report runs at the scheduled time, check the report listing. Confirm that the last run info on the report IS NOT updated. >9) Run report. >10) Export results. >11) Check the report listing. Confirm that the last run info on the report IS NOT updated AT THE TIME OF THE EXPORT. > >Questionable (I don't know if this is addressed): >12) Run report on backend through a cron job and send results via e-mail. >13) Check the report listing. Confirm that the last run info on the report IS NOT updated. > >14) Apply patch. >15) Rerun steps 2-13. Confirm that steps 3, 6, 8, 11, and 13 DO UPDATE the last run info. >--- > C4/Reports/Guided.pm | 14 +++++++----- > misc/cronjobs/runreport.pl | 8 ++++++- > opac/svc/report | 11 +++++++--- > reports/guided_reports.pl | 13 ++++++++--- > svc/report | 10 +++++++-- > t/db_dependent/Reports/Guided.t | 39 +++++++++++++++++++++++---------- > 6 files changed, 68 insertions(+), 27 deletions(-) > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index 295e1312e7..5ccf0c105c 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -541,17 +541,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; > > Koha::Logger->get->debug("Report - execute_query($sql, $offset, $limit)"); > >@@ -1053,7 +1054,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 b3ce925350..bc3c85c5d8 100755 >--- a/misc/cronjobs/runreport.pl >+++ b/misc/cronjobs/runreport.pl >@@ -262,7 +262,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 5aad6fe3bf..5e6e50d94f 100755 >--- a/opac/svc/report >+++ b/opac/svc/report >@@ -58,13 +58,18 @@ if ($cache_active) { > } > > unless ($json_text) { >- my $offset = 0; > my $limit = C4::Context->preference("SvcMaxReportRows") || 10; > > my ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params ); > >- my ( $sth, $errors ) = >- execute_query( $sql, $offset, $limit, undef, $report_id ); >+ my ( $sth, $errors ) = execute_query( >+ { >+ sql => $sql, >+ offset => 0, >+ limit => $limit, >+ report_id => $report_id, >+ } >+ ); > if ($sth) { > my $lines; > if ($report_annotation) { >diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl >index 6106fa9c03..d82094e105 100755 >--- a/reports/guided_reports.pl >+++ b/reports/guided_reports.pl >@@ -846,7 +846,14 @@ elsif ($phase eq 'Run this report'){ > } else { > my ($sql,$header_types) = $report->prep_report( \@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; > if (!$sth) { > die "execute_query failed to return sth for report $report_id: $sql"; >@@ -859,7 +866,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 }; >@@ -913,7 +920,7 @@ elsif ($phase eq 'Export'){ > my $reportfilename = $reportname ? "$reportname-reportresults.$format" : "reportresults.$format" ; > > ($sql, undef) = $report->prep_report( \@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 a1fc818f78..209df383ca 100755 >--- a/svc/report >+++ b/svc/report >@@ -61,12 +61,18 @@ if ($cache_active) { > } > > unless ($json_text) { >- my $offset = 0; > my $limit = C4::Context->preference("SvcMaxReportRows") || 10; > > my ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params ); > >- my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id ); >+ my ( $sth, $errors ) = execute_query( >+ { >+ sql => $sql, >+ offset => 0, >+ limit => $limit, >+ 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 93e0635229..6fda469ba0 100755 >--- a/t/db_dependent/Reports/Guided.t >+++ b/t/db_dependent/Reports/Guided.t >@@ -241,16 +241,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( >@@ -261,11 +269,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)' >@@ -290,14 +305,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.35.1
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