#294 - using server error message on ajax if message is provided

5 files changed, 56 insertions, 50 deletions

diff --git a/web-ui/app/js/helpers/monitored_ajax.js b/web-ui/app/js/helpers/monitored_ajax.js
index 0f851c7a..0068e10c 100644
--- a/web-ui/app/js/helpers/monitored_ajax.js
+++ b/web-ui/app/js/helpers/monitored_ajax.js
@@ -51,7 +51,9 @@ define(['page/events', 'views/i18n'], function (events, i18n) {
 
     return $.ajax(url, config).fail(function (xmlhttprequest, textstatus, message) {
       if (!config.skipErrorMessage) {
-        var msg = messages[textstatus] || 'unexpected problem while talking to server';
+        var msg =  (xmlhttprequest.responseJSON && xmlhttprequest.responseJSON.message) ||
+            messages[textstatus] ||
+            'unexpected problem while talking to server';
         on.trigger(document, events.ui.userAlerts.displayMessage, { message: i18n(msg) });
       }
     }.bind(this));
diff --git a/web-ui/app/js/mail_view/data/mail_sender.js b/web-ui/app/js/mail_view/data/mail_sender.js
index 834cb205..4c7e07ea 100644
--- a/web-ui/app/js/mail_view/data/mail_sender.js
+++ b/web-ui/app/js/mail_view/data/mail_sender.js
@@ -40,43 +40,22 @@ define(
         };
       }
 
-      function failure(on, context) {
-        return function(xhr, status, error) {
-          var contextMessage = '';
-
-          if (context) {
-            contextMessage = context + ': ';
-          }
-
-          if (xhr.status === 422) {
-            return; // ignore the fact that it failed to save the draft - it will succeed eventually
-          }
-
-          if (xhr && xhr.responseJSON && xhr.responseJSON.message) {
-            on.trigger(document, events.ui.userAlerts.displayMessage, {message: contextMessage + xhr.responseJSON.message});
-          } else {
-            on.trigger(document, events.ui.userAlerts.displayMessage, {message: 'Ops! something went wrong, try again later.'});
-          }
-          on.trigger(document, events.mail.send_failed, {xhr: xhr, error:error});
-        };
-      }
-
       this.defaultAttrs({
         mailsResource: '/mails'
       });
 
       this.sendMail = function(event, data) {
-        monitoredAjax(this, this.attr.mailsResource, {
+        monitoredAjax.call(_, this, this.attr.mailsResource, {
           type: 'POST',
           dataType: 'json',
           contentType: 'application/json; charset=utf-8',
           data: JSON.stringify(data),
-        }).done(successSendMail(this))
-          .fail(failure(this, 'Error sending mail'));
+        }).done(successSendMail(this));
+
       };
 
       this.saveMail = function(mail) {
-        return monitoredAjax(this, this.attr.mailsResource, {
+        return monitoredAjax.call(_, this, this.attr.mailsResource, {
           type: 'PUT',
           dataType: 'json',
           contentType: 'application/json; charset=utf-8',
@@ -88,8 +67,7 @@ define(
 
       this.saveDraft = function(event, data) {
         this.saveMail(data)
-          .done(successSaveDraft(this))
-          .fail(failure(this));
+          .done(successSaveDraft(this));
       };
 
       this.saveMailWithCallback = function(event, data) {
diff --git a/web-ui/test/spec/helpers/monitored_ajax_call.spec.js b/web-ui/test/spec/helpers/monitored_ajax_call.spec.js
new file mode 100644
index 00000000..bf7a2db4
--- /dev/null
+++ b/web-ui/test/spec/helpers/monitored_ajax_call.spec.js
@@ -0,0 +1,41 @@
+define(['helpers/monitored_ajax'], function (monitoredAjax) {
+  'use strict';
+  describe('monitoredAjaxCall', function () {
+    describe('when dealing with errors', function () {
+
+      _.each(
+        {
+         timeout: 'a timeout occurred',
+         error: 'problems talking to server',
+         parseerror: 'got invalid response from server'
+        }, function (errorMessage, errorType) {
+        it('shows message for a server ' + errorType, function () {
+          var component = { trigger: function () {}};
+          spyOn(component, 'trigger');
+          var d = $.Deferred();
+          spyOn($, 'ajax').and.returnValue(d);
+
+          monitoredAjax(component, '/', {});
+          d.reject({ responseJSON: {}}, errorType, '');
+
+          expect(component.trigger).toHaveBeenCalledWith(document, Pixelated.events.ui.userAlerts.displayMessage,
+              { message: errorMessage });
+        });
+      });
+
+      it('shows the error message sent by the server if it exists', function () {
+        var component = { trigger: function () {}};
+        spyOn(component, 'trigger');
+        var d = $.Deferred();
+        spyOn($, 'ajax').and.returnValue(d);
+
+        monitoredAjax(component, '/', {});
+        d.reject({ responseJSON: { message: 'Server Message'}}, 'error', '');
+
+        expect(component.trigger).toHaveBeenCalledWith(document, Pixelated.events.ui.userAlerts.displayMessage,
+            { message: 'Server Message' });
+      });
+
+    });
+  });
+});
\ No newline at end of file
diff --git a/web-ui/test/spec/mail_view/data/mail_sender.spec.js b/web-ui/test/spec/mail_view/data/mail_sender.spec.js
index 9b6c2828..6a1d356c 100644
--- a/web-ui/test/spec/mail_view/data/mail_sender.spec.js
+++ b/web-ui/test/spec/mail_view/data/mail_sender.spec.js
@@ -47,29 +47,14 @@ describeComponent('mail_view/data/mail_sender', function () {
     expect(JSON.parse($.ajax.calls.mostRecent().args[1].data).body).toEqual(mail.body);
   });
 
-  it('displays generic error message when sending an email fails in the service', function () {
-    var deferred;
-    deferred = $.Deferred();
-    deferred.reject({responseJSON: {}}, 500, 'Internal Server Error');
-
-    var messageEvent = spyOnEvent(document, Pixelated.events.ui.userAlerts.displayMessage);
-    spyOn($, 'ajax').and.returnValue(deferred);
+  it('uses the monitored ajax call to delegate for errors', function () {
+    var monitoredAjaxCall = require('helpers/monitored_ajax');
+    spyOn(monitoredAjaxCall, 'call').and.returnValue($.Deferred());
 
     this.component.trigger(Pixelated.events.mail.send, mail);
+    this.component.trigger(Pixelated.events.mail.saveDraft, mail);
 
-    expect(messageEvent).toHaveBeenTriggeredOnAndWith(document, {message: 'Ops! something went wrong, try again later.'});
+    expect(monitoredAjaxCall.call.calls.count()).toEqual(2);
   });
 
-  it('displays error message returned by the service when sending an email fails in the service', function () {
-    var deferred;
-    deferred = $.Deferred();
-    deferred.reject({responseJSON: {message: 'test: error message'}}, 422, 'Unprocessable Entity');
-
-    var messageEvent = spyOnEvent(document, Pixelated.events.ui.userAlerts.displayMessage);
-    spyOn($, 'ajax').and.returnValue(deferred);
-
-    this.component.trigger(Pixelated.events.mail.send, mail);
-
-    expect(messageEvent).toHaveBeenTriggeredOnAndWith(document, {message: 'Error sending mail: test: error message'});
-  });
 });
diff --git a/web-ui/test/spec/services/mail_service.spec.js b/web-ui/test/spec/services/mail_service.spec.js
index 685ad5bc..1a5cb9f7 100644
--- a/web-ui/test/spec/services/mail_service.spec.js
+++ b/web-ui/test/spec/services/mail_service.spec.js
@@ -164,12 +164,12 @@ describeComponent('services/mail_service', function () {
 
   it('will trigger an error message when a message cannot be deleted', function() {
     var deferred = $.Deferred();
-    var spyAjax = spyOn($, 'ajax').and.returnValue(deferred);
+    spyOn($, 'ajax').and.returnValue(deferred);
     var spyEvent = spyOnEvent(document, Pixelated.events.ui.userAlerts.displayMessage);
 
     this.component.trigger(Pixelated.events.mail.delete, {mail: {ident: '43'}});
 
-    deferred.reject();
+    deferred.reject({responseJSON: {}});
 
     expect(spyEvent).toHaveBeenTriggeredOnAndWith(document, {message: i18n('Could not delete email')} );
   });