Foundry376/Mailspring
1
1
Fork 0
mirror of https://github.com/Foundry376/Mailspring.git synced 2025-01-30 11:59:02 +08:00
Code Issues Projects Releases Packages Wiki Activity

feat(send)

Browse source 
refactor(send): split delivery from sent folder stuffing

Summary:
See explanation in https://phab.nylas.com/D3577

Depends on D3577

Test Plan: manual

Reviewers: jackie, juan, halla

Reviewed By: juan, halla

Differential Revision: https://phab.nylas.com/D3578
This commit is contained in:
Evan Morikawa 2017-01-04 15:42:19 -08:00
parent 3d12b36294
commit 16cab4272b
11 changed files with 81 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
spec/stores/draft-store-spec.es6
View file

@ -412,7 +412,7 @@ xdescribe('DraftStore', function draftStore() {
it("resets the sending state if there's an error", () => {
spyOn(NylasEnv, "isMainWindow").andReturn(false);
DraftStore._draftsSending[this.draft.clientId] = true;
Actions.sendDraftFailed({errorMessage: "boohoo", draftClientId: this.draft.clientId});
Actions.draftDeliveryFailed({errorMessage: "boohoo", draftClientId: this.draft.clientId});
expect(DraftStore.isSendingDraft(this.draft.clientId)).toBe(false);
expect(DraftStore.trigger).toHaveBeenCalledWith(this.draft.clientId);
});
@ -423,7 +423,7 @@ xdescribe('DraftStore', function draftStore() {
spyOn(remote.dialog, "showMessageBox");
spyOn(Actions, "composePopoutDraft");
DraftStore._draftsSending[this.draft.clientId] = true;
Actions.sendDraftFailed({threadId: 't1', errorMessage: "boohoo", draftClientId: this.draft.clientId});
Actions.draftDeliveryFailed({threadId: 't1', errorMessage: "boohoo", draftClientId: this.draft.clientId});
advanceClock(400);
expect(DraftStore.isSendingDraft(this.draft.clientId)).toBe(false);
expect(DraftStore.trigger).toHaveBeenCalledWith(this.draft.clientId);
@ -438,7 +438,7 @@ xdescribe('DraftStore', function draftStore() {
spyOn(FocusedContentStore, "focused").andReturn({id: "t1"});
spyOn(Actions, "composePopoutDraft");
DraftStore._draftsSending[this.draft.clientId] = true;
Actions.sendDraftFailed({threadId: 't2', errorMessage: "boohoo", draftClientId: this.draft.clientId});
Actions.draftDeliveryFailed({threadId: 't2', errorMessage: "boohoo", draftClientId: this.draft.clientId});
advanceClock(400);
expect(Actions.composePopoutDraft).toHaveBeenCalled();
const call = Actions.composePopoutDraft.calls[0];
@ -451,7 +451,7 @@ xdescribe('DraftStore', function draftStore() {
spyOn(Actions, "composePopoutDraft");
DraftStore._draftsSending[this.draft.clientId] = true;
spyOn(FocusedContentStore, "focused").andReturn(null);
Actions.sendDraftFailed({errorMessage: "boohoo", draftClientId: this.draft.clientId});
Actions.draftDeliveryFailed({errorMessage: "boohoo", draftClientId: this.draft.clientId});
advanceClock(400);
expect(Actions.composePopoutDraft).toHaveBeenCalled();
const call = Actions.composePopoutDraft.calls[0];

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

@ -133,7 +133,7 @@ xdescribe 'FileUploadStore', ->
describe "when a draft is sent", ->
it "should delete its uploads directory", ->
spyOn(FileUploadStore, '_deleteUploadsForClientId')
Actions.sendDraftSuccess({messageClientId: '123'})
Actions.ensureMessageInSentSuccess({messageClientId: '123'})
expect(FileUploadStore._deleteUploadsForClientId).toHaveBeenCalledWith('123')
describe '_getFileStats', ->

40
spec/tasks/send-draft-task-spec.es6
View file

@ -93,7 +93,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
spyOn(DBt, 'unpersistModel').andReturn(Promise.resolve());
spyOn(DBt, 'persistModel').andReturn(Promise.resolve());
spyOn(SoundRegistry, "playSound");
spyOn(Actions, "sendDraftSuccess");
spyOn(Actions, "draftDeliverySucceeded");
});
// The tests below are invoked twice, once with a new this.draft and one with a
@ -165,7 +165,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
it("should notify the draft was sent", () => {
waitsForPromise(() => this.task.performRemote().then(() => {
const args = Actions.sendDraftSuccess.calls[0].args[0];
const args = Actions.draftDeliverySucceeded.calls[0].args[0];
expect(args.message instanceof Message).toBe(true)
expect(args.messageClientId).toBe(this.draft.clientId)
}));
@ -203,7 +203,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
describe("when there are errors", () => {
beforeEach(() => {
spyOn(Actions, 'sendDraftFailed');
spyOn(Actions, 'draftDeliveryFailed');
jasmine.unspy(NylasAPIRequest.prototype, "run");
});
@ -219,7 +219,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
waitsForPromise(() => this.task.performRemote().then((status) => {
expect(status[0]).toBe(Task.Status.Failed);
expect(status[1]).toBe(thrownError);
expect(Actions.sendDraftFailed).toHaveBeenCalled();
expect(Actions.draftDeliveryFailed).toHaveBeenCalled();
expect(NylasEnv.reportError).toHaveBeenCalled();
}));
});
@ -284,7 +284,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
waitsForPromise(() => this.task.performRemote().then((status) => {
expect(status[0]).toBe(Task.Status.Failed);
expect(status[1]).toBe(thrownError);
expect(Actions.sendDraftFailed).toHaveBeenCalled();
expect(Actions.draftDeliveryFailed).toHaveBeenCalled();
}));
});
@ -296,7 +296,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
waitsForPromise(() => this.task.performRemote().then((status) => {
expect(status[0]).toBe(Task.Status.Failed);
expect(status[1]).toBe(thrownError);
expect(Actions.sendDraftFailed).toHaveBeenCalled();
expect(Actions.draftDeliveryFailed).toHaveBeenCalled();
}));
});
@ -325,9 +325,9 @@ xdescribe('SendDraftTask', function sendDraftTask() {
waitsForPromise(() => this.task.performRemote().then((status) => {
expect(status[0]).toBe(Task.Status.Failed);
expect(status[1]).toBe(thrownError);
expect(Actions.sendDraftFailed).toHaveBeenCalled();
expect(Actions.draftDeliveryFailed).toHaveBeenCalled();
const msg = Actions.sendDraftFailed.calls[0].args[0].errorMessage;
const msg = Actions.draftDeliveryFailed.calls[0].args[0].errorMessage;
expect(withoutWhitespace(msg)).toEqual(withoutWhitespace(expectedMessage));
}));
});
@ -349,9 +349,9 @@ xdescribe('SendDraftTask', function sendDraftTask() {
waitsForPromise(() => this.task.performRemote().then((status) => {
expect(status[0]).toBe(Task.Status.Failed);
expect(status[1]).toBe(thrownError);
expect(Actions.sendDraftFailed).toHaveBeenCalled();
expect(Actions.draftDeliveryFailed).toHaveBeenCalled();
const msg = Actions.sendDraftFailed.calls[0].args[0].errorMessage;
const msg = Actions.draftDeliveryFailed.calls[0].args[0].errorMessage;
expect(withoutWhitespace(msg)).toEqual(withoutWhitespace(expectedMessage));
}));
});
@ -509,7 +509,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
});
});
describe("usingMultiSend", () => {
describe("hasCustomBodyPerRecipient", () => {
beforeEach(() => {
this.task = new SendDraftTask('client-id');
this.task.allowMultiSend = true;
@ -545,13 +545,13 @@ xdescribe('SendDraftTask', function sendDraftTask() {
it("should return false if the provider is eas", () => {
this.applySpies({"AccountStore.accountForId": "eas"})
expect(this.task.usingMultiSend()).toBe(false);
expect(this.task.hasCustomBodyPerRecipient()).toBe(false);
});
it("should return false if allowMultiSend is false", () => {
this.applySpies();
this.task.allowMultiSend = false;
expect(this.task.usingMultiSend()).toBe(false);
expect(this.task.hasCustomBodyPerRecipient()).toBe(false);
});
it("should return false if the open-tracking id is null", () => {
@ -559,7 +559,7 @@ xdescribe('SendDraftTask', function sendDraftTask() {
return name === "open-tracking" ? null : name;
};
this.applySpies({"NylasEnv.packages.pluginIdFor": fake});
expect(this.task.usingMultiSend()).toBe(false);
expect(this.task.hasCustomBodyPerRecipient()).toBe(false);
});
it("should return false if the link-tracking id is null", () => {
@ -567,36 +567,36 @@ xdescribe('SendDraftTask', function sendDraftTask() {
return name === "link-tracking" ? null : name;
};
this.applySpies({"NylasEnv.packages.pluginIdFor": fake});
expect(this.task.usingMultiSend()).toBe(false);
expect(this.task.hasCustomBodyPerRecipient()).toBe(false);
});
it("should return false if neither open-tracking nor link-tracking is on", () => {
this.applySpies();
this.task.draft.applyPluginMetadata('open-tracking', false);
this.task.draft.applyPluginMetadata('link-tracking', false);
expect(this.task.usingMultiSend()).toBe(false);
expect(this.task.hasCustomBodyPerRecipient()).toBe(false);
});
it("should return true if only open-tracking is on", () => {
this.applySpies();
this.task.draft.applyPluginMetadata('link-tracking', false);
expect(this.task.usingMultiSend()).toBe(true);
expect(this.task.hasCustomBodyPerRecipient()).toBe(true);
});
it("should return true if only link-tracking is on", () => {
this.applySpies();
this.task.draft.applyPluginMetadata('open-tracking', false);
expect(this.task.usingMultiSend()).toBe(true);
expect(this.task.hasCustomBodyPerRecipient()).toBe(true);
});
it("should return false if there are too many participants", () => {
this.applySpies({"draft.participants": 15});
expect(this.task.usingMultiSend()).toBe(false);
expect(this.task.hasCustomBodyPerRecipient()).toBe(false);
});
it("should return true otherwise", () => {
this.applySpies();
expect(this.task.usingMultiSend()).toBe(true);
expect(this.task.hasCustomBodyPerRecipient()).toBe(true);
});
});
});

2
src/K2

@ -1 +1 @@
Subproject commit ab9eeaafce307976e2639cfc089ff534a7ba2082
Subproject commit 09f19d1adfec03c99fae0b4a1610b6c51e47f491

7
src/flux/actions.es6
View file

@ -423,8 +423,11 @@ class Actions {
Recieves the clientId of the message that was sent
*/
static sendDraftSuccess = ActionScopeGlobal;
static sendDraftFailed = ActionScopeGlobal;
static draftDeliverySucceeded = ActionScopeGlobal;
static draftDeliveryFailed = ActionScopeGlobal;
static ensureMessageInSentSuccess = ActionScopeGlobal;
static ensureMessageInSentFailed = ActionScopeGlobal;
static sendManyDrafts = ActionScopeWindow;
static ensureDraftSynced = ActionScopeWindow;

4
src/flux/stores/draft-store.es6
View file

@ -41,8 +41,8 @@ class DraftStore extends NylasStore {
this.listenTo(Actions.composePopoutDraft, this._onPopoutDraftClientId);
this.listenTo(Actions.composeNewBlankDraft, this._onPopoutBlankDraft);
this.listenTo(Actions.composeNewDraftToRecipient, this._onPopoutNewDraftToRecipient);
this.listenTo(Actions.sendDraftFailed, this._onSendDraftFailed);
this.listenTo(Actions.sendDraftSuccess, this._onSendDraftSuccess);
this.listenTo(Actions.draftDeliveryFailed, this._onSendDraftFailed);
this.listenTo(Actions.draftDeliverySucceeded, this._onSendDraftSuccess);
this.listenTo(Actions.didCancelSendAction, this._onDidCancelSendAction);
this.listenTo(Actions.sendQuickReply, this._onSendQuickReply);

2
src/flux/stores/file-upload-store.es6
View file

@ -49,7 +49,7 @@ class FileUploadStore extends NylasStore {
mkdirp.sync(UPLOAD_DIR);
if (NylasEnv.isMainWindow() || NylasEnv.inSpecMode()) {
this.listenTo(Actions.sendDraftSuccess, ({messageClientId}) => {
this.listenTo(Actions.ensureMessageInSentSuccess, ({messageClientId}) => {
this._deleteUploadsForClientId(messageClientId);
});
}

2
src/flux/syncback-task-api-request.es6
View file

@ -41,7 +41,7 @@ class SyncbackTaskAPIRequest {
message: failed.error.message,
data: failed.error.data,
},
statusCode: failed.error.statusCode,
statusCode: failed.error.statusCode || 500,
})
reject(error)
} else if (succeeded) {

22
src/flux/tasks/reconcile-multi-send-task.es6 → src/flux/tasks/ensure-message-in-sent-folder-task.es6
View file

@ -1,14 +1,16 @@
import Task from './task';
import {APIError} from '../errors';
import Actions from '../actions';
import NylasAPI from '../nylas-api';
import SyncbackTaskAPIRequest from '../syncback-task-api-request';
import SendDraftTask from './send-draft-task';
export default class ReconcileMultiSendTask extends Task {
export default class EnsureMessageInSentFolderTask extends Task {
constructor(opts = {}) {
super(opts);
this.message = opts.message;
this.sentPerRecipient = opts.sentPerRecipient;
}
isDependentOnTask(other) {
@ -27,18 +29,24 @@ export default class ReconcileMultiSendTask extends Task {
return new SyncbackTaskAPIRequest({
api: NylasAPI,
options: {
timeout: 1000 * 60 * 5, // We cannot hang up a send - won't know if it sent
method: "DELETE",
path: `/send-multiple/${this.message.id}`,
path: `/ensure-message-in-sent-folder/${this.message.id}`,
method: "POST",
body: {
sentPerRecipient: this.sentPerRecipient,
},
accountId: this.message.accountId,
},
})
.run()
.thenReturn(Task.Status.Success)
.then(() => {
Actions.ensureMessageInSentSuccess({messageClientId: this.message.clientId})
return Task.Status.Success
})
.catch((err) => {
const errorMessage = `We had trouble saving your message, "${this.message.subject}", to your Sent folder.\n\n${err.message}`;
const errorMessage = `Your message successfully sent; however, we had trouble saving your message, "${this.message.subject}", to your Sent folder.\n\n${err.message}`;
Actions.ensureMessageInSentFailed()
if (err instanceof APIError) {
if (SyncbackTaskAPIRequest.PermanentErrorCodes.includes(err.statusCode)) {
if (NylasAPI.PermanentErrorCodes.includes(err.statusCode)) {
NylasEnv.showErrorDialog(errorMessage, {showInMainWindow: true, detail: err.stack});
return Promise.resolve([Task.Status.Failed, err]);
}

58
src/flux/tasks/send-draft-task.es6
View file

@ -11,8 +11,7 @@ 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 ReconcileMultiSendTask from './reconcile-multi-send-task';
import EnsureMessageInSentFolderTask from './ensure-message-in-sent-folder-task';
const OPEN_TRACKING_ID = NylasEnv.packages.pluginIdFor('open-tracking')
const LINK_TRACKING_ID = NylasEnv.packages.pluginIdFor('link-tracking')
@ -38,6 +37,7 @@ export default class SendDraftTask extends BaseDraftTask {
return this.refreshDraftReference()
.then(this.assertDraftValidity)
.then(this.sendMessage)
.then(this.ensureInSentFolder)
.then(this.updatePluginMetadata)
.then(this.onSuccess)
.catch(this.onError);
@ -58,7 +58,7 @@ export default class SendDraftTask extends BaseDraftTask {
return Promise.resolve();
}
usingMultiSend = () => {
hasCustomBodyPerRecipient = () => {
if (!this.allowMultiSend) {
return false;
}
@ -73,17 +73,29 @@ export default class SendDraftTask extends BaseDraftTask {
if (!pluginsAvailable) {
return false;
}
const pluginsInUse = (this.draft.metadataForPluginId(OPEN_TRACKING_ID) || this.draft.metadataForPluginId(LINK_TRACKING_ID));
const pluginsInUse = (this.draft.metadataForPluginId(OPEN_TRACKING_ID) || this.draft.metadataForPluginId(LINK_TRACKING_ID)) || false;
const providerCompatible = (AccountStore.accountForId(this.draft.accountId).provider !== "eas");
return pluginsInUse && providerCompatible;
}
sendMessage = () => {
return this.usingMultiSend() ? this.sendWithMultipleBodies() : this.sendWithSingleBody();
sendMessage = async () => {
if (this.hasCustomBodyPerRecipient()) {
await this._sendPerRecipient();
} else {
await this._sendWithSingleBody()
}
}
sendWithSingleBody = () => {
return new SyncbackTaskAPIRequest({
ensureInSentFolder = () => {
const t = new EnsureMessageInSentFolderTask({
message: this.message,
sentPerRecipient: this.hasCustomBodyPerRecipient(),
})
Actions.queueTask(t)
}
_sendWithSingleBody = async () => {
const task = new SyncbackTaskAPIRequest({
api: NylasAPI,
options: {
path: "/send",
@ -95,17 +107,15 @@ export default class SendDraftTask extends BaseDraftTask {
requestId: this.draft.clientId,
},
})
.run()
.then((responseJSON) => {
return this.createMessageFromResponse(responseJSON)
})
const responseJSON = await task.run();
await this._createMessageFromResponse(responseJSON)
}
sendWithMultipleBodies = () => {
return new SyncbackTaskAPIRequest({
_sendPerRecipient = async () => {
const task = new SyncbackTaskAPIRequest({
api: NylasAPI,
options: {
path: "/send-multiple",
path: "/send-per-recipient",
accountId: this.draft.accountId,
method: 'POST',
body: {
@ -116,16 +126,8 @@ export default class SendDraftTask extends BaseDraftTask {
timeout: 1000 * 60 * 5, // We cannot hang up a send - won't know if it sent
},
})
.run()
.then((responseJSON) => {
return this.createMessageFromResponse(responseJSON);
})
.then(() => {
const t = new ReconcileMultiSendTask({
message: this.message,
});
Actions.queueTask(t);
})
const responseJSON = await task.run();
await this._createMessageFromResponse(responseJSON);
}
updatePluginMetadata = () => {
@ -138,7 +140,7 @@ export default class SendDraftTask extends BaseDraftTask {
return Promise.resolve();
}
createMessageFromResponse = (responseJSON) => {
_createMessageFromResponse = (responseJSON) => {
const {failedRecipients, message} = responseJSON
if (failedRecipients && failedRecipients.length > 0) {
const errorMessage = `We had trouble sending this message to all recipients. ${failedRecipients} may not have received this email.`;
@ -160,7 +162,7 @@ export default class SendDraftTask extends BaseDraftTask {
onSuccess = () => {
Actions.recordUserEvent("Draft Sent")
Actions.sendDraftSuccess({message: this.message, messageClientId: this.message.clientId, draftClientId: this.draft.clientId});
Actions.draftDeliverySucceeded({message: this.message, messageClientId: this.message.clientId, draftClientId: this.draft.clientId});
// TODO we shouldn't need to do this anymore
NylasAPIHelpers.makeDraftDeletionRequest(this.draft);
@ -197,7 +199,7 @@ export default class SendDraftTask extends BaseDraftTask {
}
if (this.emitError && !(err instanceof RequestEnsureOnceError)) {
Actions.sendDraftFailed({
Actions.draftDeliveryFailed({
threadId: this.draft.threadId,
draftClientId: this.draft.clientId,
errorMessage: message,

2
src/global/nylas-exports.es6
View file

@ -125,7 +125,7 @@ lazyLoadAndRegisterTask(`SyncbackCategoryTask`, 'syncback-category-task');
lazyLoadAndRegisterTask(`SyncbackMetadataTask`, 'syncback-metadata-task');
lazyLoadAndRegisterTask(`PerformSendActionTask`, 'perform-send-action-task');
lazyLoadAndRegisterTask(`ReprocessMailRulesTask`, 'reprocess-mail-rules-task');
lazyLoadAndRegisterTask(`ReconcileMultiSendTask`, 'reconcile-multi-send-task');
lazyLoadAndRegisterTask(`EnsureMessageInSentFolderTask`, 'ensure-message-in-sent-folder-task');
// Stores
// These need to be required immediately since some Stores are