Bug 18320: Remove calls to CGI->param from the templates in patron cards

This patch removes calls to CGI->param from within the templates
(patroncards-errors.inc) and passes error_* variables with the same
codes from the controllers to the template instead.

This way we can be sure CGI->param is not called in list context.

To test, try those pages and see they don't show warnings anymore.
Verify that error situations still show the error messages.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-errors.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-errors.inc
index 3c987d7..768c83b 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-errors.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-errors.inc
@@ -5,17 +5,17 @@
 [% BLOCK b_layout %]<span>Layout: </span>[% END %]
 [% BLOCK b_template %]<span>Template: </span>[% END %]
 
-[% IF ( error || CGI.param('pdferr') )  %]
+[% IF ( error || error_pdferr )  %]
 <div class="dialog alert">
   <p>
     <strong>WARNING:</strong>
-        [% IF CGI.param('pdferr') %]<span>Error while creating PDF file. </span>
-            [% IF CGI.param('errnocards') %]<span>No cards created (empty batch or list?) </span>[% END %]
-            [% IF CGI.param('errba') %][% INCLUDE b_batch %]<span class="ex">[%- CGI.param('errba') | html -%]</span>[% END %]
-            [% IF CGI.param('errpl') %][% INCLUDE b_patronlist %]<span class="ex">[%- CGI.param('errpl') | html -%]</span>][% END %]
-            [% IF CGI.param('errpt') %][% INCLUDE b_patronnumber %]<span class="ex">[%- CGI.param('errpt') | html -%]</span>[% END %]
-            [% IF CGI.param('errlo') %][% INCLUDE b_layout %]<span class="ex">[%- CGI.param('errlo') | html -%]</span>[% END %]
-            [% IF CGI.param('errtpl') %][% INCLUDE b_template %]<span class="ex">[%- CGI.param('errtpl') | html -%]</span>[% END %]
+        [% IF error_pdferr %]<span>Error while creating PDF file. </span>
+            [% IF error_errnocards %]<span>No cards created (empty batch or list?) </span>[% END %]
+            [% IF error_errba %][% INCLUDE b_batch %]<span class="ex">[%- error_errba | html -%]</span>[% END %]
+            [% IF error_errpl %][% INCLUDE b_patronlist %]<span class="ex">[%- error_errpl | html -%]</span>][% END %]
+            [% IF error_errpt %][% INCLUDE b_patronnumber %]<span class="ex">[%- error_errpt | html -%]</span>[% END %]
+            [% IF error_errlo %][% INCLUDE b_layout %]<span class="ex">[%- error_errlo | html -%]</span>[% END %]
+            [% IF error_errtpl %][% INCLUDE b_template %]<span class="ex">[%- error_errtpl | html -%]</span>[% END %]
             [% INCLUDE asksysadmin %]
         [% END %]
 

diff --git a/patroncards/edit-batch.pl b/patroncards/edit-batch.pl
index d90d37c..dbb3d9c 100755 (executable)
--- a/patroncards/edit-batch.pl
+++ b/patroncards/edit-batch.pl
@@ -61,6 +61,13 @@ my $errstr = $cgi->param('error') || '';
 my $bor_num_list = $cgi->param('bor_num_list') || undef;
 my $branch_code = C4::Context->userenv->{'branch'};
 
+my @errors = ( 'pdferr', 'errnocards', 'errba', 'errpl', 'errpt', 'errlo', 'errtpl', );
+foreach my $param (@errors) {
+    my $error = $cgi->param($param) ? 1 : 0;
+    $template->param( 'error_' . $param => $error )
+      if $error;
+}
+
 if ($op eq 'remove') {
     $batch = C4::Patroncards::Batch->retrieve(batch_id => $batch_id);
     foreach my $label_id (@label_ids) {

diff --git a/patroncards/image-manage.pl b/patroncards/image-manage.pl
index 3f3c288..8c2d724 100755 (executable)
--- a/patroncards/image-manage.pl
+++ b/patroncards/image-manage.pl
@@ -26,6 +26,13 @@ my $upload_file = $cgi->upload('uploadfile') || '';
 my $op = $cgi->param('op') || 'none';
 my @image_ids = $cgi->multi_param('image_id');
 
+my @errors = ( 'pdferr', 'errnocards', 'errba', 'errpl', 'errpt', 'errlo', 'errtpl', );
+foreach my $param (@errors) {
+    my $error = $cgi->param($param) ? 1 : 0;
+    $template->param( 'error_' . $param => $error )
+      if $error;
+}
+
 my $source_file = "$file_name"; # otherwise we end up with what amounts to a pointer to a filehandle rather than a user-friendly filename
 
 my $display_columns = { image =>    [  #{db column      => {label => 'col label', is link?          }},

diff --git a/patroncards/manage.pl b/patroncards/manage.pl
index f4d21c6..4366955 100755 (executable)
--- a/patroncards/manage.pl
+++ b/patroncards/manage.pl
@@ -45,6 +45,13 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
+my @errors = ( 'pdferr', 'errnocards', 'errba', 'errpl', 'errpt', 'errlo', 'errtpl', );
+foreach my $param (@errors) {
+    my $error = $cgi->param($param) ? 1 : 0;
+    $template->param( 'error_' . $param => $error )
+      if $error;
+}
+
 my $op = $cgi->param('op') || 'none';
 my $card_element = $cgi->param('card_element') || 'template';   # default to template management
 my $element_id = $cgi->param('element_id') || 0; # there should never be an element with a id of 0 so this is a safe default