[local-sync] Sync draft flag from provider to K2 & exclude drafts from Edgehill

Summary:
We've been syncing drafts messages but not the drafts flag in K2, making
them appear in Edgehill as regular old messages.

This commit makes K2 sync the drafts flag, and also correctly label
folders called "Drafts" with the 'drafts' role.

Because 2-way syncing of drafts is very complex and error-prone since
you need to add new drafts and delete the old ones on every update, and
we reaally don't want to do things like create multiplying draft copies
or accidentally lose a draft someone started composing elsewhere, we
simply exclude messages marked as drafts from being serialized to
Edgehill through the delta stream for now. This removes the confusing
behaviour and also sets a better stage for completing drafts sync later.

Eventually we will also want to add functionality to allow users to
select their drafts folder, but for now this code does the right thing
in many more cases.

While investigating this behaviour, I also discovered a bug we've never
seen before where Gmail isn't applying the \Draft flag to draft
messages, no matter which folder we fetch them from. :-/ This is very
unfortunate and there's no way for us to work around it other than to
fetch messages in the Drafts folder and manually apply the flag locally,
since "drafts" is not a label in Gmail, only another IMAP folder. Brandon
Long from the Gmail team says that this is because they've had
problems with clients which sync drafts, so the Gmail web client and
mobile apps do not set the \Draft flag on drafts. (I don't get how this
solves their problem, but okay.) Let's solve the issue on Gmail if it
comes up by user demand—should be relatively straightforward to
implement, but it adds sync work & complexity.

Fixes T7593

Test Plan: manual

Reviewers: halla, juan

Reviewed By: juan

Maniphest Tasks: T7593

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

packages/isomorphic-core/src/hook-transaction-log.js

@@ -23,6 +23,12 @@ module.exports = (db, sequelize, {only, onCreatedTransaction} = {}) => {
         return;
       }
 
+      if (name === 'message' && dataValues.isDraft) {
+        // TODO: when draft syncing support added, remove this and force
+        // transactions for all drafts in db to sync to app
+        return;
+      }
+
       if ((only && !only.includes(name)) || isTransaction($modelOptions)) {
         return;
       }

packages/local-sync/src/local-api/routes/messages.js

@@ -28,7 +28,7 @@ module.exports = (server) => {
     handler: (request, reply) => {
       request.getAccountDatabase().then((db) => {
         const {Message, Folder, Label, File} = db;
-        const wheres = {};
+        const wheres = {isDraft: false};
         if (request.query.thread_id) {
           wheres.threadId = request.query.thread_id;
         }

packages/local-sync/src/local-sync-worker/sync-tasks/fetch-folder-list.imap.es6

@@ -3,7 +3,6 @@ const SyncTask = require('./sync-task')
 const {localizedCategoryNames} = require('../sync-utils')
 
 
-const BASE_ROLES = ['inbox', 'archive', 'sent', 'trash', 'spam'];
 const GMAIL_ROLES_WITH_FOLDERS = ['all', 'trash', 'spam'];
 
 class FetchFolderListIMAP extends SyncTask {
@@ -16,12 +15,6 @@ class FetchFolderListIMAP extends SyncTask {
     return `FetchFolderListIMAP`;
   }
 
-  _getMissingRoles(categories) {
-    const currentRoles = new Set(categories.map(cat => cat.role));
-    const missingRoles = BASE_ROLES.filter(role => !currentRoles.has(role));
-    return missingRoles;
-  }
-
   _classForMailboxWithRole(role, {Folder, Label}) {
     if (this._provider === Provider.Gmail) {
       return GMAIL_ROLES_WITH_FOLDERS.includes(role) ? Folder : Label;
@@ -29,9 +22,13 @@ class FetchFolderListIMAP extends SyncTask {
     return Folder;
   }
 
+  _detectRole(boxName, box) {
+    return this._roleByAttr(box) || this._roleByName(boxName);
+  }
+
   _roleByName(boxName) {
     for (const role of Object.keys(localizedCategoryNames)) {
-      if (localizedCategoryNames[role].includes(boxName.toLowerCase().trim())) {
+      if (localizedCategoryNames[role].has(boxName.toLowerCase().trim())) {
         return role;
       }
     }
@@ -58,10 +55,10 @@ class FetchFolderListIMAP extends SyncTask {
     return null;
   }
 
-  _updateCategoriesWithBoxes(categories, boxes) {
+  async _updateCategoriesWithBoxes(categories, boxes) {
     const stack = [];
     const created = [];
-    const next = [];
+    const existing = new Set();
 
     Object.keys(boxes).forEach((boxName) => {
       stack.push([[boxName], boxes[boxName]]);
@@ -97,7 +94,7 @@ class FetchFolderListIMAP extends SyncTask {
 
       let category = categories.find((cat) => cat.name === boxName);
       if (!category) {
-        const role = this._roleByAttr(box);
+        const role = this._detectRole(boxName, box);
         const Klass = this._classForMailboxWithRole(role, this._db);
         const {accountId} = this._db
         category = Klass.build({
@@ -107,14 +104,28 @@ class FetchFolderListIMAP extends SyncTask {
           role: role,
         });
         created.push(category);
+      } else if (!category.role) {
+        // if we update the category->role mapping to include more names, we
+        // need to be able to detect newly added roles on existing categories
+        const role = this._roleByName(boxName);
+        if (role) {
+          category.role = role;
+          category.save();
+        }
       }
-      next.push(category);
+      existing.add(category);
     }
 
-    // Todo: decide whether these are renames or deletes
-    const deleted = categories.filter(cat => !next.includes(cat));
+    // TODO: decide whether these are renames or deletes
+    const deleted = categories.filter(cat => !existing.has(cat));
 
-    return {next, created, deleted};
+    for (const category of created) {
+      await category.save()
+    }
+
+    for (const category of deleted) {
+      await category.destroy()
+    }
   }
 
   // This operation is interruptible, see `SyncTask` for info on why we use
@@ -129,31 +140,8 @@ class FetchFolderListIMAP extends SyncTask {
     const folders = yield Folder.findAll()
     const labels = yield Label.findAll()
     const all = [].concat(folders, labels);
-    const {next, created, deleted} = this._updateCategoriesWithBoxes(all, boxes);
+    await this._updateCategoriesWithBoxes(all, boxes);
 
-    const categoriesByRoles = next.reduce((obj, cat) => {
-      const role = this._roleByName(cat.name);
-      if (role in obj) {
-        obj[role].push(cat);
-      } else {
-        obj[role] = [cat];
-      }
-      return obj;
-    }, {})
-
-    this._getMissingRoles(next).forEach((role) => {
-      if (categoriesByRoles[role] && categoriesByRoles[role].length === 1) {
-        categoriesByRoles[role][0].role = role;
-      }
-    })
-
-    for (const category of created) {
-      await category.save()
-    }
-
-    for (const category of deleted) {
-      await category.destroy()
-    }
     console.log(`🔚  Fetching folder list done`)
   }
 }

packages/local-sync/src/local-sync-worker/sync-utils.js

@@ -7,7 +7,7 @@ module.exports = {
   // below to test for them.
   // Make sure these are lower case! (for comparison purposes)
   localizedCategoryNames: {
-    trash: [
+    trash: new Set([
       'gel\xc3\xb6scht', 'papierkorb',
       '\xd0\x9a\xd0\xbe\xd1\x80\xd0\xb7\xd0\xb8\xd0\xbd\xd0\xb0',
       '[imap]/trash', 'papelera', 'borradores',
@@ -16,24 +16,27 @@ module.exports = {
       '\xd0\xa1\xd0\xbc\xd1\x96\xd1\x82\xd1\x82\xd1\x8f',
       'papierkorb/trash', 'gel\xc3\xb6schte elemente',
       'deleted messages', '[gmail]/trash', 'inbox/trash', 'trash',
-      'mail/trash', 'inbox.trash'],
-    spam: [
+      'mail/trash', 'inbox.trash']),
+    spam: new Set([
       'roskaposti', 'inbox.spam', 'inbox.spam', 'skr\xc3\xa4ppost',
       'spamverdacht', 'spam', 'spam', '[gmail]/spam', '[imap]/spam',
       '\xe5\x9e\x83\xe5\x9c\xbe\xe9\x82\xae\xe4\xbb\xb6', 'junk',
-      'junk mail', 'junk e-mail'],
-    inbox: [
+      'junk mail', 'junk e-mail']),
+    inbox: new Set([
       'inbox',
-    ],
-    sent: [
+    ]),
+    sent: new Set([
       'postausgang', 'inbox.gesendet', '[gmail]/sent mail',
       '\xeb\xb3\xb4\xeb\x82\xbc\xed\x8e\xb8\xec\xa7\x80\xed\x95\xa8',
       'elementos enviados', 'sent', 'sent items', 'sent messages',
       'inbox.papierkorb', 'odeslan\xc3\xa9', 'mail/sent-mail',
       'ko\xc5\xa1', 'outbox', 'outbox', 'inbox.sentmail', 'gesendet',
-      'ko\xc5\xa1/sent items', 'gesendete elemente'],
-    archive: [
+      'ko\xc5\xa1/sent items', 'gesendete elemente']),
+    archive: new Set([
       'archive',
-    ],
+    ]),
+    drafts: new Set([
+      'drafts', 'draft', 'brouillons',
+    ]),
   },
 }

packages/local-sync/src/models/message.js

@@ -72,6 +72,8 @@ module.exports = (sequelize, Sequelize) => {
       {fields: ['folderId']},
       {fields: ['threadId']},
       {fields: ['gMsgId']}, // Use in `searchThreads`
+      // TODO: when we add 2-way draft syncing, we're going to need this index
+      // {fields: ['isDraft']},
       {fields: ['folderImapUID']}, // Use in `searchThreads`
     ],
     classMethods: {
@@ -190,6 +192,7 @@ module.exports = (sequelize, Sequelize) => {
           id: this.id,
           account_id: this.accountId,
           object: this.isDraft ? 'draft' : 'message',
+          draft: this.isDraft,
           body: this.body,
           subject: this.subject,
           snippet: this.snippet,

packages/local-sync/src/models/thread.js

@@ -138,9 +138,6 @@ module.exports = (sequelize, Sequelize) => {
         const participantEmails = new Set(this.participants.map(p => p.email));
 
         for (const message of _messages) {
-          if (message.isDraft) {
-            continue;
-          }
           if (!(message.labels instanceof Array)) {
             throw new Error("Expected message.labels to be an inflated array.");
           }

packages/local-sync/src/shared/message-factory.js

@@ -256,6 +256,12 @@ async function parseFromImap(imapMessage, desiredParts, {db, accountId, folder})
     snippet: null,
     unread: !attributes.flags.includes('\\Seen'),
     starred: attributes.flags.includes('\\Flagged'),
+    // We limit drafts to the drafts and all mail folders because some clients
+    // may send messages and improperly leave the draft flag set, and also
+    // because we want to exclude drafts moved to the trash from the drafts view
+    // see https://github.com/nylas/cloud-core/commit/1433921a166ddcba7c269158d65febb7928767d8
+    // & associated phabricator bug https://phab.nylas.com/T5696
+    isDraft: attributes.flags.includes('\\Draft') && ['drafts', 'all'].includes(folder.role),
     // We prefer the date from the message headers because the date is one of
     // the fields we use for generating unique message IDs, and the server
     // INTERNALDATE, `attributes.date`, may differ across accounts for the same