Attachment 68442 Details for Bug 18606 – Bug 18606: Unify handling of success/error messages

[patch] Bug 18606: Unify handling of success/error messages

Bug-18606-Unify-handling-of-successerror-messages.patch (text/plain), 14.67 KB, created by Josef Moravec on 2017-10-23 18:19:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Josef Moravec

Created: 2017-10-23 18:19:49 UTC

Size: 14.67 KB

patch

obsolete

>From a83b8d6542ba777654931ca5dcce0daa41fc6e50 Mon Sep 17 00:00:00 2001
>From: Josef Moravec <josef.moravec@gmail.com>
>Date: Thu, 18 May 2017 09:09:33 +0000
>Subject: [PATCH] Bug 18606: Unify handling of success/error messages
>
>---
> .../en/modules/rotating_collections/addItems.tt    | 52 +++++-------
> .../rotating_collections/editCollections.tt        | 93 ++++++++--------------
> rotating_collections/editCollections.pl            | 28 +++----
> 3 files changed, 63 insertions(+), 110 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt
>index 1cfc9446af..9f60e64827 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/addItems.tt
>@@ -29,44 +29,28 @@
>               [% INCLUDE 'rotating-collections-toolbar.inc' %]
>               <h1>Collection <i>[% collection.colTitle %]</i></h1>
> 
>-              [% IF ( previousActionAdd ) %]
>-                [% IF ( addSuccess ) %]
>-                  <div class="dialog message">Item with barcode '[% barcode %]' added successfully</div>
>-                [% ELSE %]
>-                  <div class="dialog alert">
>-                    <p>Failed to add item with barcode <i>[% barcode %]</i>:
>-                    <p>
>-                        [% IF failureMessage == "NO_ITEM" %]
>-                            No item with matching barcode found
>-                        [% ELSIF failureMessage == "IN_COLLECTION_OTHER" %]
>-                            Item is already in a different rotating collection
>-                        [% ELSIF failureMessage == "IN_COLLECTION" %]
>-                            Item is already in this collection
>-                        [% ELSE %]
>-                            [% failureMessage %]
>-                        [% END %]
>-                    </p>
>+              [% IF ( messages ) %]
>+                <div class="dialog message">
>+                  [% FOR message IN messages %]
>+                    [% IF message.code == 'success_adding_item' %]
>+                      Item with barcode '[% barcode %]' added successfully<br />
>+                    [% ELSIF message.code == 'success_removing_item' %]
>+                      Item with barcode '[% barcode %]' removed successfully<br />
>+                    [% END %]
>+                  [% END %]
>                 </div>
>-                [% END %]
>               [% END %]
> 
>-              [% IF ( previousActionRemove ) %]
>-                [% IF ( removeSuccess ) %]
>-                  <div class="dialog message">Item with barcode <i>[% barcode %]</i> removed successfully</div>
>-                [% ELSE %]
>-                  <div class="dialog alert">
>-                    Failed to remove item with barcode <i>[% barcode %]</i>:
>-                    <p>
>-                        [% IF failureMessage == "NO_ITEM" %]
>-                            No item with matching barcode found
>-                        [% ELSIF failureMessage == "NOT_IN_COLLECTION" %]
>-                            Item is not in this collection
>-                        [% ELSE %]
>-                            [% failureMessage %]
>-                        [% END %]
>-                    </p>
>+              [% IF ( errors ) %]
>+                <div class="dialog alert">
>+                  [% FOR error IN errors %]
>+                    [% IF error.code == 'error_adding_item' %]
>+                      Failed to add item with barcode '[% barcode %]'<br />
>+                    [% ELSIF error.code == 'error_removing_item' %]
>+                      Failed to remove item with barcode <i>[% barcode %]</i><br />
>+                    [% END %]
>+                  [% END %]
>                 </div>
>-                [% END %]
>               [% END %]
> 
>               <div>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt
>index 9504b61e5e..9a1a9427a5 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections/editCollections.tt
>@@ -3,7 +3,7 @@
>     [%- IF ( action == "new" ) %]
>         Add a new collection
>     [% ELSE %]
>-        Edit collection [% editColTitle %]
>+        Edit collection [% collection.colTitle %]
>     [% END -%]
> </title>
> [% INCLUDE 'doc-head-close.inc' %]
>@@ -16,7 +16,7 @@
>     [%- IF ( action == "new" ) %]
>         Add a new collection
>     [% ELSE %]
>-        <a href="/cgi-bin/koha/rotating_collections/addItems.pl?colId=[% editColId %]">Collection <i>[% editColTitle %]</i></a> &rsaquo; Edit
>+        <a href="/cgi-bin/koha/rotating_collections/addItems.pl?colId=[% collection.colId %]">Collection <i>[% collection.colTitle %]</i></a> &rsaquo; Edit
>     [% END -%]
> </div>
> 
>@@ -25,68 +25,41 @@
>         <div id="yui-main">
>             <div class="yui-b">
> 
>-              [% IF ( previousActionCreate ) %]
>-                [% IF ( createSuccess ) %]
>-                    <div class="dialog message">
>-                        <p>Collection <i>[% createdTitle %]</i> added successfully</p>
>-                        <p><a href="/cgi-bin/koha/rotating_collections/rotatingCollections.pl">Return to rotating collections home</a></p>
>-                    </div>
>-                [% ELSE %]
>-                  <div class="dialog alert">
>-                    <p>Collection <i>[% createdTitle %]</i> failed to be added</p>
>-                    <p>
>-                        [% IF failureMessage == "NO_TITLE" %]
>-                            No title entered.
>-                        [% ELSIF failureMessage == "DUPLICATE_TITLE" %]
>-                            Title already in use.
>-                        [% ELSIF failureMessage == "NO_DESCRIPTION" %]
>-                            No description entered.
>-                        [% ELSE %]
>-                            [% failureMessage %]
>-                        [% END %]
>-                    </p>
>-                  </div>
>-                [% END %]
>-              [% END %]
>-
>-              [% IF ( previousActionDelete ) %]
>-                [% IF ( deleteSuccess ) %]
>-                  <div class="dialog message">
>-                    <p>Collection deleted successfully</p>
>-                    <p><a href="/cgi-bin/koha/rotating_collections/rotatingCollections.pl">Return to rotating collections home</a></p>
>-                  </div>
>-                [% ELSE %]
>-                  <div class="dialog alert">Collection failed to be deleted</div>
>-                [% END %]
>+              [% IF messages %]
>+                <div class="dialog message">
>+                  [% FOR message IN messages %]
>+                    [% IF message.code == "success_on_insert" %]
>+                      <p>Collection <i>[% createdTitle %]</i> added successfully</p>
>+                    [% ELSIF message.code == "success_on_delete" %]
>+                      <p>Collection deleted successfully</p>
>+                    [% ELSIF message.code == "success_on_update" %]
>+                      <p>Collection <i>[% updatedTitle %]</i> updated successfully</p>
>+                    [% END %]
>+                  [% END %]
>+                  <p><a href="/cgi-bin/koha/rotating_collections/rotatingCollections.pl">Return to rotating collections home</a></p>
>+                </div>
>               [% END %]
> 
>-              [% IF ( previousActionUpdate ) %]
>-                [% IF ( updateSuccess ) %]
>-                  <div class="dialog message">
>-                      <p>Collection <i>[% updatedTitle %]</i> updated successfully</p>
>-                      <p><a href="/cgi-bin/koha/rotating_collections/rotatingCollections.pl">Return to rotating collections home</a></p>
>-                  </div>
>-                [% ELSE %]
>-                  <div class="dialog alert">
>-                    <p>Collection <i>[% updatedTitle %]</i> failed to be updated</p>
>-                    <p>
>-                        [% IF failureMessage == "NO_ID" %]
>-                            No collection id given.
>-                        [% ELSIF failureMessage == "DUPLICATE_TITLE" %]
>-                            Title already in use.
>-                        [% ELSE %]
>-                            [% failureMessage %]
>-                        [% END %]
>-                    </p>
>+              [% IF errors %]
>+                <div class="dialog alert">
>+                  [% FOR error IN errors %]
>+                    [% IF error.code == "error_on_insert" %]
>+                      <p>Collection <i>[% createdTitle %]</i> failed to be added</p>
>+                    [% ELSIF  error.code == "error_on_delete" %]
>+                      <p>Collection failed to be deleted</p>
>+                    [% ELSIF  error.code == "error_on_updated" %]
>+                      <p>Collection <i>[% updatedTitle %]</i> failed to be updated</p>
>+                    [% END %]
>+                  [% END %]
>+                  <p><a href="/cgi-bin/koha/rotating_collections/rotatingCollections.pl">Return to rotating collections home</a></p>
>                 </div>
>-                [% END %]
>               [% END %]
> 
>               <div>
> 
>                 [% IF action == "new" || action == "edit" %]
>                     [% IF ( previousActionEdit ) %]
>-                      <h1>Edit collection <i>[% editColTitle %]</i></h1>
>+                      <h1>Edit collection <i>[% collection.colTitle %]</i></h1>
>                     [% ELSE %]
>                       <h1>Add new collection</h1>
>                     [% END %]
>@@ -94,7 +67,7 @@
>                     <form action="editCollections.pl" method="post" class="validated">
>                         [% IF ( previousActionEdit ) %]
>                             <input type="hidden" name="action" value="update" />
>-                            <input type="hidden" name="colId" value="[% editColId %]" />
>+                            <input type="hidden" name="colId" value="[% collection.colId %]" />
>                         [% ELSE %]
>                             <input type="hidden" name="action" value="create" />
>                         [% END %]
>@@ -103,8 +76,8 @@
>                             <ol>
>                                 <li>
>                                     <label class="required" for="title">Title: </label>
>-                                    [% IF ( editColTitle ) %]
>-                                        <input type="text" name="title" value="[% editColTitle %]" required="required" />
>+                                    [% IF ( collection.colTitle ) %]
>+                                        <input type="text" name="title" value="[% collection.colTitle %]" required="required" />
>                                         <span class="required">Required</span>
>                                     [% ELSE %]
>                                         <input type="text" name="title" required="required" />
>@@ -113,8 +86,8 @@
>                                 </li>
>                                 <li>
>                                     <label for="description">Description: </label>
>-                                    [% IF (editColDescription ) %]
>-                                        <input type="text" size="50" name="description" value="[% editColDescription %]" />
>+                                    [% IF (collection.colDesc ) %]
>+                                        <input type="text" size="50" name="description" value="[% collection.colDesc %]" />
>                                     [% ELSE %]
>                                         <input type="text" size="50" name="description" />
>                                     [% END %]
>diff --git a/rotating_collections/editCollections.pl b/rotating_collections/editCollections.pl
>index 929043248b..1427a57c20 100755
>--- a/rotating_collections/editCollections.pl
>+++ b/rotating_collections/editCollections.pl
>@@ -29,6 +29,7 @@ use Koha::RotatingCollections;
> my $query = new CGI;
> my $action = $query->param('action');
> my @messages;
>+my @errors;
> 
> my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
>     {
>@@ -45,6 +46,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
> if ( $action eq 'create' ) {
>     my $title       = $query->param('title');
>     my $description = $query->param('description');
>+    $template->param( createdTitle => $title );
> 
>     my $collection = Koha::RotatingCollection->new(
>         {   colTitle => $title,
>@@ -55,34 +57,28 @@ if ( $action eq 'create' ) {
>     eval { $collection->store; };
> 
>     if ($@) {
>-        push @messages, { type => 'error', code => 'error_on_insert' };
>+        push @errors, { code => 'error_on_insert' };
>     } else {
>-        push @messages, { type => 'message', code => 'success_on_insert' };
>+        push @messages, { code => 'success_on_insert' };
>     }
> 
>-    $action = "list";
>-
> } elsif ( $action eq 'delete' ) { # Delete collection
>     my $colId = $query->param('colId');
>     my $collection = Koha::RotatingCollections->find($colId);
>     my $deleted = eval { $collection->delete; };
> 
>     if ( $@ or not $deleted ) {
>-        push @messages, { type => 'error', code => 'error_on_delete' };
>+        push @errors, { code => 'error_on_delete' };
>     } else {
>-        push @messages, { type => 'message', code => 'success_on_delete' };
>+        push @messages, { code => 'success_on_delete' };
>     }
> 
>-    $action = "list";
>-
> } elsif ( $action eq 'edit' ) { # Edit page of collection
>     my $collection = Koha::RotatingCollections->find($query->param('colId'));
> 
>     $template->param(
>         previousActionEdit => 1,
>-        editColId          => $collection->colId,
>-        editColTitle       => $collection->colTitle,
>-        editColDescription => $collection->colDesc,
>+        collection         => $collection,
>     );
> 
> } elsif ( $action eq 'update' ) { # Update collection
>@@ -90,6 +86,8 @@ if ( $action eq 'create' ) {
>     my $title       = $query->param('title');
>     my $description = $query->param('description');
> 
>+    $template->param( updatedTitle => $title );
>+
>     if ($colId) {
>         my $collection = Koha::RotatingCollections->find($colId);
>         $collection->colTitle($title);
>@@ -98,19 +96,17 @@ if ( $action eq 'create' ) {
>         eval { $collection->store; };
> 
>         if ($@) {
>-            push @messages, { type => 'error', code => 'error_on_update' };
>+            push @errors, { code => 'error_on_update' };
>         } else {
>-            push @messages, { type => 'message', code => 'success_on_update' };
>+            push @messages, { code => 'success_on_update' };
>         }
>-
>-        $action = "list";
>     }
>-
> }
> 
> $template->param(
>     action   => $action,
>     messages => \@messages,
>+    errors   => \@errors,
> );
> 
> output_html_with_http_headers $query, $cookie, $template->output;
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 18606: 63508 | 63509 | 63510 | 63511 | 63512 | 63513 | 63514 | 63515 | 63516 | 63517 | 63518 | 63519 | 63520 | 63521 | 63522 | 63523 | 63524 | 63525 | 63526 | 63527 | 63528 | 66227 | 66228 | 66229 | 66230 | 66231 | 66232 | 66233 | 66234 | 66235 | 66236 | 66237 | 66238 | 66239 | 66240 | 66241 | 66286 | 66287 | 66288 | 66289 | 66290 | 66291 | 66292 | 66293 | 66294 | 66295 | 66296 | 66297 | 66298 | 66299 | 66300 | 67734 | 67735 | 67736 | 67737 | 67738 | 67739 | 67740 | 67741 | 67742 | 67743 | 67744 | 67745 | 67746 | 67747 | 67748 | 67749 | 68428 | 68429 | 68430 | 68431 | 68432 | 68433 | 68434 | 68435 | 68436 | 68437 | 68438 | 68439 | 68440 | 68441 | 68442 | 68443 | 69709 | 69710 | 69711 | 69712 | 69713 | 69714 | 69715 | 69716 | 69717 | 69718 | 69719 | 69720 | 69721 | 69722 | 69723 | 69759 | 72416 | 72417 | 72418 | 72419 | 72420 | 72421 | 72422 | 72423 | 72424 | 72425 | 72426 | 72427 | 72428 | 72429 | 72430 | 72431 | 72572 | 72573 | 72574 | 72575 | 72576 | 72577 | 72578 | 72579 | 72580 | 72581 | 72582 | 72583 | 72584 | 72585 | 72586 | 72587 | 72816 | 79141 | 79142 | 79143 | 79144 | 79145 | 79146 | 79147 | 79148 | 79149 | 79150 | 79151 | 79152 | 79153 | 79154 | 79155 | 79156 | 79157 | 79158 | 79161