diff --git a/internal_packages/composer/lib/composer-view.cjsx b/internal_packages/composer/lib/composer-view.cjsx index b3f380985..cfff00038 100644 --- a/internal_packages/composer/lib/composer-view.cjsx +++ b/internal_packages/composer/lib/composer-view.cjsx @@ -2,11 +2,11 @@ React = require 'react' _ = require 'underscore-plus' {Actions, + DraftStore, FileUploadStore, ComponentRegistry} = require 'inbox-exports' FileUploads = require './file-uploads.cjsx' -DraftStoreProxy = require './draft-store-proxy' ContenteditableToolbar = require './contenteditable-toolbar.cjsx' ContenteditableComponent = require './contenteditable-component.cjsx' ParticipantsTextField = require './participants-text-field.cjsx' @@ -67,8 +67,10 @@ ComposerView = React.createClass @_prepareForDraft() _prepareForDraft: -> - @_proxy = new DraftStoreProxy(@props.localId) - + @_proxy = DraftStore.sessionForLocalId(@props.localId) + if @_proxy.draft() + @_onDraftChanged() + @unlisteners = [] @unlisteners.push @_proxy.listen(@_onDraftChanged) @unlisteners.push ComponentRegistry.listen (event) => @@ -256,7 +258,6 @@ ComposerView = React.createClass @_sendDraft({force: true}) return - @_proxy.changes.commit() Actions.sendDraft(@props.localId) _destroyDraft: -> diff --git a/spec-inbox/stores/task-queue-spec.coffee b/spec-inbox/stores/task-queue-spec.coffee index 2458208ce..05d48ad94 100644 --- a/spec-inbox/stores/task-queue-spec.coffee +++ b/spec-inbox/stores/task-queue-spec.coffee @@ -119,7 +119,6 @@ describe "TaskQueue", -> taskToDie = makeRemoteFailed(new TaskSubclassA()) spyOn(TaskQueue, "dequeue").andCallThrough() - spyOn(taskToDie, "abort") TaskQueue._queue = [taskToDie, @remoteFailed] TaskQueue.enqueue(new KillsTaskA()) @@ -127,7 +126,6 @@ describe "TaskQueue", -> expect(TaskQueue._queue.length).toBe 2 expect(TaskQueue.dequeue).toHaveBeenCalledWith(taskToDie, silent: true) expect(TaskQueue.dequeue.calls.length).toBe 1 - expect(taskToDie.abort).toHaveBeenCalled() describe "dequeue", -> beforeEach -> @@ -147,37 +145,11 @@ describe "TaskQueue", -> it "throws an error if the task isn't found", -> expect( -> TaskQueue.dequeue("bad")).toThrow() - it "doesn't abort unstarted tasks", -> - spyOn(@unstartedTask, "abort") - TaskQueue.dequeue(@unstartedTask, silent: true) - expect(@unstartedTask.abort).not.toHaveBeenCalled() - - it "aborts local tasks in progress", -> - spyOn(@localStarted, "abort") - TaskQueue.dequeue(@localStarted, silent: true) - expect(@localStarted.abort).toHaveBeenCalled() - - it "aborts remote tasks in progress", -> - spyOn(@remoteStarted, "abort") - TaskQueue.dequeue(@remoteStarted, silent: true) - expect(@remoteStarted.abort).toHaveBeenCalled() - - it "calls cleanup on aborted tasks", -> + it "calls cleanup on dequeued tasks", -> spyOn(@remoteStarted, "cleanup") TaskQueue.dequeue(@remoteStarted, silent: true) expect(@remoteStarted.cleanup).toHaveBeenCalled() - it "aborts stalled remote tasks", -> - spyOn(@remoteFailed, "abort") - TaskQueue.dequeue(@remoteFailed, silent: true) - expect(@remoteFailed.abort).toHaveBeenCalled() - - it "doesn't abort if it's fully done", -> - TaskQueue._queue.push @remoteSuccess - spyOn(@remoteSuccess, "abort") - TaskQueue.dequeue(@remoteSuccess, silent: true) - expect(@remoteSuccess.abort).not.toHaveBeenCalled() - it "moves it from the queue", -> TaskQueue.dequeue(@remoteStarted, silent: true) expect(TaskQueue._queue.length).toBe 3 diff --git a/spec-inbox/tasks/file-upload-task-spec.coffee b/spec-inbox/tasks/file-upload-task-spec.coffee index 8e45c9906..2590131cc 100644 --- a/spec-inbox/tasks/file-upload-task-spec.coffee +++ b/spec-inbox/tasks/file-upload-task-spec.coffee @@ -81,18 +81,38 @@ describe "FileUploadTask", -> expect(options.method).toBe('POST') expect(options.formData.file.value).toBe("Read Stream") - it "can abort the upload with the full file path", -> - spyOn(@task, "_getBytesUploaded").andReturn(100) - waitsForPromise => @task.performRemote().then => - @task.abort() - expect(@req.abort).toHaveBeenCalled() - data = _.extend uploadData, - state: "aborted" - bytesUploaded: 100 - expect(Actions.uploadStateChanged).toHaveBeenCalledWith(data) - it "notifies when the file successfully uploaded", -> spyOn(@task, "_completedNotification").andReturn(100) waitsForPromise => @task.performRemote().then => file = (new File).fromJSON(fileJSON) expect(@task._completedNotification).toHaveBeenCalledWith(file) + + + describe "cleanup", -> + it "should not do anything if the request has finished", -> + req = jasmine.createSpyObj('req', ['abort']) + reqSuccess = null + spyOn(atom.inbox, 'makeRequest').andCallFake (reqParams) -> + reqSuccess = reqParams.success + req + + @task.performRemote() + reqSuccess([fileJSON]) + @task.cleanup() + expect(req.abort).not.toHaveBeenCalled() + + it "should cancel the request if it's in flight", -> + req = jasmine.createSpyObj('req', ['abort']) + spyOn(atom.inbox, 'makeRequest').andCallFake (reqParams) -> req + spyOn(Actions, "uploadStateChanged") + + @task.performRemote() + @task.cleanup() + + expect(req.abort).toHaveBeenCalled() + data = _.extend uploadData, + state: "aborted" + bytesUploaded: 0 + expect(Actions.uploadStateChanged).toHaveBeenCalledWith(data) + + diff --git a/spec-inbox/tasks/send-draft-spec.coffee b/spec-inbox/tasks/send-draft-spec.coffee index 22764e25f..589187b55 100644 --- a/spec-inbox/tasks/send-draft-spec.coffee +++ b/spec-inbox/tasks/send-draft-spec.coffee @@ -1,5 +1,5 @@ Actions = require '../../src/flux/actions' -SaveDraftTask = require '../../src/flux/tasks/save-draft' +SyncbackDraftTask = require '../../src/flux/tasks/syncback-draft' SendDraftTask = require '../../src/flux/tasks/send-draft' DatabaseStore = require '../../src/flux/stores/database-store' {generateTempId} = require '../../src/flux/models/utils' @@ -9,7 +9,7 @@ _ = require 'underscore-plus' describe "SendDraftTask", -> describe "shouldWaitForTask", -> - it "should return any SaveDraftTasks for the same draft", -> + it "should return any SyncbackDraftTasks for the same draft", -> @draftA = new Message version: '1' id: '1233123AEDF1' @@ -30,8 +30,8 @@ describe "SendDraftTask", -> name: 'Dummy' email: 'dummy@inboxapp.com' - @saveA = new SaveDraftTask('localid-A') - @saveB = new SaveDraftTask('localid-B') + @saveA = new SyncbackDraftTask('localid-A') + @saveB = new SyncbackDraftTask('localid-B') @sendA = new SendDraftTask('localid-A') expect(@sendA.shouldWaitForTask(@saveA)).toBe(true) @@ -40,8 +40,8 @@ describe "SendDraftTask", -> beforeEach -> TaskQueue._queue = [] TaskQueue._completed = [] - @saveTask = new SaveDraftTask('localid-A') - @saveTaskB = new SaveDraftTask('localid-B') + @saveTask = new SyncbackDraftTask('localid-A') + @saveTaskB = new SyncbackDraftTask('localid-B') @sendTask = new SendDraftTask('localid-A') @tasks = [@saveTask, @saveTaskB, @sendTask] @@ -124,13 +124,33 @@ describe "SendDraftTask", -> expect(options.path).toBe("/n/#{@draft.namespaceId}/send") expect(options.method).toBe('POST') - it "should send the draft ID and version", -> - waitsForPromise => - @task.performRemote().then => - expect(atom.inbox.makeRequest.calls.length).toBe(1) - options = atom.inbox.makeRequest.mostRecentCall.args[0] - expect(options.body.version).toBe(@draft.version) - expect(options.body.draft_id).toBe(@draft.id) + describe "when the draft has been saved", -> + it "should send the draft ID and version", -> + waitsForPromise => + @task.performRemote().then => + expect(atom.inbox.makeRequest.calls.length).toBe(1) + options = atom.inbox.makeRequest.mostRecentCall.args[0] + expect(options.body.version).toBe(@draft.version) + expect(options.body.draft_id).toBe(@draft.id) + + describe "when the draft has not been saved", -> + beforeEach -> + @draft = new Message + id: generateTempId() + namespaceId: 'A12ADE' + subject: 'New Draft' + draft: true + to: + name: 'Dummy' + email: 'dummy@inboxapp.com' + @task = new SendDraftTask(@draft) + + it "should send the draft JSON", -> + waitsForPromise => + @task.performRemote().then => + expect(atom.inbox.makeRequest.calls.length).toBe(1) + options = atom.inbox.makeRequest.mostRecentCall.args[0] + expect(options.body).toEqual(@draft.toJSON()) it "should pass returnsModel:true so that the draft is saved to the data store when returned", -> waitsForPromise => diff --git a/spec-inbox/tasks/save-draft-spec.coffee b/spec-inbox/tasks/syncback-draft-spec.coffee similarity index 69% rename from spec-inbox/tasks/save-draft-spec.coffee rename to spec-inbox/tasks/syncback-draft-spec.coffee index f6d684f6a..e4356282f 100644 --- a/spec-inbox/tasks/save-draft-spec.coffee +++ b/spec-inbox/tasks/syncback-draft-spec.coffee @@ -9,7 +9,7 @@ Contact = require '../../src/flux/models/contact' DatabaseStore = require '../../src/flux/stores/database-store' TaskQueue = require '../../src/flux/stores/task-queue' -SaveDraftTask = require '../../src/flux/tasks/save-draft' +SyncbackDraftTask = require '../../src/flux/tasks/syncback-draft' inboxError = message: "No draft with public id bvn4aydxuyqlbmzowh4wraysg", @@ -33,7 +33,7 @@ testData = localDraft = new Message _.extend {}, testData, {id: "local-id"} remoteDraft = new Message _.extend {}, testData, {id: "remoteid1234"} -describe "SaveDraftTask", -> +describe "SyncbackDraftTask", -> beforeEach -> spyOn(DatabaseStore, "findByLocalId").andCallFake (klass, localId) -> if localId is "localDraftId" then Promise.resolve(localDraft) @@ -46,53 +46,19 @@ describe "SaveDraftTask", -> spyOn(DatabaseStore, "swapModel").andCallFake -> Promise.resolve() - describe "performLocal", -> - it "rejects if it isn't constructed with a draftLocalId", -> - task = new SaveDraftTask - waitsForPromise => - task.performLocal().catch (error) -> - expect(error.message).toBeDefined() - - it "does nothing if there are no new changes", -> - task = new SaveDraftTask("localDraftId") - waitsForPromise => - task.performLocal().then -> - expect(DatabaseStore.persistModel).not.toHaveBeenCalled() - - it "persists to the Database if there are new changes", -> - task = new SaveDraftTask("localDraftId", body: "test body") - waitsForPromise => - task.performLocal().then -> - expect(DatabaseStore.persistModel).toHaveBeenCalled() - newBody = DatabaseStore.persistModel.calls[0].args[0].body - expect(newBody).toBe "test body" - - it "does nothing if no draft can be found in the db", -> - task = new SaveDraftTask("missingDraftId") - waitsForPromise => - task.performLocal().then -> - expect(DatabaseStore.persistModel).not.toHaveBeenCalled() - describe "performRemote", -> beforeEach -> spyOn(atom.inbox, 'makeRequest').andCallFake (opts) -> opts.success(remoteDraft.toJSON()) if opts.success - it "does nothing if localOnly is set to true", -> - task = new SaveDraftTask("localDraftId", {}, localOnly: true) - waitsForPromise => - task.performRemote().then -> - expect(DatabaseStore.findByLocalId).not.toHaveBeenCalled() - expect(atom.inbox.makeRequest).not.toHaveBeenCalled() - it "does nothing if no draft can be found in the db", -> - task = new SaveDraftTask("missingDraftId") + task = new SyncbackDraftTask("missingDraftId") waitsForPromise => task.performRemote().then -> expect(atom.inbox.makeRequest).not.toHaveBeenCalled() it "should start an API request with the Message JSON", -> - task = new SaveDraftTask("localDraftId") + task = new SyncbackDraftTask("localDraftId") waitsForPromise => task.performRemote().then -> expect(atom.inbox.makeRequest).toHaveBeenCalled() @@ -100,7 +66,7 @@ describe "SaveDraftTask", -> expect(reqBody.subject).toEqual testData.subject it "should do a PUT when the draft has already been saved", -> - task = new SaveDraftTask("remoteDraftId") + task = new SyncbackDraftTask("remoteDraftId") waitsForPromise => task.performRemote().then -> expect(atom.inbox.makeRequest).toHaveBeenCalled() @@ -109,7 +75,7 @@ describe "SaveDraftTask", -> expect(options.method).toBe('PUT') it "should do a POST when the draft is unsaved", -> - task = new SaveDraftTask("localDraftId") + task = new SyncbackDraftTask("localDraftId") waitsForPromise => task.performRemote().then -> expect(atom.inbox.makeRequest).toHaveBeenCalled() @@ -118,7 +84,7 @@ describe "SaveDraftTask", -> expect(options.method).toBe('POST') it "should pass returnsModel:false so that the draft can be manually removed/added to the database, accounting for its ID change", -> - task = new SaveDraftTask("localDraftId") + task = new SyncbackDraftTask("localDraftId") waitsForPromise => task.performRemote().then -> expect(atom.inbox.makeRequest).toHaveBeenCalled() @@ -126,14 +92,14 @@ describe "SaveDraftTask", -> expect(options.returnsModel).toBe(false) it "should swap the ids if we got a new one from the DB", -> - task = new SaveDraftTask("localDraftId") + task = new SyncbackDraftTask("localDraftId") waitsForPromise => task.performRemote().then -> expect(DatabaseStore.swapModel).toHaveBeenCalled() expect(DatabaseStore.persistModel).not.toHaveBeenCalled() it "should not swap the ids if we're using a persisted one", -> - task = new SaveDraftTask("remoteDraftId") + task = new SyncbackDraftTask("remoteDraftId") waitsForPromise => task.performRemote().then -> expect(DatabaseStore.swapModel).not.toHaveBeenCalled() @@ -146,7 +112,7 @@ describe "SaveDraftTask", -> opts.error(testError(opts)) if opts.error it "resets the id", -> - task = new SaveDraftTask("remoteDraftId") + task = new SyncbackDraftTask("remoteDraftId") task.onAPIError(testError({})) waitsFor -> DatabaseStore.swapModel.calls.length > 0 diff --git a/src/atom.coffee b/src/atom.coffee index 9f8c0ab7a..aa0872c5f 100644 --- a/src/atom.coffee +++ b/src/atom.coffee @@ -757,6 +757,11 @@ class Atom extends Model app.emit('will-exit') remote.process.exit(status) + showOpenDialog: (options, callback) -> + parentWindow = if process.platform is 'darwin' then null else @getCurrentWindow() + dialog = remote.require('dialog') + dialog.showOpenDialog(parentWindow, options, callback) + showSaveDialog: (defaultPath, callback) -> parentWindow = if process.platform is 'darwin' then null else @getCurrentWindow() dialog = remote.require('dialog') diff --git a/src/browser/edgehill-application.coffee b/src/browser/edgehill-application.coffee index c7f04e0be..798c49b85 100644 --- a/src/browser/edgehill-application.coffee +++ b/src/browser/edgehill-application.coffee @@ -215,9 +215,6 @@ class AtomApplication @on 'application:quit', => @quitting = true app.quit() - @on 'application:open-file-to-window', -> @promptForPath({type: 'file', to_window: true}) - @on 'application:open-dev', -> @promptForPath(devMode: true) - @on 'application:open-safe', -> @promptForPath(safeMode: true) @on 'application:inspect', ({x,y, atomWindow}) -> atomWindow ?= @focusedWindow() atomWindow?.browserWindow.inspectElement(x, y) diff --git a/src/flux/actions.coffee b/src/flux/actions.coffee index cb79763a2..d869ddaaa 100644 --- a/src/flux/actions.coffee +++ b/src/flux/actions.coffee @@ -1,4 +1,3 @@ -ipc = require 'ipc' Reflux = require 'reflux' # These actions are rebroadcast through the ActionBridge to all @@ -46,9 +45,6 @@ windowActions = [ # Fired when a dialog is opened and a file is selected "clearDeveloperConsole", - "openPathsSelected", - "savePathSelected", - # Actions for Selection State "selectNamespaceId", "selectThreadId", @@ -87,7 +83,7 @@ windowActions = [ # Some file actions only need to be processed in their current window "attachFile", "abortUpload", - "persistUploadedFile", # This touches the DB, should only be in main window + "attachFileComplete", "removeFile", "fetchAndOpenFile", "fetchAndSaveFile", @@ -106,10 +102,4 @@ Actions.windowActions = windowActions Actions.mainWindowActions = mainWindowActions Actions.globalActions = globalActions -ipc.on "paths-to-open", (pathsToOpen=[]) -> - Actions.openPathsSelected(pathsToOpen) - -ipc.on "save-file-selected", (savePath) -> - Actions.savePathSelected(savePath) - module.exports = Actions diff --git a/src/flux/inbox-api.coffee b/src/flux/inbox-api.coffee index 6f81b8102..b4d807ac9 100644 --- a/src/flux/inbox-api.coffee +++ b/src/flux/inbox-api.coffee @@ -193,11 +193,11 @@ class InboxAPI Thread = require './models/thread' return @_shouldAcceptModelIfNewer(Thread, model) - # For some reason, we occasionally get a delta with: - # delta.object = 'message', delta.attributes.object = 'draft' + # For the time being, we never accept drafts from the server. This single + # change ensures that all drafts in the system are authored locally. To + # revert, change back to use _shouldAcceptModelIfNewer if classname is "draft" or model?.object is "draft" - Message = require './models/message' - return @_shouldAcceptModelIfNewer(Message, model) + return Promise.reject() Promise.resolve() diff --git a/src/flux/inbox-long-connection.coffee b/src/flux/inbox-long-connection.coffee index a5e1cc403..8a3d3ad36 100644 --- a/src/flux/inbox-long-connection.coffee +++ b/src/flux/inbox-long-connection.coffee @@ -18,7 +18,7 @@ class InboxLongConnection @_emitter = new Emitter @_state = 'idle' @_req = null - @_reqPingInterval = null + @_reqForceReconnectInterval = null @_buffer = null @_deltas = [] @@ -86,9 +86,11 @@ class InboxLongConnection @_buffer = bufferJSONs[bufferJSONs.length - 1] start: -> - throw (new Error 'Cannot start polling without auth token.') unless @_inbox.APIToken + return if @_state is InboxLongConnection.State.Ended return if @_req + throw (new Error 'Cannot start polling without auth token.') unless @_inbox.APIToken + console.log("Long Polling Connection: Starting....") @withCursor (cursor) => return if @state is InboxLongConnection.State.Ended @@ -122,18 +124,22 @@ class InboxLongConnection req.write("1") @_req = req - @_reqPingInterval = setInterval -> - req.write("1") - ,250 - retry: -> + # Currently we have trouble identifying when the connection has closed. + # Instead of trying to fix that, just reconnect every 30 seconds. + @_reqForceReconnectInterval = setInterval => + @retry(true) + ,30000 + + retry: (immediate = false) -> return if @_state is InboxLongConnection.State.Ended @setState(InboxLongConnection.State.Retrying) - @cleanup() + + startDelay = if immediate then 0 else 10000 setTimeout => @start() - , 10000 + , startDelay end: -> console.log("Long Polling Connection: Closed.") @@ -141,8 +147,8 @@ class InboxLongConnection @cleanup() cleanup: -> - clearInterval(@_reqPingInterval) if @_reqPingInterval - @_reqPingInterval = null + clearInterval(@_reqForceReconnectInterval) if @_reqForceReconnectInterval + @_reqForceReconnectInterval = null if @_req @_req.end() @_req.abort() diff --git a/src/flux/models/utils.coffee b/src/flux/models/utils.coffee index aff66b0cb..c4ff6cb67 100644 --- a/src/flux/models/utils.coffee +++ b/src/flux/models/utils.coffee @@ -16,7 +16,7 @@ utils = SalesforceContact = require './salesforce-contact' SalesforceTask = require './salesforce-task' - SaveDraftTask = require '../tasks/save-draft' + SyncbackDraftTask = require '../tasks/syncback-draft' SendDraftTask = require '../tasks/send-draft' DestroyDraftTask = require '../tasks/destroy-draft' AddRemoveTagsTask = require '../tasks/add-remove-tags' @@ -43,7 +43,7 @@ utils = 'MarkMessageReadTask': MarkMessageReadTask 'AddRemoveTagsTask': AddRemoveTagsTask 'SendDraftTask': SendDraftTask - 'SaveDraftTask': SaveDraftTask + 'SyncbackDraftTask': SyncbackDraftTask 'DestroyDraftTask': DestroyDraftTask 'FileUploadTask': FileUploadTask } diff --git a/internal_packages/composer/lib/draft-store-proxy.coffee b/src/flux/stores/draft-store-proxy.coffee similarity index 75% rename from internal_packages/composer/lib/draft-store-proxy.coffee rename to src/flux/stores/draft-store-proxy.coffee index 4d9b7483a..dfced1878 100644 --- a/internal_packages/composer/lib/draft-store-proxy.coffee +++ b/src/flux/stores/draft-store-proxy.coffee @@ -1,4 +1,5 @@ -{Message, Actions,DraftStore} = require 'inbox-exports' +Message = require '../models/message' +Actions = require '../actions' EventEmitter = require('events').EventEmitter _ = require 'underscore-plus' @@ -15,7 +16,11 @@ _ = require 'underscore-plus' # class DraftChangeSet constructor: (@localId, @_onChange) -> + @reset() + + reset: -> @_pending = {} + clearTimeout(@_timer) if @_timer @_timer = null add: (changes, immediate) => @@ -28,9 +33,12 @@ class DraftChangeSet @_timer = setTimeout(@commit, 5000) commit: => - @_pending.localId = @localId - if Object.keys(@_pending).length > 1 - Actions.saveDraft(@_pending) + return unless Object.keys(@_pending).length > 0 + + DatabaseStore = require './database-store' + DatabaseStore.findByLocalId(Message, @localId).then (draft) => + draft = @applyToModel(draft) + DatabaseStore.persistModel(draft) @_pending = {} applyToModel: (model) => @@ -51,19 +59,32 @@ module.exports = class DraftStoreProxy constructor: (@draftLocalId) -> + DraftStore = require './draft-store' + @unlisteners = [] @unlisteners.push DraftStore.listen(@_onDraftChanged, @) @unlisteners.push Actions.didSwapModel.listen(@_onDraftSwapped, @) + @_emitter = new EventEmitter() @_draft = false - @_reloadDraft() - + @_draftPromise = null @changes = new DraftChangeSet @draftLocalId, => @_emitter.emit('trigger') + @prepare() draft: -> @changes.applyToModel(@_draft) @_draft + + prepare: -> + @_draftPromise ?= new Promise (resolve, reject) => + DatabaseStore = require './database-store' + DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => + @_draft = draft + @_emitter.emit('trigger') + resolve(@) + .catch(reject) + @_draftPromise listen: (callback, bindContext) -> eventHandler = (args) -> @@ -71,10 +92,11 @@ class DraftStoreProxy @_emitter.addListener('trigger', eventHandler) return => @_emitter.removeListener('trigger', eventHandler) - if @_emitter.listeners('trigger').length == 0 - # Unlink ourselves from the stores/actions we were listening to - # so that we can be garbage collected - unlisten() for unlisten in @unlisteners + + cleanup: -> + # Unlink ourselves from the stores/actions we were listening to + # so that we can be garbage collected + unlisten() for unlisten in @unlisteners _onDraftChanged: (change) -> # We don't accept changes unless our draft object is loaded @@ -93,12 +115,3 @@ class DraftStoreProxy if change.oldModel.id is @_draft.id @_draft = change.newModel @_emitter.emit('trigger') - - _reloadDraft: -> - promise = DraftStore.findByLocalId(@draftLocalId) - promise.catch (err) -> - console.log(err) - promise.then (draft) => - @_draft = draft - @_emitter.emit('trigger') - diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee index aa391a821..fc78842ad 100644 --- a/src/flux/stores/draft-store.coffee +++ b/src/flux/stores/draft-store.coffee @@ -2,10 +2,10 @@ _ = require 'underscore-plus' moment = require 'moment' Reflux = require 'reflux' +DraftStoreProxy = require './draft-store-proxy' DatabaseStore = require './database-store' NamespaceStore = require './namespace-store' -SaveDraftTask = require '../tasks/save-draft' SendDraftTask = require '../tasks/send-draft' DestroyDraftTask = require '../tasks/destroy-draft' @@ -21,6 +21,7 @@ Actions = require '../actions' # # Remember that a "Draft" is actually just a "Message" with draft: true. # + module.exports = DraftStore = Reflux.createStore init: -> @@ -32,18 +33,19 @@ DraftStore = Reflux.createStore @listenTo Actions.composePopoutDraft, @_onComposePopoutDraft @listenTo Actions.composeNewBlankDraft, @_onComposeNewBlankDraft - @listenTo Actions.saveDraft, @_onSaveDraft @listenTo Actions.sendDraft, @_onSendDraft @listenTo Actions.destroyDraft, @_onDestroyDraft @listenTo Actions.removeFile, @_onRemoveFile - @listenTo Actions.persistUploadedFile, @_onFileUploaded + @listenTo Actions.attachFileComplete, @_onAttachFileComplete + @_draftSessions = {} ######### PUBLIC ####################################################### # Returns a promise - findByLocalId: (localId) -> - DatabaseStore.findByLocalId(Message, localId) + sessionForLocalId: (localId) -> + @_draftSessions[localId] ?= new DraftStoreProxy(localId) + @_draftSessions[localId] ########### PRIVATE #################################################### @@ -127,55 +129,30 @@ DraftStore = Reflux.createStore atom.displayComposer(draftLocalId) _onDestroyDraft: (draftLocalId) -> + # Immediately reset any pending changes so no saves occur + @_draftSessions[draftLocalId]?.changes.reset() + delete @_draftSessions[draftLocalId] + + # Queue the task to destroy the draft Actions.queueTask(new DestroyDraftTask(draftLocalId)) atom.close() if atom.state.mode is "composer" - _onSaveDraft: (paramsWithLocalId) -> - params = _.clone(paramsWithLocalId) - draftLocalId = params.localId - - if (not draftLocalId?) then throw new Error("Must call saveDraft with a localId") - delete params.localId - - if _.size(params) > 0 - task = new SaveDraftTask(draftLocalId, params) - Actions.queueTask(task) - _onSendDraft: (draftLocalId) -> - Actions.queueTask(new SendDraftTask(draftLocalId)) - atom.close() if atom.state.mode is "composer" + # Immediately save any pending changes so we don't save after sending + save = @_draftSessions[draftLocalId]?.changes.commit() ? Promise.resolve() + save.then -> + # Queue the task to send the draft + Actions.queueTask(new SendDraftTask(draftLocalId)) + atom.close() if atom.state.mode is "composer" - _findDraft: (draftLocalId) -> - new Promise (resolve, reject) -> - DatabaseStore.findByLocalId(Message, draftLocalId) - .then (draft) -> - if not draft? then reject("Can't find draft with id #{draftLocalId}") - else resolve(draft) - .catch (error) -> reject(error) - - # Receives: - # file: - A `File` object - # uploadData: - # messageLocalId - # filePath - # fileSize - # fileName - # bytesUploaded - # state - one of "started" "progress" "completed" "aborted" "failed" - _onFileUploaded: ({file, uploadData}) -> - @_findDraft(uploadData.messageLocalId) - .then (draft) -> - draft.files ?= [] - draft.files.push(file) - DatabaseStore.persistModel(draft) - Actions.queueTask(new SaveDraftTask(uploadData.messageLocalId)) - .catch (error) -> console.error(error, error.stack) + _onAttachFileComplete: ({file, messageLocalId}) -> + @sessionForLocalId(messageLocalId).prepare().then (proxy) -> + files = proxy.draft().files ? [] + files.push(file) + proxy.changes.add({files}, true) _onRemoveFile: ({file, messageLocalId}) -> - @_findDraft(messageLocalId) - .then (draft) -> - draft.files ?= [] - draft.files = _.reject draft.files, (f) -> f.id is file.id - DatabaseStore.persistModel(draft) - Actions.queueTask(new SaveDraftTask(uploadData.messageLocalId)) - .catch (error) -> console.error(error, error.stack) + @sessionForLocalId(messageLocalId).prepare().then (proxy) -> + files = proxy.draft().files ? [] + files = _.reject files, (f) -> f.id is file.id + proxy.changes.add({files}, true) diff --git a/src/flux/stores/file-upload-store.coffee b/src/flux/stores/file-upload-store.coffee index 8b63a6750..5f5a844ee 100644 --- a/src/flux/stores/file-upload-store.coffee +++ b/src/flux/stores/file-upload-store.coffee @@ -35,17 +35,14 @@ FileUploadStore = Reflux.createStore _onAttachFile: ({messageLocalId}) -> @_verifyId(messageLocalId) - unlistenOpen = Actions.openPathsSelected.listen (pathsToOpen=[]) -> - unlistenOpen?() + # When the dialog closes, it triggers `Actions.pathsToOpen` + atom.showOpenDialog {properties: ['openFile', 'multiSelections']}, (pathsToOpen) -> pathsToOpen = [pathsToOpen] if _.isString(pathsToOpen) for path in pathsToOpen # When this task runs, we expect to hear `uploadStateChanged` # Actions. Actions.queueTask(new FileUploadTask(path, messageLocalId)) - # When the dialog closes, it triggers `Actions.pathsToOpen` - ipc.send('command', 'application:open-file-to-window') - # Receives: # uploadData: # messageLocalId - The localId of the message (draft) we're uploading to diff --git a/src/flux/stores/task-queue.coffee b/src/flux/stores/task-queue.coffee index 69c980689..bc0301e8e 100644 --- a/src/flux/stores/task-queue.coffee +++ b/src/flux/stores/task-queue.coffee @@ -55,7 +55,6 @@ TaskQueue = Reflux.createStore throw new Error("You must queue a `Task` object") @_initializeTask(task) - @_dequeueObsoleteTasks(task) @_queue.push(task) @_update() if not silent @@ -63,10 +62,7 @@ TaskQueue = Reflux.createStore dequeue: (taskOrId={}, {silent}={}) -> task = @_parseArgs(taskOrId) - task.abort() if @_shouldAbort(task) - task.queueState.isProcessing = false - task.cleanup() @_queue.splice(@_queue.indexOf(task), 1) @@ -116,14 +112,15 @@ TaskQueue = Reflux.createStore @_processQueue() _dequeueObsoleteTasks: (task) -> - for otherTask in @_queue - if otherTask? and task.shouldDequeueOtherTask(otherTask) + for otherTask in @_queue by -1 + # Do not interrupt tasks which are currently processing + continue if otherTask.queueState.isProcessing + # Do not remove ourselves from the queue + continue if otherTask is task + # Dequeue tasks which our new task indicates it makes obsolete + if task.shouldDequeueOtherTask(otherTask) @dequeue(otherTask, silent: true) - _shouldAbort: (task) -> - task.queueState.isProcessing or - (task.queueState.performedLocal and not task.queueState.performedRemote) - _taskIsBlocked: (task) -> _.any @_queue, (otherTask) -> task.shouldWaitForTask(otherTask) and task isnt otherTask diff --git a/src/flux/tasks/destroy-draft.coffee b/src/flux/tasks/destroy-draft.coffee index b0b1151d2..66fb04d92 100644 --- a/src/flux/tasks/destroy-draft.coffee +++ b/src/flux/tasks/destroy-draft.coffee @@ -3,7 +3,7 @@ Message = require '../models/message' DatabaseStore = require '../stores/database-store' Actions = require '../actions' -SaveDraftTask = require './save-draft' +SyncbackDraftTask = require './syncback-draft' SendDraftTask = require './send-draft' FileUploadTask = require './file-upload-task' @@ -12,12 +12,12 @@ class DestroyDraftTask extends Task constructor: (@draftLocalId) -> super shouldDequeueOtherTask: (other) -> - (other instanceof SaveDraftTask and other.draftLocalId is @draftLocalId) or + (other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId) or (other instanceof SendDraftTask and other.draftLocalId is @draftLocalId) or (other instanceof FileUploadTask and other.draftLocalId is @draftLocalId) shouldWaitForTask: (other) -> - (other instanceof SaveDraftTask and other.draftLocalId is @draftLocalId) + (other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId) performLocal: -> new Promise (resolve, reject) => @@ -25,8 +25,8 @@ class DestroyDraftTask extends Task return reject(new Error("Attempt to call DestroyDraftTask.performLocal without @draftLocalId")) DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - DatabaseStore.unpersistModel(draft).then(resolve) @draft = draft + DatabaseStore.unpersistModel(draft).then(resolve) performRemote: -> new Promise (resolve, reject) => @@ -47,7 +47,7 @@ class DestroyDraftTask extends Task onAPIError: (apiError) -> inboxMsg = apiError.body?.message ? "" - if inboxMsg.indexOf("No draft found") >= 0 + if apiError.statusCode is 404 # Draft has already been deleted, this is not really an error return true else if inboxMsg.indexOf("is not a draft") >= 0 diff --git a/src/flux/tasks/file-upload-task.coffee b/src/flux/tasks/file-upload-task.coffee index 0098d33b9..24cce47cc 100644 --- a/src/flux/tasks/file-upload-task.coffee +++ b/src/flux/tasks/file-upload-task.coffee @@ -11,8 +11,8 @@ DatabaseStore = require '../stores/database-store' class FileUploadTask extends Task constructor: (@filePath, @messageLocalId) -> - @progress = null # The progress checking timer. super + @progress = null # The progress checking timer. performLocal: -> return Promise.reject(new Error("Must pass an absolute path to upload")) unless @filePath?.length @@ -30,50 +30,57 @@ class FileUploadTask extends Task json: false returnsModel: true formData: @_formData() - success: (json) => @_onUploadSuccess(json, resolve) error: reject + success: (json) => + # The Inbox API returns the file json wrapped in an array + file = (new File).fromJSON(json[0]) + Actions.uploadStateChanged @_uploadData("completed") + @_completedNotification(file) + + clearInterval(@progress) + @req = null + resolve() @progress = setInterval => Actions.uploadStateChanged(@_uploadData("progress")) , 250 - abort: -> - @req?.abort() - clearInterval(@progress) - Actions.uploadStateChanged(@_uploadData("aborted")) + cleanup: -> + super - setTimeout => - Actions.fileAborted(@_uploadData("aborted")) - , 1000 # To see the aborted state for a little bit + # If the request is still in progress, notify observers that + # we've failed. + if @req + @req.abort() + clearInterval(@progress) + Actions.uploadStateChanged(@_uploadData("aborted")) + setTimeout => + # To see the aborted state for a little bit + Actions.fileAborted(@_uploadData("aborted")) + , 1000 + + onAPIError: (apiError) -> + @_rollbackLocal() + + onOtherError: (otherError) -> + @_rollbackLocal() + + onTimeoutError: -> + # Do nothing. It could take a while. Promise.resolve() - onAPIError: (apiError) -> @_rollbackLocal() - onOtherError: (otherError) -> @_rollbackLocal() - - onTimeoutError: -> Promise.resolve() # Do nothing. It could take a while. - onOfflineError: (offlineError) -> msg = "You can't upload a file while you're offline." @_rollbackLocal(msg) _rollbackLocal: (msg) -> clearInterval(@progress) + @req = null + msg ?= "There was a problem uploading this file. Please try again later." Actions.postNotification({message: msg, type: "error"}) Actions.uploadStateChanged @_uploadData("failed") - _onUploadSuccess: (json, taskCallback) -> - clearInterval(@progress) - - # The Inbox API returns the file json wrapped in an array - file = (new File).fromJSON(json[0]) - - Actions.uploadStateChanged @_uploadData("completed") - - @_completedNotification(file) - - taskCallback() - # The `persistUploadFile` action affects the Database and should only be # heard in the main window. # @@ -81,11 +88,8 @@ class FileUploadTask extends Task # composers) that the file has finished uploading. _completedNotification: (file) -> setTimeout => - Actions.persistUploadedFile - file: file - uploadData: @_uploadData("completed") - Actions.fileUploaded - uploadData: @_uploadData("completed") + Actions.attachFileComplete({file, @messageLocalId}) + Actions.fileUploaded(uploadData: @_uploadData("completed")) , 1000 # To see the success state for a little bit _formData: -> diff --git a/src/flux/tasks/send-draft.coffee b/src/flux/tasks/send-draft.coffee index 93ea132fb..8b286065e 100644 --- a/src/flux/tasks/send-draft.coffee +++ b/src/flux/tasks/send-draft.coffee @@ -4,7 +4,7 @@ Actions = require '../actions' DatabaseStore = require '../stores/database-store' Message = require '../models/message' Task = require './task' -SaveDraftTask = require './save-draft' +SyncbackDraftTask = require './syncback-draft' module.exports = class SendDraftTask extends Task @@ -15,7 +15,7 @@ class SendDraftTask extends Task other instanceof SendDraftTask and other.draftLocalId is @draftLocalId shouldWaitForTask: (other) -> - other instanceof SaveDraftTask and other.draftLocalId is @draftLocalId + other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId performLocal: -> # When we send drafts, we don't update anything in the app until @@ -31,15 +31,19 @@ class SendDraftTask extends Task # recent draft version DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) -> # The draft may have been deleted by another task. Nothing we can do. - return reject(new Error("We couldn't find the saved draft. Please try again in a couple seconds")) unless draft - return reject(new Error("Cannot send draft that is not saved!")) unless draft.isSaved() + return reject(new Error("We couldn't find the saved draft.")) unless draft + + if draft.isSaved() + body = + draft_id: draft.id + version: draft.version + else + body = draft.toJSON() atom.inbox.makeRequest path: "/n/#{draft.namespaceId}/send" method: 'POST' - body: - draft_id: draft.id - version: draft.version + body: body returnsModel: true success: -> atom.playSound('mail_sent.ogg') @@ -49,15 +53,15 @@ class SendDraftTask extends Task .catch(reject) onAPIError: -> - msg = "Our server is having problems. Your messages has NOT been sent" + msg = "Our server is having problems. Your message has not been sent." @notifyErrorMessage(msg) onOtherError: -> - msg = "We had a serious issue while sending. Your messages has NOT been sent" + msg = "We had a serious issue while sending. Your message has not been sent." @notifyErrorMessage(msg) onTimeoutError: -> - msg = "The server is taking an abnormally long time to respond. Your messages has NOT been sent" + msg = "The server is taking an abnormally long time to respond. Your message has not been sent." @notifyErrorMessage(msg) onOfflineError: -> diff --git a/src/flux/tasks/save-draft.coffee b/src/flux/tasks/syncback-draft.coffee similarity index 69% rename from src/flux/tasks/save-draft.coffee rename to src/flux/tasks/syncback-draft.coffee index 4c1457da0..4b6597c9e 100644 --- a/src/flux/tasks/save-draft.coffee +++ b/src/flux/tasks/syncback-draft.coffee @@ -9,48 +9,33 @@ Message = require '../models/message' FileUploadTask = require './file-upload-task' +# MutateDraftTask + module.exports = -class SaveDraftTask extends Task +class SyncbackDraftTask extends Task - constructor: (@draftLocalId, @changes={}, {@localOnly}={}) -> - @_saveAttempts = 0 - @queuedAt = Date.now() + constructor: (@draftLocalId) -> super + @_saveAttempts = 0 - # We also don't want to cancel any tasks that have a later timestamp - # creation than us. It's possible, because of retries, that tasks could - # get re-pushed onto the queue out of order. shouldDequeueOtherTask: (other) -> - other instanceof SaveDraftTask and - other.draftLocalId is @draftLocalId and - other.queuedAt < @queuedAt # other is an older task. + other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId and other.creationDate < @creationDate shouldWaitForTask: (other) -> - other instanceof FileUploadTask and other.draftLocalId is @draftLocalId + other instanceof SyncbackDraftTask and other.draftLocalId is @draftLocalId and other.creationDate < @creationDate - performLocal: -> new Promise (resolve, reject) => - if not @draftLocalId? - errMsg = "Attempt to call FileUploadTask.performLocal without @draftLocalId" - return reject(new Error(errMsg)) - - DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => - if not draft? - # This can happen if a save draft task is queued after it has been - # destroyed. Nothing we can really do about it, so ignore this. - resolve() - else if _.size(@changes) is 0 - resolve() - else - updatedDraft = @_applyChangesToDraft(draft, @changes) - DatabaseStore.persistModel(updatedDraft).then(resolve) - .catch(reject) + performLocal: -> + # SyncbackDraftTask does not do anything locally. You should persist your changes + # to the local database directly or using a DraftStoreProxy, and then queue a + # SyncbackDraftTask to send those changes to the server. + if not @draftLocalId? + errMsg = "Attempt to call FileUploadTask.performLocal without @draftLocalId" + Promise.reject(new Error(errMsg)) + else + Promise.resolve() performRemote: -> - if @localOnly then return Promise.resolve() - new Promise (resolve, reject) => - # Fetch the latest draft data to make sure we make the request with the most - # recent draft version DatabaseStore.findByLocalId(Message, @draftLocalId).then (draft) => # The draft may have been deleted by another task. Nothing we can do. return resolve() unless draft @@ -74,11 +59,11 @@ class SaveDraftTask extends Task body: body returnsModel: false success: (json) => - newDraft = (new Message).fromJSON(json) - if newDraft.id != initialId + if json.id != initialId + newDraft = (new Message).fromJSON(json) DatabaseStore.swapModel(oldModel: draft, newModel: newDraft, localId: @draftLocalId).then(resolve) else - DatabaseStore.persistModel(newDraft).then(resolve) + DatabaseStore.persistModel(draft).then(resolve) error: reject onAPIError: (apiError) -> @@ -139,7 +124,3 @@ class SaveDraftTask extends Task @notifyErrorMessage(msg) - _applyChangesToDraft: (draft, changes={}) -> - for key, definition of draft.attributes() - draft[key] = changes[key] if changes[key]? - return draft diff --git a/src/flux/tasks/task.coffee b/src/flux/tasks/task.coffee index b653cd7bd..56758641b 100644 --- a/src/flux/tasks/task.coffee +++ b/src/flux/tasks/task.coffee @@ -39,7 +39,9 @@ Actions = require '../actions' class Task ## These are commonly overridden ## - constructor: -> @id = generateTempId() + constructor: -> + @id = generateTempId() + @creationDate = new Date() performLocal: -> Promise.resolve()