From 43c4859592e14b665b1fd05c8df049a03476eb60 Mon Sep 17 00:00:00 2001
From: Evan Morikawa <evan@evanmorikawa.com>
Date: Wed, 18 Nov 2015 12:32:07 -0800
Subject: [PATCH] fix(draft): fix showing of incorrect body when pending send

Summary: Fixes T3712

Test Plan: new tests

Reviewers: juan, bengotow

Reviewed By: bengotow

Maniphest Tasks: T3712

Differential Revision: https://phab.nylas.com/D2273
---
 .../message-list/lib/message-item-body.cjsx   |  4 ++++
 .../worker-ui/stylesheets/worker-ui.less      |  4 ++++
 spec/stores/draft-store-spec.coffee           | 22 +++++++++++++++++++
 src/flux/stores/draft-store.coffee            |  6 ++++-
 src/flux/stores/message-body-processor.coffee |  8 ++++++-
 5 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/internal_packages/message-list/lib/message-item-body.cjsx b/internal_packages/message-list/lib/message-item-body.cjsx
index d7b2e6989..f3da8d7bf 100644
--- a/internal_packages/message-list/lib/message-item-body.cjsx
+++ b/internal_packages/message-list/lib/message-item-body.cjsx
@@ -24,6 +24,10 @@ class MessageItemBody extends React.Component
   componentWillMount: =>
     @_unsub = MessageBodyProcessor.processAndSubscribe(@props.message, @_onBodyChanged)
 
+  componentWillReceiveProps: (newProps) =>
+    @_unsub?()
+    @_unsub = MessageBodyProcessor.processAndSubscribe(newProps.message, @_onBodyChanged)
+
   componentWillUnmount: =>
     @_unsub?()
 
diff --git a/internal_packages/worker-ui/stylesheets/worker-ui.less b/internal_packages/worker-ui/stylesheets/worker-ui.less
index d204faafd..945fba336 100755
--- a/internal_packages/worker-ui/stylesheets/worker-ui.less
+++ b/internal_packages/worker-ui/stylesheets/worker-ui.less
@@ -108,6 +108,10 @@
         margin: 1em 0;
       }
     }
+    .item {
+      overflow-x: hidden;
+      text-overflow: ellipsis;
+    }
 
     &.curl-history {
       .item {
diff --git a/spec/stores/draft-store-spec.coffee b/spec/stores/draft-store-spec.coffee
index e70d44b53..77dee968b 100644
--- a/spec/stores/draft-store-spec.coffee
+++ b/spec/stores/draft-store-spec.coffee
@@ -720,7 +720,29 @@ describe "DraftStore", ->
         Actions.queueTask.calls.length > 0
       runs ->
         expect(DraftStore.isSendingDraft(draftClientId)).toBe true
+
+    # Since all changes haven't been applied yet, we want to ensure that
+    # no view of the draft renders the draft as if its sending, but with
+    # the wrong text.
+    it "does NOT trigger until the latest changes have been applied", ->
+      spyOn(NylasEnv, "isMainWindow").andReturn true
+      spyOn(Actions, "queueTask").andCallThrough()
+      runs ->
+        DraftStore._onSendDraft(draftClientId)
+        expect(DraftStore.trigger).not.toHaveBeenCalled()
+      waitsFor ->
+        Actions.queueTask.calls.length > 0
+      runs ->
+        # Normally, the session.changes.commit will persist to the
+        # Database. Since that's stubbed out, we need to manually invoke
+        # to database update event to get the trigger (which we want to
+        # test) to fire
+        DraftStore._onDataChanged
+          objectClass: "Message"
+          objects: [draft: true]
+        expect(DraftStore.isSendingDraft(draftClientId)).toBe true
         expect(DraftStore.trigger).toHaveBeenCalled()
+        expect(DraftStore.trigger.calls.length).toBe 1
 
     it "returns false if the draft hasn't been seen", ->
       spyOn(NylasEnv, "isMainWindow").andReturn true
diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee
index e6a7535cc..33c1b5b7a 100644
--- a/src/flux/stores/draft-store.coffee
+++ b/src/flux/stores/draft-store.coffee
@@ -486,7 +486,11 @@ class DraftStore
       SoundRegistry.playSound('hit-send')
 
     @_draftsSending[draftClientId] = true
-    @trigger(draftClientId)
+
+    # It's important NOT to call `trigger(draftClientId)` here. At this
+    # point there are still unpersisted changes in the DraftStoreProxy. If
+    # we `trigger`, we'll briefly display the wrong version of the draft
+    # as if it was sending.
 
     @sessionForClientId(draftClientId).then (session) =>
       @_runExtensionsBeforeSend(session)
diff --git a/src/flux/stores/message-body-processor.coffee b/src/flux/stores/message-body-processor.coffee
index 7ddd2b2bc..cf7fa8b4b 100644
--- a/src/flux/stores/message-body-processor.coffee
+++ b/src/flux/stores/message-body-processor.coffee
@@ -1,3 +1,4 @@
+crypto = require "crypto"
 MessageUtils = require '../models/message-utils'
 MessageStore = require './message-store'
 
@@ -17,8 +18,13 @@ class MessageBodyProcessor
     for {message, callback} in @_subscriptions
       callback(@process(message))
 
+  # It's far safer to key off the hash of the body then the [id, version]
+  # pair. This is because it's theoretically possible for the body to
+  # change without the version updating. When drafts sent N1 used to
+  # optimistically display the message before the latest changes
+  # persisted.
   _key: (message) ->
-    return message.id + message.version
+    return crypto.createHash('md5').update(message.body).digest('hex')
 
   version: ->
     @_version