feat(unread-notifications): Check that unread messages are in inbox

Summary: When we get unread messages, grab their threads and then make sure they're in the inbox before notifying.

Test Plan: See updated specs!

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1361

internal_packages/inbox-activity-bar/stylesheets/activity-bar.less

@@ -31,6 +31,7 @@
     input.filter {
       margin-left: 4px;
       padding: 2px;
+      color:black;
       vertical-align: middle;
       width: 400px;
     }

internal_packages/unread-notifications/lib/main.coffee

@@ -1,5 +1,5 @@
 _ = require 'underscore-plus'
-{Actions} = require 'inbox-exports'
+{Actions, DatabaseStore, Thread} = require 'inbox-exports'
 
 module.exports =
   activate: ->
@@ -12,26 +12,52 @@ module.exports =
 
   serialize: ->
 
-  _onNewMailReceived: (models) ->
-    # Display a notification if we've received new messages
-    newUnreadMessages = _.filter (models['message'] ? []), (msg) =>
-      msg.unread is true and msg.date?.valueOf() >= @activationTime
+  _onNewMailReceived: (incoming) ->
+    new Promise (resolve, reject) =>
+      incomingMessages = incoming['message'] ? []
+      incomingThreads = incoming['thread'] ? []
 
-    if newUnreadMessages.length is 1
-      msg = newUnreadMessages.pop()
-      notif = new Notification(msg.from[0].displayName(), {
-        body: msg.subject
-        tag: 'unread-update'
-      })
-      notif.onclick = ->
-        atom.displayWindow()
-        Actions.selectTagId("inbox")
-        Actions.selectThreadId(msg.threadId)
+      # Filter for new messages
+      newUnread = _.filter incomingMessages, (msg) =>
+        msg.unread is true and msg.date?.valueOf() >= @activationTime
 
-    if newUnreadMessages.length > 1
-      new Notification("#{newUnreadMessages.length} Unread Messages", {
-        tag: 'unread-update'
-      })
+      return resolve() if newUnread.length is 0
 
-    if newUnreadMessages.length > 0
-      atom.playSound('new_mail.ogg')
+      # For each message, find it's corresponding thread. First, look to see
+      # if it's already in the `incoming` payload (sent via delta sync
+      # at the same time as the message.) If it's not, try loading it from
+      # the local cache.
+      #
+      # Note we may receive multiple unread msgs for the same thread.
+      # Using a map and ?= to avoid repeating work.
+      threads = {}
+      for msg in newUnread
+        threads[msg.threadId] ?= _.findWhere(incomingThreads, {id: msg.threadId})
+        threads[msg.threadId] ?= DatabaseStore.find(Thread, msg.threadId)
+
+      Promise.props(threads).then (threads) ->
+
+        # Filter new unread messages to just the ones in the inbox
+        newUnreadInInbox = _.filter newUnread, (msg) ->
+          threads[msg.threadId]?.hasTagId('inbox')
+
+        if newUnreadInInbox.length is 1
+          msg = newUnreadInInbox.pop()
+          notif = new Notification(msg.from[0].displayName(), {
+            body: msg.subject
+            tag: 'unread-update'
+          })
+          notif.onclick = ->
+            atom.displayWindow()
+            Actions.selectTagId("inbox")
+            Actions.selectThreadId(msg.threadId)
+
+        if newUnreadInInbox.length > 1
+          new Notification("#{newUnreadInInbox.length} Unread Messages", {
+            tag: 'unread-update'
+          })
+
+        if newUnreadInInbox.length > 0
+          atom.playSound('new_mail.ogg')
+
+        resolve()

internal_packages/unread-notifications/spec/main-spec.coffee

@@ -1,57 +1,111 @@
 _ = require 'underscore-plus'
+Promise = require 'bluebird'
 Contact = require '../../../src/flux/models/contact'
 Message = require '../../../src/flux/models/message'
+Thread = require '../../../src/flux/models/thread'
+Tag = require '../../../src/flux/models/tag'
+DatabaseStore = require '../../../src/flux/stores/database-store'
 Main = require '../lib/main'
 
 describe "UnreadNotifications", ->
   beforeEach ->
     Main.activate()
-    spyOn(window, 'Notification').andCallFake ->
+
+    @threadA = new Thread
+      tags: [new Tag(id: 'inbox')]
+    @threadB = new Thread
+      tags: [new Tag(id: 'archive')]
+
     @msg1 = new Message
       unread: true
       date: new Date()
       from: [new Contact(name: 'Ben', email: 'ben@example.com')]
       subject: "Hello World"
+      threadId: "A"
     @msg2 = new Message
       unread: true
       date: new Date()
       from: [new Contact(name: 'Mark', email: 'mark@example.com')]
       subject: "Hello World 2"
+      threadId: "A"
+    @msgUnreadButArchived = new Message
+      unread: true
+      date: new Date()
+      from: [new Contact(name: 'Mark', email: 'mark@example.com')]
+      subject: "Hello World 2"
+      threadId: "B"
     @msgRead = new Message
       unread: false
       date: new Date()
       from: [new Contact(name: 'Mark', email: 'mark@example.com')]
       subject: "Hello World Read Already"
+      threadId: "A"
     @msgOld = new Message
       unread: true
       date: new Date(2000,1,1)
       from: [new Contact(name: 'Mark', email: 'mark@example.com')]
       subject: "Hello World Old"
+      threadId: "A"
+
+    spyOn(DatabaseStore, 'find').andCallFake (klass, id) =>
+      return Promise.resolve(@threadA) if id is 'A'
+      return Promise.resolve(@threadB) if id is 'B'
+      return Promise.resolve(null)
+
+    spyOn(window, 'Notification').andCallFake ->
+    spyOn(Promise, 'props').andCallFake (dict) ->
+      dictOut = {}
+      for key, val of dict
+        if val.value?
+          dictOut[key] = val.value()
+        else
+          dictOut[key] = val
+      Promise.resolve(dictOut)
 
   afterEach ->
     Main.deactivate()
 
   it "should create a Notification if there is one unread message", ->
-    Main._onNewMailReceived({message: [@msgRead, @msg1]})
-    expect(window.Notification).toHaveBeenCalled()
-    expect(window.Notification.mostRecentCall.args).toEqual([ 'Ben', { body : 'Hello World', tag : 'unread-update' } ])
+    waitsForPromise =>
+      Main._onNewMailReceived({message: [@msgRead, @msg1]})
+      .then ->
+        expect(window.Notification).toHaveBeenCalled()
+        expect(window.Notification.mostRecentCall.args).toEqual([ 'Ben', { body : 'Hello World', tag : 'unread-update' } ])
 
   it "should create a Notification if there is more than one unread message", ->
-    Main._onNewMailReceived({message: [@msg1, @msg2, @msgRead]})
-    expect(window.Notification).toHaveBeenCalled()
-    expect(window.Notification.mostRecentCall.args).toEqual([ '2 Unread Messages', { tag : 'unread-update' } ])
+    waitsForPromise =>
+      Main._onNewMailReceived({message: [@msg1, @msg2, @msgRead]})
+      .then ->
+        expect(window.Notification).toHaveBeenCalled()
+        expect(window.Notification.mostRecentCall.args).toEqual([ '2 Unread Messages', { tag : 'unread-update' } ])
 
   it "should not create a Notification if there are no new messages", ->
-    Main._onNewMailReceived({message: []})
-    expect(window.Notification).not.toHaveBeenCalled()
-    Main._onNewMailReceived({})
-    expect(window.Notification).not.toHaveBeenCalled()
+    waitsForPromise ->
+      Main._onNewMailReceived({message: []})
+      .then ->
+        expect(window.Notification).not.toHaveBeenCalled()
+
+    waitsForPromise ->
+      Main._onNewMailReceived({})
+      .then ->
+        expect(window.Notification).not.toHaveBeenCalled()
+
+  it "should not notify about unread messages that are outside the inbox", ->
+    waitsForPromise =>
+      Main._onNewMailReceived({message: [@msgUnreadButArchived, @msg1]})
+      .then ->
+        expect(window.Notification).toHaveBeenCalled()
+        expect(window.Notification.mostRecentCall.args).toEqual([ 'Ben', { body : 'Hello World', tag : 'unread-update' } ])
 
   it "should not create a Notification if the new messages are not unread", ->
-    Main._onNewMailReceived({message: [@msgRead]})
-    expect(window.Notification).not.toHaveBeenCalled()
+    waitsForPromise =>
+      Main._onNewMailReceived({message: [@msgRead]})
+      .then ->
+        expect(window.Notification).not.toHaveBeenCalled()
 
   it "should not create a Notification if the new messages are actually old ones", ->
-    Main._onNewMailReceived({message: [@msgOld]})
-    expect(window.Notification).not.toHaveBeenCalled()
+    waitsForPromise =>
+      Main._onNewMailReceived({message: [@msgOld]})
+      .then ->
+        expect(window.Notification).not.toHaveBeenCalled()
 

src/flux/inbox-api.coffee

@@ -146,14 +146,17 @@ class InboxAPI
     # each type of model in the `create` hash, waits for them all to resolve.
     create[type] = @_handleModelResponse(items) for type, items of create
     Promise.props(create).then (created) =>
-      if _.flatten(_.values(created)).length > 0
-        Actions.didPassivelyReceiveNewModels(created)
-
       # Apply all the deltas to modify objects. Gets promises for handling
       # each type of model in the `modify` hash, waits for them all to resolve.
       modify[type] = @_handleModelResponse(items) for type, items of modify
       Promise.props(modify).then (modified) ->
 
+        # Now that we've persisted creates/updates, fire an action
+        # that allows other parts of the app to update based on new models
+        # (notifications)
+        if _.flatten(_.values(created)).length > 0
+          Actions.didPassivelyReceiveNewModels(created)
+
         # Apply all of the deletions
         destroyPromises = destroy.map (delta) ->
           console.log(" - 1 #{delta.object} (#{delta.id})")