fix(unread): can mark message as unread in split mode

Summary: Fixes T4835

Test Plan: new tests

Reviewers: bengotow

Reviewed By: bengotow

Maniphest Tasks: T4835

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

spec/stores/message-store-spec.coffee

@@ -14,6 +14,7 @@ testMessage2 = new Message(id: 'b', body: '123', files: [])
 describe "MessageStore", ->
   describe "when thread focus changes", ->
     beforeEach ->
+      MessageStore._lastLoadedThreadId = null
       @focus = null
       spyOn(FocusedContentStore, 'focused').andCallFake (collection) =>
         if collection is 'thread'
@@ -75,3 +76,17 @@ describe "MessageStore", ->
         FocusedContentStore.trigger({impactsCollection: -> true})
         advanceClock(500)
         expect(Actions.queueTask).not.toHaveBeenCalled()
+
+      it "should not re-mark the thread as read when made unread", ->
+        @focus = testThread
+        testThread.unread = false
+        FocusedContentStore.trigger({impactsCollection: -> true})
+        advanceClock(500)
+        expect(Actions.queueTask).not.toHaveBeenCalled()
+
+        # This simulates a DB change or some attribute changing on the
+        # thread.
+        testThread.unread = true
+        MessageStore._fetchFromCache()
+        advanceClock(500)
+        expect(Actions.queueTask).not.toHaveBeenCalled()

spec/tasks/mark-message-read-spec.coffee

@@ -1,71 +0,0 @@
-NylasAPI = require '../../src/flux/nylas-api'
-Actions = require '../../src/flux/actions'
-{APIError} = require '../../src/flux/errors'
-MarkMessageReadTask = require '../../src/flux/tasks/mark-message-read'
-DatabaseStore = require '../../src/flux/stores/database-store'
-Message = require '../../src/flux/models/message'
-_ = require 'underscore'
-
-describe "MarkMessageReadTask", ->
-  beforeEach ->
-    @message = new Message
-      id: '1233123AEDF1'
-      accountId: 'A12ADE'
-      subject: 'New Message'
-      unread: true
-      to:
-        name: 'Dummy'
-        email: 'dummy@nylas.com'
-    @task = new MarkMessageReadTask(@message)
-
-  describe "performLocal", ->
-    it "should mark the message as read", ->
-      @task.performLocal()
-      expect(@message.unread).toBe(false)
-
-    it "should trigger an action to persist the change", ->
-      spyOn(DatabaseStore, 'persistModel').andCallFake -> Promise.resolve()
-      @task.performLocal()
-      expect(DatabaseStore.persistModel).toHaveBeenCalled()
-
-  describe "performRemote", ->
-    it "should make the PUT request to the message endpoint", ->
-      spyOn(NylasAPI, 'makeRequest').andCallFake => new Promise (resolve,reject) ->
-      @task.performRemote()
-      options = NylasAPI.makeRequest.mostRecentCall.args[0]
-      expect(options.path).toBe("/messages/#{@message.id}")
-      expect(options.accountId).toBe(@message.accountId)
-      expect(options.method).toBe('PUT')
-      expect(options.body.unread).toBe(false)
-
-  describe "when the remote API request fails", ->
-    beforeEach ->
-      spyOn(DatabaseStore, 'persistModel').andCallFake -> Promise.resolve()
-      spyOn(NylasAPI, 'makeRequest').andCallFake -> Promise.reject(new APIError(body: '', statusCode: 400))
-
-    it "should not mark the message as unread if it was not unread initially", ->
-      message = new Message
-        id: '1233123AEDF1'
-        accountId: 'A12ADE'
-        subject: 'New Message'
-        unread: false
-        to:
-          name: 'Dummy'
-          email: 'dummy@nylas.com'
-      @task = new MarkMessageReadTask(message)
-      @task.performLocal()
-      @task.performRemote()
-      advanceClock()
-      expect(message.unread).toBe(false)
-
-    it "should mark the message as unread", ->
-      @task.performLocal()
-      @task.performRemote()
-      advanceClock()
-      expect(@message.unread).toBe(true)
-
-    it "should trigger an action to persist the change", ->
-      @task.performLocal()
-      @task.performRemote()
-      advanceClock()
-      expect(DatabaseStore.persistModel).toHaveBeenCalled()

src/flux/stores/message-store.coffee

@@ -126,6 +126,26 @@ class MessageStore extends NylasStore
 
     @_fetchFromCache()
 
+  _markAsRead: ->
+    # Mark the thread as read if necessary. Make sure it's still the
+    # current thread after the timeout.
+    #
+    # Override canBeUndone to return false so that we don't see undo
+    # prompts (since this is a passive action vs. a user-triggered
+    # action.)
+    loadedThreadId = @_thread?.id
+    return unless loadedThreadId
+    return if @_lastLoadedThreadId is loadedThreadId
+    @_lastLoadedThreadId = loadedThreadId
+    if @_thread.unread
+      markAsReadDelay = atom.config.get('core.reading.markAsReadDelay')
+      setTimeout =>
+        return unless loadedThreadId is @_thread?.id and @_thread.unread
+        t = new ChangeUnreadTask(thread: @_thread, unread: false)
+        t.canBeUndone = => false
+        Actions.queueTask(t)
+      , markAsReadDelay
+
   _onToggleMessageIdExpanded: (id) =>
     item = _.findWhere(@_items, {id})
     return unless item
@@ -178,21 +198,8 @@ class MessageStore extends NylasStore
       # 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. Make sure it's still the
-        # current thread after the timeout.
-
-        # Override canBeUndone to return false so that we don't see undo prompts
-        # (since this is a passive action vs. a user-triggered action.)
-        if @_thread.unread
-          markAsReadDelay = atom.config.get('core.reading.markAsReadDelay')
-          setTimeout =>
-            return unless loadedThreadId is @_thread?.id and @_thread.unread
-            t = new ChangeUnreadTask(thread: @_thread, unread: false)
-            t.canBeUndone = => false
-            Actions.queueTask(t)
-          , markAsReadDelay
-
         @_itemsLoading = false
+        @_markAsRead()
         @trigger(@)
 
   _fetchExpandedBodies: (items) ->

src/flux/tasks/mark-message-read.coffee

@@ -1,40 +0,0 @@
-Task = require './task'
-{APIError} = require '../errors'
-DatabaseStore = require '../stores/database-store'
-Actions = require '../actions'
-NylasAPI = require '../nylas-api'
-_ = require 'underscore'
-
-module.exports =
-class MarkMessageReadTask extends Task
-
-  constructor: (@message) ->
-    super
-
-  performLocal: ->
-    # update the flag on the message
-    @_previousUnreadState = @message.unread
-    @message.unread = false
-
-    # dispatch an action to persist it
-    DatabaseStore.persistModel(@message)
-
-  performRemote: ->
-    # queue the operation to the server
-    NylasAPI.makeRequest
-      path: "/messages/#{@message.id}"
-      accountId: @message.accountId
-      method: 'PUT'
-      body:
-        unread: false
-      returnsModel: true
-    .then =>
-      return Promise.resolve(Task.Status.Success)
-    .catch APIError, (err) =>
-      if err.statusCode in NylasAPI.PermanentErrorCodes
-        # Run performLocal backwards to undo the tag changes
-        @message.unread = @_previousUnreadState
-        DatabaseStore.persistModel(@message).then =>
-          return Promise.resolve(Task.Status.Failed)
-      else
-        return Promise.resolve(Task.Status.Retry)

src/global/nylas-exports.coffee

@@ -90,7 +90,6 @@ class NylasExports
   @require "SyncbackDraftTask", 'flux/tasks/syncback-draft'
   @require "ChangeStarredTask", 'flux/tasks/change-starred-task'
   @require "CreateMetadataTask", 'flux/tasks/create-metadata-task'
-  @require "MarkMessageReadTask", 'flux/tasks/mark-message-read'
   @require "DestroyMetadataTask", 'flux/tasks/destroy-metadata-task'
 
   # Stores