Browse Source

Bug 27380: Move get_prepped_report to object and use for svc/reports

This patch moves get_prepped_report to Koha:Report->prep_report
and adds some basic tests

To test:
1 - Using the report created in last test, hit the report svc api like:
http://localhost:8081/cgi-bin/koha/svc/report?id=6&param_name=One&sql_params=One&param_name=Listy|list&sql_params=2345%0D%0A423%0D%0A3%0D%0A2%0D%0A12
2 - Note the use of %0D%0A to separate list params
3 - Test with combinations with and without param_name specified
http://localhost:8081/cgi-bin/koha/svc/report?id=6&sql_params=5&sql_params=2345%0D%0A423%0D%0A3%0D%0A2%0D%0A12

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD Amended patch: Perltidy prep_report

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
master
Nick Clemens 1 month ago
parent
commit
45ec6ba3a4
6 changed files with 113 additions and 59 deletions
  1. +12
    -0
      C4/Reports/Guided.pm
  2. +67
    -0
      Koha/Report.pm
  3. +5
    -5
      opac/svc/report
  4. +2
    -47
      reports/guided_reports.pl
  5. +5
    -6
      svc/report
  6. +22
    -1
      t/db_dependent/Koha/Reports.t

+ 12
- 0
C4/Reports/Guided.pm View File

@@ -557,6 +557,18 @@ sub execute_query {
return (undef, { queryerr => 'Missing SELECT'} );
}

foreach my $sql_param ( @$sql_params ){
if ( $sql_param =~ m/\n/ ){
my @list = split /\n/, $sql_param;
my @quoted_list;
foreach my $item ( @list ){
$item =~ s/\r//;
push @quoted_list, C4::Context->dbh->quote($item);
}
$sql_param = "(".join(",",@quoted_list).")";
}
}

my ($useroffset, $userlimit);

# Grab offset/limit from user supplied LIMIT and drop the LIMIT so we can control pagination


+ 67
- 0
Koha/Report.pm View File

@@ -90,6 +90,73 @@ sub new_from_mana {
Koha::Report->new($data)->store;
}

=head3 prep_report

Prep the report and return executable sql with parameters embedded and a list of header types
for building batch action links in the template

=cut

sub prep_report {
my ( $self, $param_names, $sql_params ) = @_;
my $sql = $self->savedsql;

# First we split out the placeholders
# This part of the code supports using [[ table.field | alias ]] in the
# query and replaces it by table.field AS alias. This is used to build
# the batch action links foir cardnumbers, itemnumbers, and biblionumbers in the template
# while allowing the library to alter the column names
my @split = split /\[\[|\]\]/, $sql;
my $headers;
for ( my $i = 0 ; $i < $#split / 2 ; $i++ )
{ #The placeholders are always the odd elements of the array
my ( $type, $name ) = split /\|/,
$split[ $i * 2 + 1 ]; # We split them on '|'
$headers->{$name} = $type; # Store as a lookup for the template
$headers->{$name} =~
s/^\w*\.//; # strip the table name just as in $sth->{NAME} array
$split[ $i * 2 + 1 ] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g
; #Quote any special characters so we can replace the placeholders
$name = C4::Context->dbh->quote($name);
$sql =~ s/\[\[$split[$i*2+1]\]\]/$type AS $name/
; # Remove placeholders from SQL
}

my %lookup;
@lookup{@$param_names} = @$sql_params;
@split = split /<<|>>/, $sql;
for ( my $i = 0 ; $i < $#split / 2 ; $i++ ) {
my $quoted =
@$param_names ? $lookup{ $split[ $i * 2 + 1 ] } : @$sql_params[$i];

# if there are special regexp chars, we must \ them
$split[ $i * 2 + 1 ] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g;
if ( $split[ $i * 2 + 1 ] =~ /\|\s*date\s*$/ ) {
$quoted = output_pref(
{
dt => dt_from_string($quoted),
dateformat => 'iso',
dateonly => 1
}
) if $quoted;
}
unless ( $split[ $i * 2 + 1 ] =~ /\|\s*list\s*$/ && $quoted ) {
$quoted = C4::Context->dbh->quote($quoted);
}
else {
my @list = split /\n/, $quoted;
my @quoted_list;
foreach my $item (@list) {
$item =~ s/\r//;
push @quoted_list, C4::Context->dbh->quote($item);
}
$quoted = "(" . join( ",", @quoted_list ) . ")";
}
$sql =~ s/<<$split[$i*2+1]>>/$quoted/;
}
return $sql, $headers;
}

=head3 _type

Returns name of corresponding DBIC resultset


+ 5
- 5
opac/svc/report View File

@@ -43,6 +43,7 @@ my $report_rec = $report_recs->next();
die "Sorry this report is not public\n" unless $report_rec->public;

my @sql_params = $query->multi_param('sql_params');
my @param_names = $query->multi_param('param_names');

my $cache = Koha::Caches->get_instance();
my $cache_active = $cache->is_cache_active;
@@ -51,7 +52,8 @@ if ($cache_active) {
$cache_key =
"opac:report:"
. ( $report_name ? "name:$report_name:" : "id:$report_id:" )
. join( '-', @sql_params );
. join( '-', @sql_params )
. join( '_'. @param_names );
$json_text = $cache->get_from_cache($cache_key);
}

@@ -60,12 +62,10 @@ unless ($json_text) {
my $limit = C4::Context->preference("SvcMaxReportRows") || 10;
my $sql = $report_rec->savedsql;

# convert SQL parameters to placeholders
$sql =~ s/(<<.*?>>)/\?/g;
$sql =~ s/\[\[(.*?)\|(.*?)\]\]/$1 AS $2/g;
my ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params );

my ( $sth, $errors ) =
execute_query( $sql, $offset, $limit, \@sql_params, $report_id );
execute_query( $sql, $offset, $limit, undef, $report_id );
if ($sth) {
my $lines;
if ($report_annotation) {


+ 2
- 47
reports/guided_reports.pl View File

@@ -829,7 +829,7 @@ elsif ($phase eq 'Run this report'){
'reports' => $report_id,
);
} else {
my ($sql,$header_types) = get_prepped_report( $sql, \@param_names, \@sql_params);
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 $total = nb_rows($sql) || 0;
@@ -894,7 +894,7 @@ elsif ($phase eq 'Export'){
my $reportname = $input->param('reportname');
my $reportfilename = $reportname ? "$reportname-reportresults.$format" : "reportresults.$format" ;

($sql, undef) = get_prepped_report( $sql, \@param_names, \@sql_params );
($sql, undef) = $report->prep_report( \@param_names, \@sql_params );
my ($sth, $q_errors) = execute_query($sql);
unless ($q_errors and @$q_errors) {
my ( $type, $content );
@@ -1090,48 +1090,3 @@ sub create_non_existing_group_and_subgroup {
}
}

# pass $sth and sql_params, get back an executable query
sub get_prepped_report {
my ($sql, $param_names, $sql_params ) = @_;

# First we split out the placeholders
# This part of the code supports using [[ table.field | alias ]] in the
# query and replaces it by table.field AS alias. Not sure why we would
# need it if we can type the latter (which is simpler)?
my @split = split /\[\[|\]\]/,$sql;
my $headers;
for(my $i=0;$i<$#split/2;$i++){ #The placeholders are always the odd elements of the array
my ($type,$name) = split /\|/,$split[$i*2+1]; # We split them on '|'
$headers->{$name} = $type; # Store as a lookup for the template
$headers->{$name} =~ s/^\w*\.//; # strip the table name just as in $sth->{NAME} array
$split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; #Quote any special characters so we can replace the placeholders
$name = C4::Context->dbh->quote($name);
$sql =~ s/\[\[$split[$i*2+1]\]\]/$type AS $name/; # Remove placeholders from SQL
}

my %lookup;
@lookup{@$param_names} = @$sql_params;
@split = split /<<|>>/,$sql;
my @tmpl_parameters;
for(my $i=0;$i<$#split/2;$i++) {
my $quoted = @$param_names ? $lookup{ $split[$i*2+1] } : @$sql_params[$i];
# if there are special regexp chars, we must \ them
$split[$i*2+1] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g;
if ($split[$i*2+1] =~ /\|\s*date\s*$/) {
$quoted = output_pref({ dt => dt_from_string($quoted), dateformat => 'iso', dateonly => 1 }) if $quoted;
}
unless( $split[$i*2+1] =~ /\|\s*list\s*$/ && $quoted ){
$quoted = C4::Context->dbh->quote($quoted);
} else {
my @list = split /\n/, $quoted;
my @quoted_list;
foreach my $item ( @list ){
$item =~ s/\r//;
push @quoted_list, C4::Context->dbh->quote($item);
}
$quoted="(".join(",",@quoted_list).")";
}
$sql =~ s/<<$split[$i*2+1]>>/$quoted/;
}
return $sql,$headers;
}

+ 5
- 6
svc/report View File

@@ -39,6 +39,7 @@ if (!$report_recs || $report_recs->count == 0 ) { die "There is no such report.\
my $report_rec = $report_recs->next();

my @sql_params = $query->multi_param('sql_params');
my @param_names = $query->multi_param('param_names');

my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
{
@@ -54,20 +55,18 @@ my $cache_active = $cache->is_cache_active;
my ($cache_key, $json_text);
if ($cache_active) {
$cache_key = "intranet:report:".($report_name ? "report_name:$report_name:" : "id:$report_id:")
. join( '-', @sql_params );
. join( '-', @sql_params )
. join( '_'. @param_names );
$json_text = $cache->get_from_cache($cache_key);
}

unless ($json_text) {
my $offset = 0;
my $limit = C4::Context->preference("SvcMaxReportRows") || 10;
my $sql = $report_rec->savedsql;

# convert SQL parameters to placeholders
$sql =~ s/(<<.*?>>)/\?/g;
$sql =~ s/\[\[(.*?)\|(.*?)\]\]/$1 AS $2/g;
my ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params );

my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, \@sql_params, $report_id );
my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id );
if ($sth) {
my $lines;
if ($report_annotation) {


+ 22
- 1
t/db_dependent/Koha/Reports.t View File

@@ -17,7 +17,7 @@

use Modern::Perl;

use Test::More tests => 4;
use Test::More tests => 5;

use Koha::Report;
use Koha::Reports;
@@ -48,4 +48,25 @@ is( $retrieved_report_1->report_name, $new_report_1->report_name, 'Find a report
$retrieved_report_1->delete;
is( Koha::Reports->search->count, $nb_of_reports + 1, 'Delete should have deleted the report' );

subtest 'prep_report' => sub {
plan tests => 3;

my $report = Koha::Report->new({
report_name => 'report_name_for_test_1',
savedsql => 'SELECT * FROM items WHERE itemnumber IN <<Test|list>>',
})->store;

my ($sql, undef) = $report->prep_report( ['Test|list'],["1\n12\n\r243"] );
is( $sql, q{SELECT * FROM items WHERE itemnumber IN ('1','12','243')},'Expected sql generated correctly with single param and name');

$report->savedsql('SELECT * FROM items WHERE itemnumber IN <<Test|list>> AND <<Another>> AND <<Test|list>>')->store;

($sql, undef) = $report->prep_report( ['Test|list','Another'],["1\n12\n\r243",'the other'] );
is( $sql, q{SELECT * FROM items WHERE itemnumber IN ('1','12','243') AND 'the other' AND ('1','12','243')},'Expected sql generated correctly with multiple params and names');

($sql, undef) = $report->prep_report( [],["1\n12\n\r243",'the other',"42\n32\n22\n12"] );
is( $sql, q{SELECT * FROM items WHERE itemnumber IN ('1','12','243') AND 'the other' AND ('42','32','22','12')},'Expected sql generated correctly with multiple params and no names');

};

$schema->storage->txn_rollback;

Loading…
Cancel
Save