Fixes to the SendDraftTask

spec/stores/file-upload-store-spec.coffee

@@ -74,16 +74,6 @@ describe 'FileUploadStore', ->
       expect(Actions.queueTask).not.toHaveBeenCalled()
       expect(FileUploadStore._onAttachFileError).toHaveBeenCalled()
 
-  describe 'when an uploading file is aborted', ->
-    it "dequeues the matching task", ->
-      spyOn(Actions, "dequeueMatchingTask")
-      Actions.abortUpload(@uploadData)
-      expect(Actions.dequeueMatchingTask).toHaveBeenCalled()
-      arg = Actions.dequeueMatchingTask.calls[0].args[0]
-      expect(arg).toEqual
-        type: "FileUploadTask"
-        matching: id: @uploadData.uploadTaskId
-
   describe 'when upload state changes', ->
     it 'updates the uploadData', ->
       Actions.uploadStateChanged(@uploadData)

spec/tasks/send-draft-spec.coffee

@@ -8,7 +8,6 @@ _ = require 'underscore'
  TaskQueue,
  SendDraftTask,
  SyncbackDraftTask,
- FileUploadTask,
  NylasAPI,
  SoundRegistry} = require 'nylas-exports'
 
@@ -300,12 +299,6 @@ describe "SendDraftTask", ->
             expect(@task._notifyUserOfError).not.toHaveBeenCalled()
             expect(NylasEnv.emitError).not.toHaveBeenCalled()
 
-        it "notifies the user that the required file upload failed", ->
-          fileUploadTask = new FileUploadTask('/dev/null', 'local-1234')
-          @task.onDependentTaskError(fileUploadTask, new Error("Oh no"))
-          expect(@task._notifyUserOfError).toHaveBeenCalled()
-          expect(@task._notifyUserOfError.calls.length).toBe 1
-
         describe "checking the promise chain halts on errors", ->
           beforeEach ->
             spyOn(@task,"_makeSendRequest").andCallThrough()

src/flux/stores/draft-store.coffee

@@ -10,6 +10,7 @@ AccountStore = require './account-store'
 ContactStore = require './contact-store'
 FocusedPerspectiveStore = require './focused-perspective-store'
 FocusedContentStore = require './focused-content-store'
+FileUploadStore = require './file-upload-store'
 
 SendDraftTask = require '../tasks/send-draft'
 DestroyDraftTask = require '../tasks/destroy-draft'
@@ -513,17 +514,11 @@ class DraftStore
       # committed to the Database since we'll look them up again just
       # before send.
       session.changes.commit(noSyncback: true).then =>
-        task = new SendDraftTask(session.draft())
-        Actions.queueTask(task)
-
-        # NOTE: We may be done with the session in this window, but there
-        # may still be {FileUploadTask}s and other pending draft mutations
-        # in the worker window.
-        #
-        # The send "pending" indicator in the main window is declaratively
-        # bound to the existence of a `@_draftSession`. We want to show
-        # the pending state immediately even as files are uploading.
+        draft = session.draft()
+        uploads = FileUploadStore.uploadsForMessage(draft.clientId)
+        Actions.queueTask(new SendDraftTask(draft, uploads))
         @_doneWithSession(session)
+
         NylasEnv.close() if @_isPopout()
 
   _isPopout: ->

src/flux/stores/file-upload-store.coffee

@@ -5,15 +5,6 @@ mkdirp = require 'mkdirp'
 NylasStore = require 'nylas-store'
 Actions = require '../actions'
 
-###
-TODO: This store uses a combination of Actions and it's own internal structures
-to keep track of uploads. It's difficult to ensure this state stays in sync as
-FileUploadTasks run.
-
-Instead, this store should observe the TaskQueueStatusStore and inspect
-FileUploadTasks themselves for state at any given moment. Refactor this!
-###
-
 UPLOAD_DIR = path.join(NylasEnv.getConfigDirPath(), 'uploads')
 
 class Upload
@@ -40,7 +31,7 @@ class FileUploadStore extends NylasStore
     mkdirp(UPLOAD_DIR)
 
   uploadsForMessage: (messageClientId) ->
-    @_fileUploads[messageClientId]
+    @_fileUploads[messageClientId] ? []
 
   _onSelectFileForUpload: ({messageClientId}) ->
     @_verifyId(messageClientId)
@@ -87,23 +78,21 @@ class FileUploadStore extends NylasStore
   _getFileStats: ({messageClientId, filePath}) ->
     fs.stat filePath, (err, stats) =>
       if err
-        reject("#{filePath} could not be found, or has invalid file permissions.")
+        Promise.reject("#{filePath} could not be found, or has invalid file permissions.")
       else
-        resolve({messageClientId, filePath, stats})
+        Promise.resolve({messageClientId, filePath, stats})
 
   _makeUpload: ({messageClientId, filePath, stats}) ->
     Promise.resolve(new Upload(messageClientId, filePath, stats))
 
   _verifyUpload: (upload) ->
-    return new Promise (resolve, reject) ->
-      {stats} = upload
-      if stats.isDirectory()
-        reject("#{filename} is a directory. Try compressing it and attaching it again.")
-      else if stats.size > 25 * 1000000
-        reject("#{filename} cannot be attached because it is larger than 25MB.")
-      else
-        resolve(upload)
-        # Actions.queueTask(new FileUploadTask(path, messageClientId))
+    {stats} = upload
+    if stats.isDirectory()
+      Promise.reject("#{filename} is a directory. Try compressing it and attaching it again.")
+    else if stats.size > 25 * 1000000
+      Promise.reject("#{filename} cannot be attached because it is larger than 25MB.")
+    else
+      Promise.resolve(upload)
 
   _prepareTargetDir: (upload) =>
     return new Promise (resolve, reject) ->

src/flux/stores/task-queue.coffee

@@ -49,9 +49,9 @@ if @_thread && @_thread.unread
 
 ```coffee
 Actions.dequeueMatchingTask({
-  type: 'FileUploadTask',
+  type: 'DestroyCategoryTask',
   matching: {
-    filePath: uploadData.filePath
+    categoryId: 'bla'
   }
 })
 ```

src/flux/tasks/send-draft.coffee

@@ -16,17 +16,17 @@ class MultiRequestProgressMonitor
     @_requests = {}
     @_expected = {}
 
-  add: (filepath, request) =>
+  add: (filepath, filesize, request) =>
     @_requests[filepath] = request
-    @_expected[filepath] = fs.statSync(filepath)["size"] ? 0
+    @_expected[filepath] = filesize ? fs.statSync(filepath)["size"] ? 0
 
   remove: (filepath) =>
     delete @_requests[filepath]
     delete @_expected[filepath]
 
-  progress: =>
+  value: =>
     sent = 0
-    expected = 0
+    expected = 1
     for filepath, req of @_requests
       sent += @req?.req?.connection?._bytesDispatched ? 0
       expected += @_expected[filepath]
@@ -36,8 +36,9 @@ class MultiRequestProgressMonitor
 module.exports =
 class SendDraftTask extends Task
 
-  constructor: (@draft, @attachmentPaths) ->
+  constructor: (@draft, @uploads) ->
     @_progress = new MultiRequestProgressMonitor()
+    Object.defineProperty(@, 'progress', { get: -> @_progress.value() })
     super
 
   label: ->
@@ -47,18 +48,22 @@ class SendDraftTask extends Task
     other instanceof SendDraftTask and other.draft.clientId is @draft.clientId
 
   performLocal: ->
-    return Promise.reject(new Error("SendDraftTask must be provided a draft.")) unless @draft
+    unless @draft and @draft instanceof Message
+      return Promise.reject(new Error("SendDraftTask must be provided a draft."))
+    unless @uploads and @uploads instanceof Array
+      return Promise.reject(new Error("SendDraftTask must be provided an array of uploads."))
+
     Promise.resolve()
 
   performRemote: ->
     @_uploadAttachments()
     .then(@_sendAndCreateMessage)
-    .then(@_deleteDraft)
+    .then(@_deleteRemoteDraft)
     .then(@_onSuccess)
     .catch(@_onError)
 
   _uploadAttachments: =>
-    Promise.all @attachmentPaths.map (filepath) =>
+    Promise.all @uploads.map ({targetPath, size}) =>
       NylasAPI.makeRequest
         path: "/files"
         accountId: @draft.accountId
@@ -66,14 +71,14 @@ class SendDraftTask extends Task
         json: false
         formData:
           file: # Must be named `file` as per the Nylas API spec
-            value: fs.createReadStream(filepath)
+            value: fs.createReadStream(targetPath)
             options:
-              filename: path.basename(filepath)
+              filename: path.basename(targetPath)
         started: (req) =>
-          @_progress.add(filepath, req)
+          @_progress.add(targetPath, size, req)
         timeout: 20 * 60 * 1000
       .finally =>
-        @_progress.remove(filepath)
+        @_progress.remove(targetPath)
       .then (file) =>
         @draft.files.push(file)
 
@@ -82,7 +87,7 @@ class SendDraftTask extends Task
       path: "/send"
       accountId: @draft.accountId
       method: 'POST'
-      body: draftModel.toJSON()
+      body: @draft.toJSON()
       timeout: 1000 * 60 * 5 # We cannot hang up a send - won't know if it sent
       returnsModel: false
 
@@ -139,7 +144,9 @@ class SendDraftTask extends Task
       SoundRegistry.playSound('send')
 
     # Remove attachments we were waiting to upload
-    @attachmentPaths.forEach(fs.unlink)
+    # Call the Action to do this
+    for upload in @uploads
+      Actions.removeFileFromUpload(upload.messageClientId, upload.id)
 
     return Promise.resolve(Task.Status.Success)
 

src/flux/tasks/task.coffee

@@ -499,9 +499,6 @@ class Task
   # Public: code to run if someone tries to dequeue your task while it is
   # in flight.
   #
-  # For example, the {FileUploadTask} implements `cancel` to `abort` the
-  # http request if someone dequeues it. Once `abort`ed, an error is
-  # thrown in `performRemote` and handled accordingly.
   cancel: ->
 
   # Public: (optional) A string displayed to users when your task is run.