Bug 9659 - Better handling of non-existent authorised value categories used in SQL reports

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.

Use cases are:

- The user creates a report from SQL
  a) Uses bad authorised values
  b) Clicks 'Save Report'
  c) Koha lists the problematic authorised values
  d) The user decides to
  e-1) Save it anyway, it gets saved
  e-2) Edit the report, it gets back to where it chose 'Save Report'

- The user edits an already saved report (Update SQL)
  a) Uses bad authorised values
  b) Clicks 'Update SQL'
  c) Koha lists the problematic authorised values
  d) The user decides to
  e-1) Save it anyway, it gets saved
  e-2) Edit the report, it gets back to where it chose 'Update SQL'

- The user tries to run a saved report that contains bad authorised values, Koha advertises the problem and provides the user with a button 'Edit SQL' to fix things.

To test, just create a report from SQL using invalid authorised values like this (misspelled 'branch'):

SELECT *
FROM itemtypes
WHERE hola=<<Test branch1|branchee>> AND
      hola2=<<Test branch2|brancha>>

Regards
To+

Notes:
- I added several comments on the code.
- Fixed an annoying warning of uninitialised variable also (refactored some tiny bits to do it).
- Added the following methods
   -  C4::Reports::Guided::GetReservedAuthorisedValues
   -  C4::Reports::Guided::GetParametersFromSQL
   -  C4::Reports::Guided::IsAuthorisedValueValid
   -  C4::Reports::Guided::ValidateSQLParameters
   -  C4::Koha::IsAuthorisedValueCategory
- Those methods could have been used to refactor this guided reports code as its *a bit messy*. I chose to do it in a new bug of course :-D.
- Fixed some trivial perlcritic -5 errors
- Removed some debugging stuff left by mistake
- Fixed some POD problems
- Optimal SQL-driven IsAuthorisedValueCategory method
- Thanks to Owen and Jared for their patience heh.

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Work as described. No koha-qa errors.

Test:
Tried with examples (from help and test plan) reports, correctly
identifies invalid authorized values, and no problem with
authorized ones.

NOTE: Online help for this does not states that partial values
need to be between '%' in a SQLish way. Perhaps this could be
addressed inserting % in values or adding a checkbox (partial|exact).
Or changing help.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
(cherry picked from commit cdc5e11159)

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

C4/Koha.pm

@@ -57,6 +57,7 @@ BEGIN {
 		&getitemtypeimagelocation
 		&GetAuthorisedValues
 		&GetAuthorisedValueCategories
+                &IsAuthorisedValueCategory
 		&GetKohaAuthorisedValues
 		&GetKohaAuthorisedValuesFromField
     &GetKohaAuthorisedValueLib
@@ -1105,6 +1106,28 @@ sub GetAuthorisedValueCategories {
     return \@results;
 }
 
+=head2 IsAuthorisedValueCategory
+
+    $is_auth_val_category = IsAuthorisedValueCategory($category);
+
+Returns whether a given category name is a valid one
+
+=cut
+
+sub IsAuthorisedValueCategory {
+    my $category = shift;
+    my $query = '
+        SELECT category
+        FROM authorised_values
+        WHERE BINARY category=?
+        LIMIT 1
+    ';
+    my $sth = C4::Context->dbh->prepare($query);
+    $sth->execute($category);
+    $sth->fetchrow ? return 1
+                   : return 0;
+}
+
 =head2 GetAuthorisedValueByCode
 
 $authhorised_value = GetAuthorisedValueByCode( $category, $authvalcode );

C4/Reports/Guided.pm

@@ -45,6 +45,10 @@ BEGIN {
       get_column_type get_distinct_values save_dictionary get_from_dictionary
       delete_definition delete_report format_results get_sql
       nb_rows update_sql build_authorised_value_list
+      GetReservedAuthorisedValues
+      GetParametersFromSQL
+      IsAuthorisedValueValid
+      ValidateSQLParameters
     );
 }
 
@@ -62,9 +66,7 @@ C4::Reports::Guided - Module for generating guided reports
 
 =head1 METHODS
 
-=over 2
-
-=item get_report_areas()
+=head2 get_report_areas
 
 This will return a list of all the available report areas
 
@@ -129,7 +131,7 @@ if ( C4::Context->preference('item-level_itypes') ) {
     unshift @{ $criteria{'CAT'} }, 'biblioitems.itemtype';
 }
 
-=item get_report_types()
+=head2 get_report_types
 
 This will return a list of all the available report types
 
@@ -151,7 +153,7 @@ sub get_report_types {
 
 }
 
-=item get_report_groups()
+=head2 get_report_groups
 
 This will return a list of all the available report areas with groups
 
@@ -180,7 +182,7 @@ sub get_report_groups {
     return \%groups_with_subgroups
 }
 
-=item get_all_tables()
+=head2 get_all_tables
 
 This will return a list of all tables in the database 
 
@@ -200,7 +202,7 @@ sub get_all_tables {
 
 }
 
-=item get_columns($area)
+=head2 get_columns($area)
 
 This will return a list of all columns for a report area
 
@@ -244,7 +246,7 @@ sub _get_columns {
     return (@columns);
 }
 
-=item build_query($columns,$criteria,$orderby,$area)
+=head2 build_query($columns,$criteria,$orderby,$area)
 
 This will build the sql needed to return the results asked for, 
 $columns is expected to be of the format tablename.columnname.
@@ -316,7 +318,7 @@ sub _build_query {
     return ($query);
 }
 
-=item get_criteria($area,$cgi);
+=head2 get_criteria($area,$cgi);
 
 Returns an arraref to hashrefs suitable for using in a tmpl_loop. With the criteria and available values.
 
@@ -388,7 +390,7 @@ sub get_criteria {
     return ( \@criteria_array );
 }
 
-sub nb_rows($) {
+sub nb_rows {
     my $sql = shift or return;
     my $sth = C4::Context->dbh->prepare($sql);
     $sth->execute();
@@ -396,7 +398,7 @@ sub nb_rows($) {
     return scalar (@$rows);
 }
 
-=item execute_query
+=head2 execute_query
 
   ($results, $error) = execute_query($sql, $offset, $limit)
 
@@ -422,7 +424,7 @@ the user in a user-supplied SQL query WILL apply in any case.
 #  ~ remove any LIMIT clause
 #  ~ repace SELECT clause w/ SELECT count(*)
 
-sub select_2_select_count ($) {
+sub select_2_select_count {
     # Modify the query passed in to create a count query... (I think this covers all cases -crn)
     my ($sql) = strip_limit(shift) or return;
     $sql =~ s/\bSELECT\W+(?:\w+\W+){1,}?FROM\b|\bSELECT\W\*\WFROM\b/SELECT count(*) FROM /ig;
@@ -512,7 +514,7 @@ sub execute_query ($;$$$) {
     # store_results($id,$xml);
 }
 
-=item save_report($sql,$name,$type,$notes)
+=head2 save_report($sql,$name,$type,$notes)
 
 Given some sql and a name this will saved it so that it can reused
 Returns id of the newly created report
@@ -700,7 +702,7 @@ sub get_saved_report {
     return $dbh->selectrow_hashref($query, undef, $report_arg);
 }
 
-=item create_compound($masterID,$subreportID)
+=head2 create_compound($masterID,$subreportID)
 
 This will take 2 reports and create a compound report using both of them
 
@@ -730,7 +732,7 @@ sub create_compound {
     return ( $mastertables, $subtables );
 }
 
-=item get_column_type($column)
+=head2 get_column_type($column)
 
 This takes a column name of the format table.column and will return what type it is
 (free text, set values, date)
@@ -758,7 +760,7 @@ sub get_column_type {
 	}
 }
 
-=item get_distinct_values($column)
+=head2 get_distinct_values($column)
 
 Given a column name, return an arrary ref of hashrefs suitable for use as a tmpl_loop 
 with the distinct values of the column
@@ -852,7 +854,7 @@ sub _get_column_defs {
 	return \%columns;
 }
 
-=item build_authorised_value_list($authorised_value)
+=head2 build_authorised_value_list($authorised_value)
 
 Returns an arrayref - hashref pair. The hashref consists of
 various code => name lists depending on the $authorised_value.
@@ -917,11 +919,98 @@ sub build_authorised_value_list {
     return (\@authorised_values, \%authorised_lib);
 }
 
+=head2 GetReservedAuthorisedValues
+
+    my %reserved_authorised_values = GetReservedAuthorisedValues();
+
+Returns a hash containig all reserved words
+
+=cut
+
+sub GetReservedAuthorisedValues {
+    my %reserved_authorised_values =
+            map { $_ => 1 } ( 'date',
+                              'branches',
+                              'itemtypes',
+                              'cn_source',
+                              'categorycode' );
+
+   return \%reserved_authorised_values;
+}
+
+
+=head2 IsAuthorisedValueValid
+
+    my $is_valid_ath_value = IsAuthorisedValueValid($authorised_value)
+
+Returns 1 if $authorised_value is on the reserved authorised values list or
+in the authorised value categories defined in
+
+=cut
+
+sub IsAuthorisedValueValid {
+
+    my $authorised_value = shift;
+    my $reserved_authorised_values = GetReservedAuthorisedValues();
+
+    if ( exists $reserved_authorised_values->{$authorised_value} ||
+         IsAuthorisedValueCategory($authorised_value)   ) {
+        return 1;
+    }
+
+    return 0;
+}
+
+=head2 GetParametersFromSQL
+
+    my @sql_parameters = GetParametersFromSQL($sql)
+
+Returns an arrayref of hashes containing the keys name and authval
+
+=cut
+
+sub GetParametersFromSQL {
+
+    my $sql = shift ;
+    my @split = split(/<<|>>/,$sql);
+    my @sql_parameters = ();
+
+    for ( my $i = 0; $i < ($#split/2) ; $i++ ) {
+        my ($name,$authval) = split(/\|/,$split[$i*2+1]);
+        push @sql_parameters, { 'name' => $name, 'authval' => $authval };
+    }
+
+    return \@sql_parameters;
+}
+
+=head2 ValidateSQLParameters
+
+    my @problematic_parameters = ValidateSQLParameters($sql)
+
+Returns an arrayref of hashes containing the keys name and authval of
+those SQL parameters that do not correspond to valid authorised names
+
+=cut
+
+sub ValidateSQLParameters {
+
+    my $sql = shift;
+    my @problematic_parameters = ();
+    my $sql_parameters = GetParametersFromSQL($sql);
+
+    foreach my $sql_parameter (@$sql_parameters) {
+        if ( defined $sql_parameter->{'authval'} ) {
+            push @problematic_parameters, $sql_parameter unless
+                IsAuthorisedValueValid($sql_parameter->{'authval'});
+        }
+    }
+
+    return \@problematic_parameters;
+}
+
 1;
 __END__
 
-=back
-
 =head1 AUTHOR
 
 Chris Cormack <crc@liblime.com>

koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt

@@ -654,28 +654,93 @@ canned reports and writing custom SQL reports.</p>
 </form>
 [% END %]
 
+[% IF ( warn_authval_problem ) %]
+     <form action='/cgi-bin/koha/reports/guided_reports.pl'>
+        <!--Every parameter the user issued is provided as a hidden field for recovery-->
+        <input type='hidden' name='id' value='[% id %]' />
+        <input type='hidden' name='sql' value='[% sql %]' />
+        <input type='hidden' name='reportname' value='[% reportname %]' />
+        <input type='hidden' name='group' value='[% group %]' />
+        <input type='hidden' name='subgroup' value='[% subgroup %]' />
+        <input type='hidden' name='notes' value='[% notes %]' />
+        <input type='hidden' name='cache_expiry' value='[% cache_expiry %]' />
+        <input type='hidden' name='cache_expiry_units' value='[% cache_expiry_units %]' />
+        <input type='hidden' name='public' value='[% public %]' />
+
+        <div class="dialog alert">
+            <h3>Errors found when processing parameters for report: [% name %]</h3>
+            [% FOREACH problematic_authval IN problematic_authvals %]
+                <p>
+                <strong>[% problematic_authval.name %]:</strong> The authorised value category (<strong>[% problematic_authval.authval %]</strong>)
+                    you selected does not exist.
+                </p>
+            [% END %]
+            <!-- Save Anyway Form -->
+            <form action='/cgi-bin/koha/reports/guided_reports.pl'>
+            <!--Every parameter the user issued is provided as a hidden field for recovery-->
+                <input type='hidden' name='id' value='[% id %]' />
+                <input type='hidden' name='sql' value='[% sql %]' />
+                <input type='hidden' name='reportname' value='[% reportname %]' />
+                <input type='hidden' name='group' value='[% group %]' />
+                <input type='hidden' name='subgroup' value='[% subgroup %]' />
+                <input type='hidden' name='notes' value='[% notes %]' />
+                <input type='hidden' name='cache_expiry' value='[% cache_expiry %]' />
+                <input type='hidden' name='cache_expiry_units' value='[% cache_expiry_units %]' />
+                <input type='hidden' name='public' value='[% public %]' />
+            [% IF ( phase_update) %]
+                <input type='hidden' name='phase' value='Update SQL' />
+                <input type="submit" name="save_anyway" class="approve" value="Save anyway" />
+            [% ELSIF ( phase_save) %]
+                <input type='hidden' name='area' value='[% area %]' />
+                <input type='hidden' name='phase' value='Save Report' />
+                <input type="submit" name="save_anyway" class="approve" value="Save anyway" />
+            [% END %]
+            </form>
+            <!-- Go back to editing -->
+            <form action='/cgi-bin/koha/reports/guided_reports.pl'>
+                <input type="button" name='back' class="deny" value="Edit SQL"
+                                    onclick="javascript:history.back()" />
+            </form>
+        </div>
+    </form>
+[% END %]
+
 [% IF ( enter_params ) %]
     <form action='/cgi-bin/koha/reports/guided_reports.pl'>
-        <input type='hidden' name='phase' value='Run this report' />
         <input type='hidden' name='reports' value="[% reports %]" />
+    [% IF ( auth_val_error ) %]
+        <input type='hidden' name='phase' value='Edit SQL' />
+        <div class="dialog alert">
+            <h3>Errors found when processing parameters for report: [% name %]</h3>
+            [% FOREACH auth_val_error IN auth_val_errors %]
+                <p>
+                    <strong>[% auth_val_error.entry %]:</strong> The authorised value category (<strong>[% auth_val_error.auth_val %]</strong>)
+                    you selected does not exist.
+                </p>
+            [% END %]
+        </div>
+        <fieldset class="action"><input type="submit" value="Edit SQL" /></fieldset>
+    [% ELSE %]
+        <input type='hidden' name='phase' value='Run this report' />
         <h1>Enter parameters for report [% name %]:</h1>
         [% IF ( notes ) %]<p>[% notes %]</p>[% END %]
         <fieldset class="rows">
             <ol>
-        [% FOREACH sql_param IN sql_params %]
-            [% IF sql_param.input == 'date' %]
-                <li>
-                <label for="date_[% sql_param_entry %][% loop.count %]">[% sql_param.entry %]:</label> <input id="date_[% sql_param_entry %][% loop.count %]" type="text" value="" size="10" name="sql_params" class="datepicker" />
-                </li>
-            [% ELSIF ( sql_param.input == 'text' ) %]
-                <li><label for="sql_params[% loop.count %]">[% sql_param.entry %]: </label><input id="sql_params[% loop.count %]" type="text" name="sql_params" /></li>
-            [% ELSE %]
-                <li><label for="sql_params_[% sql_param.labelid %]">[% sql_param.entry %]:</label> [% sql_param.input %]</li>
+            [% FOREACH sql_param IN sql_params %]
+                [% IF sql_param.input == 'date' %]
+                    <li>
+                    <label for="date_[% sql_param_entry %][% loop.count %]">[% sql_param.entry %]:</label> <input id="date_[% sql_param_entry %][% loop.count %]" type="text" value="" size="10" name="sql_params" class="datepicker" />
+                    </li>
+                [% ELSIF ( sql_param.input == 'text' ) %]
+                    <li><label for="sql_params[% loop.count %]">[% sql_param.entry %]: </label><input id="sql_params[% loop.count %]" type="text" name="sql_params" /></li>
+                [% ELSE %]
+                    <li><label for="sql_params_[% sql_param.labelid %]">[% sql_param.entry %]:</label> [% sql_param.input %]</li>
+                [% END %]
             [% END %]
-        [% END %]
             </ol>
         </fieldset>
         <fieldset class="action"><input type="submit" value="Run the report" /></fieldset>
+    [% END %]
     </form>
 [% END %]
 

reports/guided_reports.pl

@@ -27,6 +27,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/IsAuthorisedValueCategory/;
 
 =head1 NAME
 
@@ -131,7 +132,6 @@ elsif ( $phase eq 'Show SQL'){
 }
 
 elsif ( $phase eq 'Edit SQL'){
-	
     my $id = $input->param('reports');
     my $report = get_saved_report($id);
     my $group = $report->{report_group};
@@ -159,6 +159,7 @@ elsif ( $phase eq 'Update SQL'){
     my $cache_expiry = $input->param('cache_expiry');
     my $cache_expiry_units = $input->param('cache_expiry_units');
     my $public = $input->param('public');
+    my $save_anyway = $input->param('save_anyway');
 
     my @errors;
 
@@ -185,26 +186,52 @@ elsif ( $phase eq 'Update SQL'){
     elsif ($sql !~ /^(SELECT)/i) {
         push @errors, {queryerr => 1};
     }
+
     if (@errors) {
         $template->param(
             'errors'    => \@errors,
             'sql'       => $sql,
         );
     } else {
-        update_sql( $id, {
-                sql => $sql,
-                name => $reportname,
-                group => $group,
-                subgroup => $subgroup,
-                notes => $notes,
-                cache_expiry => $cache_expiry,
-                public => $public,
-        } );
-        $template->param(
-            'save_successful'       => 1,
-            'reportname'            => $reportname,
-            'id'                    => $id,
-        );
+
+        # Check defined SQL parameters for authorised value validity
+        my $problematic_authvals = ValidateSQLParameters($sql);
+
+        if ( scalar @$problematic_authvals > 0 && not $save_anyway ) {
+            # There's at least one problematic parameter, report to the
+            # GUI and provide all user input for further actions
+            $template->param(
+                'id' => $id,
+                'sql' => $sql,
+                'reportname' => $reportname,
+                'group' => $group,
+                'subgroup' => $subgroup,
+                'notes' => $notes,
+                'cache_expiry' => $cache_expiry,
+                'cache_expiry_units' => $cache_expiry_units,
+                'public' => $public,
+                'problematic_authvals' => $problematic_authvals,
+                'warn_authval_problem' => 1,
+                'phase_update' => 1
+            );
+
+        } else {
+            # No params problem found or asked to save anyway
+            update_sql( $id, {
+                    sql => $sql,
+                    name => $reportname,
+                    group => $group,
+                    subgroup => $subgroup,
+                    notes => $notes,
+                    cache_expiry => $cache_expiry,
+                    public => $public,
+                } );
+            $template->param(
+                'save_successful'       => 1,
+                'reportname'            => $reportname,
+                'id'                    => $id,
+            );
+        }
     }
 }
 
@@ -467,6 +494,7 @@ elsif ( $phase eq 'Save Report' ) {
     my $cache_expiry = $input->param('cache_expiry');
     my $cache_expiry_units = $input->param('cache_expiry_units');
     my $public = $input->param('public');
+    my $save_anyway = $input->param('save_anyway');
 
 
     # if we have the units, then we came from creating a report from SQL and thus need to handle converting units
@@ -493,6 +521,7 @@ elsif ( $phase eq 'Save Report' ) {
     elsif ($sql !~ /^(SELECT)/i) {
         push @errors, {queryerr => "No SELECT"};
     }
+
     if (@errors) {
         $template->param(
             'errors'    => \@errors,
@@ -503,25 +532,48 @@ elsif ( $phase eq 'Save Report' ) {
             'cache_expiry' => $cache_expiry,
             'public'    => $public,
         );
-    }
-    else {
-        my $id = save_report( {
-                borrowernumber => $borrowernumber,
-                sql            => $sql,
-                name           => $name,
-                area           => $area,
-                group          => $group,
-                subgroup       => $subgroup,
-                type           => $type,
-                notes          => $notes,
-                cache_expiry   => $cache_expiry,
-                public         => $public,
-            } );
-        $template->param(
-            'save_successful' => 1,
-            'reportname'      => $name,
-            'id'              => $id,
-        );
+    } else {
+        # Check defined SQL parameters for authorised value validity
+        my $problematic_authvals = ValidateSQLParameters($sql);
+
+        if ( scalar @$problematic_authvals > 0 && not $save_anyway ) {
+            # There's at least one problematic parameter, report to the
+            # GUI and provide all user input for further actions
+            $template->param(
+                'area' => $area,
+                'group' =>  $group,
+                'subgroup' => $subgroup,
+                'sql' => $sql,
+                'reportname' => $name,
+                'type' => $type,
+                'notes' => $notes,
+                'cache_expiry' => $cache_expiry,
+                'cache_expiry_units' => $cache_expiry_units,
+                'public' => $public,
+                'problematic_authvals' => $problematic_authvals,
+                'warn_authval_problem' => 1,
+                'phase_save' => 1
+            );
+        } else {
+            # No params problem found or asked to save anyway
+            my $id = save_report( {
+                    borrowernumber => $borrowernumber,
+                    sql            => $sql,
+                    name           => $name,
+                    area           => $area,
+                    group          => $group,
+                    subgroup       => $subgroup,
+                    type           => $type,
+                    notes          => $notes,
+                    cache_expiry   => $cache_expiry,
+                    public         => $public,
+                } );
+            $template->param(
+                'save_successful' => 1,
+                'reportname'      => $name,
+                'id'              => $id,
+            );
+        }
     }
 }
 
@@ -553,14 +605,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;
@@ -601,15 +658,30 @@ 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");
+                        if ( IsAuthorisedValueCategory($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);
 
-                        $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;
+                            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;
@@ -625,14 +697,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,
                             );