feat(notifications): Initial pass at new mail notifications

Summary:
Eventually, notification stuff should be moved out of InboxAPI into a separate package, and we should have some documented way of watching for "new" things. (Right now it'd be a bit hard to do on the database store...)

Additional fixes:
- AddRemoveTagsTask now optimistically updates the version. Before it would get the new version from the API response. This was bad because it could still cause local changes to be overwritten if you were changing tags really fast.

- AddRemoveTagsTask now takes a threadId, not a thread for consistency with all the rest of our Tasks

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1214
This commit is contained in:
Ben Gotow 2015-02-20 12:19:34 -08:00
parent e8dff2005d
commit c952ea3b12
20 changed files with 253 additions and 134 deletions

View file

@ -14,18 +14,13 @@ AccountSidebarStore = Reflux.createStore
sections: ->
@_sections
unreadCounts: ->
@_unreadCounts
selectedId: ->
@_selectedId
########### PRIVATE ####################################################
_setStoreDefaults: ->
@_sections = []
@_unreadCounts = {}
@_selectedId = null
_registerListeners: ->
@ -47,10 +42,13 @@ AccountSidebarStore = Reflux.createStore
# Sort the main tags so they always appear in a standard order
mainTags = _.sortBy mainTags, (tag) -> mainTagIDs.indexOf(tag.id)
lastSections = @_sections
@_sections = [
{ label: namespace.emailAddress, tags: mainTags }
]
@_populateUnreadCounts()
if _.isEqual(@_sections, lastSections) is false
@_populateUnreadCounts()
@trigger(@)
_populateUnreadCounts: ->
@ -66,10 +64,7 @@ AccountSidebarStore = Reflux.createStore
atom.inbox.makeRequest
method: 'GET'
path: "/n/#{namespace.id}/tags/#{tag.id}"
success: (json) =>
tag = (new Tag).fromJSON(json)
@_unreadCounts[tag.id] = tag.unreadCount
@trigger(@)
returnsModel: true
# Unfortunately, the joins necessary to compute unread counts are expensive.
# Rather than update unread counts every time threads change in the database,

View file

@ -4,10 +4,9 @@ React = require 'react'
module.exports =
AccountSidebarTagItem = React.createClass
render: ->
unread = if @props.unreadCount > 0 then @props.unreadCount else ""
className = "item item-tag" + if @props.select then " selected" else ""
<div className={className} onClick={@_onClick} id={@props.tag.id}>
<div className="unread"> {unread}</div>
<div className="unread"> {@props.tag.unreadCount}</div>
<span className="name"> {@props.tag.name}</span>
</div>

View file

@ -35,7 +35,6 @@ AccountSidebar = React.createClass
<SidebarTagItem
key={tag.id}
tag={tag}
unreadCount={@state.unreadCounts[tag.id]}
select={tag?.id == @state?.selected}/>
_onStoreChange: ->
@ -44,7 +43,6 @@ AccountSidebar = React.createClass
Actions.selectTagId("inbox")
_getStateFromStores: ->
unreadCounts: SidebarStore.unreadCounts()
sections: SidebarStore.sections()
selected: SidebarStore.selectedId()

View file

@ -17,7 +17,7 @@ PackageMain = proxyquire "../lib/main",
releaseVersion: stubUpdaterReleaseVersion
getState: -> stubUpdaterState
fdescribe "NotificationUpdateAvailable", ->
describe "NotificationUpdateAvailable", ->
beforeEach ->
stubUpdaterState = 'idle'
stubUpdaterReleaseVersion = undefined

View file

@ -1,7 +1,7 @@
module.exports =
activate: ->
@store = require "./app-unread-badge-store"
@store = require "./unread-badge-store"
deactivate: ->

View file

@ -1,6 +1,6 @@
Reflux = require 'reflux'
_ = require 'underscore-plus'
{DatabaseStore, NamespaceStore, Actions, Thread} = require 'inbox-exports'
{DatabaseStore, NamespaceStore, Actions, Tag} = require 'inbox-exports'
remote = require 'remote'
app = remote.require 'app'
@ -14,23 +14,17 @@ AppUnreadBadgeStore = Reflux.createStore
@_onDataChanged()
_onDataChanged: (change) ->
return if change && change.objectClass != Thread.name
return if change && change.objectClass != Tag.name
return app.dock?.setBadge?("") unless NamespaceStore.current()
@_updateBadgeDebounced()
@_updateBadge()
_updateBadge: ->
DatabaseStore.count(Thread, [
Thread.attributes.namespaceId.equal(NamespaceStore.current()?.id),
Thread.attributes.unread.equal(true),
Thread.attributes.tags.contains('inbox')
]).then (count) ->
DatabaseStore.find(Tag, 'inbox').then (inbox) ->
return unless inbox
count = inbox.unreadCount
if count > 999
app.dock?.setBadge?("\u221E")
else if count > 0
app.dock?.setBadge?("#{count}")
else
app.dock?.setBadge?("")
_updateBadgeDebounced: _.debounce ->
@_updateBadge()
, 750

View file

@ -1,5 +1,5 @@
{
"name": "app-unread-badge",
"name": "unread-badge",
"version": "0.1.0",
"main": "./lib/main",
"description": "Updates the Mac's app icon to display unread count",

View file

@ -0,0 +1,33 @@
_ = require 'underscore-plus'
{Actions} = require 'inbox-exports'
module.exports =
activate: ->
@unlisteners = []
@unlisteners.push Actions.didPassivelyReceiveNewModels.listen(@_onNewMailReceived, @)
deactivate: ->
fn() for fn in @unlisteners
serialize: ->
_onNewMailReceived: (models) ->
# Display a notification if we've received new messages
newUnreadMessages = _.filter (models['message'] ? []), (msg) ->
msg.unread is true
if newUnreadMessages.length is 1
msg = newUnreadMessages.pop()
notif = new Notification(msg.from[0].displayName(), {
body: msg.subject
tag: 'unread-update'
})
notif.onclick = -> Actions.selectThreadId(msg.threadId)
if newUnreadMessages.length > 1
new Notification("#{newUnreadMessages.length} Unread Messages", {
tag: 'unread-update'
})
if newUnreadMessages.length > 0
atom.playSound('new_mail.ogg')

View file

@ -0,0 +1,10 @@
{
"name": "unread-notifications",
"version": "0.1.0",
"main": "./lib/main",
"description": "Fires notifications when new mail is received",
"license": "Proprietary",
"private": true,
"dependencies": {
}
}

View file

@ -0,0 +1,41 @@
_ = require 'underscore-plus'
Contact = require '../../../src/flux/models/contact'
Message = require '../../../src/flux/models/message'
Main = require '../lib/main'
describe "UnreadNotifications", ->
beforeEach ->
spyOn(window, 'Notification').andCallFake ->
@msg1 = new Message
unread: true
from: [new Contact(name: 'Ben', email: 'ben@example.com')]
subject: "Hello World"
@msg2 = new Message
unread: true
from: [new Contact(name: 'Mark', email: 'mark@example.com')]
subject: "Hello World 2"
@msgRead = new Message
unread: false
from: [new Contact(name: 'Mark', email: 'mark@example.com')]
subject: "Hello World Read Already"
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' } ])
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' } ])
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()
it "should not create a Notification if the new messages are not unread", ->
Main._onNewMailReceived({message: [@msgRead]})
expect(window.Notification).not.toHaveBeenCalled()

View file

@ -5,29 +5,45 @@ Thread = require '../../src/flux/models/thread'
Tag = require '../../src/flux/models/tag'
_ = require 'underscore-plus'
testThread = null
describe "AddRemoveTagsTask", ->
beforeEach ->
spyOn(DatabaseStore, 'persistModel').andCallFake -> Promise.resolve()
spyOn(DatabaseStore, 'find').andCallFake (klass, id) =>
new Promise (resolve, reject) => resolve(new Tag({id: id, name: id}))
if klass is Thread
Promise.resolve(testThread)
else if klass is Tag
Promise.resolve(new Tag({id: id, name: id}))
else
throw new Error("Not stubbed!")
describe "rollbackLocal", ->
it "should perform the opposite changes to the thread", ->
thread = new Thread
testThread = new Thread
id: 'thread-id'
tags: [
new Tag({name: 'archive', id: 'archive'})
]
task = new AddRemoveTagsTask(thread, ['archive'], ['inbox'])
task = new AddRemoveTagsTask(testThread.id, ['archive'], ['inbox'])
task.rollbackLocal()
waitsFor ->
DatabaseStore.persistModel.callCount > 0
runs ->
expect(thread.tagIds()).toEqual(['inbox'])
expect(testThread.tagIds()).toEqual(['inbox'])
describe "performLocal", ->
it "should throw an exception if task has not been given a thread", ->
badTasks = [new AddRemoveTagsTask(), new AddRemoveTagsTask(new Object)]
goodTasks = [new AddRemoveTagsTask(new Thread)]
beforeEach ->
testThread = new Thread
id: 'thread-id'
tags: [
new Tag({name: 'inbox', id: 'inbox'}),
new Tag({name: 'unread', id: 'unread'})
]
it "should throw an exception if task has not been given a thread ID", ->
badTasks = [new AddRemoveTagsTask()]
goodTasks = [new AddRemoveTagsTask(testThread.id)]
caught = []
succeeded = []
@ -37,59 +53,66 @@ describe "AddRemoveTagsTask", ->
.then -> succeeded.push(task)
.catch (err) -> caught.push(task)
waitsFor ->
succeeded.length + caught.length == 3
succeeded.length + caught.length == 2
runs ->
expect(caught).toEqual(badTasks)
expect(succeeded).toEqual(goodTasks)
it "should trigger a persist action to commit changes to the thread to the local store", ->
task = new AddRemoveTagsTask(new Thread(), [], [])
task = new AddRemoveTagsTask(testThread.id, [], [])
task.performLocal()
expect(DatabaseStore.persistModel).toHaveBeenCalled()
it "should remove the tag IDs passed to the task", ->
thread = new Thread
tags: [
new Tag({name: 'inbox', id: 'inbox'}),
new Tag({name: 'unread', id: 'unread'})
]
task = new AddRemoveTagsTask(thread, [], ['unread'])
task.performLocal().catch (err) -> console.log(err.stack)
expect(thread.tagIds().length).toBe(1)
expect(thread.tagIds()[0]).toBe('inbox')
it "should add the tag IDs passed to the task", ->
thread = new Thread
tags: [
new Tag({name: 'inbox', id: 'inbox'})
]
task = new AddRemoveTagsTask(thread, ['archive'], ['inbox'])
task.performLocal().catch (err) -> console.log(err.stack)
waitsFor ->
DatabaseStore.persistModel.callCount > 0
runs ->
expect(thread.tagIds().length).toBe(1)
expect(thread.tagIds()[0]).toBe('archive')
expect(DatabaseStore.persistModel).toHaveBeenCalled()
it "should remove the tag IDs passed to the task", ->
task = new AddRemoveTagsTask(testThread.id, [], ['unread'])
task.performLocal()
waitsFor ->
DatabaseStore.persistModel.callCount > 0
runs ->
expect(testThread.tagIds().length).toBe(1)
expect(testThread.tagIds()[0]).toBe('inbox')
it "should add the tag IDs passed to the task", ->
testThread = new Thread
id: 'thread-id'
tags: [
new Tag({name: 'inbox', id: 'inbox'})
]
task = new AddRemoveTagsTask(testThread.id, ['archive'], ['inbox'])
task.performLocal()
waitsFor ->
DatabaseStore.persistModel.callCount > 0
runs ->
expect(testThread.tagIds().length).toBe(1)
expect(testThread.tagIds()[0]).toBe('archive')
describe "performRemote", ->
beforeEach ->
@thread = new Thread
testThread = new Thread
id: '1233123AEDF1'
namespaceId: 'A12ADE'
@task = new AddRemoveTagsTask(@thread, ['archive'], ['inbox'])
@task = new AddRemoveTagsTask(testThread.id, ['archive'], ['inbox'])
it "should start an API request with the Draft JSON", ->
spyOn(atom.inbox, 'makeRequest')
@task.performRemote().catch (err) -> console.log(err.stack)
options = atom.inbox.makeRequest.mostRecentCall.args[0]
expect(options.path).toBe("/n/#{@thread.namespaceId}/threads/#{@thread.id}")
expect(options.method).toBe('PUT')
expect(options.body.add_tags[0]).toBe('archive')
expect(options.body.remove_tags[0]).toBe('inbox')
@task.performLocal()
waitsFor ->
DatabaseStore.persistModel.callCount > 0
runs ->
@task.performRemote()
options = atom.inbox.makeRequest.mostRecentCall.args[0]
expect(options.path).toBe("/n/#{testThread.namespaceId}/threads/#{testThread.id}")
expect(options.method).toBe('PUT')
expect(options.body.add_tags[0]).toBe('archive')
expect(options.body.remove_tags[0]).toBe('inbox')
it "should pass returnsModel:true so that the draft is saved to the data store when returned", ->
spyOn(atom.inbox, 'makeRequest')
@task.performRemote().catch (err) -> console.log(err.stack)
@task.performLocal()
@task.performRemote()
options = atom.inbox.makeRequest.mostRecentCall.args[0]
expect(options.returnsModel).toBe(true)

View file

@ -650,6 +650,13 @@ class Atom extends Model
shell.beep() if @config.get('core.audioBeep')
@emitter.emit 'did-beep'
playSound: (filename) ->
{resourcePath} = atom.getLoadSettings()
a = new Audio()
a.src = path.join(resourcePath, 'static', 'sounds', filename)
a.autoplay = true
a.play()
# Essential: A flexible way to open a dialog akin to an alert dialog.
#
# ## Examples

View file

@ -7,6 +7,7 @@ Reflux = require 'reflux'
globalActions = [
"didSwapModel",
"didPassivelyReceiveNewModels",
"logout",
# File Actions

View file

@ -43,8 +43,6 @@ class InboxAPI
path: "/n"
returnsModel: true
console.log(@_streamingConnections.length)
_onNamespacesChanged: ->
return unless atom.state.mode is 'editor'
return if atom.getLoadSettings().isSpec
@ -68,7 +66,6 @@ class InboxAPI
_streamingConnectionForNamespace: (namespace) =>
connection = _.find @_streamingConnections, (c) ->
c.namespaceId() is namespace.id
console.log('Found existing connection') if connection
return connection if connection
connection = new InboxLongConnection(@, namespace.id)
@ -83,7 +80,7 @@ class InboxAPI
Actions.restartTaskQueue()
connection.onDeltas (deltas) =>
@_handleDeltas(namespace.id, deltas)
@_handleDeltas(deltas)
Actions.restartTaskQueue()
connection.start()
@ -124,54 +121,67 @@ class InboxAPI
@_handleModelResponse(body) if options.returnsModel
options.success(body) if options.success
_handleDeltas: (namespaceId, deltas) ->
console.log("Processing deltas:")
_handleDeltas: (deltas) ->
console.log("Processing Deltas")
# Group deltas by object type so we can mutate our local cache efficiently
deltasByObject = {}
deltasDeletions = []
# Group deltas by object type so we can mutate the cache efficiently
create = {}
modify = {}
destroy = []
for delta in deltas
if delta.event is 'delete'
deltasDeletions.push(delta)
else if delta.event is 'create' or delta.event is 'modify'
deltasByObject[delta.object] ||= []
deltasByObject[delta.object].push(delta.attributes)
if delta.event is 'create'
create[delta.object] ||= []
create[delta.object].push(delta.attributes)
else if delta.event is 'modify'
modify[delta.object] ||= []
modify[delta.object].push(delta.attributes)
else if delta.event is 'delete'
destroy.push(delta)
# Remove events and contacts - we don't apply deltas to them
delete deltasByObject['contact']
delete deltasByObject['event']
# Apply all the deltas to create objects. Gets promises for handling
# 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 create / modfiy events by class
for object, items of deltasByObject
console.log(" + #{items.length} #{object}")
@_handleModelResponse(items)
# 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) ->
# Apply all of the deletions
for delta in deltasDeletions
console.log(" - 1 #{delta.object} (#{delta.id})")
klass = modelClassMap()[delta.object]
return unless klass
DatabaseStore.find(klass, delta.id).then (model) ->
DatabaseStore.unpersistModel(model) if model
# Apply all of the deletions
for delta in destroy
console.log(" - 1 #{delta.object} (#{delta.id})")
klass = modelClassMap()[delta.object]
return unless klass
DatabaseStore.find(klass, delta.id).then (model) ->
DatabaseStore.unpersistModel(model) if model
_defaultErrorCallback: (apiError) ->
console.error("Unhandled Inbox API Error:", apiError.message, apiError)
_handleModelResponse: (json) ->
throw new Error("handleModelResponse with no JSON provided") unless json
json = [json] unless json instanceof Array
async.filter json
, (json, callback) =>
@_shouldAcceptModel(json.object, json).then((-> callback(true)), (-> callback(false)))
, (json) ->
# Save changes to the database, which will generate actions
# that our views are observing.
objects = []
for objectJSON in json
objects.push(modelFromJSON(objectJSON))
DatabaseStore.persistModels(objects) if objects.length > 0
new Promise (resolve, reject) =>
reject(new Error("handleModelResponse with no JSON provided")) unless json
json = [json] unless json instanceof Array
async.filter json
, (item, filterCallback) =>
@_shouldAcceptModel(item.object, item).then ->
filterCallback(true)
.catch (e) ->
filterCallback(false)
, (json) ->
# Save changes to the database, which will generate actions
# that our views are observing.
objects = []
for objectJSON in json
objects.push(modelFromJSON(objectJSON))
if objects.length > 0
DatabaseStore.persistModels(objects)
resolve(objects)
_shouldAcceptModel: (classname, model = null) ->
return Promise.resolve() unless model

View file

@ -57,7 +57,7 @@ class Thread extends Model
markAsRead: ->
MarkThreadReadTask = require '../tasks/mark-thread-read'
task = new MarkThreadReadTask(@)
task = new MarkThreadReadTask(@id)
Actions.queueTask(task)
star: ->
@ -82,5 +82,5 @@ class Thread extends Model
addRemoveTags: (tagIdsToAdd, tagIdsToRemove) ->
# start web change, which will dispatch more actions
AddRemoveTagsTask = require '../tasks/add-remove-tags'
task = new AddRemoveTagsTask(@, tagIdsToAdd, tagIdsToRemove)
task = new AddRemoveTagsTask(@id, tagIdsToAdd, tagIdsToRemove)
Actions.queueTask(task)

View file

@ -8,7 +8,7 @@ async = require 'async'
class AddRemoveTagsTask extends Task
constructor: (@thread, @tagIdsToAdd = [], @tagIdsToRemove = []) ->
constructor: (@threadId, @tagIdsToAdd = [], @tagIdsToRemove = []) ->
@
rollbackLocal: ->
@ -16,39 +16,46 @@ class AddRemoveTagsTask extends Task
a = @tagIdsToAdd
@tagIdsToAdd = @tagIdsToRemove
@tagIdsToRemove = a
@performLocal()
@performLocal(-1)
performLocal: ->
performLocal: (versionIncrement = 1) ->
new Promise (resolve, reject) =>
unless @thread instanceof Thread
return reject(new Error("Attempt to call AddRemoveTagsTask.performLocal without Thread"))
return reject(new Error("Attempt to call AddRemoveTagsTask.performLocal without Thread")) unless @threadId
# remove tags in the remove list
@thread.tags = _.filter @thread.tags, (tag) =>
@tagIdsToRemove.indexOf(tag.id) == -1
DatabaseStore.find(Thread, @threadId).then (thread) =>
return resolve() unless thread
# add tags in the add list
async.map @tagIdsToAdd, (id, callback) ->
DatabaseStore.find(Tag, id).then (tag) ->
callback(null, tag)
, (err, tags) =>
for tag in tags
@thread.tags.push(tag) if tag
DatabaseStore.persistModel(@thread).then(resolve)
@namespaceId = thread.namespaceId
# increment the thread version number
thread.version += versionIncrement
# remove tags in the remove list
thread.tags = _.filter thread.tags, (tag) =>
@tagIdsToRemove.indexOf(tag.id) == -1
# add tags in the add list
async.map @tagIdsToAdd, (id, callback) ->
DatabaseStore.find(Tag, id).then (tag) ->
callback(null, tag)
, (err, tags) ->
for tag in tags
thread.tags.push(tag) if tag
DatabaseStore.persistModel(thread).then(resolve)
performRemote: ->
new Promise (resolve, reject) =>
# queue the operation to the server
atom.inbox.makeRequest {
path: "/n/#{@thread.namespaceId}/threads/#{@thread.id}"
path: "/n/#{@namespaceId}/threads/#{@threadId}"
method: 'PUT'
body: {
add_tags: @tagIdsToAdd,
remove_tags: @tagIdsToRemove
}
returnsModel: true
success: -> resolve()
success: resolve
error: (apiError) =>
if "archive" in @tagIdsToAdd
Actions.postNotification({message: "Failed to archive thread: '#{@thread.subject}'", type: 'error'})

View file

@ -10,8 +10,8 @@ _ = require 'underscore-plus'
# thread is marked as read.
class MarkThreadReadTask extends AddRemoveTagsTask
constructor: (@thread) ->
super(@thread, [], ['unread'])
constructor: (@threadId) ->
super(@threadId, [], ['unread'])
@
performLocal: ->
@ -22,7 +22,7 @@ class MarkThreadReadTask extends AddRemoveTagsTask
# tag change is executed on the server, all of the messages in the thread
# will be marked as read. It looks bad to temporarily have unread messages
# in a read thread...
DatabaseStore.findAll(Message, threadId: @thread.id).then (messages) ->
DatabaseStore.findAll(Message, threadId: @threadId).then (messages) ->
messages = _.filter messages, (message) -> message.unread
if messages.length > 0
for message in messages

View file

@ -41,8 +41,9 @@ class SendDraftTask extends Task
version: draft.version
returnsModel: true
success: ->
atom.playSound('mail_sent.ogg')
Actions.postNotification({message: "Sent!", type: 'success'})
DatabaseStore.unpersistModel(draft).then(resolve)
error: reject
module.exports = SendDraftTask

BIN
static/sounds/mail_sent.ogg Normal file

Binary file not shown.

BIN
static/sounds/new_mail.ogg Normal file

Binary file not shown.