From 99a9ac2e270c367772b4374935e60179e78f202b Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 19 Feb 2013 15:58:53 -0300 Subject: [PATCH] Bug 9659 - Better handling of non-existent authorised value categories used in SQL reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user might create a SQL report that relies on non-existent authorised value categories. Because of a typo, or just because they copy&pasted the report from the Wiki. I added comments on the code, refactored some really tiny bits, and fixed an annoying warning of uninitialised variable also. Sponsored-by: Universidad Nacional de Córdoba --- .../en/modules/reports/guided_reports_start.tt | 48 +++++++++++------ reports/guided_reports.pl | 55 ++++++++++++++------ 2 files changed, 71 insertions(+), 32 deletions(-) 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 7f109d9..e968eef 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 @@ -551,26 +551,42 @@ canned reports and writing custom SQL reports.

[% IF ( enter_params ) %]
- -

Enter parameters for report [% name %]:

- [% IF ( notes ) %]

[% notes %]

[% END %] -
+ [% IF ( auth_val_error ) %] + +

Errors found when processing parameters for report: [% name %]

+
+
+ [% FOREACH auth_val_error IN auth_val_errors %] +

+ [% auth_val_error.entry %]: The authorised value category ([% auth_val_error.auth_val %]) + you selected does not exist. +

+ [% END %] +
+
+
+ [% ELSE %] + +

Enter parameters for report [% name %]:

+ [% IF ( notes ) %]

[% notes %]

[% END %] +
    - [% FOREACH sql_param IN sql_params %] - [% IF sql_param.input == 'date' %] -
  1. - -
  2. - [% ELSIF ( sql_param.input == 'text' ) %] -
  3. - [% ELSE %] -
  4. [% sql_param.input %]
  5. + [% FOREACH sql_param IN sql_params %] + [% IF sql_param.input == 'date' %] +
  6. + +
  7. + [% ELSIF ( sql_param.input == 'text' ) %] +
  8. + [% ELSE %] +
  9. [% sql_param.input %]
  10. + [% END %] [% END %] - [% END %]
-
-
+
+
+ [% END %]
[% END %] diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 23d85a9..ddf7fd8 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -29,6 +29,7 @@ use C4::Output; use C4::Dates qw/format_date/; use C4::Debug; use C4::Branch; # XXX subfield_is_koha_internal_p +use C4::Koha qw/GetAuthorisedValueCategories/; =head1 NAME @@ -549,14 +550,19 @@ elsif ($phase eq 'Run this report'){ # split on ??. Each odd (2,4,6,...) entry should be a parameter to fill my @split = split /<<|>>/,$sql; my @tmpl_parameters; + my @authval_errors; for(my $i=0;$i<($#split/2);$i++) { my ($text,$authorised_value) = split /\|/,$split[$i*2+1]; my $input; my $labelid; - if ($authorised_value eq "date") { - $input = 'date'; - } - elsif ($authorised_value) { + 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; @@ -596,16 +602,33 @@ elsif ($phase eq 'Run this report'){ #---- "true" authorised value } - else { - my $authorised_values_sth = $dbh->prepare("SELECT authorised_value,lib FROM authorised_values WHERE category=? ORDER BY lib"); - - $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 { + # Build a hash of authorised value categories + my %authorised_value_categories = map { $_ => 1 } (GetAuthorisedValueCategories()); + if ( exists $authorised_value_categories{$authorised_value}) { + 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; } } $labelid = $text; @@ -621,14 +644,14 @@ elsif ($phase eq 'Run this report'){ -multiple => 0, -tabindex => 1, ); - } else { - $input = "text"; } + push @tmpl_parameters, {'entry' => $text, 'input' => $input, 'labelid' => $labelid }; } $template->param('sql' => $sql, 'name' => $name, 'sql_params' => \@tmpl_parameters, + 'auth_val_errors' => \@authval_errors, 'enter_params' => 1, 'reports' => $report_id, ); -- 1.7.9.5