diff --git a/spec/stores/file-upload-store-spec.coffee b/spec/stores/file-upload-store-spec.coffee index c622f2290..d4d953dda 100644 --- a/spec/stores/file-upload-store-spec.coffee +++ b/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) diff --git a/spec/tasks/send-draft-spec.coffee b/spec/tasks/send-draft-spec.coffee index 3eec3d4e6..9c8fee8ae 100644 --- a/spec/tasks/send-draft-spec.coffee +++ b/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() diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee index b4df778b9..395145dc0 100644 --- a/src/flux/stores/draft-store.coffee +++ b/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: -> diff --git a/src/flux/stores/file-upload-store.coffee b/src/flux/stores/file-upload-store.coffee index 83d645d5f..33c5608e8 100644 --- a/src/flux/stores/file-upload-store.coffee +++ b/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) -> diff --git a/src/flux/stores/task-queue.coffee b/src/flux/stores/task-queue.coffee index eb5c21a81..b50c103cd 100644 --- a/src/flux/stores/task-queue.coffee +++ b/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' } }) ``` diff --git a/src/flux/tasks/send-draft.coffee b/src/flux/tasks/send-draft.coffee index 4ca32e973..e2067a996 100644 --- a/src/flux/tasks/send-draft.coffee +++ b/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) diff --git a/src/flux/tasks/task.coffee b/src/flux/tasks/task.coffee index 7dde1daf6..fba1376da 100644 --- a/src/flux/tasks/task.coffee +++ b/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.