[local-sync] Ensure messages always have a uid after syncback tasks

Summary:
Syncback tasks that move messages to a different folder used to leave
those messages without a uid because it was unknown at the moment, and
it would only be discovered later on in the sync loop.
This had the unwanted effect of not allowing you to perform more than 1
syncback action on the same thread back to back (e.g. undoing an archive, or
sending and archiving immediately)

This commit makes it so that `SetThreadFolderAndLabels` and
`MoveThreadToFolder` runs a new sync task to fetch the new uids for the
moved messages.

We do this via a new sync task, `FetchNewMessagesInFolder` which /only/
fetches new messages in a folder (no fetching old messages or attribute
updates). This is a first step to cleaning up the gigantic
`FetchMessagesInFolder` task into smaller parts-- but that will come in
a separate diff. For now we want to fix the immediate problem.

See D3788 and D3789 for more details

Test Plan:
manually move threads around, undo moving threads, reply on a thread
and immediately archive

Reviewers: khamidou, mark, spang, halla, evan

Reviewed By: evan

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

packages/local-sync/src/local-sync-worker/sync-task-factory.js

@@ -8,6 +8,8 @@ class SyncTaskFactory {
         Task = require('./sync-tasks/fetch-folder-list.imap'); break;
       case "FetchMessagesInFolder":
         Task = require('./sync-tasks/fetch-messages-in-folder.imap'); break;
+      case "FetchNewMessagesInFolder":
+        Task = require('./sync-tasks/fetch-new-messages-in-folder.imap'); break;
       default:
         throw new Error(`Task type not defined in syncback--factory: ${taskName}`)
     }

packages/local-sync/src/local-sync-worker/sync-tasks/fetch-new-messages-in-folder.imap.es6

@@ -0,0 +1,37 @@
+const FetchMessagesInFolderIMAP = require('./fetch-messages-in-folder.imap')
+
+
+// TODO Eventually make FetchMessagesInFolderIMAP use this class and split it up
+// into smaller parts (not a fan of the multi level inheritance tree)
+
+/**
+ * This sync task will only fetch /new/ messages in a folder
+ */
+class FetchNewMessagesInFolderIMAP extends FetchMessagesInFolderIMAP {
+
+  description() {
+    return `FetchNewMessagesInFolderIMAP (${this._folder.name} - ${this._folder.id})`;
+  }
+
+  async * runTask(db, imap) {
+    console.log(`🔜 📂  🆕  Looking for new messages in ${this._folder.name}`)
+    this._db = db;
+    this._imap = imap;
+
+    this._box = await this._imap.openBox(this._folder.name)
+
+    if (this._shouldFetchMessages(this._box)) {
+      const boxUidnext = this._box.uidnext
+      const {syncState: {fetchedmax}} = this._folder
+      yield this._fetchAndProcessMessages({min: fetchedmax, max: boxUidnext});
+    } else {
+      console.log(`🔚 📂 ${this._folder.name} has no new messages - skipping fetch messages`)
+    }
+    console.log(`🔚 📂 ${this._folder.name} done`)
+  }
+}
+
+module.exports = FetchNewMessagesInFolderIMAP;
+
+
+// boo hoo

packages/local-sync/src/local-sync-worker/syncback-tasks/move-thread-to-folder.imap.js

@@ -1,6 +1,7 @@
 const {Errors: {APIError}} = require('isomorphic-core')
 const SyncbackTask = require('./syncback-task')
 const IMAPHelpers = require('../imap-helpers')
+const SyncTaskFactory = require('../sync-task-factory');
 
 class MoveThreadToFolderIMAP extends SyncbackTask {
   description() {
@@ -12,7 +13,7 @@ class MoveThreadToFolderIMAP extends SyncbackTask {
   }
 
   async run(db, imap) {
-    const {sequelize, Thread, Folder} = db
+    const {Thread, Folder} = db
     const threadId = this.syncbackRequestObject().props.threadId
     const targetFolderId = this.syncbackRequestObject().props.folderId
     if (!threadId) {
@@ -46,14 +47,15 @@ class MoveThreadToFolderIMAP extends SyncbackTask {
       },
     })
 
-    // If IMAP succeeds, save the model updates
-    await sequelize.transaction(async (transaction) => {
-      await Promise.all(threadMessages.map(async (m) => {
-        await m.update({folderImapUID: null}, {transaction})
-        await m.setFolder(targetFolder, {transaction})
-      }))
-      await thread.setFolders([targetFolder], {transaction})
+    // If IMAP succeeds, fetch any new messages in the target folder which
+    // should include the messages we just moved there
+    // The sync operation will save the changes to the database.
+    // TODO add transaction
+    const syncOperation = SyncTaskFactory.create('FetchNewMessagesInFolder', {
+      account: this._account,
+      folder: targetFolder,
     })
+    await syncOperation.run(db, imap)
   }
 }
 module.exports = MoveThreadToFolderIMAP

packages/local-sync/src/local-sync-worker/syncback-tasks/set-thread-folder-and-labels.imap.js

@@ -1,6 +1,8 @@
 const {Errors: {APIError}} = require('isomorphic-core')
 const SyncbackTask = require('./syncback-task')
 const IMAPHelpers = require('../imap-helpers')
+const SyncTaskFactory = require('../sync-task-factory');
+
 
 class SetThreadFolderAndLabelsIMAP extends SyncbackTask {
   description() {
@@ -13,7 +15,7 @@ class SetThreadFolderAndLabelsIMAP extends SyncbackTask {
 
 
   async run(db, imap) {
-    const {sequelize, Thread, Folder} = db
+    const {Thread, Folder} = db
     const threadId = this.syncbackRequestObject().props.threadId
     const labelIds = this.syncbackRequestObject().props.labelIds
     const targetFolderId = this.syncbackRequestObject().props.folderId
@@ -50,16 +52,15 @@ class SetThreadFolderAndLabelsIMAP extends SyncbackTask {
       },
     })
 
-    // If IMAP succeeds, save the model updates
-    await sequelize.transaction(async (transaction) => {
-      await Promise.all(threadMessages.map(async (m) => {
-        await m.update({folderImapUID: null}, {transaction})
-        await m.setLabels(labelIds, {transaction})
-        await m.setFolder(targetFolder, {transaction})
-      }))
-      await thread.setLabels(labelIds, {transaction})
-      await thread.setFolders([targetFolder], {transaction})
+    // If IMAP succeeds, fetch any new messages in the target folder which
+    // should include the messages we just moved there
+    // The sync operation will save the changes to the database.
+    // TODO add transaction
+    const syncOperation = SyncTaskFactory.create('FetchNewMessagesInFolder', {
+      account: this._account,
+      folder: targetFolder,
     })
+    await syncOperation.run(db, imap)
   }
 }
 module.exports = SetThreadFolderAndLabelsIMAP