From 2b9e96fc7273b349363237e96f7119a137803661 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Thu, 7 Sep 2017 02:36:25 -0700 Subject: [PATCH] Swap replyToMessageId for replyToHeaderMessageId --- .../composer/lib/composer-header.jsx | 2 +- .../composer/specs/composer-header-spec.jsx | 2 +- .../composer/specs/composer-view-spec.cjsx | 4 ++-- .../draft-list/lib/draft-list-store.coffee | 1 + app/spec/stores/draft-factory-spec.es6 | 15 ++++++++++----- app/spec/stores/draft-helpers-spec.es6 | 4 ++-- app/spec/stores/draft-store-spec.es6 | 2 +- app/spec_disabled/tasks/send-draft-task-spec.es6 | 4 ++-- app/src/flux/models/message.es6 | 8 ++++---- app/src/flux/stores/draft-factory.coffee | 6 +++--- app/src/flux/stores/draft-helpers.es6 | 15 +++++++++------ app/src/flux/stores/message-store.coffee | 6 +++--- 12 files changed, 39 insertions(+), 30 deletions(-) diff --git a/app/internal_packages/composer/lib/composer-header.jsx b/app/internal_packages/composer/lib/composer-header.jsx index 3cdec195e..2ac0c83b7 100644 --- a/app/internal_packages/composer/lib/composer-header.jsx +++ b/app/internal_packages/composer/lib/composer-header.jsx @@ -127,7 +127,7 @@ export default class ComposerHeader extends React.Component { if (DraftHelpers.isForwardedMessage(this.props.draft)) { return true; } - if (this.props.draft.replyToMessageId) { + if (this.props.draft.replyToHeaderMessageId) { return false; } return true; diff --git a/app/internal_packages/composer/specs/composer-header-spec.jsx b/app/internal_packages/composer/specs/composer-header-spec.jsx index 2e3acab2a..8189f7f07 100644 --- a/app/internal_packages/composer/specs/composer-header-spec.jsx +++ b/app/internal_packages/composer/specs/composer-header-spec.jsx @@ -111,7 +111,7 @@ describe('ComposerHeader', function composerHeader() { }); it("should be hidden if the message is a reply", () => { - const draft = new Message({draft: true, subject: 'Re: 1234', replyToMessageId: '123', accountId: TEST_ACCOUNT_ID, headerMessageId: DRAFT_HEADER_MSG_ID}); + const draft = new Message({draft: true, subject: 'Re: 1234', replyToHeaderMessageId: '123', accountId: TEST_ACCOUNT_ID, headerMessageId: DRAFT_HEADER_MSG_ID}); this.createWithDraft(draft); expect(this.component.state.enabledFields).toEqual(['textFieldTo', 'fromField']) }); diff --git a/app/internal_packages/composer/specs/composer-view-spec.cjsx b/app/internal_packages/composer/specs/composer-view-spec.cjsx index 0e2ebd656..5176921d9 100644 --- a/app/internal_packages/composer/specs/composer-view-spec.cjsx +++ b/app/internal_packages/composer/specs/composer-view-spec.cjsx @@ -62,7 +62,7 @@ useFullDraft = -> files: [f1, f2] subject: "Test Message 1" body: "Hello World
This is a test" - replyToMessageId: null + replyToHeaderMessageId: null makeComposer = (props={}) -> @composer = NylasTestUtils.renderIntoDocument( @@ -118,7 +118,7 @@ describe "ComposerView", -> to: [u2] subject: "Test Reply Message 1" body: "" - replyToMessageId: "1") + replyToHeaderMessageId: "1") .then( => sessionSetupComplete = true ) diff --git a/app/internal_packages/draft-list/lib/draft-list-store.coffee b/app/internal_packages/draft-list/lib/draft-list-store.coffee index 883e84330..d78169ffe 100644 --- a/app/internal_packages/draft-list/lib/draft-list-store.coffee +++ b/app/internal_packages/draft-list/lib/draft-list-store.coffee @@ -56,6 +56,7 @@ class DraftListStore extends NylasStore # where we set thread.__messages to the message array. resultSetWithTasks = new MutableQueryResultSet(resultSet) + # TODO BG modelWithId: task.headerMessageId does not work mailboxPerspective.accountIds.forEach (aid) => OutboxStore.itemsForAccount(aid).forEach (task) => draft = resultSet.modelWithId(task.headerMessageId) diff --git a/app/spec/stores/draft-factory-spec.es6 b/app/spec/stores/draft-factory-spec.es6 index 8f089400a..e26bfaa44 100644 --- a/app/spec/stores/draft-factory-spec.es6 +++ b/app/spec/stores/draft-factory-spec.es6 @@ -60,6 +60,7 @@ describe('DraftFactory', function draftFactory() { fakeMessage1 = new Message({ id: 'fake-message-1', + headerMessageId: 'fake-message-1@localhost', accountId: account.id, to: [new Contact({email: 'ben@nylas.com'}), new Contact({email: 'evan@nylas.com'})], cc: [new Contact({email: 'mg@nylas.com'}), account.me()], @@ -73,6 +74,7 @@ describe('DraftFactory', function draftFactory() { fakeMessageWithFiles = new Message({ id: 'fake-message-with-files', + headerMessageId: 'fake-message-with-files@localhost', accountId: account.id, to: [new Contact({email: 'ben@nylas.com'}), new Contact({email: 'evan@nylas.com'})], cc: [new Contact({email: 'mg@nylas.com'}), account.me()], @@ -87,6 +89,7 @@ describe('DraftFactory', function draftFactory() { msgFromMe = new Message({ id: 'fake-message-3', + headerMessageId: 'fake-message-3@localhost', accountId: account.id, to: [new Contact({email: '1@1.com'}), new Contact({email: '2@2.com'})], cc: [new Contact({email: '3@3.com'}), new Contact({email: '4@4.com'})], @@ -100,6 +103,7 @@ describe('DraftFactory', function draftFactory() { msgWithReplyTo = new Message({ id: 'fake-message-reply-to', + headerMessageId: 'fake-message-reply-to@localhost', accountId: account.id, to: [new Contact({email: '1@1.com'}), new Contact({email: '2@2.com'})], cc: [new Contact({email: '3@3.com'}), new Contact({email: '4@4.com'})], @@ -122,6 +126,7 @@ describe('DraftFactory', function draftFactory() { msgWithReplyToDuplicates = new Message({ id: 'fake-message-reply-to-duplicates', + headerMessageId: 'fake-message-reply-to-duplicates@localhost', accountId: account.id, to: [new Contact({email: '1@1.com'}), new Contact({email: '2@2.com'})], cc: [new Contact({email: '1@1.com'}), new Contact({email: '4@4.com'})], @@ -152,10 +157,10 @@ describe('DraftFactory', function draftFactory() { }); }); - it("should set the replyToMessageId to the previous message's ids", () => { + it("should set the replyToHeaderMessageId to the previous message's ids", () => { waitsForPromise(() => { return DraftFactory.createDraftForReply({thread: fakeThread, message: fakeMessage1, type: 'reply'}).then((draft) => { - expect(draft.replyToMessageId).toEqual(fakeMessage1.id); + expect(draft.replyToHeaderMessageId).toEqual(fakeMessage1.headerMessageId); }); }); }); @@ -378,8 +383,8 @@ describe('DraftFactory', function draftFactory() { expect(this.model.bcc).toEqual([]); }); - it("should not set the replyToMessageId", () => { - expect(this.model.replyToMessageId).toEqual(undefined); + it("should not set the replyToHeaderMessageId", () => { + expect(this.model.replyToHeaderMessageId).toEqual(undefined); }); it("should sanitize the HTML", () => { @@ -433,7 +438,7 @@ describe('DraftFactory', function draftFactory() { this.existingDraft = new Message({ id: 'asd', accountId: TEST_ACCOUNT_ID, - replyToMessageId: fakeMessage1.id, + replyToHeaderMessageId: fakeMessage1.headerMessageId, threadId: fakeMessage1.threadId, draft: true, }); diff --git a/app/spec/stores/draft-helpers-spec.es6 b/app/spec/stores/draft-helpers-spec.es6 index ced90b832..6d9b217ad 100644 --- a/app/spec/stores/draft-helpers-spec.es6 +++ b/app/spec/stores/draft-helpers-spec.es6 @@ -54,7 +54,7 @@ xdescribe('DraftHelpers', function describeBlock() { describe('shouldAppendQuotedText', () => { it('returns true if message is reply and has no marker', () => { const draft = { - replyToMessageId: 1, + replyToHeaderMessageId: 1, body: `
hello!
`, } expect(DraftHelpers.shouldAppendQuotedText(draft)).toBe(true) @@ -62,7 +62,7 @@ xdescribe('DraftHelpers', function describeBlock() { it('returns false if message is reply and has marker', () => { const draft = { - replyToMessageId: 1, + replyToHeaderMessageId: 1, body: `
hello!
Quoted Text`, } expect(DraftHelpers.shouldAppendQuotedText(draft)).toBe(false) diff --git a/app/spec/stores/draft-store-spec.es6 b/app/spec/stores/draft-store-spec.es6 index 5e657368d..8de08303b 100644 --- a/app/spec/stores/draft-store-spec.es6 +++ b/app/spec/stores/draft-store-spec.es6 @@ -302,7 +302,7 @@ xdescribe('DraftStore', function draftStore() { this.draft = new Message({ headerMessageId: "local-123", threadId: "thread-123", - replyToMessageId: "message-123", + replyToHeaderMessageId: "message-123@localhost", files: ['stub'], }); DraftStore._draftSessions = {}; diff --git a/app/spec_disabled/tasks/send-draft-task-spec.es6 b/app/spec_disabled/tasks/send-draft-task-spec.es6 index 7d566e8f0..872108712 100644 --- a/app/spec_disabled/tasks/send-draft-task-spec.es6 +++ b/app/spec_disabled/tasks/send-draft-task-spec.es6 @@ -205,7 +205,7 @@ xdescribe('SendDraftTask', function sendDraftTask() { return Promise.resolve(this.response); }); - this.draft.replyToMessageId = "reply-123"; + this.draft.replyToHeaderMessageId = "reply-123"; this.draft.threadId = "thread-123"; waitsForPromise(() => { return this.task.performRemote(this.draft) @@ -232,7 +232,7 @@ xdescribe('SendDraftTask', function sendDraftTask() { return Promise.resolve(this.response); }); - this.draft.replyToMessageId = "reply-123"; + this.draft.replyToHeaderMessageId = "reply-123"; this.draft.threadId = "thread-123"; waitsForPromise(() => this.task.performRemote(this.draft).then(() => { expect(NylasAPIRequest.prototype.run).toHaveBeenCalled(); diff --git a/app/src/flux/models/message.es6 b/app/src/flux/models/message.es6 index 0d10ab889..1a57661f6 100644 --- a/app/src/flux/models/message.es6 +++ b/app/src/flux/models/message.es6 @@ -56,8 +56,7 @@ All messages are part of a thread, even if that thread has only one message. `threadId`: {AttributeString} The ID of the Message's parent {Thread}. Queryable. -`replyToMessageId`: {AttributeString} The ID of a {Message} that this message - === in reply to. +`replyToHeaderMessageId`: {AttributeString} The headerMessageID of a {Message} that this message is in reply to. This class also inherits attributes from {Model} @@ -156,8 +155,9 @@ export default class Message extends ModelWithMetadata { queryable: true, }), - replyToMessageId: Attributes.String({ - modelKey: 'replyToMessageId', + replyToHeaderMessageId: Attributes.String({ + jsonKey: 'rthMsgId', + modelKey: 'replyToHeaderMessageId', }), folder: Attributes.Object({ diff --git a/app/src/flux/stores/draft-factory.coffee b/app/src/flux/stores/draft-factory.coffee index 27ea8530f..7f7306ca2 100644 --- a/app/src/flux/stores/draft-factory.coffee +++ b/app/src/flux/stores/draft-factory.coffee @@ -120,8 +120,8 @@ class DraftFactory from: [@_fromContactForReply(message)], threadId: thread.id, accountId: message.accountId, - replyToMessageId: message.id, - body: "" # quoted html is managed by the composer via the replyToMessageId + replyToHeaderMessageId: message.headerMessageId, + body: "" # quoted html is managed by the composer via the replyToHeaderMessageId ) createDraftForForward: ({thread, message}) => @@ -166,7 +166,7 @@ class DraftFactory getMessages.then (messages) => candidateDrafts = messages.filter (other) => - other.replyToMessageId is message.id and other.draft is true + other.replyToHeaderMessageId is message.id and other.draft is true if candidateDrafts.length is 0 return Promise.resolve(null) diff --git a/app/src/flux/stores/draft-helpers.es6 b/app/src/flux/stores/draft-helpers.es6 index 1088d6376..20050ba96 100644 --- a/app/src/flux/stores/draft-helpers.es6 +++ b/app/src/flux/stores/draft-helpers.es6 @@ -37,10 +37,10 @@ class DraftHelpers { return bodyForwarded || bodyFwd || subjectFwd } - shouldAppendQuotedText({body = '', replyToMessageId = false} = {}) { - return replyToMessageId && + shouldAppendQuotedText({body = '', replyToHeaderMessageId = false} = {}) { + return replyToHeaderMessageId && !body.includes('
') && - !body.includes(`nylas-quote-id-${replyToMessageId}`) + !body.includes(`nylas-quote-id-${replyToHeaderMessageId}`) } prepareBodyForQuoting(body = "") { @@ -75,7 +75,10 @@ class DraftHelpers { } appendQuotedTextToDraft(draft) { - const query = DatabaseStore.find(Message, draft.replyToMessageId).include(Message.attributes.body); + const query = DatabaseStore.findBy(Message, { + headerMessageId: draft.replyToHeaderMessageId, + accountId: draft.accountId, + }).include(Message.attributes.body); return query.then((prevMessage) => { if (!prevMessage) { @@ -83,7 +86,7 @@ class DraftHelpers { } return this.prepareBodyForQuoting(prevMessage.body).then((prevBodySanitized) => { draft.body = `${draft.body} -
+

${DOMUtils.escapeHTMLCharacters(prevMessage.replyAttributionLine())}
@@ -127,7 +130,7 @@ class DraftHelpers { draft = await this.applyExtensionTransforms(draft) draft = await this.pruneRemovedInlineFiles(draft); - if (draft.replyToMessageId && this.shouldAppendQuotedText(draft)) { + if (draft.replyToHeaderMessageId && this.shouldAppendQuotedText(draft)) { draft = await this.appendQuotedTextToDraft(draft); } return draft; diff --git a/app/src/flux/stores/message-store.coffee b/app/src/flux/stores/message-store.coffee index 71fef22a3..497ff74b4 100644 --- a/app/src/flux/stores/message-store.coffee +++ b/app/src/flux/stores/message-store.coffee @@ -280,10 +280,10 @@ class MessageStore extends NylasStore _sortItemsForDisplay: (items) -> # Re-sort items in the list so that drafts appear after the message that # they are in reply to, when possible. First, identify all the drafts - # with a replyToMessageId and remove them + # with a replyToHeaderMessageId and remove them itemsInReplyTo = [] for item, index in items by -1 - if item.draft and item.replyToMessageId + if item.draft and item.replyToHeaderMessageId itemsInReplyTo.push(item) items.splice(index, 1) @@ -291,7 +291,7 @@ class MessageStore extends NylasStore # the message which it was in reply to. If we can't find it, put it at the end. for item in itemsInReplyTo for other, index in items - if item.replyToMessageId is other.id + if item.replyToHeaderMessageId is other.id items.splice(index+1, 0, item) item = null break