diff --git a/docs/ComposerExtensions.md b/docs/ComposerExtensions.md index 27885cce1..7ef6d625e 100644 --- a/docs/ComposerExtensions.md +++ b/docs/ComposerExtensions.md @@ -35,12 +35,12 @@ class ProductsExtension extends ComposerExtension return ["with the word '#{word}'?"] return [] - @finalizeSessionBeforeSending: ({session}) -> - draft = session.draft() - if @warningsForSending({draft}) - bodyWithWarning = draft.body += "
This email \ - contains competitor's product names \ - or trademarks used in context." - return session.changes.add(body: bodyWithWarning) - else return Promise.resolve() + @applyTransformsToDraft: ({draft}) -> + if @warningsForSending({draft}) + updated = draft.clone() + updated.body += "
This email \ + contains competitor's product names \ + or trademarks used in context." + return updated + return draft ``` diff --git a/internal_packages/composer-spellcheck/lib/spellcheck-composer-extension.es6 b/internal_packages/composer-spellcheck/lib/spellcheck-composer-extension.es6 index aef5308c5..68384d3af 100644 --- a/internal_packages/composer-spellcheck/lib/spellcheck-composer-extension.es6 +++ b/internal_packages/composer-spellcheck/lib/spellcheck-composer-extension.es6 @@ -148,14 +148,11 @@ export default class SpellcheckComposerExtension extends ComposerExtension { }); } - static finalizeSessionBeforeSending = ({session}) => { - const body = session.draft().body; - const clean = body.replace(/<\/?spelling[^>]*>/g, ''); - - if (body !== clean) { - return session.changes.add({body: clean}); - } - - return Promise.resolve(); + static applyTransformsToDraft = ({draft}) => { + const nextDraft = draft.clone(); + nextDraft.body = nextDraft.body.replace(/<\/?spelling[^>]*>/g, ''); + return nextDraft; } + + static unapplyTransformsToDraft = () => 'unnecessary' } diff --git a/internal_packages/composer-spellcheck/spec/spellcheck-composer-extension-spec.es6 b/internal_packages/composer-spellcheck/spec/spellcheck-composer-extension-spec.es6 index 31d58b35d..7344edc7a 100644 --- a/internal_packages/composer-spellcheck/spec/spellcheck-composer-extension-spec.es6 +++ b/internal_packages/composer-spellcheck/spec/spellcheck-composer-extension-spec.es6 @@ -4,28 +4,29 @@ import fs from 'fs'; import path from 'path'; import SpellcheckComposerExtension from '../lib/spellcheck-composer-extension'; -import {NylasSpellchecker} from 'nylas-exports'; +import {NylasSpellchecker, Message} from 'nylas-exports'; -const initialHTML = fs.readFileSync(path.join(__dirname, 'fixtures', 'california-with-misspellings-before.html')).toString(); -const expectedHTML = fs.readFileSync(path.join(__dirname, 'fixtures', 'california-with-misspellings-after.html')).toString(); +const initialPath = path.join(__dirname, 'fixtures', 'california-with-misspellings-before.html'); +const initialHTML = fs.readFileSync(initialPath).toString(); +const expectedPath = path.join(__dirname, 'fixtures', 'california-with-misspellings-after.html'); +const expectedHTML = fs.readFileSync(expectedPath).toString(); -describe("SpellcheckComposerExtension", ()=> { - beforeEach(()=> { +describe("SpellcheckComposerExtension", () => { + beforeEach(() => { // Avoid differences between node-spellcheck on different platforms - const spellings = JSON.parse(fs.readFileSync(path.join(__dirname, 'fixtures', 'california-spelling-lookup.json'))); - spyOn(NylasSpellchecker, 'isMisspelled').andCallFake(word=> spellings[word]) + const lookupPath = path.join(__dirname, 'fixtures', 'california-spelling-lookup.json'); + const spellings = JSON.parse(fs.readFileSync(lookupPath)); + spyOn(NylasSpellchecker, 'isMisspelled').andCallFake(word => spellings[word]) }); - describe("update", ()=> { - it("correctly walks a DOM tree and surrounds mispelled words", ()=> { + describe("update", () => { + it("correctly walks a DOM tree and surrounds mispelled words", () => { const node = document.createElement('div'); node.innerHTML = initialHTML; const editor = { rootNode: node, - whilePreservingSelection: (cb)=> { - return cb(); - }, + whilePreservingSelection: (cb) => cb(), }; SpellcheckComposerExtension.update(editor); @@ -33,24 +34,19 @@ describe("SpellcheckComposerExtension", ()=> { }); }); - describe("finalizeSessionBeforeSending", ()=> { - it("removes the annotations it inserted", ()=> { - const session = { - draft: ()=> { - return { - body: expectedHTML, - }; - }, - changes: { - add: jasmine.createSpy('add').andReturn(Promise.resolve()), - }, - }; + describe("applyTransformsToDraft", () => { + it("removes the spelling annotations it inserted", () => { + const draft = new Message({ body: expectedHTML }); + const out = SpellcheckComposerExtension.applyTransformsToDraft({draft}); + expect(out.body).toEqual(initialHTML); + }); + }); - waitsForPromise(()=> { - return SpellcheckComposerExtension.finalizeSessionBeforeSending({session}).then(()=> { - expect(session.changes.add).toHaveBeenCalledWith({body: initialHTML}); - }); - }); + describe("unapplyTransformsToDraft", () => { + it("returns the magic no-op option", () => { + const draft = new Message({ body: expectedHTML }); + const out = SpellcheckComposerExtension.unapplyTransformsToDraft({draft}); + expect(out).toEqual('unnecessary'); }); }); }); diff --git a/internal_packages/composer-templates/lib/template-composer-extension.es6 b/internal_packages/composer-templates/lib/template-composer-extension.es6 index 83d499a91..961849417 100644 --- a/internal_packages/composer-templates/lib/template-composer-extension.es6 +++ b/internal_packages/composer-templates/lib/template-composer-extension.es6 @@ -10,12 +10,20 @@ class TemplatesComposerExtension extends ComposerExtension { return warnings; } - static finalizeSessionBeforeSending({session}) { - const body = session.draft().body; - const clean = body.replace(/<\/?code[^>]*>/g, ''); - if (body !== clean) { - return session.changes.add({body: clean}); - } + static applyTransformsToDraft = ({draft}) => { + const nextDraft = draft.clone(); + nextDraft.body = nextDraft.body.replace(/<\/?code[^>]*>/g, (match) => + `` + ); + return nextDraft; + } + + static unapplyTransformsToDraft = ({draft}) => { + const nextDraft = draft.clone(); + nextDraft.body = nextDraft.body.replace(//g, (match, node) => + node + ); + return nextDraft; } static onClick({editor, event}) { @@ -87,7 +95,7 @@ class TemplatesComposerExtension extends ComposerExtension { static onContentChanged({editor}) { const editableNode = editor.rootNode; const selection = editor.currentSelection().rawSelection; - const isWithinNode = (node)=> { + const isWithinNode = (node) => { let test = selection.baseNode; while (test !== editableNode) { if (test === node) { return true; } @@ -97,19 +105,14 @@ class TemplatesComposerExtension extends ComposerExtension { }; const codeTags = editableNode.querySelectorAll('code.var.empty'); - return (() => { - const result = []; - for (let i = 0, codeTag; i < codeTags.length; i++) { - codeTag = codeTags[i]; - codeTag.textContent = codeTag.textContent; // sets node contents to just its textContent, strips HTML - result.push((() => { - if (selection.containsNode(codeTag) || isWithinNode(codeTag)) { - return codeTag.classList.remove('empty'); - } - })()); + for (let i = 0, codeTag; i < codeTags.length; i++) { + codeTag = codeTags[i]; + // sets node contents to just its textContent, strips HTML + codeTag.textContent = codeTag.textContent; + if (selection.containsNode(codeTag) || isWithinNode(codeTag)) { + codeTag.classList.remove('empty'); } - return result; - })(); + } } } diff --git a/internal_packages/link-tracking/lib/link-tracking-composer-extension.es6 b/internal_packages/link-tracking/lib/link-tracking-composer-extension.es6 index 962a603db..510b7418c 100644 --- a/internal_packages/link-tracking/lib/link-tracking-composer-extension.es6 +++ b/internal_packages/link-tracking/lib/link-tracking-composer-extension.es6 @@ -16,33 +16,57 @@ class DraftBody { } export default class LinkTrackingComposerExtension extends ComposerExtension { - static finalizeSessionBeforeSending({session}) { - const draft = session.draft(); - + static applyTransformsToDraft({draft}) { // grab message metadata, if any - const metadata = draft.metadataForPluginId(PLUGIN_ID); + const nextDraft = draft.clone(); + const metadata = nextDraft.metadataForPluginId(PLUGIN_ID); if (metadata) { const draftBody = new DraftBody(draft); const links = []; const messageUid = uuid.v4().replace(/-/g, ""); - // loop through all elements, replace with redirect links and save mappings - draftBody.unquoted = draftBody.unquoted.replace(RegExpUtils.urlLinkTagRegex(), (match, prefix, url, suffix, content, closingTag) => { - const encoded = encodeURIComponent(url); - // the links param is an index of the link array. - const redirectUrl = `${PLUGIN_URL}/link/${draft.accountId}/${messageUid}/${links.length}?redirect=${encoded}`; - links.push({url: url, click_count: 0, click_data: [], redirect_url: redirectUrl}); - return prefix + redirectUrl + suffix + content + closingTag; - }); + // loop through all elements, replace with redirect links and save + // mappings. The links component of the path is an index of the link array. + draftBody.unquoted = draftBody.unquoted.replace( + RegExpUtils.urlLinkTagRegex(), + (match, prefix, url, suffix, content, closingTag) => { + const encoded = encodeURIComponent(url); + const redirectUrl = `${PLUGIN_URL}/link/${draft.accountId}/${messageUid}/${links.length}?redirect=${encoded}`; + links.push({ + url, + click_count: 0, + click_data: [], + redirect_url: redirectUrl, + }); + return prefix + redirectUrl + suffix + content + closingTag; + } + ); // save the draft - session.changes.add({body: draftBody.body}); + nextDraft.body = draftBody.body; // save the link info to draft metadata metadata.uid = messageUid; metadata.links = links; - Actions.setMetadata(draft, PLUGIN_ID, metadata); } + return nextDraft; + } + + static unapplyTransformsToDraft({draft}) { + const nextDraft = draft.clone(); + const draftBody = new DraftBody(draft); + draftBody.unquoted = draftBody.unquoted.replace( + RegExpUtils.urlLinkTagRegex(), + (match, prefix, url, suffix, content, closingTag) => { + if (url.indexOf(PLUGIN_URL) !== -1) { + const userURLEncoded = url.split('?redirect=')[1]; + return prefix + decodeURIComponent(userURLEncoded) + suffix + content + closingTag; + } + return prefix + url + suffix + content + closingTag; + } + ) + nextDraft.body = draftBody.body; + return nextDraft; } } diff --git a/internal_packages/link-tracking/spec/link-tracking-composer-extension-spec.es6 b/internal_packages/link-tracking/spec/link-tracking-composer-extension-spec.es6 index 7c6745dd5..bddfb45ed 100644 --- a/internal_packages/link-tracking/spec/link-tracking-composer-extension-spec.es6 +++ b/internal_packages/link-tracking/spec/link-tracking-composer-extension-spec.es6 @@ -18,68 +18,71 @@ const replacedContent = (accountId, messageUid) => `TEST_BODY
http://www.stillhere.com`; -const quote = `
On Feb 25 2016, at 3:38 pm, Drew <drew@nylas.com> wrote:
twst
`; -const testBody = `${testContent}${quote}`; -const replacedBody = (accountId, messageUid, unquoted) => `${replacedContent(accountId, messageUid)}${unquoted ? "" : quote}`; +const quote = `
twst
`; +const testBody = `${testContent}${quote}`; -describe("Open tracking composer extension", () => { +const replacedBody = (accountId, messageUid, unquoted) => + `${replacedContent(accountId, messageUid)}${unquoted ? "" : quote}`; + +describe("Link tracking composer extension", () => { // Set up a draft, session that returns the draft, and metadata - beforeEach(()=>{ + beforeEach(() => { this.draft = new Message({accountId: "test"}); this.draft.body = testBody; - this.session = { - draft: () => this.draft, - changes: jasmine.createSpyObj('changes', ['add', 'commit']), - }; }); - it("takes no action if there is no metadata", ()=>{ - LinkTrackingComposerExtension.finalizeSessionBeforeSending({session: this.session}); - expect(this.session.changes.add).not.toHaveBeenCalled(); - expect(this.session.changes.commit).not.toHaveBeenCalled(); + describe("applyTransformsToDraft", () => { + it("takes no action if there is no metadata", () => { + const out = LinkTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + expect(out.body).toEqual(this.draft.body); + }); + + describe("With properly formatted metadata and correct params", () => { + beforeEach(() => { + this.metadata = {tracked: true}; + this.draft.applyPluginMetadata(PLUGIN_ID, this.metadata); + }); + + it("replaces links in the unquoted portion of the body", () => { + const out = LinkTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + const outUnquoted = QuotedHTMLTransformer.removeQuotedHTML(out.body); + + expect(outUnquoted).toContain(replacedBody(this.draft.accountId, this.metadata.uid, true)); + expect(out.body).toContain(replacedBody(this.draft.accountId, this.metadata.uid, false)); + }); + + it("sets a uid and list of links on the metadata", () => { + LinkTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + + expect(this.metadata.uid).not.toBeUndefined(); + expect(this.metadata.links).not.toBeUndefined(); + expect(this.metadata.links.length).toEqual(2); + + for (const link of this.metadata.links) { + expect(link.click_count).toEqual(0); + } + }); + }); }); - describe("With properly formatted metadata and correct params", () => { - // Set metadata on the draft and call finalizeSessionBeforeSending - beforeEach(()=>{ + describe("unapplyTransformsToDraft", () => { + it("takes no action if there are no tracked links in the body", () => { + const out = LinkTrackingComposerExtension.unapplyTransformsToDraft({ + draft: this.draft.clone(), + }); + expect(out.body).toEqual(this.draft.body); + }); + + it("replaces tracked links with the original links, restoring the body exactly", () => { this.metadata = {tracked: true}; this.draft.applyPluginMetadata(PLUGIN_ID, this.metadata); - LinkTrackingComposerExtension.finalizeSessionBeforeSending({session: this.session}); + const withImg = LinkTrackingComposerExtension.applyTransformsToDraft({ + draft: this.draft.clone(), + }); + const withoutImg = LinkTrackingComposerExtension.unapplyTransformsToDraft({ + draft: withImg.clone(), + }); + expect(withoutImg.body).toEqual(this.draft.body); }); - - it("adds (but does not commit) the changes to the session", ()=>{ - expect(this.session.changes.add).toHaveBeenCalled(); - expect(this.session.changes.add.mostRecentCall.args[0].body).toBeDefined(); - expect(this.session.changes.commit).not.toHaveBeenCalled(); - }); - - describe("On the unquoted body", () => { - beforeEach(()=>{ - this.body = this.session.changes.add.mostRecentCall.args[0].body; - this.unquoted = QuotedHTMLTransformer.removeQuotedHTML(this.body); - - waitsFor(()=>this.metadata.uid) - }); - - it("sets a uid and list of links on the metadata", ()=>{ - runs(() => { - expect(this.metadata.uid).not.toBeUndefined(); - expect(this.metadata.links).not.toBeUndefined(); - expect(this.metadata.links.length).toEqual(2); - - for (const link of this.metadata.links) { - expect(link.click_count).toEqual(0); - } - }) - }); - - it("replaces all the valid href URLs with redirects", ()=>{ - runs(() => { - expect(this.unquoted).toContain(replacedBody(this.draft.accountId, this.metadata.uid, true)); - expect(this.body).toContain(replacedBody(this.draft.accountId, this.metadata.uid, false)); - }) - }); - }) }); }); - diff --git a/internal_packages/open-tracking/lib/open-tracking-composer-extension.es6 b/internal_packages/open-tracking/lib/open-tracking-composer-extension.es6 index 369253c6f..f148ba71f 100644 --- a/internal_packages/open-tracking/lib/open-tracking-composer-extension.es6 +++ b/internal_packages/open-tracking/lib/open-tracking-composer-extension.es6 @@ -10,27 +10,33 @@ class DraftBody { } export default class OpenTrackingComposerExtension extends ComposerExtension { - static finalizeSessionBeforeSending({session}) { - const draft = session.draft(); + static applyTransformsToDraft({draft}) { // grab message metadata, if any + const nextDraft = draft.clone(); const metadata = draft.metadataForPluginId(PLUGIN_ID); if (!metadata) { - return; + return nextDraft; } if (!metadata.uid) { NylasEnv.reportError(new Error("Open tracking composer extension could not find 'uid' in metadata!")); - return; + return nextDraft; } // insert a tracking pixel into the message const serverUrl = `${PLUGIN_URL}/open/${draft.accountId}/${metadata.uid}`; - const img = ``; + const img = ``; const draftBody = new DraftBody(draft); - draftBody.unquoted = draftBody.unquoted + "
" + img; - // save the draft - session.changes.add({body: draftBody.body}); + draftBody.unquoted = `${draftBody.unquoted}${img}`; + nextDraft.body = draftBody.body; + return nextDraft; + } + + static unapplyTransformsToDraft({draft}) { + const nextDraft = draft.clone(); + nextDraft.body = draft.body.replace(/]*>/g, ''); + return nextDraft; } } diff --git a/internal_packages/open-tracking/spec/open-tracking-composer-extension-spec.es6 b/internal_packages/open-tracking/spec/open-tracking-composer-extension-spec.es6 index fe639ca11..4b3ba77ec 100644 --- a/internal_packages/open-tracking/spec/open-tracking-composer-extension-spec.es6 +++ b/internal_packages/open-tracking/spec/open-tracking-composer-extension-spec.es6 @@ -6,58 +6,57 @@ const quote = `
{ // Set up a draft, session that returns the draft, and metadata - beforeEach(()=>{ - this.draft = new Message(); - this.draft.body = `TEST_BODY ${quote}`; - this.session = { - draft: () => this.draft, - changes: jasmine.createSpyObj('changes', ['add', 'commit']), - }; + beforeEach(() => { + this.draft = new Message({ + body: `TEST_BODY ${quote}`, + }); }); - it("takes no action if there is no metadata", ()=>{ - OpenTrackingComposerExtension.finalizeSessionBeforeSending({session: this.session}); - expect(this.session.changes.add.calls.length).toEqual(0); - expect(this.session.changes.commit.calls.length).toEqual(0); - }); - - describe("With properly formatted metadata and correct params", () => { - // Set metadata on the draft and call finalizeSessionBeforeSending - beforeEach(()=>{ - this.metadata = {uid: "TEST_UID"}; - this.draft.applyPluginMetadata(PLUGIN_ID, this.metadata); - OpenTrackingComposerExtension.finalizeSessionBeforeSending({session: this.session}); + describe("applyTransformsToDraft", () => { + it("takes no action if there is no metadata", () => { + const out = OpenTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + expect(out.body).toEqual(this.draft.body); }); - it("adds (but does not commit) the changes to the session", ()=>{ - expect(this.session.changes.add).toHaveBeenCalled(); - expect(this.session.changes.add.mostRecentCall.args[0].body).toBeDefined(); - expect(this.session.changes.add.mostRecentCall.args[0].body).toContain("TEST_BODY"); - expect(this.session.changes.commit).not.toHaveBeenCalled(); + it("reports an error if the metadata is missing required fields", () => { + this.draft.applyPluginMetadata(PLUGIN_ID, {}); + spyOn(NylasEnv, "reportError"); + OpenTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + expect(NylasEnv.reportError).toHaveBeenCalled() }); - describe("On the unquoted body", () => { - beforeEach(()=>{ - const body = this.session.changes.add.mostRecentCall.args[0].body; - this.unquoted = QuotedHTMLTransformer.removeQuotedHTML(body); + describe("With properly formatted metadata and correct params", () => { + beforeEach(() => { + this.metadata = {uid: "TEST_UID"}; + this.draft.applyPluginMetadata(PLUGIN_ID, this.metadata); + const out = OpenTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + this.unquoted = QuotedHTMLTransformer.removeQuotedHTML(out.body); }); - it("appends an image to the body", ()=>{ + it("appends an image to the unquoted body", () => { expect(this.unquoted).toMatch(//); }); - it("has the right server URL", ()=>{ + it("has the right server URL", () => { const img = this.unquoted.match(//)[0]; expect(img).toContain(`${PLUGIN_URL}/open/${this.draft.accountId}/${this.metadata.uid}`); }); - }) + }); }); - it("reports an error if the metadata is missing required fields", ()=>{ - this.draft.applyPluginMetadata(PLUGIN_ID, {}); - spyOn(NylasEnv, "reportError"); - OpenTrackingComposerExtension.finalizeSessionBeforeSending({session: this.session}); - expect(NylasEnv.reportError).toHaveBeenCalled() + describe("unapplyTransformsToDraft", () => { + it("takes no action if the img tag is missing", () => { + const out = OpenTrackingComposerExtension.unapplyTransformsToDraft({draft: this.draft}); + expect(out.body).toEqual(this.draft.body); + }); + + it("removes the image from the body and restore the body to it's exact original content", () => { + this.metadata = {uid: "TEST_UID"}; + this.draft.applyPluginMetadata(PLUGIN_ID, this.metadata); + const withImg = OpenTrackingComposerExtension.applyTransformsToDraft({draft: this.draft}); + + const withoutImg = OpenTrackingComposerExtension.unapplyTransformsToDraft({draft: withImg}); + expect(withoutImg.body).toEqual(this.draft.body); + }); }); }); - diff --git a/internal_packages/send-later/lib/send-later-button.jsx b/internal_packages/send-later/lib/send-later-button.jsx index 77321cde9..001663a9d 100644 --- a/internal_packages/send-later/lib/send-later-button.jsx +++ b/internal_packages/send-later/lib/send-later-button.jsx @@ -32,36 +32,36 @@ class SendLaterButton extends Component { this._subscription.dispose(); } - onSendLater = (formattedDate, dateLabel)=> { + onSendLater = (formattedDate, dateLabel) => { SendLaterActions.sendLater(this.props.draftClientId, formattedDate, dateLabel); this.setState({scheduledDate: 'saving'}); - Actions.closePopover() }; - onCancelSendLater = ()=> { + onCancelSendLater = () => { SendLaterActions.cancelSendLater(this.props.draftClientId); - Actions.closePopover() }; - onClick = ()=> { + onClick = () => { const buttonRect = React.findDOMNode(this).getBoundingClientRect() Actions.openPopover( , + onCancelSendLater={this.onCancelSendLater} + />, {originRect: buttonRect, direction: 'up'} ) }; - onMessageChanged = (message)=> { + onMessageChanged = (message) => { if (!message) return; + const {scheduledDate} = this.state; const messageMetadata = message.metadataForPluginId(PLUGIN_ID) || {} const nextScheduledDate = messageMetadata.sendLaterDate - if (nextScheduledDate !== this.state.scheduledDate) { + if (nextScheduledDate !== scheduledDate) { const isComposer = NylasEnv.isComposerWindow() - const isFinishedSelecting = ((this.state.scheduledDate === 'saving') && (nextScheduledDate !== null)); + const isFinishedSelecting = ((scheduledDate === 'saving') && (nextScheduledDate !== null)); if (isComposer && isFinishedSelecting) { NylasEnv.close(); } @@ -79,7 +79,8 @@ class SendLaterButton extends Component { + style={{width: 14, height: 14}} + /> ); } @@ -94,10 +95,10 @@ class SendLaterButton extends Component { } return ( ); } diff --git a/internal_packages/send-later/lib/send-later-store.js b/internal_packages/send-later/lib/send-later-store.js index 7210f9d5c..8809b2c0d 100644 --- a/internal_packages/send-later/lib/send-later-store.js +++ b/internal_packages/send-later/lib/send-later-store.js @@ -5,7 +5,6 @@ import { Actions, Message, DatabaseStore, - SyncbackDraftTask, } from 'nylas-exports' import SendLaterActions from './send-later-actions' import {PLUGIN_ID, PLUGIN_NAME} from './send-later-constants' @@ -26,19 +25,17 @@ class SendLaterStore extends NylasStore { ]; } - setMetadata = (draftClientId, metadata)=> { + setMetadata = (draftClientId, metadata) => { return DatabaseStore.modelify(Message, [draftClientId]) - .then((messages)=> { - const {accountId} = messages[0]; + .then((messages) => { + const message = messages[0]; - return NylasAPI.authPlugin(this.pluginId, this.pluginName, accountId) - .then(()=> { - Actions.setMetadata(messages, this.pluginId, metadata); - - // Important: Do not remove this unless N1 is syncing drafts by default. - Actions.queueTask(new SyncbackDraftTask(draftClientId)); + return NylasAPI.authPlugin(this.pluginId, this.pluginName, message.accountId) + .then(() => { + Actions.setMetadata(message, this.pluginId, metadata); + Actions.ensureDraftSynced(message.clientId); }) - .catch((error)=> { + .catch((error) => { NylasEnv.reportError(error); NylasEnv.showErrorDialog(`Sorry, we were unable to schedule this message. ${error.message}`); }); @@ -61,17 +58,21 @@ class SendLaterStore extends NylasStore { } } - onSendLater = (draftClientId, sendLaterDate, dateLabel)=> { + onSendLater = (draftClientId, sendLaterDate, dateLabel) => { this.recordAction(sendLaterDate, dateLabel) - this.setMetadata(draftClientId, {sendLaterDate}); + this.setMetadata(draftClientId, {sendLaterDate}).then(() => { + if (NylasEnv.isComposerWindow()) { + Actions.closePopover(); + } + }); }; - onCancelSendLater = (draftClientId)=> { + onCancelSendLater = (draftClientId) => { this.recordAction(null) this.setMetadata(draftClientId, {sendLaterDate: null}); }; - deactivate = ()=> { + deactivate = () => { this.unsubscribers.forEach(unsub => unsub()); }; } diff --git a/internal_packages/send-later/spec/send-later-button-spec.jsx b/internal_packages/send-later/spec/send-later-button-spec.jsx index 237c0a0bd..5f9108cfd 100644 --- a/internal_packages/send-later/spec/send-later-button-spec.jsx +++ b/internal_packages/send-later/spec/send-later-button-spec.jsx @@ -71,12 +71,10 @@ describe('SendLaterButton', ()=> { it('sets scheduled date to "saving" and dispatches action', ()=> { const button = makeButton() spyOn(button, 'setState') - spyOn(Actions, 'closePopover') button.onSendLater({utc: ()=> 'utc'}) expect(SendLaterActions.sendLater).toHaveBeenCalled() expect(button.setState).toHaveBeenCalledWith({scheduledDate: 'saving'}) - expect(Actions.closePopover).toHaveBeenCalled() }); }); diff --git a/internal_packages/send-later/spec/send-later-store-spec.es6 b/internal_packages/send-later/spec/send-later-store-spec.es6 index 4290bd372..0ea9fb94a 100644 --- a/internal_packages/send-later/spec/send-later-store-spec.es6 +++ b/internal_packages/send-later/spec/send-later-store-spec.es6 @@ -7,13 +7,24 @@ import { import SendLaterStore from '../lib/send-later-store' -describe('SendLaterStore', ()=> { - beforeEach(()=> { +describe('SendLaterStore', () => { + beforeEach(() => { this.store = new SendLaterStore('plug-id', 'plug-name') }); - describe('setMetadata', ()=> { - beforeEach(()=> { + describe('onSendLater', () => { + it("should call setMetadata and then close the popover (if in a composer)", () => { + spyOn(Actions, 'closePopover'); + spyOn(NylasEnv, 'isComposerWindow').andReturn(true); + spyOn(this.store, 'setMetadata').andReturn(Promise.resolve()); + this.store.onSendLater('client-id', new Date(), 'asd'); + advanceClock(); + expect(Actions.closePopover).toHaveBeenCalled(); + }); + }); + + describe('setMetadata', () => { + beforeEach(() => { this.message = new Message({accountId: 123, clientId: 'c-1'}) this.metadata = {sendLaterDate: 'the future'} spyOn(this.store, 'recordAction') @@ -24,10 +35,10 @@ describe('SendLaterStore', ()=> { spyOn(NylasEnv, 'showErrorDialog') }); - it('auths the plugin correctly', ()=> { - waitsForPromise(()=> { + it('auths the plugin correctly', () => { + waitsForPromise(() => { return this.store.setMetadata('c-1', this.metadata) - .then(()=> { + .then(() => { expect(NylasAPI.authPlugin).toHaveBeenCalled() expect(NylasAPI.authPlugin).toHaveBeenCalledWith( 'plug-id', @@ -38,12 +49,12 @@ describe('SendLaterStore', ()=> { }) }); - it('sets the correct metadata', ()=> { - waitsForPromise(()=> { + it('sets the correct metadata', () => { + waitsForPromise(() => { return this.store.setMetadata('c-1', this.metadata) - .then(()=> { + .then(() => { expect(Actions.setMetadata).toHaveBeenCalledWith( - [this.message], + this.message, 'plug-id', this.metadata ) @@ -52,12 +63,12 @@ describe('SendLaterStore', ()=> { }) }); - it('displays dialog if an error occurs', ()=> { + it('displays dialog if an error occurs', () => { jasmine.unspy(NylasAPI, 'authPlugin') spyOn(NylasAPI, 'authPlugin').andReturn(Promise.reject(new Error('Oh no!'))) - waitsForPromise(()=> { + waitsForPromise(() => { return this.store.setMetadata('c-1', this.metadata) - .finally(()=> { + .finally(() => { expect(Actions.setMetadata).not.toHaveBeenCalled() expect(NylasEnv.reportError).toHaveBeenCalled() expect(NylasEnv.showErrorDialog).toHaveBeenCalled() diff --git a/internal_packages/thread-list/lib/draft-list-send-status.jsx b/internal_packages/thread-list/lib/draft-list-send-status.jsx index 458fd3423..c5603b252 100644 --- a/internal_packages/thread-list/lib/draft-list-send-status.jsx +++ b/internal_packages/thread-list/lib/draft-list-send-status.jsx @@ -2,7 +2,6 @@ import React, {Component, PropTypes} from 'react' import {Flexbox} from 'nylas-component-kit' import {timestamp} from './formatting-utils' import SendingProgressBar from './sending-progress-bar' -import SendingCancelButton from './sending-cancel-button' export default class DraftListSendStatus extends Component { static displayName = 'DraftListSendStatus'; @@ -18,8 +17,10 @@ export default class DraftListSendStatus extends Component { if (draft.uploadTaskId) { return ( - - + ) } diff --git a/internal_packages/thread-list/lib/draft-list-store.coffee b/internal_packages/thread-list/lib/draft-list-store.coffee index 85057886a..9b6e36480 100644 --- a/internal_packages/thread-list/lib/draft-list-store.coffee +++ b/internal_packages/thread-list/lib/draft-list-store.coffee @@ -49,7 +49,7 @@ class DraftListStore extends NylasStore mailboxPerspective.accountIds.forEach (aid) => OutboxStore.itemsForAccount(aid).forEach (task) => - draft = resultSet.modelWithId(task.draft.clientId) + draft = resultSet.modelWithId(task.draftClientId) if draft draft = draft.clone() draft.uploadTaskId = task.id diff --git a/internal_packages/thread-list/stylesheets/thread-list.less b/internal_packages/thread-list/stylesheets/thread-list.less index f9824c6c9..64ebb7ea7 100644 --- a/internal_packages/thread-list/stylesheets/thread-list.less +++ b/internal_packages/thread-list/stylesheets/thread-list.less @@ -508,7 +508,7 @@ body.is-blurred { .sending-progress { display: block; height:7px; - margin-top:10px; + align-self: center; background-color: @background-primary; border-bottom:1px solid @border-color-divider; position: relative; diff --git a/internal_packages/worker-ui/lib/developer-bar-task.cjsx b/internal_packages/worker-ui/lib/developer-bar-task.cjsx index b19facabb..55d89ac1e 100644 --- a/internal_packages/worker-ui/lib/developer-bar-task.cjsx +++ b/internal_packages/worker-ui/lib/developer-bar-task.cjsx @@ -15,7 +15,7 @@ class DeveloperBarTask extends React.Component if @state.expanded # This could be a potentially large amount of JSON. # Do not render unless it's actually being displayed! - details =
{JSON.stringify(@props.task.toJSON())}
+ details =
{JSON.stringify(@props.task.toJSON(), null, 2)}
@setState(expanded: not @state.expanded)}>
diff --git a/internal_packages/worker-ui/stylesheets/worker-ui.less b/internal_packages/worker-ui/stylesheets/worker-ui.less index 945fba336..eceab4969 100755 --- a/internal_packages/worker-ui/stylesheets/worker-ui.less +++ b/internal_packages/worker-ui/stylesheets/worker-ui.less @@ -217,7 +217,7 @@ .task-details { display: none; } &.task-expanded{ - .task-details { display: block; } + .task-details { display: block; white-space: pre; } } } diff --git a/package.json b/package.json index 6af5680fb..62a3e1ece 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "event-kit": "^1.0.2", "fs-plus": "^2.3.2", "fstream": "0.1.24", + "rimraf": "2.5.2", "grim": "1.5.0", "guid": "0.0.10", "immutable": "3.7.5", diff --git a/spec/nylas-api-spec.coffee b/spec/nylas-api-spec.coffee index 21b0d1b1d..ac24b7851 100644 --- a/spec/nylas-api-spec.coffee +++ b/spec/nylas-api-spec.coffee @@ -3,6 +3,7 @@ fs = require 'fs' Actions = require '../src/flux/actions' NylasAPI = require '../src/flux/nylas-api' Thread = require '../src/flux/models/thread' +Message = require '../src/flux/models/message' AccountStore = require '../src/flux/stores/account-store' DatabaseStore = require '../src/flux/stores/database-store' DatabaseTransaction = require '../src/flux/stores/database-transaction' @@ -334,3 +335,33 @@ describe "NylasAPI", -> verifyUpdate = _.partial(verifyUpdateHappened, klass) waitsForPromise => NylasAPI._handleModelResponse(json).then verifyUpdate + + describe "makeDraftDeletionRequest", -> + it "should make an API request to delete the draft", -> + draft = new Message(accountId: TEST_ACCOUNT_ID, draft: true, clientId: 'asd', serverId: 'asd') + spyOn(NylasAPI, 'makeRequest') + NylasAPI.makeDraftDeletionRequest(draft) + expect(NylasAPI.makeRequest).toHaveBeenCalled() + expect(NylasAPI.makeRequest.callCount).toBe 1 + req = NylasAPI.makeRequest.calls[0].args[0] + expect(req.path).toBe "/drafts/#{draft.serverId}" + expect(req.accountId).toBe TEST_ACCOUNT_ID + expect(req.method).toBe "DELETE" + expect(req.returnsModel).toBe false + + it "should increment the change tracker, preventing any further deltas about the draft", -> + draft = new Message(accountId: TEST_ACCOUNT_ID, draft: true, clientId: 'asd', serverId: 'asd') + spyOn(NylasAPI, 'incrementRemoteChangeLock') + NylasAPI.makeDraftDeletionRequest(draft) + expect(NylasAPI.incrementRemoteChangeLock).toHaveBeenCalledWith(Message, draft.serverId) + + it "should not return a promise or anything else, to avoid accidentally making things dependent on the request", -> + draft = new Message(accountId: TEST_ACCOUNT_ID, draft: true, clientId: 'asd', serverId: 'asd') + a = NylasAPI.makeDraftDeletionRequest(draft) + expect(a).toBe(undefined) + + it "should not do anything if the draft is missing a serverId", -> + draft = new Message(accountId: TEST_ACCOUNT_ID, draft: true, clientId: 'asd', serverId: null) + spyOn(NylasAPI, 'makeRequest') + NylasAPI.makeDraftDeletionRequest(draft) + expect(NylasAPI.makeRequest).not.toHaveBeenCalled() diff --git a/spec/stores/draft-store-proxy-spec.coffee b/spec/stores/draft-store-proxy-spec.coffee index b626dfb60..60cc4e27e 100644 --- a/spec/stores/draft-store-proxy-spec.coffee +++ b/spec/stores/draft-store-proxy-spec.coffee @@ -116,8 +116,10 @@ describe "DraftStoreProxy", -> it "should not make a query for the draft", -> expect(DatabaseStore.run).not.toHaveBeenCalled() - it "should immediately make the draft available", -> - expect(@proxy.draft()).toEqual(@draft) + it "prepare should resolve without querying for the draft", -> + waitsForPromise => @proxy.prepare().then => + expect(@proxy.draft()).toEqual(@draft) + expect(DatabaseStore.run).not.toHaveBeenCalled() describe "teardown", -> it "should mark the session as destroyed", -> @@ -131,7 +133,7 @@ describe "DraftStoreProxy", -> @draft = new Message(draft: true, body: '123', clientId: 'client-id') spyOn(DraftStoreProxy.prototype, "prepare") @proxy = new DraftStoreProxy('client-id') - spyOn(@proxy, '_setDraft') + spyOn(@proxy, '_setDraft').andCallThrough() spyOn(DatabaseStore, 'run').andCallFake (modelQuery) => Promise.resolve(@draft) jasmine.unspy(DraftStoreProxy.prototype, "prepare") @@ -167,6 +169,7 @@ describe "DraftStoreProxy", -> beforeEach -> @draft = new Message(draft: true, clientId: 'client-id', body: 'A', subject: 'initial') @proxy = new DraftStoreProxy('client-id', @draft) + advanceClock() spyOn(DatabaseTransaction.prototype, "persistModel").andReturn Promise.resolve() spyOn(Actions, "queueTask").andReturn Promise.resolve() diff --git a/spec/stores/draft-store-spec.coffee b/spec/stores/draft-store-spec.coffee index 682fed840..25f818f9d 100644 --- a/spec/stores/draft-store-spec.coffee +++ b/spec/stores/draft-store-spec.coffee @@ -16,6 +16,7 @@ FocusedContentStore, DatabaseTransaction, SanitizeTransformer, + SyncbackDraftFilesTask, InlineStyleTransformer} = require 'nylas-exports' ModelQuery = require '../../src/flux/models/query' @@ -685,6 +686,7 @@ describe "DraftStore", -> DraftStore._draftSessions[@draft.clientId] = proxy spyOn(DraftStore, "_doneWithSession").andCallThrough() + spyOn(DraftStore, "_prepareForSyncback").andReturn(Promise.resolve()) spyOn(DraftStore, "trigger") spyOn(SoundRegistry, "playSound") spyOn(Actions, "queueTask") @@ -692,18 +694,21 @@ describe "DraftStore", -> it "plays a sound immediately when sending draft", -> spyOn(NylasEnv.config, "get").andReturn true DraftStore._onSendDraft(@draft.clientId) + advanceClock() expect(NylasEnv.config.get).toHaveBeenCalledWith("core.sending.sounds") expect(SoundRegistry.playSound).toHaveBeenCalledWith("hit-send") it "doesn't plays a sound if the setting is off", -> spyOn(NylasEnv.config, "get").andReturn false DraftStore._onSendDraft(@draft.clientId) + advanceClock() expect(NylasEnv.config.get).toHaveBeenCalledWith("core.sending.sounds") expect(SoundRegistry.playSound).not.toHaveBeenCalled() it "sets the sending state when sending", -> spyOn(NylasEnv, "isMainWindow").andReturn true DraftStore._onSendDraft(@draft.clientId) + advanceClock() expect(DraftStore.isSendingDraft(@draft.clientId)).toBe true # Since all changes haven't been applied yet, we want to ensure that @@ -753,27 +758,19 @@ describe "DraftStore", -> runs -> expect(NylasEnv.close).not.toHaveBeenCalled() - it "queues the correct SendDraftTask", -> + it "queues tasks to upload files and send the draft", -> runs -> DraftStore._onSendDraft(@draft.clientId) waitsFor -> DraftStore._doneWithSession.calls.length > 0 runs -> expect(Actions.queueTask).toHaveBeenCalled() - task = Actions.queueTask.calls[0].args[0] - expect(task instanceof SendDraftTask).toBe true - expect(task.draft).toBe @draft - - it "queues a SendDraftTask", -> - runs -> - DraftStore._onSendDraft(@draft.clientId) - waitsFor -> - DraftStore._doneWithSession.calls.length > 0 - runs -> - expect(Actions.queueTask).toHaveBeenCalled() - task = Actions.queueTask.calls[0].args[0] - expect(task instanceof SendDraftTask).toBe true - expect(task.draft).toBe(@draft) + saveAttachments = Actions.queueTask.calls[0].args[0] + expect(saveAttachments instanceof SyncbackDraftFilesTask).toBe true + expect(saveAttachments.draftClientId).toBe(@draft.clientId) + sendDraft = Actions.queueTask.calls[1].args[0] + expect(sendDraft instanceof SendDraftTask).toBe true + expect(sendDraft.draftClientId).toBe(@draft.clientId) it "resets the sending state if there's an error", -> spyOn(NylasEnv, "isMainWindow").andReturn false diff --git a/spec/stores/file-upload-store-spec.coffee b/spec/stores/file-upload-store-spec.coffee index 83ee5b824..681036ca3 100644 --- a/spec/stores/file-upload-store-spec.coffee +++ b/spec/stores/file-upload-store-spec.coffee @@ -93,6 +93,12 @@ describe 'FileUploadStore', -> .then => expect(FileUploadStore._deleteUpload).toHaveBeenCalled() + describe "when a draft is sent", -> + it "should delete its uploads directory", -> + spyOn(FileUploadStore, '_deleteUploadsForClientId') + Actions.sendDraftSuccess({messageClientId: '123'}) + expect(FileUploadStore._deleteUploadsForClientId).toHaveBeenCalledWith('123') + describe '_getFileStats', -> it 'returns the correct stats', -> diff --git a/spec/tasks/base-draft-task-spec.es6 b/spec/tasks/base-draft-task-spec.es6 new file mode 100644 index 000000000..2a94748ce --- /dev/null +++ b/spec/tasks/base-draft-task-spec.es6 @@ -0,0 +1,117 @@ +import { + Message, + DatabaseStore, +} from 'nylas-exports'; + +import BaseDraftTask from '../../src/flux/tasks/base-draft-task'; + +describe("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 draftClientId") + }); + }); + }); + + 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/spec/tasks/send-draft-task-spec.coffee b/spec/tasks/send-draft-task-spec.coffee index 4f887c618..d2bf1523c 100644 --- a/spec/tasks/send-draft-task-spec.coffee +++ b/spec/tasks/send-draft-task-spec.coffee @@ -1,5 +1,4 @@ _ = require 'underscore' -fs = require 'fs' {APIError, Actions, DatabaseStore, @@ -17,51 +16,27 @@ DBt = DatabaseTransaction.prototype describe "SendDraftTask", -> - describe "isDependentOnTask", -> - it "is not dependent on any pending SyncbackDraftTasks", -> - draftA = new Message - version: '1' - clientId: 'localid-A' - serverId: '1233123AEDF1' - accountId: 'A12ADE' - subject: 'New Draft' - draft: true - to: - name: 'Dummy' - email: 'dummy@nylas.com' - - saveA = new SyncbackDraftTask(draftA, []) - sendA = new SendDraftTask(draftA, []) - - expect(sendA.isDependentOnTask(saveA)).toBe(false) - - describe "performLocal", -> - it "rejects if we we don't pass a draft", -> - badTask = new SendDraftTask() - badTask.performLocal().then -> + describe "assertDraftValidity", -> + it "rejects if there are still uploads on the draft", -> + badTask = new SendDraftTask('1') + badTask.draft = new Message(from: [new Contact(email: TEST_ACCOUNT_EMAIL)], accountId: TEST_ACCOUNT_ID, clientId: '1', uploads: ['123']) + badTask.assertDraftValidity().then -> throw new Error("Shouldn't succeed") .catch (err) -> - expect(err.message).toBe "SendDraftTask - must be provided a draft." - - it "rejects if we we don't pass uploads", -> - message = new Message(from: [new Contact(email: TEST_ACCOUNT_EMAIL)]) - message.uploads = null - badTask = new SendDraftTask(message) - badTask.performLocal().then -> - throw new Error("Shouldn't succeed") - .catch (err) -> - expect(err.message).toBe "SendDraftTask - must be provided an array of uploads." + expect(err.message).toBe "Files have been added since you started sending this draft. Double-check the draft and click 'Send' again.." it "rejects if no from address is specified", -> - badTask = new SendDraftTask(new Message(from: [], uploads: [])) - badTask.performLocal().then -> + badTask = new SendDraftTask('1') + badTask.draft = new Message(from: [], uploads: [], accountId: TEST_ACCOUNT_ID, clientId: '1') + badTask.assertDraftValidity().then -> throw new Error("Shouldn't succeed") .catch (err) -> expect(err.message).toBe "SendDraftTask - you must populate `from` before sending." it "rejects if the from address does not map to any account", -> - badTask = new SendDraftTask(new Message(from: [new Contact(email: 'not-configured@nylas.com')], uploads: null)) - badTask.performLocal().then -> + badTask = new SendDraftTask('1') + badTask.draft = new Message(from: [new Contact(email: 'not-configured@nylas.com')], accountId: TEST_ACCOUNT_ID, clientId: '1') + badTask.assertDraftValidity().then -> throw new Error("Shouldn't succeed") .catch (err) -> expect(err.message).toBe "SendDraftTask - you can only send drafts from a configured account." @@ -84,13 +59,12 @@ describe "SendDraftTask", -> return Promise.resolve(@response) spyOn(NylasAPI, 'incrementRemoteChangeLock') spyOn(NylasAPI, 'decrementRemoteChangeLock') + spyOn(NylasAPI, 'makeDraftDeletionRequest') spyOn(DBt, 'unpersistModel').andReturn Promise.resolve() spyOn(DBt, 'persistModel').andReturn Promise.resolve() spyOn(SoundRegistry, "playSound") spyOn(Actions, "postNotification") spyOn(Actions, "sendDraftSuccess") - spyOn(Actions, "attachmentUploaded") - spyOn(fs, 'createReadStream').andReturn "stub" # The tests below are invoked twice, once with a new @draft and one with a # persisted @draft. @@ -103,7 +77,7 @@ describe "SendDraftTask", -> expect(status).toBe Task.Status.Success it "makes a send request with the correct data", -> - waitsForPromise => @task._sendAndCreateMessage().then => + waitsForPromise => @task.performRemote().then => expect(NylasAPI.makeRequest).toHaveBeenCalled() expect(NylasAPI.makeRequest.callCount).toBe(1) options = NylasAPI.makeRequest.mostRecentCall.args[0] @@ -113,47 +87,43 @@ describe "SendDraftTask", -> expect(options.body).toEqual @draft.toJSON() it "should pass returnsModel:false", -> - waitsForPromise => @task._sendAndCreateMessage().then -> + waitsForPromise => @task.performRemote().then => expect(NylasAPI.makeRequest.calls.length).toBe(1) options = NylasAPI.makeRequest.mostRecentCall.args[0] expect(options.returnsModel).toBe(false) it "should always send the draft body in the request body (joined attribute check)", -> - waitsForPromise => - @task._sendAndCreateMessage().then => - expect(NylasAPI.makeRequest.calls.length).toBe(1) - options = NylasAPI.makeRequest.mostRecentCall.args[0] - expect(options.body.body).toBe('hello world') + waitsForPromise => @task.performRemote().then => + expect(NylasAPI.makeRequest.calls.length).toBe(1) + options = NylasAPI.makeRequest.mostRecentCall.args[0] + expect(options.body.body).toBe('hello world') describe "saving the sent message", -> it "should preserve the draft client id", -> - waitsForPromise => - @task._sendAndCreateMessage().then => - expect(DBt.persistModel).toHaveBeenCalled() - model = DBt.persistModel.mostRecentCall.args[0] - expect(model.clientId).toEqual(@draft.clientId) - expect(model.serverId).toEqual(@response.id) - expect(model.draft).toEqual(false) + waitsForPromise => @task.performRemote().then => + expect(DBt.persistModel).toHaveBeenCalled() + model = DBt.persistModel.mostRecentCall.args[0] + expect(model.clientId).toEqual(@draft.clientId) + expect(model.serverId).toEqual(@response.id) + expect(model.draft).toEqual(false) it "should preserve metadata, but not version numbers", -> - waitsForPromise => - @task._sendAndCreateMessage().then => - expect(DBt.persistModel).toHaveBeenCalled() - model = DBt.persistModel.mostRecentCall.args[0] + waitsForPromise => @task.performRemote().then => + expect(DBt.persistModel).toHaveBeenCalled() + model = DBt.persistModel.mostRecentCall.args[0] - expect(model.pluginMetadata.length).toEqual(@draft.pluginMetadata.length) + expect(model.pluginMetadata.length).toEqual(@draft.pluginMetadata.length) - for {pluginId, value, version} in @draft.pluginMetadata - updated = model.metadataObjectForPluginId(pluginId) - expect(updated.value).toEqual(value) - expect(updated.version).toEqual(0) + for {pluginId, value, version} in @draft.pluginMetadata + updated = model.metadataObjectForPluginId(pluginId) + expect(updated.value).toEqual(value) + expect(updated.version).toEqual(0) it "should notify the draft was sent", -> - waitsForPromise => - @task.performRemote().then => - args = Actions.sendDraftSuccess.calls[0].args[0] - expect(args.message instanceof Message).toBe(true) - expect(args.messageClientId).toBe(@draft.clientId) + waitsForPromise => @task.performRemote().then => + args = Actions.sendDraftSuccess.calls[0].args[0] + expect(args.message instanceof Message).toBe(true) + expect(args.messageClientId).toBe(@draft.clientId) it "should queue tasks to sync back the metadata on the new message", -> waitsForPromise => @@ -168,13 +138,13 @@ describe "SendDraftTask", -> it "should play a sound", -> spyOn(NylasEnv.config, "get").andReturn true - waitsForPromise => @task.performRemote().then -> + waitsForPromise => @task.performRemote().then => expect(NylasEnv.config.get).toHaveBeenCalledWith("core.sending.sounds") expect(SoundRegistry.playSound).toHaveBeenCalledWith("send") it "shouldn't play a sound if the config is disabled", -> spyOn(NylasEnv.config, "get").andReturn false - waitsForPromise => @task.performRemote().then -> + waitsForPromise => @task.performRemote().then => expect(NylasEnv.config.get).toHaveBeenCalledWith("core.sending.sounds") expect(SoundRegistry.playSound).not.toHaveBeenCalled() @@ -209,7 +179,7 @@ describe "SendDraftTask", -> @draft.replyToMessageId = "reply-123" @draft.threadId = "thread-123" - waitsForPromise => @task._sendAndCreateMessage(@draft).then => + waitsForPromise => @task.performRemote(@draft).then => expect(NylasAPI.makeRequest).toHaveBeenCalled() expect(NylasAPI.makeRequest.callCount).toEqual 2 req1 = NylasAPI.makeRequest.calls[0].args[0] @@ -231,7 +201,7 @@ describe "SendDraftTask", -> @draft.replyToMessageId = "reply-123" @draft.threadId = "thread-123" - waitsForPromise => @task._sendAndCreateMessage(@draft).then => + waitsForPromise => @task.performRemote(@draft).then => expect(NylasAPI.makeRequest).toHaveBeenCalled() expect(NylasAPI.makeRequest.callCount).toEqual 2 req1 = NylasAPI.makeRequest.calls[0].args[0] @@ -317,16 +287,14 @@ describe "SendDraftTask", -> describe "checking the promise chain halts on errors", -> beforeEach -> spyOn(NylasEnv, 'reportError') - spyOn(@task,"_sendAndCreateMessage").andCallThrough() - spyOn(@task,"_deleteRemoteDraft").andCallThrough() - spyOn(@task,"_onSuccess").andCallThrough() - spyOn(@task,"_onError").andCallThrough() + spyOn(@task, "sendMessage").andCallThrough() + spyOn(@task, "onSuccess").andCallThrough() + spyOn(@task, "onError").andCallThrough() @expectBlockedChain = => - expect(@task._sendAndCreateMessage).toHaveBeenCalled() - expect(@task._deleteRemoteDraft).not.toHaveBeenCalled() - expect(@task._onSuccess).not.toHaveBeenCalled() - expect(@task._onError).toHaveBeenCalled() + expect(@task.sendMessage).toHaveBeenCalled() + expect(@task.onSuccess).not.toHaveBeenCalled() + expect(@task.onError).toHaveBeenCalled() it "halts on 500s", -> thrownError = new APIError(statusCode: 500, body: "err") @@ -370,10 +338,9 @@ describe "SendDraftTask", -> waitsForPromise => @task.performRemote().then (status) => expect(status).toBe Task.Status.Success - expect(@task._sendAndCreateMessage).toHaveBeenCalled() - expect(@task._deleteRemoteDraft).toHaveBeenCalled() - expect(@task._onSuccess).toHaveBeenCalled() - expect(@task._onError).not.toHaveBeenCalled() + expect(@task.sendMessage).toHaveBeenCalled() + expect(@task.onSuccess).toHaveBeenCalled() + expect(@task.onError).not.toHaveBeenCalled() describe "with a new draft", -> beforeEach -> @@ -391,9 +358,9 @@ describe "SendDraftTask", -> @draft.applyPluginMetadata('pluginIdB', {a: true, b: 2}) @draft.metadataObjectForPluginId('pluginIdA').version = 2 - @task = new SendDraftTask(@draft) + @task = new SendDraftTask('client-id') @calledBody = "ERROR: The body wasn't included!" - spyOn(DatabaseStore, "findBy").andCallFake => + spyOn(DatabaseStore, "run").andCallFake => Promise.resolve(@draft) sharedTests() @@ -407,13 +374,6 @@ describe "SendDraftTask", -> expect(model.serverId).toBe @response.id expect(model.draft).toBe false - describe "deleteRemoteDraft", -> - it "should not make an API request", -> - waitsForPromise => - @task._deleteRemoteDraft(@draft).then => - expect(NylasAPI.makeRequest).not.toHaveBeenCalled() - - describe "with an existing persisted draft", -> beforeEach -> @draft = new Message @@ -434,13 +394,18 @@ describe "SendDraftTask", -> @draft.applyPluginMetadata('pluginIdB', {a: true, b: 2}) @draft.metadataObjectForPluginId('pluginIdA').version = 2 - @task = new SendDraftTask(@draft) + @task = new SendDraftTask('client-id') @calledBody = "ERROR: The body wasn't included!" - spyOn(DatabaseStore, "findBy").andCallFake => + spyOn(DatabaseStore, "run").andCallFake => Promise.resolve(@draft) sharedTests() + it "should call makeDraftDeletionRequest to delete the draft after sending", -> + @task.performLocal() + waitsForPromise => @task.performRemote().then => + expect(NylasAPI.makeDraftDeletionRequest).toHaveBeenCalled() + it "should locally convert the existing draft to a message on send", -> expect(@draft.clientId).toBe @draft.clientId expect(@draft.serverId).toBe "server-123" @@ -452,135 +417,3 @@ describe "SendDraftTask", -> expect(model.clientId).toBe @draft.clientId expect(model.serverId).toBe @response.id expect(model.draft).toBe false - - describe "deleteRemoteDraft", -> - it "should make an API request to delete the draft", -> - @task.performLocal() - waitsForPromise => - @task._deleteRemoteDraft(@draft).then => - expect(NylasAPI.makeRequest).toHaveBeenCalled() - expect(NylasAPI.makeRequest.callCount).toBe 1 - req = NylasAPI.makeRequest.calls[0].args[0] - expect(req.path).toBe "/drafts/#{@draft.serverId}" - expect(req.accountId).toBe TEST_ACCOUNT_ID - expect(req.method).toBe "DELETE" - expect(req.returnsModel).toBe false - - it "should increment the change tracker, preventing any further deltas about the draft", -> - @task.performLocal() - waitsForPromise => - @task._deleteRemoteDraft(@draft).then => - expect(NylasAPI.incrementRemoteChangeLock).toHaveBeenCalledWith(Message, @draft.serverId) - - it "should continue if the request fails", -> - jasmine.unspy(NylasAPI, "makeRequest") - spyOn(NylasAPI, 'makeRequest').andCallFake (options) => - Promise.reject(new APIError(body: "Boo", statusCode: 500)) - - @task.performLocal() - waitsForPromise => - @task._deleteRemoteDraft(@draft) - .then => - expect(NylasAPI.makeRequest).toHaveBeenCalled() - expect(NylasAPI.makeRequest.callCount).toBe 1 - .catch => - throw new Error("Shouldn't fail the promise") - - describe "with uploads", -> - beforeEach -> - @uploads = [ - {targetPath: '/test-file-1.png', size: 100}, - {targetPath: '/test-file-2.png', size: 100} - ] - @draft = new Message - version: 1 - clientId: 'client-id' - accountId: TEST_ACCOUNT_ID - from: [new Contact(email: TEST_ACCOUNT_EMAIL)] - subject: 'New Draft' - draft: true - body: 'hello world' - uploads: [].concat(@uploads) - - @task = new SendDraftTask(@draft) - jasmine.unspy(NylasAPI, 'makeRequest') - - @resolves = [] - @resolveAll = => - resolve() for resolve in @resolves - @resolves = [] - advanceClock() - - spyOn(NylasAPI, 'makeRequest').andCallFake (options) => - response = @response - - if options.path is '/files' - response = JSON.stringify([{ - id: '1234' - account_id: TEST_ACCOUNT_ID - filename: options.formData.file.options.filename - }]) - - new Promise (resolve, reject) => - @resolves.push => - options.success?(response) - resolve(response) - - spyOn(DatabaseStore, 'findBy').andCallFake => - Promise.resolve(@draft) - - it "should begin file uploads and not hit /send until they complete", -> - @task.performRemote() - advanceClock() - - # uploads should be queued, but not the send - expect(NylasAPI.makeRequest.callCount).toEqual(2) - expect(NylasAPI.makeRequest.calls[0].args[0].formData).toEqual({ file : { value : 'stub', options : { filename : 'test-file-1.png' } } }) - expect(NylasAPI.makeRequest.calls[1].args[0].formData).toEqual({ file : { value : 'stub', options : { filename : 'test-file-2.png' } } }) - - # finish all uploads - @resolveAll() - - # send should now be queued - expect(NylasAPI.makeRequest.callCount).toEqual(3) - expect(NylasAPI.makeRequest.calls[2].args[0].path).toEqual('/send') - - it "should convert the uploads to files", -> - @task.performRemote() - advanceClock() - expect(@task.draft.files.length).toEqual(0) - expect(@task.draft.uploads.length).toEqual(2) - @resolves[0]() - advanceClock() - expect(@task.draft.files.length).toEqual(1) - expect(@task.draft.uploads.length).toEqual(1) - - {filename, accountId, id} = @task.draft.files[0] - expect({filename, accountId, id}).toEqual({ - filename: 'test-file-1.png', - accountId: TEST_ACCOUNT_ID, - id: '1234' - }) - - describe "cancel, during attachment upload", -> - it "should make the task resolve early, before making the /send call", -> - exitStatus = null - @task.performRemote().then (status) => exitStatus = status - advanceClock() - @task.cancel() - NylasAPI.makeRequest.reset() - @resolveAll() - advanceClock() - expect(NylasAPI.makeRequest).not.toHaveBeenCalled() - expect(exitStatus).toEqual(Task.Status.Continue) - - describe "after the message sends", -> - it "should notify the attachments were uploaded (so they can be deleted)", -> - @task.performRemote() - advanceClock() - @resolveAll() # uploads - @resolveAll() # send - expect(Actions.attachmentUploaded).toHaveBeenCalled() - expect(Actions.attachmentUploaded.callCount).toEqual(@uploads.length) - for upload, idx in @uploads - expect(Actions.attachmentUploaded.calls[idx].args[0]).toBe(upload) diff --git a/spec/tasks/syncback-draft-files-task-spec.coffee b/spec/tasks/syncback-draft-files-task-spec.coffee new file mode 100644 index 000000000..9537b2ad6 --- /dev/null +++ b/spec/tasks/syncback-draft-files-task-spec.coffee @@ -0,0 +1,95 @@ +_ = require 'underscore' +fs = require 'fs' +{APIError, + Actions, + DatabaseStore, + DatabaseTransaction, + Message, + Contact, + Task, + TaskQueue, + SyncbackDraftFilesTask, + NylasAPI, + SoundRegistry} = require 'nylas-exports' + +DBt = DatabaseTransaction.prototype + +describe "SyncbackDraftFilesTask", -> + describe "with uploads", -> + beforeEach -> + @uploads = [ + {targetPath: '/test-file-1.png', size: 100}, + {targetPath: '/test-file-2.png', size: 100} + ] + @draft = new Message + version: 1 + clientId: 'client-id' + accountId: TEST_ACCOUNT_ID + from: [new Contact(email: TEST_ACCOUNT_EMAIL)] + subject: 'New Draft' + draft: true + body: 'hello world' + uploads: [].concat(@uploads) + + @task = new SyncbackDraftFilesTask(@draft.clientId) + + @resolves = [] + @resolveAll = => + resolve() for resolve in @resolves + @resolves = [] + advanceClock() + + spyOn(DBt, 'persistModel') + spyOn(fs, 'createReadStream').andReturn "stub" + spyOn(NylasAPI, 'makeRequest').andCallFake (options) => + response = @response + + if options.path is '/files' + response = JSON.stringify([{ + id: '1234' + account_id: TEST_ACCOUNT_ID + filename: options.formData.file.options.filename + }]) + + new Promise (resolve, reject) => + @resolves.push => + options.success?(response) + resolve(response) + + spyOn(DatabaseStore, 'run').andCallFake => + Promise.resolve(@draft) + + it "should begin file uploads and not resolve until they complete", -> + taskPromise = @task.performRemote() + advanceClock() + + # uploads should be queued, but not the send + expect(NylasAPI.makeRequest.callCount).toEqual(2) + expect(NylasAPI.makeRequest.calls[0].args[0].formData).toEqual({ file : { value : 'stub', options : { filename : 'test-file-1.png' } } }) + expect(NylasAPI.makeRequest.calls[1].args[0].formData).toEqual({ file : { value : 'stub', options : { filename : 'test-file-2.png' } } }) + + # finish all uploads + expect(taskPromise.isFulfilled()).toBe(false) + @resolveAll() + expect(taskPromise.isFulfilled()).toBe(true) + + it "should update the draft, removing uploads and adding files", -> + taskPromise = @task.performRemote() + advanceClock() + @resolveAll() + advanceClock() + expect(DBt.persistModel).toHaveBeenCalled() + draft = DBt.persistModel.mostRecentCall.args[0] + expect(draft.files.length).toBe(2) + expect(draft.uploads.length).toBe(0) + + it "should not interfere with other uploads added to the draft during task execution", -> + taskPromise = @task.performRemote() + advanceClock() + @draft.uploads.push({targetPath: '/test-file-3.png', size: 100}) + @resolveAll() + advanceClock() + expect(DBt.persistModel).toHaveBeenCalled() + draft = DBt.persistModel.mostRecentCall.args[0] + expect(draft.files.length).toBe(2) + expect(draft.uploads.length).toBe(1) diff --git a/spec/tasks/syncback-draft-task-spec.coffee b/spec/tasks/syncback-draft-task-spec.coffee index a44e60488..c3cca6160 100644 --- a/spec/tasks/syncback-draft-task-spec.coffee +++ b/spec/tasks/syncback-draft-task-spec.coffee @@ -164,74 +164,30 @@ describe "SyncbackDraftTask", -> draft = remoteDraft() draft.pluginMetadata = [{pluginId: 1, value: {a: 1}}] task = new SyncbackDraftTask(draft.clientId) - spyOn(task, 'getLatestLocalDraft').andReturn Promise.resolve(draft) + spyOn(task, 'refreshDraftReference').andCallFake -> + task.draft = draft + Promise.resolve(draft) spyOn(Actions, 'queueTask') waitsForPromise => - task.updateLocalDraft(draft).then => + task.applyResponseToDraft(draft).then => expect(Actions.queueTask).not.toHaveBeenCalled() it "should save metadata associated to the draft when the draft is syncbacked for the first time", -> draft = localDraft() draft.pluginMetadata = [{pluginId: 1, value: {a: 1}}] task = new SyncbackDraftTask(draft.clientId) - spyOn(task, 'getLatestLocalDraft').andReturn Promise.resolve(draft) + spyOn(task, 'refreshDraftReference').andCallFake => + task.draft = draft + Promise.resolve() spyOn(Actions, 'queueTask') waitsForPromise => - task.updateLocalDraft(draft).then => + task.applyResponseToDraft(draft).then => metadataTask = Actions.queueTask.mostRecentCall.args[0] expect(metadataTask instanceof SyncbackMetadataTask).toBe true expect(metadataTask.clientId).toEqual draft.clientId expect(metadataTask.modelClassName).toEqual 'Message' expect(metadataTask.pluginId).toEqual 1 - describe 'when `from` value does not match the account associated to the draft', -> - beforeEach -> - @serverId = 'remote123' - @draft = remoteDraft() - @draft.serverId = 'remote123' - @draft.from = [{email: 'another@email.com'}] - @task = new SyncbackDraftTask(@draft.clientId) - jasmine.unspy(AccountStore, 'accountForEmail') - spyOn(AccountStore, "accountForEmail").andReturn {id: 'other-account'} - spyOn(Actions, "queueTask") - spyOn(@task, 'getLatestLocalDraft').andReturn Promise.resolve(@draft) - - it "should delete the remote draft if it was already saved", -> - waitsForPromise => - @task.checkDraftFromMatchesAccount(@draft).then => - expect(NylasAPI.makeRequest).toHaveBeenCalled() - params = NylasAPI.makeRequest.mostRecentCall.args[0] - expect(params.method).toEqual "DELETE" - expect(params.path).toEqual "/drafts/#{@serverId}" - - it "should increment the change tracker for the deleted serverId, preventing any further deltas about the draft", -> - waitsForPromise => - @task.checkDraftFromMatchesAccount(@draft).then => - expect(NylasAPI.incrementRemoteChangeLock).toHaveBeenCalledWith(Message, 'remote123') - - it "should change the accountId and clear server fields", -> - waitsForPromise => - @task.checkDraftFromMatchesAccount(@draft).then (updatedDraft) => - expect(updatedDraft.serverId).toBeUndefined() - expect(updatedDraft.version).toBeUndefined() - expect(updatedDraft.threadId).toBeUndefined() - expect(updatedDraft.replyToMessageId).toBeUndefined() - expect(updatedDraft.accountId).toEqual 'other-account' - - it "should syncback any metadata associated with the original draft", -> - @draft.pluginMetadata = [{pluginId: 1, value: {a: 1}}] - @task = new SyncbackDraftTask(@draft.clientId) - spyOn(@task, 'getLatestLocalDraft').andReturn Promise.resolve(@draft) - spyOn(@task, 'saveDraft').andCallFake (d) -> Promise.resolve(d) - waitsForPromise => - @task.performRemote().then => - metadataTask = Actions.queueTask.mostRecentCall.args[0] - expect(metadataTask instanceof SyncbackMetadataTask).toBe true - expect(metadataTask.clientId).toEqual @draft.clientId - expect(metadataTask.modelClassName).toEqual 'Message' - expect(metadataTask.pluginId).toEqual 1 - - describe "When the api throws errors", -> stubAPI = (code, method) -> spyOn(NylasAPI, "makeRequest").andCallFake (opts) -> @@ -245,7 +201,9 @@ describe "SyncbackDraftTask", -> beforeEach -> @task = new SyncbackDraftTask("removeDraftId") - spyOn(@task, "getLatestLocalDraft").andCallFake -> Promise.resolve(remoteDraft()) + spyOn(@task, 'refreshDraftReference').andCallFake => + @task.draft = remoteDraft() + Promise.resolve() NylasAPI.PermanentErrorCodes.forEach (code) -> it "fails on API status code #{code}", -> @@ -253,8 +211,8 @@ describe "SyncbackDraftTask", -> waitsForPromise => @task.performRemote().then ([status, err]) => expect(status).toBe Task.Status.Failed - expect(@task.getLatestLocalDraft).toHaveBeenCalled() - expect(@task.getLatestLocalDraft.calls.length).toBe 1 + expect(@task.refreshDraftReference).toHaveBeenCalled() + expect(@task.refreshDraftReference.calls.length).toBe 1 expect(err.statusCode).toBe code [NylasAPI.TimeoutErrorCode].forEach (code) -> @@ -269,5 +227,5 @@ describe "SyncbackDraftTask", -> waitsForPromise => @task.performRemote().then ([status, err]) => expect(status).toBe Task.Status.Failed - expect(@task.getLatestLocalDraft).toHaveBeenCalled() - expect(@task.getLatestLocalDraft.calls.length).toBe 1 + expect(@task.refreshDraftReference).toHaveBeenCalled() + expect(@task.refreshDraftReference.calls.length).toBe 1 diff --git a/src/flux/actions.coffee b/src/flux/actions.coffee index fa667efb4..0e9a96d32 100644 --- a/src/flux/actions.coffee +++ b/src/flux/actions.coffee @@ -376,6 +376,7 @@ class Actions ``` ### @sendDraft: ActionScopeWindow + @ensureDraftSynced: ActionScopeWindow ### Public: Destroys the draft with the given ID. This Action is handled by the {DraftStore}, @@ -471,7 +472,6 @@ class Actions @addAttachment: ActionScopeWindow @selectAttachment: ActionScopeWindow @removeAttachment: ActionScopeWindow - @attachmentUploaded: ActionScopeWindow @fetchAndOpenFile: ActionScopeWindow @fetchAndSaveFile: ActionScopeWindow diff --git a/src/flux/nylas-api.coffee b/src/flux/nylas-api.coffee index 8319200c9..18307e20a 100644 --- a/src/flux/nylas-api.coffee +++ b/src/flux/nylas-api.coffee @@ -2,6 +2,7 @@ _ = require 'underscore' request = require 'request' Utils = require './models/utils' Account = require './models/account' +Message = require './models/message' Actions = require './actions' {APIError} = require './errors' PriorityUICoordinator = require '../priority-ui-coordinator' @@ -279,7 +280,7 @@ class NylasAPI # problems downstream when we try to write to the database. uniquedJSONs = _.uniq jsons, false, (model) -> model.id if uniquedJSONs.length < jsons.length - console.warn("NylasAPI.handleModelResponse: called with non-unique object set. Maybe an API request returned the same object more than once?") + console.warn("NylasAPI::handleModelResponse: called with non-unique object set. Maybe an API request returned the same object more than once?") # Step 2: Filter out any objects we've locked (usually because we successfully) # deleted them moments ago. @@ -375,6 +376,17 @@ class NylasAPI if requestSuccess requestSuccess(jsons) + makeDraftDeletionRequest: (draft) -> + return unless draft.serverId + @incrementRemoteChangeLock(Message, draft.serverId) + @makeRequest + path: "/drafts/#{draft.serverId}" + accountId: draft.accountId + method: "DELETE" + body: {version: draft.version} + returnsModel: false + return + incrementRemoteChangeLock: (klass, id) -> @_lockTracker.increment(klass, id) diff --git a/src/flux/stores/draft-store-proxy.coffee b/src/flux/stores/draft-store-proxy.coffee index c9dc93d20..13459ae16 100644 --- a/src/flux/stores/draft-store-proxy.coffee +++ b/src/flux/stores/draft-store-proxy.coffee @@ -58,8 +58,8 @@ class DraftChangeSet applyToModel: (model) => if model - model.fromJSON(@_saving) - model.fromJSON(@_pending) + model[key] = val for key, val of @_saving + model[key] = val for key, val of @_pending model ### @@ -92,8 +92,7 @@ class DraftStoreProxy @changes = new DraftChangeSet(@_changeSetTrigger, @_changeSetCommit) if draft - @_setDraft(draft) - @_draftPromise = Promise.resolve(@) + @_draftPromise = @_setDraft(draft) @prepare() @@ -115,9 +114,7 @@ class DraftStoreProxy @_draftPromise ?= DatabaseStore.findBy(Message, clientId: @draftClientId).include(Message.attributes.body).then (draft) => return Promise.reject(new Error("Draft has been destroyed.")) if @_destroyed return Promise.reject(new Error("Assertion Failure: Draft #{@draftClientId} not found.")) if not draft - @_setDraft(draft) - Promise.resolve(@) - @_draftPromise + return @_setDraft(draft) teardown: -> @stopListeningToAll() @@ -133,8 +130,20 @@ class DraftStoreProxy # to send with an empty body?" if draft.pristine @_draftPristineBody = draft.body - @_draft = draft - @trigger() + + # Reverse draft transformations performed by third-party plugins when the draft + # was last saved to disk + DraftStore = require './draft-store' + + return Promise.each DraftStore.extensions(), (ext) -> + if ext.applyTransformsToDraft and ext.unapplyTransformsToDraft + Promise.resolve(ext.unapplyTransformsToDraft({draft})).then (untransformed) -> + unless untransformed is 'unnecessary' + draft = untransformed + .then => + @_draft = draft + @trigger() + Promise.resolve(@) _onDraftChanged: (change) -> return if not change? @@ -184,8 +193,7 @@ class DraftStoreProxy # once they have a serverId we sync them periodically here. # return unless @_draft.serverId - - Actions.queueTask(new SyncbackDraftTask(@draftClientId)) + Actions.ensureDraftSynced(@draftClientId) DraftStoreProxy.DraftChangeSet = DraftChangeSet diff --git a/src/flux/stores/draft-store.coffee b/src/flux/stores/draft-store.coffee index 640c68f40..8e9fd245a 100644 --- a/src/flux/stores/draft-store.coffee +++ b/src/flux/stores/draft-store.coffee @@ -4,6 +4,7 @@ moment = require 'moment' {ipcRenderer} = require 'electron' +NylasAPI = require '../nylas-api' DraftStoreProxy = require './draft-store-proxy' DatabaseStore = require './database-store' AccountStore = require './account-store' @@ -12,7 +13,10 @@ TaskQueueStatusStore = require './task-queue-status-store' FocusedPerspectiveStore = require './focused-perspective-store' FocusedContentStore = require './focused-content-store' +BaseDraftTask = require '../tasks/base-draft-task' SendDraftTask = require '../tasks/send-draft-task' +SyncbackDraftFilesTask = require '../tasks/syncback-draft-files-task' +SyncbackDraftTask = require '../tasks/syncback-draft-task' DestroyDraftTask = require '../tasks/destroy-draft-task' InlineStyleTransformer = require '../../services/inline-style-transformer' @@ -71,6 +75,7 @@ class DraftStore # Remember that these two actions only fire in the current window and # are picked up by the instance of the DraftStore in the current # window. + @listenTo Actions.ensureDraftSynced, @_onEnsureDraftSynced @listenTo Actions.sendDraft, @_onSendDraft @listenTo Actions.destroyDraft, @_onDestroyDraft @@ -171,7 +176,7 @@ class DraftStore # window.close() within on onbeforeunload could do weird things. for key, session of @_draftSessions if session.draft()?.pristine - Actions.queueTask(new DestroyDraftTask(draftClientId: session.draftClientId)) + Actions.queueTask(new DestroyDraftTask(session.draftClientId)) else promises.push(session.changes.commit()) @@ -492,55 +497,87 @@ class DraftStore if session @_doneWithSession(session) - # Stop any pending SendDraftTasks + # Stop any pending tasks related ot the draft for task in TaskQueueStatusStore.queue() - if task instanceof SendDraftTask and task.draft.clientId is draftClientId + if task instanceof BaseDraftTask and task.draftClientId is draftClientId Actions.dequeueTask(task.id) # Queue the task to destroy the draft - Actions.queueTask(new DestroyDraftTask(draftClientId: draftClientId)) + Actions.queueTask(new DestroyDraftTask(draftClientId)) NylasEnv.close() if @_isPopout() # The user request to send the draft - _onSendDraft: (draftClientId) => - if NylasEnv.config.get("core.sending.sounds") - SoundRegistry.playSound('hit-send') + _onEnsureDraftSynced: (draftClientId) => + @sessionForClientId(draftClientId).then (session) => + @_prepareForSyncback(session).then => + Actions.queueTask(new SyncbackDraftFilesTask(draftClientId)) + Actions.queueTask(new SyncbackDraftTask(draftClientId)) + _onSendDraft: (draftClientId) => @_draftsSending[draftClientId] = true - # It's important NOT to call `trigger(draftClientId)` here. At this - # point there are still unpersisted changes in the DraftStoreProxy. If - # we `trigger`, we'll briefly display the wrong version of the draft - # as if it was sending. - @sessionForClientId(draftClientId) - .then(@_runExtensionsBeforeSend) - .then (session) => - # Immediately save any pending changes so we don't save after - # sending - # - # We do NOT queue a final {SyncbackDraftTask} before sending because - # we're going to send the full raw body with the Send are are about - # to delete the draft anyway. - # - # We do, however, need to ensure that all of the pending changes are - # committed to the Database since we'll look them up again just - # before send. - session.changes.commit(noSyncback: true).then => - draft = session.draft() - Actions.queueTask(new SendDraftTask(draft)) + @sessionForClientId(draftClientId).then (session) => + @_prepareForSyncback(session).then => + if NylasEnv.config.get("core.sending.sounds") + SoundRegistry.playSound('hit-send') + Actions.queueTask(new SyncbackDraftFilesTask(draftClientId)) + Actions.queueTask(new SendDraftTask(draftClientId)) @_doneWithSession(session) - NylasEnv.close() if @_isPopout() + if @_isPopout() + NylasEnv.close() _isPopout: -> NylasEnv.getWindowType() is "composer" - # Give third-party plugins an opportunity to sanitize draft data - _runExtensionsBeforeSend: (session) => + _prepareForSyncback: (session) => + draft = session.draft() + + # Make sure the draft is attached to a valid account, and change it's + # accountId if the from address does not match the current account. + account = AccountStore.accountForEmail(draft.from[0].email) + unless account + return Promise.reject(new Error("DraftStore._finalizeForSending - you can only send drafts from a configured account.")) + + if account.id isnt draft.accountId + NylasAPI.makeDraftDeletionRequest(draft) + session.changes.add({ + accountId: account.id + version: null + serverId: null + threadId: null + replyToMessageId: null + }) + + # Run draft transformations registered by third-party plugins + allowedFields = ['to', 'from', 'cc', 'bcc', 'subject', 'body'] + Promise.each @extensions(), (ext) -> - ext.finalizeSessionBeforeSending?({session}) - .return(session) + extApply = ext.applyTransformsToDraft + extUnapply = ext.unapplyTransformsToDraft + unless extApply and extUnapply + return Promise.resolve() + + draft = session.draft().clone() + Promise.resolve(extUnapply({draft})).then (cleaned) => + cleaned = draft if cleaned is 'unnecessary' + Promise.resolve(extApply({draft: cleaned})).then (transformed) => + Promise.resolve(extUnapply({draft: transformed.clone()})).then (untransformed) => + untransformed = cleaned if untransformed is 'unnecessary' + + if not _.isEqual(_.pick(untransformed, allowedFields), _.pick(cleaned, allowedFields)) + console.log("-- BEFORE --") + console.log(draft.body) + console.log("-- TRANSFORMED --") + console.log(transformed.body) + console.log("-- UNTRANSFORMED (should match BEFORE) --") + console.log(untransformed.body) + NylasEnv.reportError(new Error("An extension applied a tranform to the draft that it could not reverse.")) + session.changes.add(_.pick(transformed, allowedFields)) + + .then => + session.changes.commit(noSyncback: true) _onRemoveFile: ({file, messageClientId}) => @sessionForClientId(messageClientId).then (session) -> diff --git a/src/flux/stores/file-upload-store.coffee b/src/flux/stores/file-upload-store.coffee index 1a131685f..d282f8e47 100644 --- a/src/flux/stores/file-upload-store.coffee +++ b/src/flux/stores/file-upload-store.coffee @@ -32,10 +32,12 @@ class FileUploadStore extends NylasStore @listenTo Actions.addAttachment, @_onAddAttachment @listenTo Actions.selectAttachment, @_onSelectAttachment @listenTo Actions.removeAttachment, @_onRemoveAttachment - @listenTo Actions.attachmentUploaded, @_onAttachmentUploaded @listenTo DatabaseStore, @_onDataChanged mkdirp.sync(UPLOAD_DIR) + if NylasEnv.isMainWindow() or NylasEnv.inSpecMode() + @listenTo Actions.sendDraftSuccess, ({messageClientId}) => + @_deleteUploadsForClientId(messageClientId) # Handlers @@ -79,10 +81,6 @@ class FileUploadStore extends NylasStore @_deleteUpload(upload).catch(@_onAttachFileError) - _onAttachmentUploaded: (upload) -> - return Promise.resolve() unless upload - @_deleteUpload(upload) - _onAttachFileError: (error) -> NylasEnv.showErrorDialog(error.message) @@ -136,6 +134,11 @@ class FileUploadStore extends NylasStore .catch -> Promise.reject(new Error("Error cleaning up file #{upload.filename}")) + _deleteUploadsForClientId: (messageClientId) => + rimraf = require('rimraf') + rimraf path.join(UPLOAD_DIR, messageClientId), {disableGlob: true}, (err) => + console.warn(err) if err + _applySessionChanges: (messageClientId, changeFunction) => DraftStore.sessionForClientId(messageClientId).then (session) => uploads = changeFunction(session.draft().uploads) diff --git a/src/flux/stores/outbox-store.es6 b/src/flux/stores/outbox-store.es6 index f7021f26b..94974f137 100644 --- a/src/flux/stores/outbox-store.es6 +++ b/src/flux/stores/outbox-store.es6 @@ -1,9 +1,9 @@ import NylasStore from 'nylas-store'; import SendDraftTask from '../tasks/send-draft-task'; +import SyncbackDraftTask from '../tasks/syncback-draft-task'; import TaskQueueStatusStore from './task-queue-status-store'; class OutboxStore extends NylasStore { - constructor() { super(); this._tasks = []; @@ -12,9 +12,9 @@ class OutboxStore extends NylasStore { } _populate() { - const nextTasks = TaskQueueStatusStore.queue().filter((task)=> { - return task instanceof SendDraftTask; - }); + const nextTasks = TaskQueueStatusStore.queue().filter((task) => + (task instanceof SendDraftTask) || (task instanceof SyncbackDraftTask) + ); if ((this._tasks.length === 0) && (nextTasks.length === 0)) { return; } @@ -23,9 +23,7 @@ class OutboxStore extends NylasStore { } itemsForAccount(accountId) { - return this._tasks.filter((task)=> { - return task.draft.accountId === accountId; - }); + return this._tasks.filter((task) => task.draftAccountId === accountId); } } module.exports = new OutboxStore(); diff --git a/src/flux/tasks/base-draft-task.coffee b/src/flux/tasks/base-draft-task.coffee new file mode 100644 index 000000000..02c3a910d --- /dev/null +++ b/src/flux/tasks/base-draft-task.coffee @@ -0,0 +1,57 @@ +_ = require 'underscore' +DatabaseStore = require '../stores/database-store' +Task = require './task' +Message = require '../models/message' +{APIError} = require '../errors' + +class DraftNotFoundError extends Error + constructor: -> + super + +class BaseDraftTask extends Task + + constructor: (@draftClientId) -> + @draft = null + super + + shouldDequeueOtherTask: (other) -> + isSameDraft = other.draftClientId is @draftClientId + isOlderTask = other.sequentialId < @sequentialId + isExactClass = other.constructor.name is @constructor.name + return isSameDraft and isOlderTask and isExactClass + + isDependentOnTask: (other) -> + # Set this task to be dependent on any SyncbackDraftTasks and + # SendDraftTasks for the same draft that were created first. + # This, in conjunction with this method on SendDraftTask, ensures + # that a send and a syncback never run at the same time for a draft. + + # Require here rather than on top to avoid a circular dependency + isSameDraft = other.draftClientId is @draftClientId + isOlderTask = other.sequentialId < @sequentialId + isSaveOrSend = other instanceof BaseDraftTask + + return isSameDraft and isOlderTask and isSaveOrSend + + 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 @draftClientId + errMsg = "Attempt to call #{@constructor.name}.performLocal without a draftClientId" + return Promise.reject(new Error(errMsg)) + Promise.resolve() + + refreshDraftReference: => + DatabaseStore + .findBy(Message, clientId: @draftClientId) + .include(Message.attributes.body) + .then (message) => + unless message and message.draft + return Promise.reject(new DraftNotFoundError()) + @draft = message + return Promise.resolve(message) + + +BaseDraftTask.DraftNotFoundError = DraftNotFoundError +module.exports = BaseDraftTask diff --git a/src/flux/tasks/destroy-draft-task.coffee b/src/flux/tasks/destroy-draft-task.coffee index 505fe59f9..60f88a5a5 100644 --- a/src/flux/tasks/destroy-draft-task.coffee +++ b/src/flux/tasks/destroy-draft-task.coffee @@ -4,32 +4,21 @@ Message = require '../models/message' DatabaseStore = require '../stores/database-store' Actions = require '../actions' NylasAPI = require '../nylas-api' - -SyncbackDraftTask = require './syncback-draft-task' -SendDraftTask = require './send-draft-task' +BaseDraftTask = require './base-draft-task' module.exports = -class DestroyDraftTask extends Task - constructor: ({@draftClientId} = {}) -> - super +class DestroyDraftTask extends BaseDraftTask + constructor: (@draftClientId) -> + super(@draftClientId) shouldDequeueOtherTask: (other) -> - (other instanceof DestroyDraftTask and other.draftClientId is @draftClientId) or - (other instanceof SyncbackDraftTask and other.draftClientId is @draftClientId) or - (other instanceof SendDraftTask and other.draftClientId is @draftClientId) - - isDependentOnTask: (other) -> - (other instanceof SyncbackDraftTask and other.draftClientId is @draftClientId) + other instanceof BaseDraftTask and other.draftClientId is @draftClientId performLocal: -> - unless @draftClientId - return Promise.reject(new Error("Attempt to call DestroyDraftTask.performLocal without draftClientId")) - - DatabaseStore.findBy(Message, clientId: @draftClientId).include(Message.attributes.body).then (draft) => - return Promise.resolve() unless draft - @draft = draft + super + @refreshDraftReference().then => DatabaseStore.inTransaction (t) => - t.unpersistModel(draft) + t.unpersistModel(@draft) performRemote: -> # We don't need to do anything if we weren't able to find the draft diff --git a/src/flux/tasks/send-draft-task.coffee b/src/flux/tasks/send-draft-task.coffee index 1b56b8e7b..567073a5f 100644 --- a/src/flux/tasks/send-draft-task.coffee +++ b/src/flux/tasks/send-draft-task.coffee @@ -3,76 +3,45 @@ fs = require 'fs' path = require 'path' Task = require './task' Actions = require '../actions' -File = require '../models/file' Message = require '../models/message' NylasAPI = require '../nylas-api' -TaskQueue = require '../stores/task-queue' {APIError} = require '../errors' SoundRegistry = require '../../sound-registry' DatabaseStore = require '../stores/database-store' AccountStore = require '../stores/account-store' +BaseDraftTask = require './base-draft-task' SyncbackMetadataTask = require './syncback-metadata-task' -SyncbackDraftTask = require './syncback-draft-task' - -class MultiRequestProgressMonitor - - constructor: -> - @_requests = {} - @_expected = {} - - add: (filepath, filesize, request) => - @_requests[filepath] = request - @_expected[filepath] = filesize ? fs.statSync(filepath)["size"] ? 0 - - remove: (filepath) => - delete @_requests[filepath] - delete @_expected[filepath] - - requests: => - _.values(@_requests) - - value: => - sent = 0 - expected = 1 - for filepath, request of @_requests - sent += request.req?.connection?._bytesDispatched ? 0 - expected += @_expected[filepath] - - return sent / expected module.exports = -class SendDraftTask extends Task +class SendDraftTask extends BaseDraftTask - constructor: (@draft) -> + constructor: (@draftClientId) -> @uploaded = [] + @draft = null + @message = null super label: -> "Sending message..." - shouldDequeueOtherTask: (other) -> - # A new send action should knock any other sends that are not - # currently executing out of the queue. It should also knock out - # any SyncbackDraftTasks - running these concurrently with a send - # results in weird behavior. - (other instanceof SendDraftTask and other.draft.clientId is @draft.clientId) or - (other instanceof SyncbackDraftTask and other.draftClientId is @draft.clientId) + performRemote: -> + @refreshDraftReference() + .then(@assertDraftValidity) + .then(@sendMessage) + .then (responseJSON) => + @message = new Message().fromJSON(responseJSON) + @message.clientId = @draft.clientId + @message.draft = false + @message.clonePluginMetadataFrom(@draft) + DatabaseStore.inTransaction (t) => + @refreshDraftReference().then => + t.persistModel(@message) - isDependentOnTask: (other) -> - # Set this task to be dependent on any SyncbackDraftTasks for the - # same draft that were created first, to ensure this task does not - # execute at the same time as a syncback. Works in conjunction with - # similar restrictions in this method on the SyncbackDraftTask. - other instanceof SyncbackDraftTask and - other.draftClientId is @draft.clientId and - other.sequentialId < @sequentialId + .then(@onSuccess) + .catch(@onError) - performLocal: -> - unless @draft and @draft instanceof Message - return Promise.reject(new Error("SendDraftTask - must be provided a draft.")) - unless @draft.uploads and @draft.uploads instanceof Array - return Promise.reject(new Error("SendDraftTask - must be provided an array of uploads.")) + assertDraftValidity: => unless @draft.from[0] return Promise.reject(new Error("SendDraftTask - you must populate `from` before sending.")) @@ -80,74 +49,17 @@ class SendDraftTask extends Task unless account return Promise.reject(new Error("SendDraftTask - you can only send drafts from a configured account.")) - if account.id isnt @draft.accountId - @draft.accountId = account.id - delete @draft.serverId - delete @draft.version - delete @draft.threadId - delete @draft.replyToMessageId + unless @draft.accountId is account.id + return Promise.reject(new Error("The from address has changed since you started sending this draft. Double-check the draft and click 'Send' again.")) - Promise.resolve() + if @draft.uploads and @draft.uploads.length > 0 + return Promise.reject(new Error("Files have been added since you started sending this draft. Double-check the draft and click 'Send' again..")) - performRemote: -> - @_uploadAttachments().then => - return Promise.resolve(Task.Status.Continue) if @_cancelled - @_sendAndCreateMessage() - .then(@_deleteRemoteDraft) - .then(@_onSuccess) - .catch(@_onError) - - cancel: => - # Note that you can only cancel during the uploadAttachments phase. Once - # we hit sendAndCreateMessage, nothing checks the cancelled bit and - # performRemote will continue through to success. - @_cancelled = true - for request in @_attachmentUploadsMonitor.requests() - request.abort() - @ - - _uploadAttachments: => - @_attachmentUploadsMonitor = new MultiRequestProgressMonitor() - Object.defineProperty(@, 'progress', { - configurable: true, - enumerable: true, - get: => @_attachmentUploadsMonitor.value() - }) - - Promise.all @draft.uploads.map (upload) => - {targetPath, size} = upload - - formData = - file: # Must be named `file` as per the Nylas API spec - value: fs.createReadStream(targetPath) - options: - filename: path.basename(targetPath) - - NylasAPI.makeRequest - path: "/files" - accountId: @draft.accountId - method: "POST" - json: false - formData: formData - started: (req) => - @_attachmentUploadsMonitor.add(targetPath, size, req) - timeout: 20 * 60 * 1000 - .finally => - @_attachmentUploadsMonitor.remove(targetPath) - .then (rawResponseString) => - json = JSON.parse(rawResponseString) - file = (new File).fromJSON(json[0]) - @uploaded.push(upload) - @draft.uploads.splice(@draft.uploads.indexOf(upload), 1) - @draft.files.push(file) - - # Note: We don't actually delete uploaded files until send completes, - # because it's possible for the app to quit without saving state and - # need to re-upload the file. + return Promise.resolve() # This function returns a promise that resolves to the draft when the draft has # been sent successfully. - _sendAndCreateMessage: => + sendMessage: => NylasAPI.makeRequest path: "/send" accountId: @draft.accountId @@ -160,58 +72,18 @@ class SendDraftTask extends Task # If the message you're "replying to" were deleted if err.message?.indexOf('Invalid message public id') is 0 @draft.replyToMessageId = null - return @_sendAndCreateMessage() + return @sendMessage() # If the thread was deleted else if err.message?.indexOf('Invalid thread') is 0 @draft.threadId = null @draft.replyToMessageId = null - return @_sendAndCreateMessage() + return @sendMessage() else return Promise.reject(err) - .then (newMessageJSON) => - @message = new Message().fromJSON(newMessageJSON) - @message.clientId = @draft.clientId - @message.draft = false - # Create new metadata objs on the message based on the existing ones in the draft - @message.clonePluginMetadataFrom(@draft) - - return DatabaseStore.inTransaction (t) => - DatabaseStore.findBy(Message, {clientId: @draft.clientId}) - .then (draft) => - t.persistModel(@message).then => - Promise.resolve(draft) - - - # We DON'T need to delete the local draft because we turn it into a message - # by writing the new message into the database with the same clientId. - # - # We DO, need to make sure that the remote draft has been cleaned up. - # - _deleteRemoteDraft: ({accountId, version, serverId}) => - return Promise.resolve() unless serverId - NylasAPI.incrementRemoteChangeLock(Message, serverId) - NylasAPI.makeRequest - path: "/drafts/#{serverId}" - accountId: accountId - method: "DELETE" - body: {version} - returnsModel: false - .catch APIError, (err) => - # If the draft failed to delete remotely, we don't really care. It - # shouldn't stop the send draft task from continuing. - - # Deliberately do not decrement the change count so that deltas about - # this (deleted) draft are ignored. - Promise.resolve() - - _onSuccess: => - # Delete attachments from the uploads folder - for upload in @uploaded - Actions.attachmentUploaded(upload) - + onSuccess: => # Queue a task to save metadata on the message @message.pluginMetadata.forEach((m)=> task = new SyncbackMetadataTask(@message.clientId, @message.constructor.name, m.pluginId) @@ -219,6 +91,7 @@ class SendDraftTask extends Task ) Actions.sendDraftSuccess(message: @message, messageClientId: @message.clientId) + NylasAPI.makeDraftDeletionRequest(@draft) # Play the sending sound if NylasEnv.config.get("core.sending.sounds") @@ -226,10 +99,16 @@ class SendDraftTask extends Task return Promise.resolve(Task.Status.Success) - _onError: (err) => - if err instanceof APIError and not (err.statusCode in NylasAPI.PermanentErrorCodes) - return Promise.resolve(Task.Status.Retry) - else + onError: (err) => + if err instanceof BaseDraftTask.DraftNotFoundError + return Promise.resolve(Task.Status.Continue) + + message = err.message + + if err instanceof APIError + if err.statusCode not in NylasAPI.PermanentErrorCodes + return Promise.resolve(Task.Status.Retry) + message = "Sorry, this message could not be sent. Please try again, and make sure your message is addressed correctly and is not too large." if err.statusCode is 402 and err.body.message if err.body.message.indexOf('at least one recipient') isnt -1 @@ -239,9 +118,9 @@ class SendDraftTask extends Task if err.body.server_error message += "\n\n" + err.body.server_error - Actions.draftSendingFailed - threadId: @draft.threadId - draftClientId: @draft.clientId, - errorMessage: message - NylasEnv.reportError(err) - return Promise.resolve([Task.Status.Failed, err]) + Actions.draftSendingFailed + threadId: @draft.threadId + draftClientId: @draft.clientId, + errorMessage: message + NylasEnv.reportError(err) + return Promise.resolve([Task.Status.Failed, err]) diff --git a/src/flux/tasks/syncback-draft-files-task.coffee b/src/flux/tasks/syncback-draft-files-task.coffee new file mode 100644 index 000000000..7055ff15b --- /dev/null +++ b/src/flux/tasks/syncback-draft-files-task.coffee @@ -0,0 +1,87 @@ +_ = require 'underscore' +fs = require 'fs' +path = require 'path' + +Task = require './task' +Actions = require '../actions' +{APIError} = require '../errors' +File = require '../models/file' +NylasAPI = require '../nylas-api' +Message = require '../models/message' +BaseDraftTask = require './base-draft-task' +DatabaseStore = require '../stores/database-store' +MultiRequestProgressMonitor = require '../../multi-request-progress-monitor' + +module.exports = +class SyncbackDraftFilesTask extends BaseDraftTask + + constructor: (@draftClientId) -> + super(@draftClientId) + @_appliedUploads = null + @_appliedFiles = null + + label: -> + "Uploading attachments..." + + performRemote: -> + @refreshDraftReference() + .then(@uploadAttachments) + .then(@applyChangesToDraft) + .thenReturn(Task.Status.Success) + .catch (err) => + if err instanceof BaseDraftTask.DraftNotFoundError + return Promise.resolve(Task.Status.Continue) + if err instanceof APIError and not (err.statusCode in NylasAPI.PermanentErrorCodes) + return Promise.resolve(Task.Status.Retry) + return Promise.resolve([Task.Status.Failed, err]) + + uploadAttachments: => + @_attachmentUploadsMonitor = new MultiRequestProgressMonitor() + Object.defineProperty(@, 'progress', { + configurable: true, + enumerable: true, + get: => @_attachmentUploadsMonitor.value() + }) + + uploaded = [].concat(@draft.uploads) + Promise.all(uploaded.map(@uploadAttachment)).then (files) => + # Note: We don't actually delete uploaded files until send completes, + # because it's possible for the app to quit without saving state and + # need to re-upload the file. + @_appliedUploads = uploaded + @_appliedFiles = files + + uploadAttachment: (upload) => + {targetPath, size} = upload + + formData = + file: # Must be named `file` as per the Nylas API spec + value: fs.createReadStream(targetPath) + options: + filename: path.basename(targetPath) + + NylasAPI.makeRequest + path: "/files" + accountId: @draft.accountId + method: "POST" + json: false + formData: formData + started: (req) => + @_attachmentUploadsMonitor.add(targetPath, size, req) + timeout: 20 * 60 * 1000 + .finally => + @_attachmentUploadsMonitor.remove(targetPath) + .then (rawResponseString) => + json = JSON.parse(rawResponseString) + file = (new File).fromJSON(json[0]) + Promise.resolve(file) + + applyChangesToDraft: => + DatabaseStore.inTransaction (t) => + @refreshDraftReference().then => + @draft.files = @draft.files.concat(@_appliedFiles) + if @draft.uploads instanceof Array + uploadedPaths = @_appliedUploads.map (upload) => upload.targetPath + @draft.uploads = @draft.uploads.filter (upload) => + upload.targetPath not in uploadedPaths + t.persistModel(@draft) diff --git a/src/flux/tasks/syncback-draft-task.coffee b/src/flux/tasks/syncback-draft-task.coffee index db6640139..20057aab8 100644 --- a/src/flux/tasks/syncback-draft-task.coffee +++ b/src/flux/tasks/syncback-draft-task.coffee @@ -1,137 +1,70 @@ _ = require 'underscore' - -Actions = require '../actions' -AccountStore = require '../stores/account-store' -DatabaseStore = require '../stores/database-store' -TaskQueueStatusStore = require '../stores/task-queue-status-store' -NylasAPI = require '../nylas-api' +fs = require 'fs' +path = require 'path' Task = require './task' +Actions = require '../actions' +DatabaseStore = require '../stores/database-store' +TaskQueueStatusStore = require '../stores/task-queue-status-store' +MultiRequestProgressMonitor = require '../../multi-request-progress-monitor' +NylasAPI = require '../nylas-api' + +BaseDraftTask = require './base-draft-task' SyncbackMetadataTask = require './syncback-metadata-task' {APIError} = require '../errors' -Message = require '../models/message' -Account = require '../models/account' +class DraftNotFoundError extends Error module.exports = -class SyncbackDraftTask extends Task - - constructor: (@draftClientId) -> - super - - shouldDequeueOtherTask: (other) -> - # A new syncback action should knock any other syncbacks that are - # not currently executing out of the queue. - other instanceof SyncbackDraftTask and - other.draftClientId is @draftClientId and - other.sequentialId <= @sequentialId - - isDependentOnTask: (other) -> - # Set this task to be dependent on any SyncbackDraftTasks and - # SendDraftTasks for the same draft that were created first. - # This, in conjunction with this method on SendDraftTask, ensures - # that a send and a syncback never run at the same time for a draft. - - # Require here rather than on top to avoid a circular dependency - SendDraftTask = require './send-draft-task' - - (other instanceof SyncbackDraftTask and - other.draftClientId is @draftClientId and - other.sequentialId < @sequentialId) or - (other instanceof SendDraftTask and - other.draft.clientId is @draftClientId and - other.sequentialId < @sequentialId) - - 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 @draftClientId - errMsg = "Attempt to call SyncbackDraftTask.performLocal without @draftClientId" - return Promise.reject(new Error(errMsg)) - Promise.resolve() +class SyncbackDraftTask extends BaseDraftTask performRemote: -> - @getLatestLocalDraft().then (draft) => - return Promise.resolve() unless draft + @refreshDraftReference() + .then => + if @draft.serverId + requestPath = "/drafts/#{@draft.serverId}" + requestMethod = 'PUT' + else + requestPath = "/drafts" + requestMethod = 'POST' - @checkDraftFromMatchesAccount(draft) - .then(@saveDraft) - .then(@updateLocalDraft) + NylasAPI.makeRequest + accountId: @draft.accountId + path: requestPath + method: requestMethod + body: @draft.toJSON() + returnsModel: false + .then(@applyResponseToDraft) .thenReturn(Task.Status.Success) - .catch (err) => - if err instanceof APIError and not (err.statusCode in NylasAPI.PermanentErrorCodes) - return Promise.resolve(Task.Status.Retry) - return Promise.resolve([Task.Status.Failed, err]) - saveDraft: (draft) => - if draft.serverId - path = "/drafts/#{draft.serverId}" - method = 'PUT' - else - path = "/drafts" - method = 'POST' + .catch (err) => + if err instanceof DraftNotFoundError + return Promise.resolve(Task.Status.Continue) + if err instanceof APIError and not (err.statusCode in NylasAPI.PermanentErrorCodes) + return Promise.resolve(Task.Status.Retry) + return Promise.resolve([Task.Status.Failed, err]) - NylasAPI.makeRequest - accountId: draft.accountId - path: path - method: method - body: draft.toJSON() - returnsModel: false - - updateLocalDraft: ({version, id, thread_id}) => + applyResponseToDraft: (response) => # Important: There could be a significant delay between us initiating the save # and getting JSON back from the server. Our local copy of the draft may have # already changed more. # # The only fields we want to update from the server are the `id` and `version`. # - draftIsNew = false + draftWasCreated = false DatabaseStore.inTransaction (t) => - @getLatestLocalDraft().then (draft) => - # Draft may have been deleted. Oh well. - return Promise.resolve() unless draft - if draft.serverId isnt id - draft.threadId = thread_id - draft.serverId = id - draftIsNew = true - draft.version = version - t.persistModel(draft).then => - Promise.resolve(draft) - .then (draft) => - if draftIsNew - for {pluginId, value} in draft.pluginMetadata - task = new SyncbackMetadataTask(@draftClientId, draft.constructor.name, pluginId) + @refreshDraftReference().then => + if @draft.serverId isnt response.id + @draft.threadId = response.thread_id + @draft.serverId = response.id + draftWasCreated = true + @draft.version = response.version + t.persistModel(@draft) + + .then => + if draftWasCreated + for {pluginId, value} in @draft.pluginMetadata + task = new SyncbackMetadataTask(@draftClientId, @draft.constructor.name, pluginId) Actions.queueTask(task) return true - - getLatestLocalDraft: => - DatabaseStore.findBy(Message, clientId: @draftClientId).include(Message.attributes.body) - .then (message) -> - if not message?.draft - return Promise.resolve() - return Promise.resolve(message) - - checkDraftFromMatchesAccount: (draft) -> - account = AccountStore.accountForEmail(draft.from[0].email) - if draft.accountId is account.id - return Promise.resolve(draft) - else - if draft.serverId - NylasAPI.incrementRemoteChangeLock(Message, draft.serverId) - NylasAPI.makeRequest - path: "/drafts/#{draft.serverId}" - accountId: draft.accountId - method: "DELETE" - body: {version: draft.version} - returnsModel: false - - draft.accountId = account.id - delete draft.serverId - delete draft.version - delete draft.threadId - delete draft.replyToMessageId - DatabaseStore.inTransaction (t) => - t.persistModel(draft) - .thenReturn(draft) diff --git a/src/global/nylas-exports.coffee b/src/global/nylas-exports.coffee index 22ece2510..edc28d1a8 100644 --- a/src/global/nylas-exports.coffee +++ b/src/global/nylas-exports.coffee @@ -105,6 +105,7 @@ class NylasExports @require "SyncbackCategoryTask", 'flux/tasks/syncback-category-task' @require "DestroyCategoryTask", 'flux/tasks/destroy-category-task' @require "ChangeUnreadTask", 'flux/tasks/change-unread-task' + @require "SyncbackDraftFilesTask", 'flux/tasks/syncback-draft-files-task' @require "SyncbackDraftTask", 'flux/tasks/syncback-draft-task' @require "ChangeStarredTask", 'flux/tasks/change-starred-task' @require "DestroyModelTask", 'flux/tasks/destroy-model-task' diff --git a/src/multi-request-progress-monitor.coffee b/src/multi-request-progress-monitor.coffee new file mode 100644 index 000000000..43fc5fc77 --- /dev/null +++ b/src/multi-request-progress-monitor.coffee @@ -0,0 +1,27 @@ +class MultiRequestProgressMonitor + + constructor: -> + @_requests = {} + @_expected = {} + + add: (filepath, filesize, request) => + @_requests[filepath] = request + @_expected[filepath] = filesize ? fs.statSync(filepath)["size"] ? 0 + + remove: (filepath) => + delete @_requests[filepath] + delete @_expected[filepath] + + requests: => + _.values(@_requests) + + value: => + sent = 0 + expected = 1 + for filepath, request of @_requests + sent += request.req?.connection?._bytesDispatched ? 0 + expected += @_expected[filepath] + + return sent / expected + +module.exports = MultiRequestProgressMonitor