fix(mark-as-read): Wait for messages to load, then 700msec, before marking as read

Summary: Resolves T1984 fixes T1984 closes T1984

Test Plan: Run two new test cases

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T1984

Differential Revision: https://phab.nylas.com/D1654

spec-nylas/stores/focused-content-store-spec.coffee

@@ -26,17 +26,6 @@ describe "FocusedContentStore", ->
       FocusedContentStore._onFocus({collection: 'thread', item: testThread})
       expect(FocusedContentStore.trigger).toHaveBeenCalled()
 
-    describe "when the thread is unread", ->
-      beforeEach ->
-        FocusedContentStore._onFocus({collection: 'thread', item: null})
-        spyOn(testThread, 'isUnread').andCallFake -> true
-
-      it "should queue a task to mark the thread as read", ->
-        spyOn(Actions, 'queueTask')
-        FocusedContentStore._onFocus({collection: 'thread', item: testThread})
-        expect(Actions.queueTask).toHaveBeenCalled()
-        expect(Actions.queueTask.mostRecentCall.args[0] instanceof MarkThreadReadTask).toBe(true)
-
   describe "threadId", ->
     it "should return the id of the focused thread", ->
       FocusedContentStore._onFocus({collection: 'thread', item: testThread})

spec-nylas/stores/message-store-spec.coffee

@@ -0,0 +1,65 @@
+_ = require 'underscore'
+Thread = require '../../src/flux/models/thread'
+Message = require '../../src/flux/models/message'
+FocusedContentStore = require '../../src/flux/stores/focused-content-store'
+MessageStore = require '../../src/flux/stores/message-store'
+DatabaseStore = require '../../src/flux/stores/database-store'
+MarkThreadReadTask = require '../../src/flux/tasks/mark-thread-read'
+Actions = require '../../src/flux/actions'
+
+testThread = new Thread(id: '123')
+testMessage1 = new Message(id: 'a', body: '123', files: [])
+testMessage2 = new Message(id: 'b', body: '123', files: [])
+
+describe "MessageStore", ->
+  describe "when thread focus changes", ->
+    beforeEach ->
+      @focus = null
+      spyOn(FocusedContentStore, 'focused').andCallFake (collection) =>
+        if collection is 'thread'
+          @focus
+        else
+          null
+
+      spyOn(FocusedContentStore, 'focusedId').andCallFake (collection) =>
+        if collection is 'thread'
+          @focus?.id
+        else
+          null
+
+      spyOn(DatabaseStore, 'findAll').andCallFake ->
+        include: -> @
+        evaluateImmediately: -> @
+        then: (callback) -> callback([testMessage1, testMessage2])
+
+    it "should retrieve the focused thread", ->
+      @focus = testThread
+      FocusedContentStore.trigger({impactsCollection: -> true})
+      expect(DatabaseStore.findAll).toHaveBeenCalled()
+      expect(DatabaseStore.findAll.mostRecentCall.args[0]).toBe(Message)
+
+    describe "when the thread is unread", ->
+      beforeEach ->
+        @focus = null
+        FocusedContentStore.trigger({impactsCollection: -> true})
+        spyOn(testThread, 'isUnread').andCallFake -> true
+        spyOn(Actions, 'queueTask')
+
+      it "should queue a task to mark the thread as read", ->
+        @focus = testThread
+        FocusedContentStore.trigger({impactsCollection: -> true})
+        advanceClock(500)
+        expect(Actions.queueTask).not.toHaveBeenCalled()
+        advanceClock(500)
+        expect(Actions.queueTask).toHaveBeenCalled()
+        expect(Actions.queueTask.mostRecentCall.args[0] instanceof MarkThreadReadTask).toBe(true)
+
+      it "should not queue a task to mark the thread as read if the thread is no longer selected 500msec later", ->
+        @focus = testThread
+        FocusedContentStore.trigger({impactsCollection: -> true})
+        advanceClock(500)
+        expect(Actions.queueTask).not.toHaveBeenCalled()
+        @focus = null
+        FocusedContentStore.trigger({impactsCollection: -> true})
+        advanceClock(500)
+        expect(Actions.queueTask).not.toHaveBeenCalled()

src/flux/models/message.coffee

@@ -139,6 +139,7 @@ class Message extends Model
     @cc ||= []
     @bcc ||= []
     @replyTo ||= []
+    @files ||= []
     @
 
   toJSON: ->

src/flux/stores/message-store.coffee

@@ -100,16 +100,10 @@ MessageStore = Reflux.createStore
 
     @trigger()
 
-  _markAsReadIfNecessary: ->
-    if @_thread && @_thread.isUnread()
-      Actions.queueTask(new MarkThreadReadTask(@_thread))
-
   _fetchFromCache: (options = {}) ->
     return unless @_thread
     loadedThreadId = @_thread.id
 
-    @_markAsReadIfNecessary()
-
     query = DatabaseStore.findAll(Message, threadId: loadedThreadId)
     query.include(Message.attributes.body)
     query.evaluateImmediately()
@@ -123,7 +117,7 @@ MessageStore = Reflux.createStore
       , =>
         # Check to make sure that our thread is still the thread we were
         # loading items for. Necessary because this takes a while.
-        return unless loadedThreadId == @_thread?.id
+        return unless loadedThreadId is @_thread?.id
 
         loaded = true
 
@@ -159,6 +153,15 @@ MessageStore = Reflux.createStore
         # and once when ready. Many third-party stores will observe
         # MessageStore and they'll be stupid and re-render constantly.
         if loaded
+          # Mark the thread as read if necessary. Wait 700msec so that flipping
+          # through threads doens't mark them all. Make sure it's still the
+          # current thread after the timeout.
+          if @_thread.isUnread()
+            setTimeout =>
+              return unless loadedThreadId is @_thread?.id
+              Actions.queueTask(new MarkThreadReadTask(@_thread))
+            ,700
+
           @_itemsLoading = false
           @trigger(@)