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 <lucas@bywatersolutions.com>
This commit is contained in:
parent
9d0d9715e8
commit
1162d2d67c
|
|
@ -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<user_id> (a borrowernumber) and C<report_id> are optional. The lookup
|
||||
relies on the SQL comment embedded by L<Koha::Report/prep_report> being
|
||||
visible in C<information_schema.processlist>.
|
||||
|
||||
=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
|
||||
|
|
|
|||
|
|
@ -1638,7 +1638,7 @@
|
|||
<div class="alert alert-warning">
|
||||
<strong>The following error was encountered:</strong><br />
|
||||
[% 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 <strong>[% error.sqlerr | html %]</strong>.<br />
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
|
|
|
|||
Loading…
Reference in New Issue