Ensure v1 models reach the client, fix notification issues #160, #154

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

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({

mailsync

@@ -1 +1 @@
-Subproject commit dd26a2758c38019b39ca30a68cd36cde721a918a
+Subproject commit 8841ba0989bcc6179a88f166a24a0634e22a41c6