Bug 4461: Better error handling

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reportproblem.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reportproblem.tt
index 038e846..3b5ffe3 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reportproblem.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reportproblem.tt
@@ -27,7 +27,22 @@
                 [% END %]
                 <h1>Report a problem</h1>
 
-                [% IF ( successfuladd ) %]
+                [% FOR m IN messages %]
+                    <div class="alert alert-[% m.type | html %]">
+                        [% SWITCH m.code %]
+                        [% CASE 'success_on_send' %]
+                            [% IF recipient == 'admin' %]
+                                Your problem report has been sent to the Koha administrator.
+                            [% ELSE %]
+                                Your problem report has been sent to the library.
+                            [% END %]
+                        [% CASE 'error_on_send' %][#% We really should avoid reaching this! %]
+                            Something wrong happened when sending the report. Please contact your library.
+                        [% END %]
+                    </div>
+                [% END %]
+
+                [% IF success_on_send %]
                     <div class="alert alert-info">
                         [% IF recipient == 'admin' %]
                             Your problem report has been sent to the Koha administrator.
@@ -36,6 +51,7 @@
                         [% END %]
                     </div>
                 [% END %]
+
                 <div id="reportproblem" class="maincontent toptabs">
                     <form name="reportlibform" action="/cgi-bin/koha/opac-reportproblem.pl" method="post">
                         <input type="hidden" name="op" value="addreport">
@@ -53,9 +69,9 @@
                                     [% END %]
                                 </li>
                                 <li>
-                                    <label for="place">Problem found on page: </label>
-                                    <input type="hidden" name="place" id="place" value="[% probpage | html %]">
-                                    [% probpage | html %]
+                                    <label for="problempage">Problem found on page: </label>
+                                    <input type="hidden" name="problempage" id="problempage" value="[% problempage | html %]">
+                                    [% problempage | html %]
                                 </li>
                                 <li>
                                     <label for="user">Username: </label>

diff --git a/opac/opac-reportproblem.pl b/opac/opac-reportproblem.pl
index 2d4d8b1..c8e0d2a 100644 (file)
--- a/opac/opac-reportproblem.pl
+++ b/opac/opac-reportproblem.pl
@@ -19,12 +19,12 @@
 
 use Modern::Perl;
 use CGI qw ( -utf8 );
+use Try::Tiny;
+
 use C4::Auth;    # get_template_and_user
 use C4::Output;
-use C4::Members;
 use C4::Letters;
 use Koha::ProblemReport;
-use Koha::DateUtils;
 use Koha::Libraries;
 use Koha::Patrons;
 use Koha::Util::Navigation;
@@ -48,15 +48,16 @@ if (   !C4::Context->preference('OPACReportProblem')
 
 my $problempage = C4::Context->preference('OPACBaseURL') . Koha::Util::Navigation::local_referer($input );
 
-my $member = Koha::Patrons->find($borrowernumber);
-my $username = $member->userid;
-my $branchcode = $member->branchcode;
+my $patron = Koha::Patrons->find($borrowernumber);
+my $username = $patron->userid;
+my $branchcode = $patron->branchcode;
 my $library = Koha::Libraries->find($branchcode);
+my @messages;
 
 $template->param(
-    username => $username,
-    probpage => $problempage,
-    library => $library,
+    username    => $username,
+    problempage => $problempage,
+    library     => $library,
 );
 
 my $op = $input->param('op') || '';
@@ -64,55 +65,76 @@ if ( $op eq 'addreport' ) {
 
     my $subject = $input->param('subject');
     my $message = $input->param('message');
-    my $place = $input->param('place');
+    my $problempage = $input->param('problempage');
     my $recipient = $input->param('recipient') || 'admin';
-    my $problem = Koha::ProblemReport->new(
-        {
-            title          => $subject,
-            content        => $message,
-            borrowernumber => $borrowernumber,
-            branchcode     => $branchcode,
-            username       => $username,
-            problempage    => $place,
-            recipient      => $recipient,
-        }
-    )->store;
-    $template->param(
-        recipient => $recipient,
-        successfuladd => 1,
-        probpage => $place,
-    );
-
-    # send notice to library
-    my $letter = C4::Letters::GetPreparedLetter(
-        module => 'members',
-        letter_code => 'PROBLEM_REPORT',
-        branchcode => $problem->branchcode,
-        tables => {
-            'problem_reports', $problem->reportid
-        }
-    );
-
-    my $from_address = C4::Context->preference('KohaAdminEmailAddress');
-    my $transport = 'email';
-
-    if ( $recipient eq 'admin' ) {
-        C4::Letters::EnqueueLetter({
-            letter                 => $letter,
-            borrowernumber         => $borrowernumber,
-            message_transport_type => $transport,
-            to_address             => C4::Context->preference('KohaAdminEmailAddress'),
-            from_address           => $from_address,
-        });
-    } else {
-        C4::Letters::EnqueueLetter({
-            letter                 => $letter,
-            borrowernumber         => $borrowernumber,
-            message_transport_type => $transport,
-            to_address             => $library->branchemail,
-            from_address           => $from_address,
-        });
+
+    try {
+        my $schema = Koha::Database->new->schema;
+        $schema->txn_do(
+            sub {
+                my $problem = Koha::ProblemReport->new(
+                    {
+                        title          => $subject,
+                        content        => $message,
+                        borrowernumber => $borrowernumber,
+                        branchcode     => $branchcode,
+                        username       => $username,
+                        problempage    => $problempage,
+                        recipient      => $recipient,
+                    }
+                )->store;
+
+                # send notice to library
+                my $letter = C4::Letters::GetPreparedLetter(
+                    module => 'members',
+                    letter_code => 'PROBLEM_REPORT',
+                    branchcode => $problem->branchcode,
+                    tables => {
+                        'problem_reports', $problem->reportid
+                    }
+                );
+
+                my $from_address = C4::Context->preference('KohaAdminEmailAddress');
+                my $transport = 'email';
+
+                if ( $recipient eq 'admin' ) {
+                    C4::Letters::EnqueueLetter({
+                        letter                 => $letter,
+                        borrowernumber         => $borrowernumber,
+                        message_transport_type => $transport,
+                        to_address             => C4::Context->preference('KohaAdminEmailAddress'),
+                        from_address           => $from_address,
+                    });
+                } else {
+                    C4::Letters::EnqueueLetter({
+                        letter                 => $letter,
+                        borrowernumber         => $borrowernumber,
+                        message_transport_type => $transport,
+                        to_address             => $library->branchemail,
+                        from_address           => $from_address,
+                    });
+                }
+
+                push @messages, {
+                    type => 'info',
+                    code => 'success_on_send',
+                };
+
+                $template->param(
+                    recipient => $recipient,
+                );
+            }
+        );
+    }
+    catch {
+        warn "Something wrong happened when sending the report problem: $_";
+        push @messages, {
+            type => 'error',
+            code => 'error_on_send',
+        };
     }
 }
 
+$template->param( messages => \@messages );
+
 output_html_with_http_headers $input, $cookie, $template->output;