[local-sync] Refactor tasks and /actually/ fix moving between Gmail folders

Moving to between gmail folders (all, spam, trash) or moving to inbox,
involves changing labels /and/ folders, simultaneously. For this I added
a task to perform both operations, and apply labels first before
attempting to move the folder

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

@@ -76,7 +76,16 @@ module.exports = (server) => {
     },
     handler: (request, reply) => {
       const payload = request.payload
-      if (payload.folder_id || payload.folder) {
+      if ((payload.folder_id || payload.folder) && (payload.label_ids || payload.labels)) {
+        createSyncbackRequest(request, reply, {
+          type: "SetThreadFolderAndLabels",
+          props: {
+            folderId: payload.folder_id || payload.folder,
+            labelIds: payload.label_ids || payload.labels,
+            threadId: request.params.id,
+          },
+        })
+      } else if (payload.folder_id || payload.folder) {
         createSyncbackRequest(request, reply, {
           type: "MoveThreadToFolder",
           props: {

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

@@ -11,6 +11,8 @@ class SyncbackTaskFactory {
         Task = require('./syncback_tasks/move-thread-to-folder.imap'); break;
       case "SetThreadLabels":
         Task = require('./syncback_tasks/set-thread-labels.imap'); break;
+      case "SetThreadFolderAndLabels":
+        Task = require('./syncback_tasks/set-thread-folder-and-labels.imap'); break;
       case "MarkThreadAsRead":
         Task = require('./syncback_tasks/mark-thread-as-read.imap'); break;
       case "MarkThreadAsUnread":

packages/local-sync/src/local-sync-worker/syncback_tasks/move-message-to-folder.imap.js

@@ -6,16 +6,12 @@ class MoveMessageToFolderIMAP extends SyncbackTask {
     return `MoveMessageToFolder`;
   }
 
-  run(db, imap) {
+  async run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
-    const toFolderId = this.syncbackRequestObject().props.folderId
+    const targetFolderId = this.syncbackRequestObject().props.folderId
 
-    return TaskHelpers.openMessageBox({messageId, db, imap})
+    const {box, message} = await TaskHelpers.openMessageBox({messageId, db, imap})
-      .then(({box, message}) => {
+    return TaskHelpers.moveMessageToFolder({db, box, message, targetFolderId})
-        return db.Folder.findById(toFolderId).then((newFolder) => {
-          return box.moveFromBox(message.folderImapUID, newFolder.name)
-        })
-      })
   }
 }
 module.exports = MoveMessageToFolderIMAP

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

@@ -6,17 +6,18 @@ class MoveThreadToFolderIMAP extends SyncbackTask {
     return `MoveThreadToFolder`;
   }
 
-  run(db, imap) {
+  async run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
-    const toFolderId = this.syncbackRequestObject().props.folderId
+    const targetFolderId = this.syncbackRequestObject().props.folderId
 
-    const eachMsg = ({message, box}) => {
+    return TaskHelpers.forEachMessageInThread({
-      return db.Folder.findById(toFolderId).then((category) => {
+      db,
-        return box.moveFromBox(message.folderImapUID, category.name)
+      imap,
+      threadId,
+      async callback({message, box}) {
+        return TaskHelpers.moveMessageToFolder({db, box, message, targetFolderId})
+      },
     })
   }
-
-    return TaskHelpers.forEachMessageInThread({threadId, db, imap, callback: eachMsg})
-  }
 }
 module.exports = MoveThreadToFolderIMAP

packages/local-sync/src/local-sync-worker/syncback_tasks/set-message-labels.imap.js

@@ -6,28 +6,12 @@ class SetMessageLabelsIMAP extends SyncbackTask {
     return `SetMessageLabels`;
   }
 
-  run(db, imap) {
+  async run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
     const labelIds = this.syncbackRequestObject().props.labelIds
 
-    return TaskHelpers.openMessageBox({messageId, db, imap})
+    const {box, message} = await TaskHelpers.openMessageBox({messageId, db, imap})
-    .then(({box, message}) => {
+    return TaskHelpers.setMessageLabels({message, db, box, labelIds})
-      if (!labelIds || labelIds.length === 0) {
-        return message.getLabels().then((labels) => {
-          const labelNames = labels.map(({name}) => name)
-          return box.removeLabels(message.folderImapUID, labelNames)
-        })
-      }
-      return db.Label.findAll({
-        where: {
-          id: {'in': labelIds},
-        },
-      })
-      .then((labels) => {
-        const labelNames = labels.map(({name}) => name)
-        return box.setLabels(message.folderImapUID, labelNames)
-      })
-    })
   }
 }
 module.exports = SetMessageLabelsIMAP

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

@@ -0,0 +1,29 @@
+const SyncbackTask = require('./syncback-task')
+const TaskHelpers = require('./task-helpers')
+
+class SetThreadFolderAndLabelsIMAP extends SyncbackTask {
+  description() {
+    return `SetThreadFolderAndLabels`;
+  }
+
+  async run(db, imap) {
+    const threadId = this.syncbackRequestObject().props.threadId
+    const labelIds = this.syncbackRequestObject().props.labelIds
+    const targetFolderId = this.syncbackRequestObject().props.folderId
+
+    // Ben TODO this is super inefficient because it makes IMAP requests
+    // one UID at a time, rather than gathering all the UIDs and making
+    // a single removeLabels call.
+    return TaskHelpers.forEachMessageInThread({
+      db,
+      imap,
+      threadId,
+      async callback({message, box}) {
+        await TaskHelpers.setMessageLabels({message, db, box, labelIds})
+        return TaskHelpers.moveMessageToFolder({db, box, message, targetFolderId})
+      },
+    })
+  }
+}
+module.exports = SetThreadFolderAndLabelsIMAP
+

packages/local-sync/src/local-sync-worker/syncback_tasks/set-thread-labels.imap.js

@@ -10,23 +10,6 @@ class SetThreadLabelsIMAP extends SyncbackTask {
     const threadId = this.syncbackRequestObject().props.threadId
     const labelIds = this.syncbackRequestObject().props.labelIds
 
-    if (!labelIds || labelIds.length === 0) {
-      return TaskHelpers.forEachMessageInThread({
-        db,
-        imap,
-        threadId,
-        callback: ({message, box}) => {
-          return message.getLabels().then((labels) => {
-            const labelIdentifiers = labels.map(label => label.imapLabelIdentifier())
-            return box.removeLabels(message.folderImapUID, labelIdentifiers)
-          })
-        },
-      })
-    }
-
-    const labels = await db.Label.findAll({where: {id: labelIds}});
-    const labelIdentifiers = labels.map(label => label.imapLabelIdentifier());
-
     // Ben TODO this is super inefficient because it makes IMAP requests
     // one UID at a time, rather than gathering all the UIDs and making
     // a single removeLabels call.
@@ -34,8 +17,8 @@ class SetThreadLabelsIMAP extends SyncbackTask {
       db,
       imap,
       threadId,
-      callback: ({message, box}) => {
+      async callback({message, box}) {
-        return box.setLabels(message.folderImapUID, labelIdentifiers)
+        return TaskHelpers.setMessageLabels({message, db, box, labelIds})
       },
     })
   }

packages/local-sync/src/local-sync-worker/syncback_tasks/task-helpers.js

@@ -33,5 +33,34 @@ const TaskHelpers = {
       })
     }))
   },
+
+  async moveMessageToFolder({db, box, message, targetFolderId}) {
+    if (!targetFolderId) {
+      throw new Error('TaskHelpers.moveMessageToFolder: targetFolderId is required')
+    }
+    if (targetFolderId === message.folderId) {
+      return Promise.resolve()
+    }
+    const targetFolder = await db.Folder.findById(targetFolderId)
+    if (!targetFolder) {
+      return Promise.resolve()
+    }
+    return box.moveFromBox(message.folderImapUID, targetFolder.name)
+  },
+
+  async setMessageLabels({db, box, message, labelIds}) {
+    if (!labelIds || labelIds.length === 0) {
+      const labels = await message.getLabels()
+      if (labels.length === 0) {
+        return Promise.resolve()
+      }
+      const labelIdentifiers = labels.map(label => label.imapLabelIdentifier())
+      return box.removeLabels(message.folderImapUID, labelIdentifiers)
+    }
+
+    const labels = await db.Label.findAll({where: {id: labelIds}});
+    const labelIdentifiers = labels.map(label => label.imapLabelIdentifier());
+    return box.setLabels(message.folderImapUID, labelIdentifiers)
+  },
 }
 module.exports = TaskHelpers