Attachment 53352 Details for Bug 14138 – [PASSED QA] Bug 14138: Patroncard: Warn user if PDF creation fails

[patch] [PASSED QA] Bug 14138: Patroncard: Warn user if PDF creation fails

PASSED-QA-Bug-14138-Patroncard-Warn-user-if-PDF-cr.patch (text/plain), 6.87 KB, created by Katrin Fischer on 2016-07-12 21:15:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2016-07-12 21:15:04 UTC

Size: 6.87 KB

patch

obsolete

>From 96f6c30e86cac13e2218714347d183c6464eba9f Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Marc=20V=C3=A9ron?= <veron@veron.ch>
>Date: Tue, 5 Jul 2016 19:23:59 +0200
>Subject: [PATCH] [PASSED QA] 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>
>---
> .../prog/en/includes/patroncards-errors.inc | 13 ++++++-
> .../prog/en/modules/patroncards/manage.tt | 1 +
> patroncards/create-pdf.pl | 45 ++++++++++++++++------
> 3 files changed, 47 insertions(+), 12 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..8dfe56d 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,18 @@
>-[% IF ( error ) %]
>+[% IF ( error || CGI.param('pdferr') ) %]
> <div class="dialog alert">
> 
> WARNING:
>+ [% 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: [% '' _ CGI.param('errba') _ '' %][% END %]
>+ [% IF CGI.param('errpl') %]Patron list: [% '' _ CGI.param('errpl') _ '' %][% END %]
>+ [% IF CGI.param('errpt') %]Patron number: [% '' _ CGI.param('errpt') _ '' %][% 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..e660af6 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,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;
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 14138: 38854 | 53228 | 53233 | 53242 | 53245 | 53248 | 53307 | 53352