From 0089fcac2a2f1059b3d860c4e1ee00db433abe7f Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Mon, 15 Jun 2015 18:19:52 -0700 Subject: [PATCH] fix(T1886): When sending, strip off fields if they result in specific, known errors on the API Summary: Fixes T1886 Test Plan: Run 2 new tests Reviewers: evan Reviewed By: evan Maniphest Tasks: T1886 Differential Revision: https://phab.nylas.com/D1633 --- spec-nylas/tasks/send-draft-spec.coffee | 41 +++++++++++- src/flux/tasks/send-draft.coffee | 86 +++++++++++++++---------- 2 files changed, 92 insertions(+), 35 deletions(-) diff --git a/spec-nylas/tasks/send-draft-spec.coffee b/spec-nylas/tasks/send-draft-spec.coffee index 52ef3e0a0..8a00b0408 100644 --- a/spec-nylas/tasks/send-draft-spec.coffee +++ b/spec-nylas/tasks/send-draft-spec.coffee @@ -10,7 +10,7 @@ _ = require 'underscore' describe "SendDraftTask", -> describe "shouldWaitForTask", -> - it "should return any SyncbackDraftTasks for the same draft", -> + it "should return true if there are SyncbackDraftTasks for the same draft", -> @draftA = new Message version: '1' id: '1233123AEDF1' @@ -190,13 +190,52 @@ describe "SendDraftTask", -> version: '1' id: '1233123AEDF1' namespaceId: 'A12ADE' + threadId: 'threadId' + replyToMessageId: 'replyToMessageId' subject: 'New Draft' + body: 'body' draft: true to: name: 'Dummy' email: 'dummy@nylas.com' @task = new SendDraftTask(@draft.id) spyOn(Actions, "dequeueTask") + spyOn(DatabaseStore, 'unpersistModel').andCallFake (draft) -> + Promise.resolve() + + describe "when the server responds with `Invalid message public ID`", -> + it "should resend the draft without the reply_to_message_id key set", -> + @draft.id = generateTempId() + spyOn(DatabaseStore, 'findByLocalId').andCallFake => Promise.resolve(@draft) + spyOn(NylasAPI, 'makeRequest').andCallFake ({body, success, error}) -> + if body.reply_to_message_id + err = new Error("Invalid message public id") + error(err) + else + success(body) + + waitsForPromise => + @task.performRemote().then => + expect(NylasAPI.makeRequest.calls.length).toBe(2) + expect(NylasAPI.makeRequest.calls[1].args[0].body.thread_id).toBe('threadId') + expect(NylasAPI.makeRequest.calls[1].args[0].body.reply_to_message_id).toBe(null) + + describe "when the server responds with `Invalid thread ID`", -> + it "should resend the draft without the thread_id or reply_to_message_id keys set", -> + @draft.id = generateTempId() + spyOn(DatabaseStore, 'findByLocalId').andCallFake => Promise.resolve(@draft) + spyOn(NylasAPI, 'makeRequest').andCallFake ({body, success, error}) -> + if body.thread_id + err = new Error("Invalid thread public id") + error(err) + else + success(body) + + waitsForPromise => + @task.performRemote().then => + expect(NylasAPI.makeRequest.calls.length).toBe(2) + expect(NylasAPI.makeRequest.calls[1].args[0].body.thread_id).toBe(null) + expect(NylasAPI.makeRequest.calls[1].args[0].body.reply_to_message_id).toBe(null) it "throws an error if the draft can't be found", -> spyOn(DatabaseStore, 'findByLocalId').andCallFake (klass, localId) -> diff --git a/src/flux/tasks/send-draft.coffee b/src/flux/tasks/send-draft.coffee index b077bb2bc..fe9cd1996 100644 --- a/src/flux/tasks/send-draft.coffee +++ b/src/flux/tasks/send-draft.coffee @@ -27,44 +27,62 @@ class SendDraftTask extends Task # When we send drafts, we don't update anything in the app until # it actually succeeds. We don't want users to think messages have # already sent when they haven't! - return Promise.reject("Attempt to call SendDraftTask.performLocal without @draftLocalId") unless @draftLocalId - + if not @draftLocalId + return Promise.reject(new Error("Attempt to call SendDraftTask.performLocal without @draftLocalId.")) Promise.resolve() performRemote: -> + # Fetch the latest draft data to make sure we make the request with the most + # recent draft version + DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => + # The draft may have been deleted by another task. Nothing we can do. + @draft = draft + if not draft + return Promise.reject(new Error("We couldn't find the saved draft.")) + + if draft.isSaved() + body = + draft_id: draft.id + version: draft.version + else + # Pass joined:true so the draft body is included + body = draft.toJSON(joined: true) + + return @_performRemoteSend(body) + + # Returns a promise which resolves when the draft is sent. There are several + # failure cases where this method may call itself, stripping bad fields out of + # the body. This promise only rejects when these changes have been tried. + _performRemoteSend: (body) -> + @_performRemoteAPIRequest(body) + .then (json) => + message = (new Message).fromJSON(json) + atom.playSound('mail_sent.ogg') + Actions.sendDraftSuccess + draftLocalId: @draftLocalId + newMessage: message + return DatabaseStore.unpersistModel(@draft) + + .catch (err) => + if err.message?.indexOf('Invalid message public id') is 0 + body.reply_to_message_id = null + return @_performRemoteSend(body) + else if err.message?.indexOf('Invalid thread') is 0 + body.thread_id = null + body.reply_to_message_id = null + return @_performRemoteSend(body) + else + return Promise.reject(err) + + _performRemoteAPIRequest: (body) -> new Promise (resolve, reject) => - # Fetch the latest draft data to make sure we make the request with the most - # recent draft version - DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - # The draft may have been deleted by another task. Nothing we can do. - return reject(new Error("We couldn't find the saved draft.")) unless draft - - NylasAPI.makeRequest - path: "/n/#{draft.namespaceId}/send" - method: 'POST' - body: @_prepareBody(draft) - returnsModel: true - success: @_onSendDraftSuccess(draft, resolve, reject) - error: reject - .catch(reject) - - _prepareBody: (draft) -> - if draft.isSaved() - body = - draft_id: draft.id - version: draft.version - else - # Pass joined:true so the draft body is included - body = draft.toJSON(joined: true) - return body - - _onSendDraftSuccess: (draft, resolve, reject) => (newMessage) => - newMessage = (new Message).fromJSON(newMessage) - atom.playSound('mail_sent.ogg') - Actions.sendDraftSuccess - draftLocalId: @draftLocalId - newMessage: newMessage - DatabaseStore.unpersistModel(draft).then(resolve).catch(reject) + NylasAPI.makeRequest + path: "/n/#{@draft.namespaceId}/send" + method: 'POST' + body: body + returnsModel: true + success: resolve + error: reject onAPIError: (apiError) -> msg = apiError.message ? "Our server is having problems. Your message has not been sent."