#216 when trying to save a draft too fast (before the previous version is saved) simply ignore the attempt

8 files changed, 59 insertions, 23 deletions

diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py
index f4f37d77..cb92228c 100644
--- a/service/pixelated/adapter/services/mail_service.py
+++ b/service/pixelated/adapter/services/mail_service.py
@@ -41,6 +41,9 @@ class MailService:
     def mail(self, mail_id):
         return self.mailboxes.mail(mail_id)
 
+    def mail_exists(self, mail_id):
+        return not(not(self.querier.get_header_by_chash(mail_id)))
+
     def send(self, last_draft_ident, mail):
         self.mail_sender.sendmail(mail)
         if last_draft_ident:
diff --git a/service/pixelated/adapter/soledad/soledad_facade_mixin.py b/service/pixelated/adapter/soledad/soledad_facade_mixin.py
index 33588d1e..1df038ea 100644
--- a/service/pixelated/adapter/soledad/soledad_facade_mixin.py
+++ b/service/pixelated/adapter/soledad/soledad_facade_mixin.py
@@ -23,17 +23,20 @@ class SoledadDbFacadeMixin(object):
     def get_all_flags_by_mbox(self, mbox):
         return self.soledad.get_from_index('by-type-and-mbox', 'flags', mbox)
 
-    def get_all_headers_by_chash(self, chash):
-        return self.soledad.get_from_index('by-type-and-contenthash', 'head', chash)
-
     def get_content_by_phash(self, phash):
-        return self.soledad.get_from_index('by-type-and-payloadhash', 'cnt', phash)
+        content = self.soledad.get_from_index('by-type-and-payloadhash', 'cnt', phash)
+        if len(content):
+            return content[0]
 
     def get_flags_by_chash(self, chash):
-        return self.soledad.get_from_index('by-type-and-contenthash', 'flags', chash)[0]
+        flags = self.soledad.get_from_index('by-type-and-contenthash', 'flags', chash)
+        if len(flags):
+            return flags[0]
 
     def get_header_by_chash(self, chash):
-        return self.soledad.get_from_index('by-type-and-contenthash', 'head', chash)[0]
+        header = self.soledad.get_from_index('by-type-and-contenthash', 'head', chash)
+        if len(header):
+            return header[0]
 
     def get_recent_by_mbox(self, mbox):
         return self.soledad.get_from_index('by-type-and-mbox', 'rct', mbox)
diff --git a/service/pixelated/adapter/soledad/soledad_reader_mixin.py b/service/pixelated/adapter/soledad/soledad_reader_mixin.py
index 9cb20a9a..24010e2d 100644
--- a/service/pixelated/adapter/soledad/soledad_reader_mixin.py
+++ b/service/pixelated/adapter/soledad/soledad_reader_mixin.py
@@ -34,26 +34,29 @@ class SoledadReaderMixin(SoledadDbFacadeMixin, object):
 
         fdocs_hdocs = []
         for fdoc, chash in fdocs_chash:
-            hdoc = self.get_all_headers_by_chash(chash)
-            if len(hdoc) == 0:
+            hdoc = self.get_header_by_chash(chash)
+            if not hdoc:
                 continue
-            fdocs_hdocs.append((fdoc, hdoc[0]))
+            fdocs_hdocs.append((fdoc, hdoc))
 
         fdocs_hdocs_bodyphash = [(f[0], f[1], f[1].content.get('body')) for f in fdocs_hdocs]
         fdocs_hdocs_bdocs_parts = []
         for fdoc, hdoc, body_phash in fdocs_hdocs_bodyphash:
             bdoc = self.get_content_by_phash(body_phash)
-            if len(bdoc) == 0:
+            if not bdoc:
                 continue
             parts = self._extract_parts(hdoc.content)
-            fdocs_hdocs_bdocs_parts.append((fdoc, hdoc, bdoc[0], parts))
+            fdocs_hdocs_bdocs_parts.append((fdoc, hdoc, bdoc, parts))
 
         return [PixelatedMail.from_soledad(*raw_mail, soledad_querier=self) for raw_mail in fdocs_hdocs_bdocs_parts]
 
+    def mail_exists(self, ident):
+        return self.get_flags_by_chash(ident)
+
     def mail(self, ident):
         fdoc = self.get_flags_by_chash(ident)
         hdoc = self.get_header_by_chash(ident)
-        bdoc = self.get_content_by_phash(hdoc.content['body'])[0]
+        bdoc = self.get_content_by_phash(hdoc.content['body'])
         parts = self._extract_parts(hdoc.content)
 
         return PixelatedMail.from_soledad(fdoc, hdoc, bdoc, parts=parts, soledad_querier=self)
@@ -65,7 +68,7 @@ class SoledadReaderMixin(SoledadDbFacadeMixin, object):
         return self._build_mails_from_fdocs(fdocs_chash)
 
     def attachment(self, attachment_ident, encoding):
-        bdoc = self.get_content_by_phash(attachment_ident)[0]
+        bdoc = self.get_content_by_phash(attachment_ident)
         return {'content': self._try_decode(bdoc.content['raw'], encoding),
                 'content-type': bdoc.content['content-type']}
 
@@ -94,7 +97,7 @@ class SoledadReaderMixin(SoledadDbFacadeMixin, object):
         return parts
 
     def _extract_alternative(self, hdoc, headers_dict):
-        bdoc = self.get_content_by_phash(hdoc['phash'])[0]
+        bdoc = self.get_content_by_phash(hdoc['phash'])
         raw_content = bdoc.content['raw']
         return {'headers': headers_dict, 'content': raw_content}
 
diff --git a/service/pixelated/controllers/mails_controller.py b/service/pixelated/controllers/mails_controller.py
index 9d649a1f..29e25d76 100644
--- a/service/pixelated/controllers/mails_controller.py
+++ b/service/pixelated/controllers/mails_controller.py
@@ -15,6 +15,8 @@
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
 import json
+import random
+import time
 
 from pixelated.adapter.model.mail import InputMail
 from pixelated.controllers import respond_json
@@ -105,11 +107,15 @@ class MailsController:
         return respond_json(mail.as_dict(), request)
 
     def update_draft(self, request):
+        if bool(random.getrandbits(1)):
+            time.sleep(3)
         content_dict = json.loads(request.content.read())
         _mail = InputMail.from_dict(content_dict)
         draft_id = content_dict.get('ident')
 
         if draft_id:
+            if not self._mail_service.mail_exists(draft_id):
+                return respond_json("", request, status_code=422)
             pixelated_mail = self._draft_service.update_draft(draft_id, _mail)
             self._search_engine.remove_from_index(draft_id)
         else:
diff --git a/service/test/integration/drafts_test.py b/service/test/integration/drafts_test.py
index 2ba14dfd..d4fde099 100644
--- a/service/test/integration/drafts_test.py
+++ b/service/test/integration/drafts_test.py
@@ -28,7 +28,7 @@ class DraftsTest(SoledadTestBase):
     def test_post_sends_mail_and_deletes_previous_draft_if_it_exists(self):
         # creates one draft
         first_draft = MailBuilder().with_subject('First draft').build_json()
-        first_draft_ident = self.put_mail(first_draft)
+        first_draft_ident = self.put_mail(first_draft)[0]['ident']
 
         # sends an updated version of the draft
         second_draft = MailBuilder().with_subject('Second draft').with_ident(first_draft_ident).build_json()
@@ -64,7 +64,7 @@ class DraftsTest(SoledadTestBase):
 
     def test_put_updates_draft_if_it_already_exists(self):
         draft = MailBuilder().with_subject('First draft').build_json()
-        draft_ident = self.put_mail(draft)
+        draft_ident = self.put_mail(draft)[0]['ident']
 
         updated_draft = MailBuilder().with_subject('First draft edited').with_ident(draft_ident).build_json()
         self.put_mail(updated_draft)
@@ -73,3 +73,12 @@ class DraftsTest(SoledadTestBase):
 
         self.assertEquals(1, len(drafts))
         self.assertEquals('First draft edited', drafts[0].subject)
+
+    def test_respond_unprocessable_entity_if_draft_to_remove_doesnt_exist(self):
+        draft = MailBuilder().with_subject('First draft').build_json()
+        self.put_mail(draft)
+
+        updated_draft = MailBuilder().with_subject('First draft edited').with_ident('NOTFOUND').build_json()
+        _, request = self.put_mail(updated_draft)
+
+        self.assertEquals(422, request.code)
diff --git a/service/test/support/integration/soledad_test_base.py b/service/test/support/integration/soledad_test_base.py
index 6368d3e8..4149462c 100644
--- a/service/test/support/integration/soledad_test_base.py
+++ b/service/test/support/integration/soledad_test_base.py
@@ -56,7 +56,7 @@ class SoledadTestBase(unittest.TestCase):
 
     def put_mail(self, data):
         res, req = self.client.put('/mails', data)
-        return res['ident']
+        return res, req
 
     def post_tags(self, mail_ident, tags_json):
         res, req = self.client.post("/mail/%s/tags" % mail_ident, tags_json)
diff --git a/web-ui/app/js/helpers/monitored_ajax.js b/web-ui/app/js/helpers/monitored_ajax.js
index e8454ee6..42b2dcf7 100644
--- a/web-ui/app/js/helpers/monitored_ajax.js
+++ b/web-ui/app/js/helpers/monitored_ajax.js
@@ -32,7 +32,9 @@ define(['page/events', 'views/i18n'], function (events, i18n) {
 
     var originalBeforeSend = config.beforeSend;
     config.beforeSend = function () {
-      $('#loading').show();
+      if (!config.skipLoadingWarning) {
+        $('#loading').show();
+      }
       if (originalBeforeSend) {
         originalBeforeSend();
       }
@@ -40,15 +42,19 @@ define(['page/events', 'views/i18n'], function (events, i18n) {
 
     var originalComplete = config.complete;
     config.complete = function () {
-      $('#loading').fadeOut(500);
+      if (!config.skipLoadingWarning) {
+        $('#loading').fadeOut(500);
+      }
       if (originalComplete) {
         originalComplete();
       }
     };
 
     return $.ajax(url, config).fail(function (xmlhttprequest, textstatus, message) {
-      var msg = messages[textstatus] || 'unexpected problem while talking to server';
-      on.trigger(document, events.ui.userAlerts.displayMessage, { message: i18n(msg) });
+      if (!config.skipErrorMessage) {
+        var msg = 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 a5a28209..834cb205 100644
--- a/web-ui/app/js/mail_view/data/mail_sender.js
+++ b/web-ui/app/js/mail_view/data/mail_sender.js
@@ -48,6 +48,10 @@ define(
             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 {
@@ -66,7 +70,7 @@ define(
           type: 'POST',
           dataType: 'json',
           contentType: 'application/json; charset=utf-8',
-          data: JSON.stringify(data)
+          data: JSON.stringify(data),
         }).done(successSendMail(this))
           .fail(failure(this, 'Error sending mail'));
       };
@@ -76,7 +80,9 @@ define(
           type: 'PUT',
           dataType: 'json',
           contentType: 'application/json; charset=utf-8',
-          data: JSON.stringify(mail)
+          data: JSON.stringify(mail),
+          skipLoadingWarning: true,
+          skipErrorMessage: true
         });
       };