Bug 14138: Patroncard: Warn user if PDF creation fails

Change patroncards/create-pdf.pl to redirect with an error message
instead of writing an invalid pdf that does not open in pdf viewer.

To test:
- Apply patch

- Test that pdf creator behaves as before (with valid batches and
  patron lists)

- While testing, copy pdf link address from window with title 'Click
  the following link(s) to download...'

- Open another staff client browser tab

- Paste link to browser address field, change batch id rsp. patron
  list id to an invalid value and submit

- The window should redirect to cgi-bin/koha/patroncards/create-pdf.pl
  and display an error message

- Bonus test 1: Create an empty patron list and test patron card
  creation. You should get an error message as appropriate.

- Bonus test 2: Use a link with params like the following:
  ...create-pdf.pl?borrower_number=61&template_id=2&layout_id=1&start_card=1
  Verify that you can create a pdf with a valid borrower_number and that
  you get the error message with an invalid borrower number

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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 136cfc4..8dfe56d 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-errors.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-errors.inc
@@ -1,7 +1,18 @@
-[% IF ( error ) %]
+[% IF ( error || CGI.param('pdferr') )  %]
 <div class="dialog alert">
   <p>
     <strong>WARNING:</strong>
+        [% IF CGI.param('pdferr') %]
+        Error while creating PDF file.
+        [% IF CGI.param('errnocards') %]No cards created (empty batch or list?)[% END %]
+        [% IF CGI.param('errba') %]Batch: [% '<span class="ex">' _ CGI.param('errba') _ '</span>' %][% END %]
+        [% IF CGI.param('errpl') %]Patron list: [% '<span class="ex">' _ CGI.param('errpl') _ '</span>' %][% END %]
+        [% IF CGI.param('errpt') %]Patron number: [% '<span class="ex">' _ CGI.param('errpt') _ '</span>' %][% END %]
+        [% IF CGI.param('errlo') %]Layout: [% '<span class="ex">' _ CGI.param('errlo') _ '</span>' %][% END %]
+        [% IF CGI.param('errtpl') %]Template: [% '<span class="ex">' _ CGI.param('errtpl') _ '</span>' %][% END %]
+
+        Please have your system administrator check the error log for details.
+        [% END %]
         [% IF ( error == 101 ) %]
         The database returned an error while [% IF ( card_element ) %]saving [% card_element %] [% element_id %][% ELSE %]attempting a save operation[% END %]. Please have your system administrator check the error log for details.
         [% ELSIF ( error == 102 ) %]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt
index a75a924..efdf9fb 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt
@@ -1,3 +1,4 @@
+[% USE CGI %]
 [% BLOCK translate_card_element %]
 [%-  SWITCH element -%]
 [%-  CASE 'layout'    -%]layout

diff --git a/patroncards/create-pdf.pl b/patroncards/create-pdf.pl
index 1811f0f..e660af6 100755 (executable)
--- a/patroncards/create-pdf.pl
+++ b/patroncards/create-pdf.pl
@@ -45,8 +45,6 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user({
                                                                      flagsrequired   => { tools => 'label_creator' },
                                                                      debug           => 1,
                                                                      });
-
-
 my $batch_id    = $cgi->param('batch_id') if $cgi->param('batch_id');
 my $template_id = $cgi->param('template_id') || undef;
 my $layout_id   = $cgi->param('layout_id') || undef;
@@ -58,13 +56,17 @@ my $patronlist_id = $cgi->param('patronlist_id');
 my $items = undef; # items = cards
 my $new_page = 0;
 
-my $pdf_file = (@label_ids || @borrower_numbers ? "card_single_" . scalar(@label_ids || @borrower_numbers) : "card_batch_$batch_id");
-print $cgi->header( -type       => 'application/pdf',
-                    -encoding   => 'utf-8',
-                    -attachment => "$pdf_file.pdf",
-                  );
+# Wrap pdf creation part into an eval, some vars need scope outside eval
+my $pdf_ok;
+my $pdf;
+my $pdf_file;
+my $cardscount = 0;
+
+#Note fo bug 14138: Indenting follows in separate patch to ease review
+eval {
+$pdf_file = (@label_ids || @borrower_numbers ? "card_single_" . scalar(@label_ids || @borrower_numbers) : "card_batch_$batch_id");
 
-my $pdf = C4::Creators::PDF->new(InitVars => 0);
+$pdf = C4::Creators::PDF->new(InitVars => 0);
 my $batch = C4::Patroncards::Batch->retrieve(batch_id => $batch_id);
 my $pc_template = C4::Patroncards::Template->retrieve(template_id => $template_id, profile_id => 1);
 my $layout = C4::Patroncards::Layout->retrieve(layout_id => $layout_id);
@@ -125,6 +127,7 @@ if ($layout_xml->{'page_side'} eq 'B') { # rearrange items on backside of page t
 CARD_ITEMS:
 foreach my $item (@{$items}) {
     if ($item) {
+        $cardscount ++;
         my $borrower_number = $item->{'borrower_number'};
         my $card_number = GetMember(borrowernumber => $borrower_number)->{'cardnumber'};
 
@@ -231,9 +234,29 @@ foreach my $item (@{$items}) {
     ($llx, $lly, $new_page) = $pc_template->get_next_label_pos();
     $pdf->Page() if $new_page;
 }
+# No errors occurred within eval, we can issue the pdf
+$pdf_ok = 1 if ($cardscount > 0);
+}; # end of eval block
 
-$pdf->End();
-
-# FIXME: Possibly do a redirect here if there were error encountered during PDF creation.
+if ($pdf_ok) {
+    #issue the pdf
+    print $cgi->header( -type       => 'application/pdf',
+                    -encoding   => 'utf-8',
+                    -attachment => "$pdf_file.pdf",
+                  );
+    $pdf->End();
+}
+else {
+    # warn user that pdf is not created
+    my $errparams = '&pdferr=1';
+    $errparams .= "&errba=$batch_id" if $batch_id;
+    $errparams .= "&errpl=$patronlist_id" if $patronlist_id;
+    $errparams =  $errparams.'&errpt='.$cgi->param('borrower_number') if $cgi->param('borrower_number');
+    $errparams .= "&errlo=$layout_id" if $layout_id;
+    $errparams .= "&errtpl=$template_id" if $template_id;
+    $errparams .= "&errnocards=1" if !$cardscount;
+
+    print $cgi->redirect("/cgi-bin/koha/patroncards/manage.pl?card_element=batch$errparams");
+}
 
 1;