diff --git a/app/internal_packages/unread-notifications/lib/main.es6 b/app/internal_packages/unread-notifications/lib/main.es6 index 44e76f292..355de3978 100644 --- a/app/internal_packages/unread-notifications/lib/main.es6 +++ b/app/internal_packages/unread-notifications/lib/main.es6 @@ -2,12 +2,15 @@ import _ from 'underscore'; import { Thread, Actions, + AccountStore, Message, SoundRegistry, NativeNotifications, DatabaseStore, } from 'mailspring-exports'; +const WAIT_FOR_CHANGES_DELAY = 400; + export class Notifier { constructor() { this.activationTime = Date.now(); @@ -26,38 +29,79 @@ export class Notifier { // async for testing async _onDatabaseChanged({ objectClass, objects }) { + if (AppEnv.config.get('core.notifications.enabled') === false) { + return; + } + if (objectClass === Thread.name) { - // Ensure notifications are dismissed when the user reads a thread - objects.forEach(({ id, unread }) => { - if (!unread && this.activeNotifications[id]) { - this.activeNotifications[id].forEach(n => n.close()); - delete this.activeNotifications[id]; - } - }); + return this._onThreadsChanged(objects); } if (objectClass === Message.name) { - if (AppEnv.config.get('core.notifications.enabled') === false) { - return; - } - const newUnread = objects.filter(msg => { - // ensure the message is unread - if (msg.unread !== true) return false; - // ensure the message was just saved (eg: this is not a modification) - if (msg.version !== 1) return false; - // ensure the message was received after the app launched (eg: not syncing an old email) - if (!msg.date || msg.date.valueOf() < this.activationTime) return false; - // ensure the message is from someone else - if (msg.isFromMe()) return false; - return true; - }); - - if (newUnread.length > 0) { - await this._onNewMessagesReceived(newUnread); - } + return this._onMessagesChanged(objects); } } + // async for testing + async _onMessagesChanged(msgs) { + const notifworthy = {}; + + for (const msg of msgs) { + // ensure the message is unread + if (msg.unread !== true) continue; + // ensure the message was just created (eg: this is not a modification) + if (msg.version !== 1) continue; + // ensure the message was received after the app launched (eg: not syncing an old email) + if (!msg.date || msg.date.valueOf() < this.activationTime) continue; + // ensure the message is not a loopback + const account = msg.from[0] && AccountStore.accountForEmail(msg.from[0].email); + if (msg.accountId === (account || {}).id) continue; + + notifworthy[msg.id] = msg; + } + + if (Object.keys(notifworthy).length === 0) { + return; + } + + if (!AppEnv.inSpecMode()) { + await new Promise(resolve => { + // wait a couple hundred milliseconds and collect any updates to these + // new messages. This gets us message bodies, messages impacted by mail rules, etc. + // while ensuring notifications are never too delayed. + const unlisten = DatabaseStore.listen(({ objectClass, objects }) => { + if (objectClass !== Message.name) { + return; + } + for (const msg of objects) { + if (notifworthy[msg.id]) { + notifworthy[msg.id] = msg; + if (msg.unread === false) { + delete notifworthy[msg.id]; + } + } + } + }); + setTimeout(() => { + unlisten(); + resolve(); + }, WAIT_FOR_CHANGES_DELAY); + }); + } + + await this._onNewMessagesReceived(Object.values(notifworthy)); + } + + _onThreadsChanged(threads) { + // Ensure notifications are dismissed when the user reads a thread + threads.forEach(({ id, unread }) => { + if (!unread && this.activeNotifications[id]) { + this.activeNotifications[id].forEach(n => n.close()); + delete this.activeNotifications[id]; + } + }); + } + _notifyAll() { NativeNotifications.displayNotification({ title: `${this.unnotifiedQueue.length} Unread Messages`, @@ -123,6 +167,10 @@ export class Notifier { } _onNewMessagesReceived(newMessages) { + if (newMessages.length === 0) { + return Promise.resolve(); + } + // 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 diff --git a/app/internal_packages/unread-notifications/specs/main-spec.es6 b/app/internal_packages/unread-notifications/specs/main-spec.es6 index 8bc16f994..ce1d1c758 100644 --- a/app/internal_packages/unread-notifications/specs/main-spec.es6 +++ b/app/internal_packages/unread-notifications/specs/main-spec.es6 @@ -25,96 +25,130 @@ describe('UnreadNotifications', function UnreadNotifications() { this.threadA = new Thread({ id: 'A', + accountId: 'a', folders: [inbox], }); this.threadB = new Thread({ id: 'B', + accountId: 'a', folders: [archive], }); this.msg1 = new Message({ + id: '1', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Ben', email: 'benthis.example.com' })], subject: 'Hello World', threadId: 'A', version: 1, }); this.msgNoSender = new Message({ + id: 'no', unread: true, date: new Date(), from: [], + accountId: 'a', subject: 'Hello World', threadId: 'A', version: 1, }); this.msg2 = new Message({ + id: '2', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Mark', email: 'markthis.example.com' })], subject: 'Hello World 2', threadId: 'A', version: 1, }); this.msg3 = new Message({ + id: '3', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Ben', email: 'benthis.example.com' })], subject: 'Hello World 3', threadId: 'A', version: 1, }); this.msg4 = new Message({ + id: '4', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Ben', email: 'benthis.example.com' })], subject: 'Hello World 4', threadId: 'A', version: 1, }); this.msg5 = new Message({ + id: '5', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Ben', email: 'benthis.example.com' })], subject: 'Hello World 5', threadId: 'A', version: 1, }); this.msgUnreadButArchived = new Message({ + id: 'uba', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Mark', email: 'markthis.example.com' })], subject: 'Hello World 2', threadId: 'B', version: 1, }); this.msgRead = new Message({ + id: 'read', unread: false, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Mark', email: 'markthis.example.com' })], subject: 'Hello World Read Already', threadId: 'A', version: 1, }); this.msgOld = new Message({ + id: 'old', unread: true, date: new Date(2000, 1, 1), + accountId: 'a', from: [new Contact({ name: 'Mark', email: 'markthis.example.com' })], subject: 'Hello World Old', threadId: 'A', version: 1, }); - this.msgFromMe = new Message({ + this.msgFromMeSameAccount = new Message({ + id: 'from-me', unread: true, date: new Date(), + accountId: account.id, + from: [account.me()], + subject: 'A Sent Mail!', + threadId: 'A', + version: 1, + }); + this.msgFromMeDiffAccount = new Message({ + id: 'from-me-diff', + unread: true, + date: new Date(), + accountId: 'other', from: [account.me()], subject: 'A Sent Mail!', threadId: 'A', version: 1, }); this.msgHigherVersion = new Message({ + id: 'hv', unread: true, date: new Date(), + accountId: 'a', from: [new Contact({ name: 'Ben', email: 'benthis.example.com' })], subject: 'Hello World', threadId: 'A', @@ -308,16 +342,26 @@ describe('UnreadNotifications', function UnreadNotifications() { }); }); - it('should not create a Notification if the new message is one I sent', () => { + it('should not create a Notification if the new message is one I sent from the same account', () => { waitsForPromise(async () => { await this.notifier._onDatabaseChanged({ objectClass: Message.name, - objects: [this.msgFromMe], + objects: [this.msgFromMeSameAccount], }); expect(NativeNotifications.displayNotification).not.toHaveBeenCalled(); }); }); + it('should xcreate a Notification if the new message is one I sent from a different linked account', () => { + waitsForPromise(async () => { + await this.notifier._onDatabaseChanged({ + objectClass: Message.name, + objects: [this.msgFromMeDiffAccount], + }); + expect(NativeNotifications.displayNotification).toHaveBeenCalled(); + }); + }); + it('clears notifications when a thread is read', () => { waitsForPromise(async () => { await this.notifier._onDatabaseChanged({ diff --git a/mailsync b/mailsync index dd26a2758..8841ba098 160000 --- a/mailsync +++ b/mailsync @@ -1 +1 @@ -Subproject commit dd26a2758c38019b39ca30a68cd36cde721a918a +Subproject commit 8841ba0989bcc6179a88f166a24a0634e22a41c6