Attachment 38854 Details for Bug 14138 – Bug 14138 - Patroncard: POC for pre-testing parameters for create-pdf.pl

[patch] Bug 14138 - Patroncard: POC for pre-testing parameters for create-pdf.pl

Bug-14138---Patroncard-POC-for-pre-testing-paramet.patch (text/plain), 5.96 KB, created by Marc Véron on 2015-05-05 14:45:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marc Véron

Created: 2015-05-05 14:45:24 UTC

Size: 5.96 KB

patch

obsolete

>From 0d3225f95bc25a25088b176f4ad6930862396cf1 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Marc=20V=C3=A9ron?= <veron@veron.ch>
>Date: Tue, 5 May 2015 16:24:10 +0200
>Subject: [PATCH] Bug 14138 - Patroncard: POC for pre-testing parameters for
> create-pdf.pl
>
>If wrong parameters are given, create-pdf.pl displays a "Software error" or creates an invalid PDF (it is a text file containing  "Software error....").
>
>This patch is a proof of concept with the aim to prevent such a situation.
>
>It implements pre-testing the parameters provided for create-pdf.pl.
>If wrong parameters are given, a error message is displayed instead of creating the PDF.
>The user has then the possibility to go back to the page she came from. (Patron card tool, maybe later patron's detail page? )
>
>To test:
>
>1)
>Apply on top of Bug 14122 (otherwise it is not possible to test for borrower)
>
>2)
>Test with links similar to the following
>...cgi-bin/koha/patroncards/create-pdf.pl?borrower_number=61&template_id=24&layout_id=21&start_card=1
>or:
>...cgi-bin/koha/patroncards/create-pdf.pl?batch_id=8&template_id=24&layout_id=21&start_card=1
>
>3)
>Change values for existing / not existing Ids. If a parameter is not valid, a new HTML page is displayed with a JavaScript alert.
>(This will need optimization. Ideas and/or patterns for doing this part are highly welcome).
>
>Comments are welcome - is this the way to go?
>
>Marc
>---
> C4/Creators/Layout.pm     |    1 +
> C4/Creators/Template.pm   |    1 +
> patroncards/create-pdf.pl |   70 +++++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 67 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Creators/Layout.pm b/C4/Creators/Layout.pm
>index a12532f..0a0eda4 100644
>--- a/C4/Creators/Layout.pm
>+++ b/C4/Creators/Layout.pm
>@@ -108,6 +108,7 @@ sub retrieve {
>         return -1;
>     }
>     my $self = $sth->fetchrow_hashref;
>+    return -2 unless ( $self ); # Make it behave like C4::Patroncards::Batch->retrieve()
>     bless ($self, $type);
>     return $self;
> }
>diff --git a/C4/Creators/Template.pm b/C4/Creators/Template.pm
>index b0b7da0..d76120c 100644
>--- a/C4/Creators/Template.pm
>+++ b/C4/Creators/Template.pm
>@@ -127,6 +127,7 @@ sub retrieve {
>         return -1;
>     }
>     my $self = $sth->fetchrow_hashref;
>+    return -2 unless ( $self ); # Make it behave like C4::Patroncards::Batch->retrieve() 
>     $self = _conv_points($self) if (($opts{convert} && $opts{convert} == 1) || $opts{profile_id});
>     $self = _apply_profile($self, $opts{'creator'}) if $opts{profile_id} && $self->{'profile_id'};        # don't bother if there is no profile_id
>     $self->{'template_stat'} = 1;
>diff --git a/patroncards/create-pdf.pl b/patroncards/create-pdf.pl
>index 1df3e27..4b7631a 100755
>--- a/patroncards/create-pdf.pl
>+++ b/patroncards/create-pdf.pl
>@@ -45,13 +45,76 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user({
>                                                                      });
> 
> 
>-my $batch_id    = $cgi->param('batch_id') if $cgi->param('batch_id');
>+my $batch_id    = $cgi->param('batch_id') || undef;
> my $template_id = $cgi->param('template_id') || undef;
> my $layout_id   = $cgi->param('layout_id') || undef;
>-my $start_card = $cgi->param('start_card') || 1;
>+my $start_card  = $cgi->param('start_card') || 1;
> my @label_ids   = $cgi->param('label_id') if $cgi->param('label_id');
> my @borrower_numbers  = $cgi->param('borrower_number') if $cgi->param('borrower_number');
> 
>+
>+# Proof of concept pretesting see Bug 14138
>+# Prepare for pre-tests
>+
>+my $batch_failed = '';
>+my $patron_failed = '';
>+my $template_failed = '';
>+my $layout_failed = '';
>+
>+
>+my $batch = C4::Patroncards::Batch->retrieve(batch_id => $batch_id) if ($batch_id);
>+# returns -1 or -2 if error occured
>+$batch_failed = $batch_id if ( ( $batch && $batch < 0 ) || ! $batch );
>+
>+# Later add code to handle lists (Bug 14131)
>+# Later prevent that more than one of patron / batch / list is given
>+
>+if ( @borrower_numbers ) {
>+   my $test_number = GetMember(borrowernumber => @borrower_numbers);
>+   $patron_failed = @borrower_numbers unless ( $test_number );
>+}
>+
>+my $pc_template;
>+my $template_failed = '';
>+if ( $template_id ) {
>+    $pc_template = C4::Patroncards::Template->retrieve(template_id => $template_id, profile_id => 1);
>+    # returns -1 or -2 if error occured
>+    $template_failed = $template_id if ( $pc_template && $pc_template < 0 );
>+} else {
>+    $template_failed = 'missing';
>+}
>+
>+my $layout;
>+if ( $layout_id ) {
>+    $layout = C4::Patroncards::Layout->retrieve(layout_id => $layout_id);
>+    # returns -1 or -2 if error occured
>+    $layout_failed = $layout_id if ( $layout && $layout < 0 );
>+} else {
>+    $layout_failed = 'missing';
>+}
>+
>+
>+# Display the errors and go back to previous page
>+# Later: Fit better into Koha look and feel (Template?) / Translation issues (is hard coded) 
>+if ( $batch_failed || $patron_failed || $template_failed || $layout_failed  ) {
>+    my $errmessage = '';
>+    $errmessage .= "ERROR ";
>+    $errmessage .= "Template $template_failed " if ( $template_failed );
>+    $errmessage .= "Layout $layout_failed " if ( $layout_failed );
>+    $errmessage .= "Batch $batch_failed " if ( $batch_failed );
>+    $errmessage .= "Patron $patron_failed" if ( $patron_failed );
>+
>+    print $cgi->header( -type => 'text/html', -encoding => 'utf-8', );
>+    my $errtitle = 'Error preparing PDF document';
>+    my $errscript = "alert(\" $errmessage \"); window.history.back();";
>+    print $cgi-> start_html( -title => $errtitle, -onLoad => $errscript, );
>+    print $cgi->h1($errtitle);
>+    print $cgi->end_html();
>+    exit 1;
>+}
>+
>+
>+# Do the work
> my $items = undef; # items = cards
> my $new_page = 0;
> 
>@@ -62,9 +125,6 @@ print $cgi->header( -type       => 'application/pdf',
>                   );
> 
> 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);
> 
> $| = 1;
> 
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

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