Bugzilla – Attachment 77781 Details for
Bug 21215
Saved SQL reports code improvements [Omnibus]
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21215: Saved SQL reports code improvements
0001-Bug-21215-Saved-SQL-reports-code-improvements.patch (text/plain), 27.42 KB, created by
paxed
on 2018-08-14 12:04:05 UTC
(
hide
)
Description:
Bug 21215: Saved SQL reports code improvements
Filename:
MIME Type:
Creator:
paxed
Created:
2018-08-14 12:04:05 UTC
Size:
27.42 KB
patch
obsolete
>From 49d831fad92e3ad787832dc49c6e324b68566dd6 Mon Sep 17 00:00:00 2001 >From: Pasi Kallinen <pasi.kallinen@joensuu.fi> >Date: Tue, 14 Aug 2018 08:40:27 +0300 >Subject: [PATCH] Bug 21215: Saved SQL reports code improvements > >The saved SQL reports code is a bit warty, so improve it: > >Instead of using the same code in multiple places to split the SQL >move that code into the Guided.pm module. > >Add several tests for parameter substitution and SQL validation. > >Make it much easier to add more parameter types for substitution. > >Test plan: > >1) Run your saved SQL reports and check that they work correctly >2) Create new SQL reports and check that the parameters are queried > correctly. >3) Prove the t/db_dependent/Reports/Guided.t > >Signed-off-by: Pasi Kallinen <pasi.kallinen@joensuu.fi> >--- > C4/Reports/Guided.pm | 246 ++++++++++++++++++++++++++++++++++++++-- > reports/guided_reports.pl | 150 +++++------------------- > t/db_dependent/Reports/Guided.t | 133 +++++++++++++++++++++- > 3 files changed, 389 insertions(+), 140 deletions(-) > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index f8d98e6f1e..6849c5f1a4 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -45,8 +45,11 @@ BEGIN { > nb_rows update_sql > GetReservedAuthorisedValues > GetParametersFromSQL >+ GetSQLAuthValueResult >+ GetPreppedSQLReport > IsAuthorisedValueValid > ValidateSQLParameters >+ ValidateSQLQuery > nb_rows update_sql > ); > } >@@ -508,11 +511,9 @@ sub execute_query { > $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} ); >- } elsif ($sql !~ /^\s*SELECT\b\s*/i) { >- return (undef, { queryerr => 'Missing SELECT'} ); >- } >+ >+ my $errors = ValidateSQLQuery($sql); >+ return (undef, @{$errors}[0]) if (scalar(@$errors)); > > my ($useroffset, $userlimit); > >@@ -838,6 +839,135 @@ sub _get_column_defs { > return \%columns; > } > >+sub _authval_resultvalue_authval { >+ my $parm = shift; >+ my %tmp = ( 'input' => $parm->{'authval'} ); >+ return \%tmp; >+} >+ >+sub _authval_resultvalue_branches { >+ my $parm = shift; >+ >+ my @authorised_values; >+ my %authorised_lib; >+ >+ # builds list, depending on authorised value... >+ my $libraries = Koha::Libraries->search( {}, { order_by => ['branchname'] } ); >+ while ( my $library = $libraries->next ) { >+ push @authorised_values, $library->branchcode; >+ $authorised_lib{$library->branchcode} = $library->branchname; >+ } >+ >+ my %tmp = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ return \%tmp; >+} >+ >+sub _authval_resultvalue_itemtypes { >+ my $parm = shift; >+ >+ my @authorised_values; >+ my %authorised_lib; >+ my $dbh=C4::Context->dbh; >+ # builds list, depending on authorised value... >+ my $sth = $dbh->prepare("SELECT itemtype,description FROM itemtypes ORDER BY description"); >+ $sth->execute; >+ while ( my ( $itemtype, $description ) = $sth->fetchrow_array ) { >+ push @authorised_values, $itemtype; >+ $authorised_lib{$itemtype} = $description; >+ } >+ >+ my %tmp = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ return \%tmp; >+} >+ >+sub _authval_resultvalue_biblio_framework { >+ my $parm = shift; >+ >+ my @authorised_values; >+ my %authorised_lib; >+ >+ # builds list, depending on authorised value... >+ my @frameworks = Koha::BiblioFrameworks->search({}, { order_by => ['frameworktext'] }); >+ my $default_source = ''; >+ push @authorised_values,$default_source; >+ $authorised_lib{$default_source} = 'Default'; >+ foreach my $framework (@frameworks) { >+ push @authorised_values, $framework->frameworkcode; >+ $authorised_lib{$framework->frameworkcode} = $framework->frameworktext; >+ } >+ >+ my %tmp = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ return \%tmp; >+} >+ >+sub _authval_resultvalue_biblio_cn_source { >+ my $parm = shift; >+ >+ my @authorised_values; >+ my %authorised_lib; >+ >+ # builds list, depending on authorised value... >+ my $class_sources = GetClassSources(); >+ my $default_source = C4::Context->preference("DefaultClassificationSource"); >+ foreach my $class_source (sort keys %$class_sources) { >+ next unless $class_sources->{$class_source}->{'used'} or >+ ($class_source eq $default_source); >+ push @authorised_values, $class_source; >+ $authorised_lib{$class_source} = $class_sources->{$class_source}->{'description'}; >+ } >+ >+ my %tmp = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ return \%tmp; >+} >+ >+sub _authval_resultvalue_biblio_categorycode { >+ my $parm = shift; >+ >+ my @authorised_values; >+ my %authorised_lib; >+ >+ # builds list, depending on authorised value... >+ my @patron_categories = Koha::Patron::Categories->search({}, { order_by => ['description']}); >+ %authorised_lib = map { $_->categorycode => $_->description } @patron_categories; >+ push @authorised_values, $_->categorycode for @patron_categories; >+ >+ my %tmp = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ return \%tmp; >+} >+ >+sub _authval_prepvalue_date { >+ my ($parm, $quoted) = @_; >+ $quoted = output_pref({ dt => dt_from_string($quoted), dateformat => 'iso', dateonly => 1 }) if $quoted; >+ return $quoted; >+} >+ >+# reserved_savedsql_auth_values contains the special replacement authority values >+# used in saved SQL queries. >+# >+# The 'auth' function takes a hashref - one element returned from GetParametersFromSQL - and >+# must return a hashref with one of: >+# - 'values' and 'labels' >+# (for a dropdown menu) >+# - 'input' >+# (for an input field) >+# - 'auth_val_error' and 'data' >+# (in case of error) >+# >+# The 'prep' function is used to reformat the input we got from the user, so it can >+# be used in SQL query. It takes in a hashref (one element from GetParametersFromSQL) and >+# the input string, and must return the correctly formatted string which will be used in >+# the SQL query. >+# >+my %reserved_savedsql_auth_values = ( >+ 'date' => {'auth' => \&_authval_resultvalue_authval, 'prep' => \&_authval_prepvalue_date}, >+ 'text' => {'auth' => \&_authval_resultvalue_authval}, >+ 'branches' => {'auth' => \&_authval_resultvalue_branches}, >+ 'itemtypes' => {'auth' => \&_authval_resultvalue_itemtypes}, >+ 'cn_source' => {'auth' => \&_authval_resultvalue_biblio_cn_source}, >+ 'categorycode' => {'auth' => \&_authval_resultvalue_biblio_categorycode}, >+ 'biblio_framework' => {'auth' => \&_authval_resultvalue_biblio_framework}, >+ ); >+ > =head2 GetReservedAuthorisedValues > > my %reserved_authorised_values = GetReservedAuthorisedValues(); >@@ -848,16 +978,52 @@ Returns a hash containig all reserved words > > sub GetReservedAuthorisedValues { > my %reserved_authorised_values = >- map { $_ => 1 } ( 'date', >- 'branches', >- 'itemtypes', >- 'cn_source', >- 'categorycode', >- 'biblio_framework' ); >+ map { $_ => 1 } ( keys(%reserved_savedsql_auth_values) ); > > return \%reserved_authorised_values; > } > >+=head2 GetSQLAuthValueResult >+ >+ my $params = GetParametersFromSQL($sql); >+ my $authdata = GetSQLAuthValueResult(@{$params}[0]); >+ >+=cut >+ >+sub GetSQLAuthValueResult { >+ my $authparam = shift; >+ my $authorised_value = $authparam->{'authval'}; >+ my %ret; >+ >+ if (defined($reserved_savedsql_auth_values{$authorised_value})) { >+ return &{$reserved_savedsql_auth_values{$authorised_value}{'auth'}}($authparam); >+ } elsif ( Koha::AuthorisedValues->search({ category => $authorised_value })->count ) { >+ my @authorised_values; >+ my %authorised_lib; >+ my $query = ' >+ SELECT authorised_value,lib >+ FROM authorised_values >+ WHERE category=? >+ ORDER BY lib >+ '; >+ my $dbh=C4::Context->dbh; >+ my $authorised_values_sth = $dbh->prepare($query); >+ $authorised_values_sth->execute( $authorised_value); >+ >+ while ( my ( $value, $lib ) = $authorised_values_sth->fetchrow_array ) { >+ push @authorised_values, $value; >+ $authorised_lib{$value} = $lib; >+ } >+ %ret = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ } else { >+ %ret = ( 'auth_val_error' => 1, >+ 'data' => { >+ 'entry' => $authparam->{'name'}, >+ 'auth_val' => $authorised_value >+ } ); >+ } >+ return \%ret; >+} > > =head2 IsAuthorisedValueValid > >@@ -895,14 +1061,46 @@ sub GetParametersFromSQL { > my @split = split(/<<|>>/,$sql); > my @sql_parameters = (); > >+ # split on ??. Each odd (2,4,6,...) entry should be a parameter to fill > for ( my $i = 0; $i < ($#split/2) ; $i++ ) { > my ($name,$authval) = split(/\|/,$split[$i*2+1]); >- push @sql_parameters, { 'name' => $name, 'authval' => $authval }; >+ $authval ||= 'text'; >+ push @sql_parameters, { 'name' => $name, 'authval' => $authval, 'rawparam' => $split[$i*2+1] }; > } > > return \@sql_parameters; > } > >+=head2 GetPreppedSQLReport >+ >+ my $sql = GetPreppedSQLReport( $sql, \@param_names, \@sql_params ); >+ >+Returns an executable query >+ >+=cut >+ >+sub GetPreppedSQLReport { >+ my ($sql, $param_names, $sql_params ) = @_; >+ my %lookup; >+ @lookup{@$param_names} = @$sql_params; >+ my $split = GetParametersFromSQL( $sql ); >+ my @tmpl_parameters; >+ my $i = 0; >+ foreach my $parm (@$split) { >+ my $raw = $parm->{'rawparam'}; >+ my $quoted = @$param_names ? $lookup{ $raw } : @$sql_params[$i]; >+ # if there are special regexp chars, we must \ them >+ $raw =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g; >+ if (defined($reserved_savedsql_auth_values{$parm->{'authval'}}{'prep'})) { >+ $quoted = &{$reserved_savedsql_auth_values{$parm->{'authval'}}{'prep'}}($parm, $quoted); >+ } >+ $quoted = C4::Context->dbh->quote($quoted); >+ $sql =~ s/<<$raw>>/$quoted/; >+ $i++; >+ } >+ return $sql; >+} >+ > =head2 ValidateSQLParameters > > my @problematic_parameters = ValidateSQLParameters($sql) >@@ -928,6 +1126,30 @@ sub ValidateSQLParameters { > return \@problematic_parameters; > } > >+=head2 ValidateSQLQuery >+ >+ my $errors = ValidateSQLQuery( $sql ); >+ >+Validates SQL query string so it only contains a select, >+not any of the harmful queries. >+ >+=cut >+ >+sub ValidateSQLQuery { >+ my $sql = shift; >+ >+ my @errors; >+ >+ if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) { >+ push @errors, {sqlerr => $1}; >+ } >+ elsif ($sql !~ /^(SELECT)/i) { >+ push @errors, {queryerr => "No SELECT"}; >+ } >+ >+ return \@errors; >+} >+ > sub _get_display_value { > my ( $original_value, $column ) = @_; > if ( $column eq 'periodicity' ) { >diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl >index 0989dfdbd1..7070e58d5c 100755 >--- a/reports/guided_reports.pl >+++ b/reports/guided_reports.pl >@@ -232,12 +232,7 @@ elsif ( $phase eq 'Update SQL'){ > > create_non_existing_group_and_subgroup($input, $group, $subgroup); > >- if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) { >- push @errors, {sqlerr => $1}; >- } >- elsif ($sql !~ /^(SELECT)/i) { >- push @errors, {queryerr => "No SELECT"}; >- } >+ push(@errors, @{ValidateSQLQuery($sql)}); > > if (@errors) { > $template->param( >@@ -594,12 +589,7 @@ elsif ( $phase eq 'Save Report' ) { > create_non_existing_group_and_subgroup($input, $group, $subgroup); > > ## FIXME this is AFTER entering a name to save the report under >- if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) { >- push @errors, {sqlerr => $1}; >- } >- elsif ($sql !~ /^(SELECT)/i) { >- push @errors, {queryerr => "No SELECT"}; >- } >+ push(@errors, @{ValidateSQLQuery($sql)}); > > if (@errors) { > $template->param( >@@ -693,109 +683,43 @@ elsif ($phase eq 'Run this report'){ > $notes = $report->notes; > > my @rows = (); >+ my $split = GetParametersFromSQL( $sql ); >+ > # if we have at least 1 parameter, and it's not filled, then don't execute but ask for parameters >- if ($sql =~ /<</ && !@sql_params) { >- # split on ??. Each odd (2,4,6,...) entry should be a parameter to fill >- my @split = split /<<|>>/,$sql; >+ if (scalar(@$split) && !@sql_params) { > my @tmpl_parameters; > my @authval_errors; > my %uniq_params; >- for(my $i=0;$i<($#split/2);$i++) { >- my ($text,$authorised_value) = split /\|/,$split[$i*2+1]; >+ my $i = 0; >+ foreach my $parm (@$split) { >+ my $text = $parm->{'name'}; >+ my $authorised_value = $parm->{'authval'}; > my $sep = $authorised_value ? "|" : ""; >+ $i++; > if( defined $uniq_params{$text.$sep.$authorised_value} ){ > next; > } else { $uniq_params{$text.$sep.$authorised_value} = "$i"; } > my $input; > my $labelid; >- if ( not defined $authorised_value ) { >- # no authorised value input, provide a text box >- $input = "text"; >- } elsif ( $authorised_value eq "date" ) { >- # require a date, provide a date picker >- $input = 'date'; >- } else { >- # defined $authorised_value, and not 'date' >- my $dbh=C4::Context->dbh; >- my @authorised_values; >- my %authorised_lib; >- # builds list, depending on authorised value... >- if ( $authorised_value eq "branches" ) { >- my $libraries = Koha::Libraries->search( {}, { order_by => ['branchname'] } ); >- while ( my $library = $libraries->next ) { >- push @authorised_values, $library->branchcode; >- $authorised_lib{$library->branchcode} = $library->branchname; >- } >- } >- elsif ( $authorised_value eq "itemtypes" ) { >- my $sth = $dbh->prepare("SELECT itemtype,description FROM itemtypes ORDER BY description"); >- $sth->execute; >- while ( my ( $itemtype, $description ) = $sth->fetchrow_array ) { >- push @authorised_values, $itemtype; >- $authorised_lib{$itemtype} = $description; >- } >- } >- elsif ( $authorised_value eq "biblio_framework" ) { >- my @frameworks = Koha::BiblioFrameworks->search({}, { order_by => ['frameworktext'] }); >- my $default_source = ''; >- push @authorised_values,$default_source; >- $authorised_lib{$default_source} = 'Default'; >- foreach my $framework (@frameworks) { >- push @authorised_values, $framework->frameworkcode; >- $authorised_lib{$framework->frameworkcode} = $framework->frameworktext; >- } >- } >- elsif ( $authorised_value eq "cn_source" ) { >- my $class_sources = GetClassSources(); >- my $default_source = C4::Context->preference("DefaultClassificationSource"); >- foreach my $class_source (sort keys %$class_sources) { >- next unless $class_sources->{$class_source}->{'used'} or >- ($class_source eq $default_source); >- push @authorised_values, $class_source; >- $authorised_lib{$class_source} = $class_sources->{$class_source}->{'description'}; >- } >- } >- elsif ( $authorised_value eq "categorycode" ) { >- my @patron_categories = Koha::Patron::Categories->search({}, { order_by => ['description']}); >- %authorised_lib = map { $_->categorycode => $_->description } @patron_categories; >- push @authorised_values, $_->categorycode for @patron_categories; >- } >- else { >- if ( Koha::AuthorisedValues->search({ category => $authorised_value })->count ) { >- my $query = ' >- SELECT authorised_value,lib >- FROM authorised_values >- WHERE category=? >- ORDER BY lib >- '; >- my $authorised_values_sth = $dbh->prepare($query); >- $authorised_values_sth->execute( $authorised_value); >- >- while ( my ( $value, $lib ) = $authorised_values_sth->fetchrow_array ) { >- push @authorised_values, $value; >- $authorised_lib{$value} = $lib; >- # For item location, we show the code and the libelle >- $authorised_lib{$value} = $lib; >- } >- } else { >- # not exists $authorised_value_categories{$authorised_value}) >- push @authval_errors, {'entry' => $text, >- 'auth_val' => $authorised_value }; >- # tell the template there's an error >- $template->param( auth_val_error => 1 ); >- # skip scrolling list creation and params push >- next; >- } >- } >+ >+ my $ret = GetSQLAuthValueResult($parm); >+ >+ if (defined($ret->{'values'})) { > $labelid = $text; > $labelid =~ s/\W//g; > $input = { > name => "sql_params", > id => "sql_params_".$labelid, >- values => \@authorised_values, >- labels => \%authorised_lib, >+ values => $ret->{'values'}, >+ labels => $ret->{'labels'}, > }; >- } >+ } elsif (defined($ret->{'auth_val_error'})) { >+ push(@authval_errors, $ret->{'data'}); >+ $template->param( auth_val_error => 1 ); >+ next; >+ } else { >+ $input = $ret->{'input'}; >+ } > > push @tmpl_parameters, {'entry' => $text, 'input' => $input, 'labelid' => $labelid, 'name' => $text.$sep.$authorised_value }; > } >@@ -807,11 +731,13 @@ elsif ($phase eq 'Run this report'){ > 'reports' => $report_id, > ); > } else { >- my $sql = get_prepped_report( $sql, \@param_names, \@sql_params); >+ my $sql = GetPreppedSQLReport( $sql, \@param_names, \@sql_params); > my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id ); > my $total = nb_rows($sql) || 0; > unless ($sth) { >- die "execute_query failed to return sth for report $report_id: $sql"; >+ if (!defined($errors)) { >+ die "execute_query failed to return sth for report $report_id: $sql"; >+ } > } else { > my $headers = header_cell_loop($sth); > $template->param(header_row => $headers); >@@ -859,7 +785,7 @@ elsif ($phase eq 'Export'){ > my $reportname = $input->param('reportname'); > my $reportfilename = $reportname ? "$reportname-reportresults.$format" : "reportresults.$format" ; > >- $sql = get_prepped_report( $sql, \@param_names, \@sql_params ); >+ $sql = GetPreppedSQLReport( $sql, \@param_names, \@sql_params ); > my ($sth, $q_errors) = execute_query($sql); > unless ($q_errors and @$q_errors) { > my ( $type, $content ); >@@ -1052,23 +978,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 ) = @_; >- my %lookup; >- @lookup{@$param_names} = @$sql_params; >- my @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; >- } >- $quoted = C4::Context->dbh->quote($quoted); >- $sql =~ s/<<$split[$i*2+1]>>/$quoted/; >- } >- return $sql; >-} >diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t >index 827cfc17a6..e2b9df7799 100644 >--- a/t/db_dependent/Reports/Guided.t >+++ b/t/db_dependent/Reports/Guided.t >@@ -18,7 +18,7 @@ > > use Modern::Perl; > >-use Test::More tests => 9; >+use Test::More tests => 12; > use Test::Warn; > > use t::lib::TestBuilder; >@@ -110,6 +110,7 @@ subtest 'GetReservedAuthorisedValues' => sub { > # to GetReservedAuthorisedValues > my %test_authval = ( > 'date' => 1, >+ 'text' => 1, > 'branches' => 1, > 'itemtypes' => 1, > 'cn_source' => 1, >@@ -123,7 +124,7 @@ subtest 'GetReservedAuthorisedValues' => sub { > }; > > subtest 'IsAuthorisedValueValid' => sub { >- plan tests => 8; >+ plan tests => 9; > ok( IsAuthorisedValueValid('LOC'), > 'User defined authorised value category is valid'); > >@@ -137,6 +138,26 @@ subtest 'IsAuthorisedValueValid' => sub { > } > }; > >+subtest 'ValidateSQLQuery' => sub { >+ plan tests => 2 + 6 * 2; >+ >+ is_deeply( ValidateSQLQuery("SELECT * FROM bar"), [], 'Query starts with select'); >+ >+ my @result = ( { queryerr => "No SELECT"} ); >+ is_deeply( ValidateSQLQuery("ELECT * FROM bar"), \@result, 'Query does not start with select'); >+ >+ my @banned_words = ("UPDATE", "DELETE", "DROP", "INSERT", "SHOW", "CREATE"); >+ foreach my $banned (@banned_words) { >+ @result = ( { sqlerr => $banned } ); >+ my $sql = $banned . " foo WHERE bar"; >+ is_deeply( ValidateSQLQuery($sql), \@result, 'Query starts with ' . $banned); >+ >+ @result = ( { sqlerr => $banned } ); >+ $sql = "baz;" . $banned . " foo WHERE bar"; >+ is_deeply( ValidateSQLQuery($sql), \@result, 'Query has colon and ' . $banned); >+ } >+}; >+ > subtest 'GetParametersFromSQL+ValidateSQLParameters' => sub { > plan tests => 3; > my $test_query_1 = " >@@ -148,16 +169,16 @@ subtest 'GetParametersFromSQL+ValidateSQLParameters' => sub { > "; > > my @test_parameters_with_custom_list = ( >- { 'name' => 'Year', 'authval' => 'custom_list' }, >- { 'name' => 'Branch', 'authval' => 'branches' }, >- { 'name' => 'Borrower', 'authval' => undef } >+ { 'name' => 'Year', 'authval' => 'custom_list', 'rawparam' => 'Year|custom_list' }, >+ { 'name' => 'Branch', 'authval' => 'branches', 'rawparam' => 'Branch|branches' }, >+ { 'name' => 'Borrower', 'authval' => 'text', 'rawparam' => 'Borrower' } > ); > > is_deeply( GetParametersFromSQL($test_query_1), \@test_parameters_with_custom_list, > 'SQL params are correctly parsed'); > > my @problematic_parameters = (); >- push @problematic_parameters, { 'name' => 'Year', 'authval' => 'custom_list' }; >+ push @problematic_parameters, { 'name' => 'Year', 'authval' => 'custom_list', 'rawparam' => 'Year|custom_list' }; > is_deeply( ValidateSQLParameters( $test_query_1 ), > \@problematic_parameters, > '\'custom_list\' not a valid category' ); >@@ -176,6 +197,106 @@ subtest 'GetParametersFromSQL+ValidateSQLParameters' => sub { > ); > }; > >+subtest 'GetSQLAuthValueResult' => sub { >+ plan tests => 4; >+ >+ my $test_query_1 = "<<Name>> <<Surname|text>> foo <<Year|date>> <<Branch|branches>> <<Temp|LOC>> <<bar|baz>>"; >+ my $params = GetParametersFromSQL( $test_query_1 ); >+ >+ >+ my %test_1_return = ( 'input' => 'text' ); >+ >+ is_deeply( GetSQLAuthValueResult(@{$params}[0]), >+ \%test_1_return, >+ 'Returned implicit text parameter correctly'); >+ >+ >+ my %test_2_return = ( 'input' => 'text' ); >+ >+ is_deeply( GetSQLAuthValueResult(@{$params}[1]), >+ \%test_1_return, >+ 'Returned explicit text parameter correctly'); >+ >+ >+ my %test_3_return = ( 'input' => 'date' ); >+ >+ is_deeply( GetSQLAuthValueResult(@{$params}[2]), >+ \%test_3_return, >+ 'Returned date parameter correctly'); >+ >+ >+ # same as _authval_resultvalue_branches() in C4/Reports/Guided.pm >+ my @authorised_values; >+ my %authorised_lib; >+ my $libraries = Koha::Libraries->search( {}, { order_by => ['branchname'] } ); >+ while ( my $library = $libraries->next ) { >+ push @authorised_values, $library->branchcode; >+ $authorised_lib{$library->branchcode} = $library->branchname; >+ } >+ my %test_4_return = ( 'values' => \@authorised_values, 'labels' => \%authorised_lib ); >+ >+ is_deeply( GetSQLAuthValueResult(@{$params}[3]), >+ \%test_4_return, >+ 'Returned branches parameter correctly'); >+}; >+ >+subtest 'GetPreppedSQLReport' => sub { >+ plan tests => 5; >+ >+ # Test without parameters >+ my @paramnames = (); >+ my @sql_params = (); >+ >+ my $test_query_1 = "SELECT foo FROM bar WHERE qux"; >+ >+ is( GetPreppedSQLReport( $test_query_1, \@paramnames, \@sql_params ), $test_query_1, >+ 'Returns exact same SQL when no parameters to substitute'); >+ >+ >+ # Test completely unknown parameters >+ my $test_query_2 = "SELECT foo FROM bar WHERE <<qux>>"; >+ >+ is( GetPreppedSQLReport( $test_query_2, \@paramnames, \@sql_params ), >+ "SELECT foo FROM bar WHERE NULL", >+ 'Returns completely unknown parameters as NULLs'); >+ >+ >+ # Test unknown parameters >+ @paramnames = ( "qux" ); >+ my $test_query_3 = "SELECT foo FROM bar WHERE <<qux>>"; >+ >+ is( GetPreppedSQLReport( $test_query_3, \@paramnames, \@sql_params ), >+ "SELECT foo FROM bar WHERE NULL", >+ 'Returns parameters with unknown sql_param as NULLs'); >+ >+ >+ >+ # Test parameter substitution >+ @paramnames = ( "qux" ); >+ @sql_params = ( "XXXX" ); >+ >+ my $test_query_4 = "foo qux <<qux>> bar"; >+ my $test_query_4_temp = "foo qux ? bar"; >+ >+ my $dbh = C4::Context->dbh; >+ my $test_query_4_ok = "foo qux " . $dbh->quote($sql_params[0]) . " bar"; >+ >+ is( GetPreppedSQLReport( $test_query_4, \@paramnames, \@sql_params ), >+ $test_query_4_ok, >+ 'Returns parameter substituted'); >+ >+ >+ # Test character escaping (and NULL params, too) >+ @paramnames = ( "qu*x" ); >+ @sql_params = ( "XXX" ); >+ my $test_query_5 = "foo <<qx|text>> <<quux>> <<qu*x>> bar"; >+ my $test_query_5_ok = "foo NULL NULL " . $dbh->quote($sql_params[0]) . " bar"; >+ >+ is( GetPreppedSQLReport( $test_query_5, \@paramnames, \@sql_params ), >+ $test_query_5_ok, >+ 'Handles escaping parameters'); >+}; >+ > subtest 'get_saved_reports' => sub { > plan tests => 16; > my $dbh = C4::Context->dbh; >-- >2.11.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 21215
:
77776
|
77781
|
79060
|
80585
|
80710
|
80711