From 92b8c61f2fe0b4540ae8a12a9712019fe1e7d24c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20V=C3=A9ron?= Date: Tue, 5 Jul 2016 19:23:59 +0200 Subject: [PATCH] Bug 14138: Patroncard: Warn user if PDF creation fails First step: Change patroncards/create-pdf.pl to redirect with an an error code instead of writing an ivalid pdf. Updated patch to display error and test plan follows. --- .../prog/en/includes/patroncards-errors.inc | 12 +- .../prog/en/modules/patroncards/manage.tt | 1 + patroncards/create-pdf.pl | 317 +++++++++++--------- 3 files changed, 181 insertions(+), 149 deletions(-) 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..1907eb0 100644 --- 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,17 @@ -[% IF ( error ) %] +[% IF ( error || CGI.param('pdferr') ) %]

WARNING: + [% IF CGI.param('pdferr') %] + Error while creating the PDF file. + [% IF CGI.param('errnocards') %]No cards created (empty batch or list?)[% END %] + [% IF CGI.param('errba') %]Batch: [% CGI.param('errba') %][% END %] + [% IF CGI.param('errpl') %]Patron list: [% CGI.param('errpl') %][% END %] + [% IF CGI.param('errlo') %]Layout: [% CGI.param('errlo') %][% END %] + [% IF CGI.param('errtpl') %]Template: [% CGI.param('errtpl') %][% 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 --- 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..51019cc 100755 --- 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,182 +56,205 @@ 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; -my $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); +eval { + $pdf_file = (@label_ids || @borrower_numbers ? "card_single_" . scalar(@label_ids || @borrower_numbers) : "card_batch_$batch_id"); -$| = 1; + $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); -# set the paper size -my $lower_left_x = 0; -my $lower_left_y = 0; -my $upper_right_x = $pc_template->get_attr('page_width'); -my $upper_right_y = $pc_template->get_attr('page_height'); + $| = 1; -$pdf->Compress(1); # comment this out to debug pdf files, but be sure to uncomment it in production or you may be very sorry... -$pdf->Mbox($lower_left_x, $lower_left_y, $upper_right_x, $upper_right_y); + # set the paper size + my $lower_left_x = 0; + my $lower_left_y = 0; + my $upper_right_x = $pc_template->get_attr('page_width'); + my $upper_right_y = $pc_template->get_attr('page_height'); -my ($llx, $lly) = 0,0; -(undef, undef, $llx, $lly) = $pc_template->get_label_position($start_card); + $pdf->Compress(1); # comment this out to debug pdf files, but be sure to uncomment it in production or you may be very sorry... + $pdf->Mbox($lower_left_x, $lower_left_y, $upper_right_x, $upper_right_y); -if (@label_ids) { + my ($llx, $lly) = 0,0; + (undef, undef, $llx, $lly) = $pc_template->get_label_position($start_card); + + if (@label_ids) { my $batch_items = $batch->get_attr('items'); - grep { - my $label_id = $_; - push(@{$items}, grep{$_->{'label_id'} == $label_id;} @{$batch_items}); - } @label_ids; -} -elsif (@borrower_numbers) { - grep { - push(@{$items}, {borrower_number => $_}); - } @borrower_numbers; -} -elsif ( $patronlist_id ) { - my ($list) = GetPatronLists( { patron_list_id => $patronlist_id } ); - my @borrowerlist = $list->patron_list_patrons()->search_related('borrowernumber') - ->get_column('borrowernumber')->all(); - grep { - push(@{$items}, {borrower_number => $_}); - } @borrowerlist; -} -else { - $items = $batch->get_attr('items'); -} + grep { + my $label_id = $_; + push(@{$items}, grep{$_->{'label_id'} == $label_id;} @{$batch_items}); + } @label_ids; + } + elsif (@borrower_numbers) { + grep { + push(@{$items}, {borrower_number => $_}); + } @borrower_numbers; + } + elsif ( $patronlist_id ) { + my ($list) = GetPatronLists( { patron_list_id => $patronlist_id } ); + my @borrowerlist = $list->patron_list_patrons()->search_related('borrowernumber') + ->get_column('borrowernumber')->all(); + grep { + push(@{$items}, {borrower_number => $_}); + } @borrowerlist; + } + else { + $items = $batch->get_attr('items'); + } -my $layout_xml = XMLin($layout->get_attr('layout_xml'), ForceArray => 1); - -if ($layout_xml->{'page_side'} eq 'B') { # rearrange items on backside of page to swap columns - my $even = 1; - my $odd = 0; - my @swap_array = (); - while ($even <= (scalar(@{$items})+1)) { - push (@swap_array, @{$items}[$even]); - push (@swap_array, @{$items}[$odd]); - $even += 2; - $odd += 2; + my $layout_xml = XMLin($layout->get_attr('layout_xml'), ForceArray => 1); + + if ($layout_xml->{'page_side'} eq 'B') { # rearrange items on backside of page to swap columns + my $even = 1; + my $odd = 0; + my @swap_array = (); + while ($even <= (scalar(@{$items})+1)) { + push (@swap_array, @{$items}[$even]); + push (@swap_array, @{$items}[$odd]); + $even += 2; + $odd += 2; + } + @{$items} = @swap_array; } - @{$items} = @swap_array; -} -CARD_ITEMS: -foreach my $item (@{$items}) { - if ($item) { - my $borrower_number = $item->{'borrower_number'}; - my $card_number = GetMember(borrowernumber => $borrower_number)->{'cardnumber'}; - -# Set barcode data - $layout_xml->{'barcode'}->[0]->{'data'} = $card_number if $layout_xml->{'barcode'}; - -# Create a new patroncard object - my $patron_card = C4::Patroncards::Patroncard->new( - batch_id => 1, - borrower_number => $borrower_number, - llx => $llx, # lower left corner of the card - lly => $lly, - height => $pc_template->get_attr('label_height'), # of the card - width => $pc_template->get_attr('label_width'), - layout => $layout_xml, - text_wrap_cols => 30, #FIXME: hardcoded, - ); - - $patron_card->draw_guide_box($pdf) if $layout_xml->{'guide_box'}; - $patron_card->draw_barcode($pdf) if $layout_xml->{'barcode'}; - -# Do image foo and place binary image data into layout hash - my $image_data = {}; - my $error = undef; - my $images = $layout_xml->{'images'}; - PROCESS_IMAGES: - foreach (keys %{$images}) { - if (grep{m/source/} keys(%{$images->{$_}->{'data_source'}->[0]})) { - if ($images->{$_}->{'data_source'}->[0]->{'image_source'} eq 'none') { - next PROCESS_IMAGES; - } - elsif ($images->{$_}->{'data_source'}->[0]->{'image_source'} eq 'patronimages') { - my $patron_image = Koha::Patron::Images->find($borrower_number); - if ($patron_image) { - $image_data->{'imagefile'} = $patron_image->imagefile; + CARD_ITEMS: + foreach my $item (@{$items}) { + if ($item) { + $cardscount ++; + my $borrower_number = $item->{'borrower_number'}; + my $card_number = GetMember(borrowernumber => $borrower_number)->{'cardnumber'}; + + # Set barcode data + $layout_xml->{'barcode'}->[0]->{'data'} = $card_number if $layout_xml->{'barcode'}; + + # Create a new patroncard object + my $patron_card = C4::Patroncards::Patroncard->new( + batch_id => 1, + borrower_number => $borrower_number, + llx => $llx, # lower left corner of the card + lly => $lly, + height => $pc_template->get_attr('label_height'), # of the card + width => $pc_template->get_attr('label_width'), + layout => $layout_xml, + text_wrap_cols => 30, #FIXME: hardcoded, + ); + + $patron_card->draw_guide_box($pdf) if $layout_xml->{'guide_box'}; + $patron_card->draw_barcode($pdf) if $layout_xml->{'barcode'}; + + # Do image foo and place binary image data into layout hash + my $image_data = {}; + my $error = undef; + my $images = $layout_xml->{'images'}; + PROCESS_IMAGES: + foreach (keys %{$images}) { + if (grep{m/source/} keys(%{$images->{$_}->{'data_source'}->[0]})) { + if ($images->{$_}->{'data_source'}->[0]->{'image_source'} eq 'none') { + next PROCESS_IMAGES; + } + elsif ($images->{$_}->{'data_source'}->[0]->{'image_source'} eq 'patronimages') { + my $patron_image = Koha::Patron::Images->find($borrower_number); + if ($patron_image) { + $image_data->{'imagefile'} = $patron_image->imagefile; + } + else { + warn sprintf('No image exists for borrower number %s.', $borrower_number); + } + next PROCESS_IMAGES unless $patron_image; + } + elsif ($images->{$_}->{'data_source'}->[0]->{'image_source'} eq 'creator_images') { + my $dbh = C4::Context->dbh(); + $dbh->{LongReadLen} = 1000000; # allows us to read approx 1MB + $image_data = $dbh->selectrow_hashref("SELECT imagefile FROM creator_images WHERE image_name = \'$images->{$_}->{'data_source'}->[0]->{'image_name'}\'"); + warn sprintf('Database returned the following error: %s.', $error) if $error; + warn sprintf('Image does not exists in db table %s.', $images->{$_}->{'data_source'}->[0]->{'image_name'}) if !$image_data; + next PROCESS_IMAGES if !$image_data; } else { - warn sprintf('No image exists for borrower number %s.', $borrower_number); + warn sprintf('No retrieval method for image source %s.', $images->{$_}->{'data_source'}->[0]->{'image_source'}); + next PROCESS_IMAGES; } - next PROCESS_IMAGES unless $patron_image; - } - elsif ($images->{$_}->{'data_source'}->[0]->{'image_source'} eq 'creator_images') { - my $dbh = C4::Context->dbh(); - $dbh->{LongReadLen} = 1000000; # allows us to read approx 1MB - $image_data = $dbh->selectrow_hashref("SELECT imagefile FROM creator_images WHERE image_name = \'$images->{$_}->{'data_source'}->[0]->{'image_name'}\'"); - warn sprintf('Database returned the following error: %s.', $error) if $error; - warn sprintf('Image does not exists in db table %s.', $images->{$_}->{'data_source'}->[0]->{'image_name'}) if !$image_data; - next PROCESS_IMAGES if !$image_data; } else { - warn sprintf('No retrieval method for image source %s.', $images->{$_}->{'data_source'}->[0]->{'image_source'}); + warn sprintf("Unrecognized image data source: %s", $images->{$_}->{'data_source'}); next PROCESS_IMAGES; } - } - else { - warn sprintf("Unrecognized image data source: %s", $images->{$_}->{'data_source'}); - next PROCESS_IMAGES; - } - my $binary_data = $image_data->{'imagefile'}; + my $binary_data = $image_data->{'imagefile'}; -# invoke the display image object... - my $image = Graphics::Magick->new; - $image->BlobToImage($binary_data); + # invoke the display image object... + my $image = Graphics::Magick->new; + $image->BlobToImage($binary_data); -# invoke the alt (aka print) image object... - my $alt_image = Graphics::Magick->new; - $alt_image->BlobToImage($binary_data); - $alt_image->Set(magick => 'jpg', quality => 100); + # invoke the alt (aka print) image object... + my $alt_image = Graphics::Magick->new; + $alt_image->BlobToImage($binary_data); + $alt_image->Set(magick => 'jpg', quality => 100); - #To avoid pixelation have the image 5 times bigger and - #scale it down in PDF itself - my $oversize_factor = 8; - my $pdf_scale_factor = 1 / $oversize_factor; + #To avoid pixelation have the image 5 times bigger and + #scale it down in PDF itself + my $oversize_factor = 8; + my $pdf_scale_factor = 1 / $oversize_factor; - my $alt_width = ceil($image->Get('width')); # the rounding up is important: Adobe reader does not handle long decimal numbers well - my $alt_height = ceil($image->Get('height')); - my $ratio = $alt_width / $alt_height; - my $display_height = ceil($images->{$_}->{'Dx'}); - my $display_width = ceil($ratio * $display_height); + my $alt_width = ceil($image->Get('width')); # the rounding up is important: Adobe reader does not handle long decimal numbers well + my $alt_height = ceil($image->Get('height')); + my $ratio = $alt_width / $alt_height; + my $display_height = ceil($images->{$_}->{'Dx'}); + my $display_width = ceil($ratio * $display_height); - $image->Resize(width => $oversize_factor * $display_width, height => $oversize_factor * $display_height); - $image->Set(magick => 'jpg', quality => 100); + $image->Resize(width => $oversize_factor * $display_width, height => $oversize_factor * $display_height); + $image->Set(magick => 'jpg', quality => 100); -# Write param for downsizing in pdf - $images->{$_}->{'scale'} = $pdf_scale_factor; + # Write param for downsizing in pdf + $images->{$_}->{'scale'} = $pdf_scale_factor; -# Write params for alt image... - $images->{$_}->{'alt'}->{'Sx'} = $oversize_factor * $alt_width; - $images->{$_}->{'alt'}->{'Sy'} = $oversize_factor * $alt_height; - $images->{$_}->{'alt'}->{'data'} = $alt_image->ImageToBlob(); + # Write params for alt image... + $images->{$_}->{'alt'}->{'Sx'} = $oversize_factor * $alt_width; + $images->{$_}->{'alt'}->{'Sy'} = $oversize_factor * $alt_height; + $images->{$_}->{'alt'}->{'data'} = $alt_image->ImageToBlob(); -# Write params for display image... - $images->{$_}->{'Sx'} = $oversize_factor * $display_width; - $images->{$_}->{'Sy'} = $oversize_factor * $display_height; - $images->{$_}->{'data'} = $image->ImageToBlob(); + # Write params for display image... + $images->{$_}->{'Sx'} = $oversize_factor * $display_width; + $images->{$_}->{'Sy'} = $oversize_factor * $display_height; + $images->{$_}->{'data'} = $image->ImageToBlob(); - my $err = $patron_card->draw_image($pdf); - warn sprintf ("Error encountered while attempting to draw image %s, %s", $_, $err) if $err; + my $err = $patron_card->draw_image($pdf); + warn sprintf ("Error encountered while attempting to draw image %s, %s", $_, $err) if $err; + } + $patron_card->draw_text($pdf); } - $patron_card->draw_text($pdf); + ($llx, $lly, $new_page) = $pc_template->get_next_label_pos(); + $pdf->Page() if $new_page; } - ($llx, $lly, $new_page) = $pc_template->get_next_label_pos(); - $pdf->Page() if $new_page; -} - -$pdf->End(); + # No errors occured within eval, we can issue the pdf + $pdf_ok = 1 if ($cardscount > 0); +}; # end of eval block -# 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 .= "&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; -- 1.7.10.4