From 1162d2d67cafa277d3e2a0d7690620e2b3150631 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 27 Apr 2026 17:39:49 +0100 Subject: [PATCH] Bug 41918: (QA follow-up) Refactor Koha::Reports->running - Fix POD synopsis: invalid syntax and wrong field (userid vs borrowernumber) replaced with a correct example and parameter list. - Build the WHERE clause with parameterised bindings via $schema->storage->dbh, removing string interpolation of $user_id / $report_id into LIKE patterns and the regex match. IDs are formatted with %d so non-integer input becomes 0 and cannot inject regex/LIKE metacharacters. - Return a Koha::Reports resultset instead of a list of MySQL process ids. The method name now matches what it returns and callers can use the standard ->count / iteration interface. The single in-tree caller in reports/guided_reports.pl is updated; the template error key is simplified from duplicate_running_report_ids to a boolean duplicate_running_report (the ids were never displayed). Test plan: 1) Apply the previous Bug 41918 patches and this follow-up. 2) Set duplicate_running_reports_per_user_limit to 1 in koha-conf.xml. 3) Restart all the things. 4) Open a long-running saved report in two tabs and run it from both. 5) Verify the second tab shows "This report is already running..." 6) Set the limit to 0 and confirm both tabs run normally. 7) prove t/db_dependent/Reports.t still passes. Signed-off-by: Lucas Gass --- Koha/Reports.pm | 60 +++++++++++-------- .../modules/reports/guided_reports_start.tt | 2 +- reports/guided_reports.pl | 16 +++-- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/Koha/Reports.pm b/Koha/Reports.pm index 7ee5ccc19e..deeaf8f33e 100644 --- a/Koha/Reports.pm +++ b/Koha/Reports.pm @@ -35,42 +35,54 @@ Koha::Reports - Koha Report Object set class =head3 running -Returns a list of reports that are currently running +Returns a Koha::Reports resultset of reports currently being executed, +optionally filtered by the patron that started them and/or by report id. -my @query_ids = Koha::Reports->running({ [ user_id => $user->userid ] }); + my $running = Koha::Reports->running( + { + user_id => $patron->borrowernumber, + report_id => $report->id, + } + ); + + if ( $running->count ) { ... } + +Both C (a borrowernumber) and C are optional. The lookup +relies on the SQL comment embedded by L being +visible in C. =cut sub running { - my ( $self, $params ) = @_; + my ( $class, $params ) = @_; my $user_id = $params->{user_id}; my $report_id = $params->{report_id}; - my $dbh = Koha::Database->dbh; + my @where = ( 'command != ?', 'info LIKE ?' ); + my @binds = ( 'Sleep', '%saved_sql.id:%' ); - my $query = q{ - SELECT id, info - FROM information_schema.processlist - WHERE command != 'Sleep' - AND info LIKE '%saved_sql.id%' - }; - - my @ids; - - my $sth = $dbh->prepare($query); - $sth->execute(); - while ( my $row = $sth->fetchrow_hashref ) { - if ($user_id) { - next unless $row->{info} =~ /{ user_id: $user_id }/; - } - if ($report_id) { - next unless $row->{info} =~ /{ saved_sql.id: $report_id }/; - } - push @ids, $row->{id}; + if ($user_id) { + push @where, 'info LIKE ?'; + push @binds, sprintf( '%%{ user_id: %d }%%', $user_id ); + } + if ($report_id) { + push @where, 'info LIKE ?'; + push @binds, sprintf( '%%{ saved_sql.id: %d }%%', $report_id ); } - return @ids; + my $sql = 'SELECT info FROM information_schema.processlist WHERE ' . join( ' AND ', @where ); + + my $schema = Koha::Database->new->schema; + my $rows = + $schema->storage->dbh->selectall_arrayref( $sql, { Slice => {} }, @binds ); + + my %report_ids; + for my $row (@$rows) { + $report_ids{$1} = 1 if $row->{info} =~ /saved_sql\.id:\s*(\d+)/; + } + + return $class->search( { id => { -in => [ keys %report_ids ] } } ); } =head3 _type 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 74ffefbca6..8e08b53447 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 @@ -1638,7 +1638,7 @@
The following error was encountered:
[% FOREACH error IN errors %] - [% IF ( error.duplicate_running_report_ids ) %] + [% IF ( error.duplicate_running_report ) %] This report is already running. Please wait for it to finish before running it again. [% ELSIF ( error.sqlerr ) %] This report contains the SQL keyword [% error.sqlerr | html %].
diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 45bf150eda..414372bfb6 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -824,11 +824,15 @@ if ( $op eq 'run' ) { my $template_id = $input->param('template'); my $want_full_chart = $input->param('want_full_chart') || 0; - my @duplicate_running_report_ids; my $duplicate_running_reports_per_user_limit = C4::Context->config('duplicate_running_reports_per_user_limit'); + my $duplicate_running_reports; if ( $duplicate_running_reports_per_user_limit && C4::Context->userenv ) { - my $user_id = C4::Context->userenv ? C4::Context->userenv->{number} : undef; - @duplicate_running_report_ids = Koha::Reports->running( { report_id => $report_id, user_id => $user_id } ); + $duplicate_running_reports = Koha::Reports->running( + { + report_id => $report_id, + user_id => C4::Context->userenv->{number}, + } + ); } # offset algorithm @@ -1016,8 +1020,8 @@ if ( $op eq 'run' ) { 'id' => $report_id, 'template_id' => $template_id, ); - } elsif ( $duplicate_running_reports_per_user_limit - && ( scalar @duplicate_running_report_ids >= $duplicate_running_reports_per_user_limit ) ) + } elsif ( $duplicate_running_reports + && $duplicate_running_reports->count >= $duplicate_running_reports_per_user_limit ) { $template->param( 'sql' => $sql, @@ -1026,7 +1030,7 @@ if ( $op eq 'run' ) { 'execute' => 1, 'name' => $name, 'notes' => $notes, - 'errors' => [ { duplicate_running_report_ids => \@duplicate_running_report_ids } ], + 'errors' => [ { duplicate_running_report => 1 } ], 'sql_params' => \@sql_params, 'param_names' => \@param_names, );