From 1aa88636a6af0d10ac1df01ca41ce3a6eb8bbc81 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 20 Oct 2016 08:58:05 +0100
Subject: [PATCH] Bug 5260: simplify script and error handling

No need to redirect, just sent the params to the template directly

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 acqui/basket.pl                               | 35 ++++++++-----------
 .../prog/en/modules/acqui/basket.tt           | 28 +++++++--------
 2 files changed, 28 insertions(+), 35 deletions(-)

diff --git a/acqui/basket.pl b/acqui/basket.pl
index 054d743e88..c9e92994be 100755
--- a/acqui/basket.pl
+++ b/acqui/basket.pl
@@ -108,17 +108,12 @@ unless (CanUserManageBasket($loggedinuser, $basket, $userflags)) {
 # FIXME : the query->param('booksellerid') below is probably useless. The bookseller is always known from the basket
 # if no booksellerid in parameter, get it from basket
 # warn "=>".$basket->{booksellerid};
-my $op = $query->param('op');
-if (!defined $op) {
-    $op = q{};
-}
+my $op = $query->param('op') // 'list';
 
 my $confirm_pref= C4::Context->preference("BasketConfirmations") || '1';
 $template->param( skip_confirm_reopen => 1) if $confirm_pref eq '2';
 
-$template->param( email_ok => 1 ) if defined $query->param('email_ok');
-$template->param( email_error => $query->param('email_error') ) if defined $query->param('email_error');
-
+my @messages;
 
 if ( $op eq 'delete_confirm' ) {
     my $basketno = $query->param('basketno');
@@ -170,22 +165,18 @@ if ( $op eq 'delete_confirm' ) {
     print GetBasketAsCSV($query->param('basketno'), $query);
     exit;
 } elsif ($op eq 'email') {
-    my $redirect_url = '/cgi-bin/koha/acqui/basket.pl?basketno='.$basket->{'basketno'};
-    my $err;
-
-    eval {
-        $err = SendAlerts( 'orderacquisition', $query->param('basketno'), 'ACQORDER' );
+    my $err = eval {
+        SendAlerts( 'orderacquisition', $query->param('basketno'), 'ACQORDER' );
     };
     if ( $@ ) {
-    $redirect_url .= '&email_error='.$@;
+        push @messages, { type => 'error', code => $@ };
     } elsif ( ref $err and exists $err->{error} ) {
-        $redirect_url .= '&email_error=' . $err->{error};
+        push @messages, { type => 'error', code => $err->{error} };
     } else {
-        $redirect_url .= '&email_ok=1';
+        push @messages, { type => 'message', code => 'email_sent' };
     }
 
-    print $query->redirect($redirect_url)
-
+    $op = 'list';
 } elsif ($op eq 'close') {
     my $confirm = $query->param('confirm') || $confirm_pref eq '2';
     if ($confirm) {
@@ -242,7 +233,9 @@ elsif ( $op eq 'ediorder' ) {
     });
     print $query->redirect("/cgi-bin/koha/acqui/basket.pl?basketno=$basketno");
     exit;
-} else {
+}
+
+if ( $op eq 'list' ) {
     my @branches_loop;
     # get librarian branch...
     if ( C4::Context->preference("IndependentBranches") ) {
@@ -427,6 +420,9 @@ elsif ( $op eq 'ediorder' ) {
     );
 }
 
+$template->param( messages => \@messages );
+output_html_with_http_headers $query, $cookie, $template->output;
+
 sub get_order_infos {
     my $order = shift;
     my $bookseller = shift;
@@ -502,9 +498,6 @@ sub get_order_infos {
     return \%line;
 }
 
-output_html_with_http_headers $query, $cookie, $template->output;
-
-
 sub edi_close_and_order {
     my $confirm = $query->param('confirm') || $confirm_pref eq '2';
     if ($confirm) {
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
index 8a114fdb07..d4622e7db3 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
@@ -295,22 +295,22 @@
             [% END %]
         [% ELSE %]
 
-        [% IF email_error %]
-            <div class="dialog alert">
-            [% IF ( email_error == "no_email" ) %]
-                This vendor has no contact selected for sending orders to or is missing an e-mail address.
-            [% ELSIF ( email_error == "no_basketno" ) %]
-                No basket given.
-            [% ELSIF ( email_error == "no_letter" ) %]
-                There is no notice template with code ACQORDER defined.
-            [% ELSE %]
-                ERROR! - [% email_error %]
-            [% END %]
+        [% FOR m IN messages %]
+            <div class="dialog [% m.type %]">
+                [% SWITCH m.code %]
+                [% CASE 'no_email' %]
+                    This vendor has no contact selected for sending orders to or is missing an e-mail address.
+                [% CASE 'no_basketno' %]
+                    No basket given.
+                [% CASE 'no_letter' %]
+                    There is no notice template with code ACQORDER defined.
+                [% CASE 'email_sent' %]
+                    Order e-mail was sent to the vendor.
+                [% CASE %]
+                    ERROR! - [% m.code %]
+                [% END %]
             </div>
         [% END %]
-        [% IF ( email_ok ) %]
-            <div class="dialog message">Order e-mail was sent to the vendor.</div>
-        [% END %]
         <h1>[% UNLESS ( basketno ) %]New [% END %]Basket [% basketname|html %] ([% basketno %]) for <a href="supplier.pl?booksellerid=[% booksellerid %]">[% name|html %]</a></h1>
         [% IF ( basketno ) %]
             <div id="acqui_basket_summary" class="yui-g">
-- 
2.39.5