From da2dbe43c96b35b7c751f3de3b9cb92ed720232b Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Wed, 5 Jul 2017 10:34:10 -0700 Subject: [PATCH] Send drafts through the new mailsync process --- .../composer/lib/composer-view.jsx | 3 +- .../draft-list/lib/draft-list.cjsx | 2 +- .../draft-list/lib/draft-toolbar-buttons.cjsx | 2 +- .../lib/page-account-settings-imap.jsx | 9 +- .../lib/send-and-archive-extension.es6 | 2 +- .../spec/tasks/base-draft-task-spec.es6 | 117 --------------- .../decorators/inflates-draft-client-id.jsx | 3 +- .../src/flux/stores/draft-store.es6 | 13 +- .../src/flux/stores/send-actions-store.es6 | 2 +- .../src/flux/tasks/base-draft-task.es6 | 10 -- .../src/flux/tasks/destroy-draft-task.es6 | 7 +- .../src/flux/tasks/send-draft-task.es6 | 134 ++---------------- .../src/flux/tasks/syncback-draft-task.es6 | 11 +- packages/client-app/src/nylas-env.es6 | 2 +- 14 files changed, 44 insertions(+), 273 deletions(-) delete mode 100644 packages/client-app/spec/tasks/base-draft-task-spec.es6 delete mode 100644 packages/client-app/src/flux/tasks/base-draft-task.es6 diff --git a/packages/client-app/internal_packages/composer/lib/composer-view.jsx b/packages/client-app/internal_packages/composer/lib/composer-view.jsx index f5ca6541f..b919e3ac8 100644 --- a/packages/client-app/internal_packages/composer/lib/composer-view.jsx +++ b/packages/client-app/internal_packages/composer/lib/composer-view.jsx @@ -559,7 +559,8 @@ export default class ComposerView extends React.Component { } _onDestroyDraft = () => { - Actions.destroyDraft(this.props.draft.headerMessageId); + const {draft} = this.props; + Actions.destroyDraft(draft.accountId, draft.headerMessageId); } _onSelectAttachment = () => { diff --git a/packages/client-app/internal_packages/draft-list/lib/draft-list.cjsx b/packages/client-app/internal_packages/draft-list/lib/draft-list.cjsx index ddf37fd52..24321f7ae 100644 --- a/packages/client-app/internal_packages/draft-list/lib/draft-list.cjsx +++ b/packages/client-app/internal_packages/draft-list/lib/draft-list.cjsx @@ -47,6 +47,6 @@ class DraftList extends React.Component _onRemoveFromView: => drafts = DraftListStore.dataSource().selection.items() - Actions.destroyDraft(draft.headerMessageId) for draft in drafts + Actions.destroyDraft(draft.accountId, draft.headerMessageId) for draft in drafts module.exports = DraftList diff --git a/packages/client-app/internal_packages/draft-list/lib/draft-toolbar-buttons.cjsx b/packages/client-app/internal_packages/draft-list/lib/draft-toolbar-buttons.cjsx index c6fc6ceef..81894ba52 100644 --- a/packages/client-app/internal_packages/draft-list/lib/draft-toolbar-buttons.cjsx +++ b/packages/client-app/internal_packages/draft-list/lib/draft-toolbar-buttons.cjsx @@ -19,7 +19,7 @@ class DraftDeleteButton extends React.Component _destroySelected: => for item in @props.selection.items() - Actions.destroyDraft(item.headerMessageId) + Actions.destroyDraft(item.accountId, item.headerMessageId) @props.selection.clear() return diff --git a/packages/client-app/internal_packages/onboarding/lib/page-account-settings-imap.jsx b/packages/client-app/internal_packages/onboarding/lib/page-account-settings-imap.jsx index e330131d8..a743dd532 100644 --- a/packages/client-app/internal_packages/onboarding/lib/page-account-settings-imap.jsx +++ b/packages/client-app/internal_packages/onboarding/lib/page-account-settings-imap.jsx @@ -39,10 +39,11 @@ class AccountIMAPSettingsForm extends React.Component { errorMessage = "Please provide a valid hostname or IP adddress."; errorFieldNames.push(`${type}_host`); } - if (accountInfo[`${type}_host`] === 'imap.gmail.com') { - errorMessage = "Please link Gmail accounts by choosing 'Google' on the account type screen."; - errorFieldNames.push(`${type}_host`); - } + // todo bg + // if (accountInfo[`${type}_host`] === 'imap.gmail.com') { + // errorMessage = "Please link Gmail accounts by choosing 'Google' on the account type screen."; + // errorFieldNames.push(`${type}_host`); + // } if (!Number.isInteger(accountInfo[`${type}_port`] / 1)) { errorMessage = "Please provide a valid port number."; errorFieldNames.push(`${type}_port`); diff --git a/packages/client-app/internal_packages/send-and-archive/lib/send-and-archive-extension.es6 b/packages/client-app/internal_packages/send-and-archive/lib/send-and-archive-extension.es6 index fdc883495..01ae8c2d7 100644 --- a/packages/client-app/internal_packages/send-and-archive/lib/send-and-archive-extension.es6 +++ b/packages/client-app/internal_packages/send-and-archive/lib/send-and-archive-extension.es6 @@ -17,7 +17,7 @@ export function sendActions() { return draft.threadId != null }, performSendAction({draft}) { - Actions.queueTask(new SendDraftTask(draft.id)) + Actions.queueTask(new SendDraftTask(draft)) return DatabaseStore.modelify(Thread, [draft.threadId]) .then((threads) => { const tasks = TaskFactory.tasksForArchiving({ diff --git a/packages/client-app/spec/tasks/base-draft-task-spec.es6 b/packages/client-app/spec/tasks/base-draft-task-spec.es6 deleted file mode 100644 index 3ad7780e0..000000000 --- a/packages/client-app/spec/tasks/base-draft-task-spec.es6 +++ /dev/null @@ -1,117 +0,0 @@ -import { - Message, - DatabaseStore, -} from 'nylas-exports'; - -import BaseDraftTask from '../../src/flux/tasks/base-draft-task'; - -xdescribe('BaseDraftTask', function baseDraftTask() { - describe("shouldDequeueOtherTask", () => { - it("should dequeue instances of the same subclass for the same draft which are older", () => { - class ATask extends BaseDraftTask { - - } - class BTask extends BaseDraftTask { - - } - - const A = new ATask('localid-A'); - A.sequentialId = 1; - const B1 = new BTask('localid-A'); - B1.sequentialId = 2; - const B2 = new BTask('localid-A'); - B2.sequentialId = 3; - const BOther = new BTask('localid-other'); - BOther.sequentialId = 4; - - expect(B1.shouldDequeueOtherTask(A)).toBe(false); - expect(A.shouldDequeueOtherTask(B1)).toBe(false); - - expect(B2.shouldDequeueOtherTask(B1)).toBe(true); - expect(B1.shouldDequeueOtherTask(B2)).toBe(false); - - expect(BOther.shouldDequeueOtherTask(B2)).toBe(false); - expect(B2.shouldDequeueOtherTask(BOther)).toBe(false); - }); - }); - - describe("isDependentOnTask", () => { - it("should always wait on older tasks for the same draft", () => { - const A = new BaseDraftTask('localid-A'); - A.sequentialId = 1; - const B = new BaseDraftTask('localid-A'); - B.sequentialId = 2; - expect(B.isDependentOnTask(A)).toBe(true); - }); - - it("should not wait on newer tasks for the same draft", () => { - const A = new BaseDraftTask('localid-A'); - A.sequentialId = 1; - const B = new BaseDraftTask('localid-A'); - B.sequentialId = 2; - expect(A.isDependentOnTask(B)).toBe(false) - }); - - it("should not wait on older tasks for other drafts", () => { - const A = new BaseDraftTask('localid-other'); - A.sequentialId = 1; - const B = new BaseDraftTask('localid-A'); - B.sequentialId = 2; - expect(A.isDependentOnTask(B)).toBe(false); - expect(B.isDependentOnTask(A)).toBe(false); - }); - }); - - describe("performLocal", () => { - it("rejects if we we don't pass a draft", () => { - const badTask = new BaseDraftTask(null) - badTask.performLocal().then(() => { - throw new Error("Shouldn't succeed") - }).catch((err) => { - expect(err.message).toBe("Attempt to call BaseDraftTask.performLocal without a headerMessageId") - }); - }); - }); - - describe("refreshDraftReference", () => { - it("should retrieve the draft by client ID, with the body, and assign it to @draft", () => { - const draft = new Message({draft: true}); - const A = new BaseDraftTask('localid-other'); - spyOn(DatabaseStore, 'run').andReturn(Promise.resolve(draft)); - waitsForPromise(() => { - return A.refreshDraftReference().then((resolvedValue) => { - expect(A.draft).toEqual(draft); - expect(resolvedValue).toEqual(draft); - - const query = DatabaseStore.run.mostRecentCall.args[0]; - expect(query.sql()).toEqual("SELECT `Message`.`data`, IFNULL(`MessageBody`.`value`, '!NULLVALUE!') AS `body` FROM `Message` LEFT OUTER JOIN `MessageBody` ON `MessageBody`.`id` = `Message`.`id` WHERE `Message`.`client_id` = 'localid-other' ORDER BY `Message`.`date` ASC LIMIT 1"); - }); - }); - }); - - it("should throw a DraftNotFoundError error if it the response was no longer a draft", () => { - const message = new Message({draft: false}); - const A = new BaseDraftTask('localid-other'); - spyOn(DatabaseStore, 'run').andReturn(Promise.resolve(message)); - waitsForPromise(() => { - return A.refreshDraftReference().then(() => { - throw new Error("Should not have resolved"); - }).catch((err) => { - expect(err instanceof BaseDraftTask.DraftNotFoundError).toBe(true); - }) - }); - }); - - it("should throw a DraftNotFoundError error if nothing was returned", () => { - const A = new BaseDraftTask('localid-other'); - spyOn(DatabaseStore, 'run').andReturn(Promise.resolve(null)); - waitsForPromise(() => { - return A.refreshDraftReference().then(() => { - throw new Error("Should not have resolved"); - }).catch((err) => { - expect(err instanceof BaseDraftTask.DraftNotFoundError).toBe(true); - }) - }); - }); - }); -}); diff --git a/packages/client-app/src/decorators/inflates-draft-client-id.jsx b/packages/client-app/src/decorators/inflates-draft-client-id.jsx index 5e02f3715..24120baa6 100644 --- a/packages/client-app/src/decorators/inflates-draft-client-id.jsx +++ b/packages/client-app/src/decorators/inflates-draft-client-id.jsx @@ -80,7 +80,8 @@ function InflatesDraftClientId(ComposedComponent) { return; } if (this.state.draft.pristine) { - Actions.destroyDraft(this.props.headerMessageId); + const {accountId, headerMessageId} = this.state.draft; + Actions.destroyDraft(accountId, headerMessageId); } } diff --git a/packages/client-app/src/flux/stores/draft-store.es6 b/packages/client-app/src/flux/stores/draft-store.es6 index 5d6a80148..cbd2f4765 100644 --- a/packages/client-app/src/flux/stores/draft-store.es6 +++ b/packages/client-app/src/flux/stores/draft-store.es6 @@ -7,8 +7,8 @@ import DraftFactory from './draft-factory'; import DatabaseStore from './database-store'; import SendActionsStore from './send-actions-store'; import FocusedContentStore from './focused-content-store'; -import BaseDraftTask from '../tasks/base-draft-task'; import SyncbackDraftTask from '../tasks/syncback-draft-task'; +import SendDraftTask from '../tasks/send-draft-task'; import DestroyDraftTask from '../tasks/destroy-draft-task'; import Thread from '../models/thread'; import Message from '../models/message'; @@ -127,7 +127,7 @@ class DraftStore extends NylasStore { _.each(this._draftSessions, (session) => { const draft = session.draft() if (draft && draft.pristine) { - Actions.queueTask(new DestroyDraftTask(session.headerMessageId)); + Actions.queueTask(new DestroyDraftTask(draft.accountId, draft.headerMessageId)); } else { promises.push(session.changes.commit()); } @@ -335,7 +335,7 @@ class DraftStore extends NylasStore { }); } - _onDestroyDraft = (headerMessageId) => { + _onDestroyDraft = (accountId, headerMessageId) => { const session = this._draftSessions[headerMessageId]; // Immediately reset any pending changes so no saves occur @@ -345,13 +345,16 @@ class DraftStore extends NylasStore { // Stop any pending tasks related ot the draft TaskQueue.queue().forEach((task) => { - if (task instanceof BaseDraftTask && task.headerMessageId === headerMessageId) { + if (task instanceof SyncbackDraftTask && task.headerMessageId === headerMessageId) { + Actions.dequeueTask(task.id); + } + if (task instanceof SendDraftTask && task.headerMessageId === headerMessageId) { Actions.dequeueTask(task.id); } }) // Queue the task to destroy the draft - Actions.queueTask(new DestroyDraftTask(headerMessageId)); + Actions.queueTask(new DestroyDraftTask(accountId, headerMessageId)); if (NylasEnv.isComposerWindow()) { NylasEnv.close(); diff --git a/packages/client-app/src/flux/stores/send-actions-store.es6 b/packages/client-app/src/flux/stores/send-actions-store.es6 index fe005a5a8..2de003842 100644 --- a/packages/client-app/src/flux/stores/send-actions-store.es6 +++ b/packages/client-app/src/flux/stores/send-actions-store.es6 @@ -13,7 +13,7 @@ const DefaultSendAction = { iconUrl: null, configKey: DefaultSendActionKey, isAvailableForDraft: () => true, - performSendAction: ({draft}) => Actions.queueTask(new SendDraftTask(draft.id)), + performSendAction: ({draft}) => Actions.queueTask(new SendDraftTask(draft)), } function verifySendAction(sendAction = {}, extension = {}) { diff --git a/packages/client-app/src/flux/tasks/base-draft-task.es6 b/packages/client-app/src/flux/tasks/base-draft-task.es6 deleted file mode 100644 index 607b80fda..000000000 --- a/packages/client-app/src/flux/tasks/base-draft-task.es6 +++ /dev/null @@ -1,10 +0,0 @@ -import Task from './task'; -import DraftHelpers from '../stores/draft-helpers'; - -export default class BaseDraftTask extends Task { - - constructor(draft) { - super(); - this.draft = draft; - } -} diff --git a/packages/client-app/src/flux/tasks/destroy-draft-task.es6 b/packages/client-app/src/flux/tasks/destroy-draft-task.es6 index f5761ecda..b596f67b6 100644 --- a/packages/client-app/src/flux/tasks/destroy-draft-task.es6 +++ b/packages/client-app/src/flux/tasks/destroy-draft-task.es6 @@ -1,8 +1,9 @@ -import BaseDraftTask from './base-draft-task'; +import Task from './task'; -export default class DestroyDraftTask extends BaseDraftTask { - constructor(headerMessageId) { +export default class DestroyDraftTask extends Task { + constructor(accountId, headerMessageId) { super(); + this.accountId = accountId; this.headerMessageId = headerMessageId; } } diff --git a/packages/client-app/src/flux/tasks/send-draft-task.es6 b/packages/client-app/src/flux/tasks/send-draft-task.es6 index fb7d4d565..5f62d15d9 100644 --- a/packages/client-app/src/flux/tasks/send-draft-task.es6 +++ b/packages/client-app/src/flux/tasks/send-draft-task.es6 @@ -1,27 +1,19 @@ /* eslint global-require: 0 */ -import Task from './task'; -import Actions from '../actions'; -import Message from '../models/message'; -import Account from '../models/account'; -import NylasAPI from '../nylas-api'; -import * as NylasAPIHelpers from '../nylas-api-helpers'; -import {APIError, RequestEnsureOnceError} from '../errors'; -import SoundRegistry from '../../registries/sound-registry'; -import DatabaseStore from '../stores/database-store'; import AccountStore from '../stores/account-store'; -import BaseDraftTask from './base-draft-task'; -import SyncbackMetadataTask from './syncback-metadata-task'; -import EnsureMessageInSentFolderTask from './ensure-message-in-sent-folder-task'; +import Task from './task'; const OPEN_TRACKING_ID = NylasEnv.packages.pluginIdFor('open-tracking') const LINK_TRACKING_ID = NylasEnv.packages.pluginIdFor('link-tracking') -export default class SendDraftTask extends BaseDraftTask { - constructor(headerMessageId, {playSound = true, emitError = true, allowMultiSend = true} = {}) { - super(headerMessageId); - this.draft = null; - this.message = null; +export default class SendDraftTask extends Task { + + constructor(draft, {playSound = true, emitError = true, allowMultiSend = true} = {}) { + super(); + this.draft = draft; + this.accountId = (draft || {}).accountId; + this.headerMessageId = (draft || {}).headerMessageId; + this.emitError = emitError this.playSound = playSound this.allowMultiSend = allowMultiSend @@ -31,16 +23,6 @@ export default class SendDraftTask extends BaseDraftTask { return "Sending message"; } - performRemote() { - return this.refreshDraftReference() - .then(this.assertDraftValidity) - .then(this.sendMessage) - .then(this.ensureInSentFolder) - .then(this.updatePluginMetadata) - .then(this.onSuccess) - .catch(this.onError); - } - assertDraftValidity = () => { if (!this.draft.from[0]) { return Promise.reject(new Error("SendDraftTask - you must populate `from` before sending.")); @@ -64,99 +46,6 @@ export default class SendDraftTask extends BaseDraftTask { return (!!this.draft.metadataForPluginId(OPEN_TRACKING_ID) || !!this.draft.metadataForPluginId(LINK_TRACKING_ID)) || false; } - hasCustomBodyPerRecipient = () => { - if (!this.allowMultiSend) { - return false; - } - - // Sending individual bodies for too many participants can cause us - // to hit the smtp rate limit. - const participants = this.draft.participants({includeFrom: false, includeBcc: true}) - if (participants.length === 1 || participants.length > 10) { - return false; - } - - const providerCompatible = (AccountStore.accountForId(this.draft.accountId).provider !== "eas"); - return this._trackingPluginsInUse() && providerCompatible; - } - - sendMessage = async () => { - if (this.hasCustomBodyPerRecipient()) { - await this._sendPerRecipient(); - } else { - await this._sendWithSingleBody() - } - } - - ensureInSentFolder = () => { - const t = new EnsureMessageInSentFolderTask({ - message: this.message, - customSentMessage: this.hasCustomBodyPerRecipient() || this._trackingPluginsInUse(), - }) - Actions.queueTask(t) - } - - _sendWithSingleBody = async () => { - let responseJSON = {} - if (this._syncbackRequestId) { - responseJSON = await SyncbackTaskAPIRequest.waitForQueuedRequest(this._syncbackRequestId) - } else { - const task = new SyncbackTaskAPIRequest({ - api: NylasAPI, - options: { - path: "/send", - accountId: this.draft.accountId, - method: 'POST', - body: this.draft.toJSON(), - timeout: 1000 * 60 * 5, // We cannot hang up a send - won't know if it sent - requestId: this.draft.id, - onSyncbackRequestCreated: (syncbackRequest) => { - this._syncbackRequestId = syncbackRequest.id - }, - }, - }) - responseJSON = await task.run(); - } - await this._createMessageFromResponse(responseJSON) - } - - _sendPerRecipient = async () => { - let responseJSON = {} - if (this._syncbackRequestId) { - responseJSON = await SyncbackTaskAPIRequest.waitForQueuedRequest(this._syncbackRequestId) - } else { - const task = new SyncbackTaskAPIRequest({ - api: NylasAPI, - options: { - path: "/send-per-recipient", - accountId: this.draft.accountId, - method: 'POST', - body: { - message: this.draft.toJSON(), - uses_open_tracking: this.draft.metadataForPluginId(OPEN_TRACKING_ID) != null, - uses_link_tracking: this.draft.metadataForPluginId(LINK_TRACKING_ID) != null, - }, - timeout: 1000 * 60 * 5, // We cannot hang up a send - won't know if it sent - onSyncbackRequestCreated: (syncbackRequest) => { - this._syncbackRequestId = syncbackRequest.id - }, - }, - }) - responseJSON = await task.run(); - } - await this._createMessageFromResponse(responseJSON); - } - - updatePluginMetadata = () => { - this.message.pluginMetadata.forEach((m) => { - const t1 = new SyncbackMetadataTask(this.message.id, - this.message.constructor.name, m.pluginId); - Actions.queueTask(t1); - }); - - return Promise.resolve(); - } - _createMessageFromResponse = (responseJSON) => { const {failedRecipients, message} = responseJSON if (failedRecipients && failedRecipients.length > 0) { @@ -201,10 +90,6 @@ export default class SendDraftTask extends BaseDraftTask { } onError = (err) => { - if (err instanceof BaseDraftTask.DraftNotFoundError) { - return Promise.resolve(Task.Status.Continue); - } - let message = err.message; // TODO Handle errors in a cleaner way @@ -259,4 +144,5 @@ export default class SendDraftTask extends BaseDraftTask { return Promise.resolve([Task.Status.Failed, err]); } + } diff --git a/packages/client-app/src/flux/tasks/syncback-draft-task.es6 b/packages/client-app/src/flux/tasks/syncback-draft-task.es6 index 0e22f3ed1..8ef2b460a 100644 --- a/packages/client-app/src/flux/tasks/syncback-draft-task.es6 +++ b/packages/client-app/src/flux/tasks/syncback-draft-task.es6 @@ -1,5 +1,10 @@ -import BaseDraftTask from './base-draft-task'; - -export default class SyncbackDraftTask extends BaseDraftTask { +import Task from './task'; +export default class SyncbackDraftTask extends Task { + constructor(draft) { + super(); + this.draft = draft; + this.accountId = (draft || {}).accountId; + this.headerMessageId = (draft || {}).headerMessageId; + } } diff --git a/packages/client-app/src/nylas-env.es6 b/packages/client-app/src/nylas-env.es6 index c87d5ec1b..b5237b4d2 100644 --- a/packages/client-app/src/nylas-env.es6 +++ b/packages/client-app/src/nylas-env.es6 @@ -371,7 +371,7 @@ export default class NylasEnvConstructor { if (event.defaultPrevented) { return; } this.lastUncaughtError = error; - + try { extra.pluginIds = this._findPluginsFromError(error); } catch (err) {