From fa69b07eaa2903a5a9ab03d057b294a4ce899e68 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Fri, 13 Mar 2015 10:55:28 -0700 Subject: [PATCH] fix(attachments): T427 - show attachments with contentIds not in body Summary: Resolves an issue where an attachment with a contentId was excluded from the attachments list on the message, even though the cid was not found in the message body. Test Plan: New test case Reviewers: evan Reviewed By: evan Maniphest Tasks: T427 Differential Revision: https://review.inboxapp.com/D1293 --- .../message-list/lib/message-item.cjsx | 6 +- .../message-list/spec/message-item-spec.cjsx | 76 ++++++++++--------- 2 files changed, 45 insertions(+), 37 deletions(-) diff --git a/internal_packages/message-list/lib/message-item.cjsx b/internal_packages/message-list/lib/message-item.cjsx index 86bae8b3d..f44773476 100644 --- a/internal_packages/message-list/lib/message-item.cjsx +++ b/internal_packages/message-list/lib/message-item.cjsx @@ -164,8 +164,10 @@ MessageItem = React.createClass _attachmentComponents: -> AttachmentComponent = @state.AttachmentComponent - attachments = _.filter @props.message.files, (f) -> - not f.contentId? and f.filename.length > 0 + attachments = _.filter @props.message.files, (f) => + inBody = f.contentId? and @props.message.body.indexOf(f.contentId) > 0 + not inBody and f.filename.length > 0 + attachments.map (file) => diff --git a/internal_packages/message-list/spec/message-item-spec.cjsx b/internal_packages/message-list/spec/message-item-spec.cjsx index 5ec0a70db..9edd7a1e2 100644 --- a/internal_packages/message-list/spec/message-item-spec.cjsx +++ b/internal_packages/message-list/spec/message-item-spec.cjsx @@ -38,6 +38,12 @@ file_inline_not_downloaded = new File contentId: 'file_inline_not_downloaded_id' contentType: 'image/png' size: 10 +file_cid_but_not_referenced = new File + id: 'file_cid_but_not_referenced' + filename: 'f.png' + contentId: 'file_cid_but_not_referenced' + contentType: 'image/png' + size: 10 download = fileId: 'file_1_id' @@ -153,34 +159,17 @@ describe "MessageItem", -> it "should not have the `collapsed` class", -> expect(@component.getDOMNode().className.indexOf('collapsed') >= 0).toBe(false) - + describe "when the message contains attachments", -> - beforeEach -> - @message.files = [file, file_not_downloaded] - @createComponent() - - it "should include the attachments area", -> - attachments = ReactTestUtils.findRenderedDOMComponentWithClass(@component, 'attachments-area') - expect(attachments).toBeDefined() - - it "should render the registered AttachmentComponent for each attachment", -> - attachments = ReactTestUtils.scryRenderedComponentsWithType(@component, AttachmentStub) - expect(attachments.length).toEqual(2) - expect(attachments[0].props.file).toBe(file) - expect(attachments[1].props.file).toBe(file_not_downloaded) - - it "should provide file download state to each AttachmentComponent", -> - attachments = ReactTestUtils.scryRenderedComponentsWithType(@component, AttachmentStub) - expect(attachments[0].props.download).toBe(download) - expect(attachments[1].props.download).toBe(undefined) - - describe "when the message contains inline attachments", -> beforeEach -> @message.files = [ file, + file_not_downloaded, + file_cid_but_not_referenced, + file_inline, file_inline_downloading, - file_inline_not_downloaded + file_inline_not_downloaded, ] @message.body = """ \"A\" @@ -190,23 +179,40 @@ describe "MessageItem", -> """ @createComponent() - it "should never include src=cid:// in the message body", -> - body = @component._formatBody() - expect(body.indexOf('cid')).toEqual(-1) + it "should include the attachments area", -> + attachments = ReactTestUtils.findRenderedDOMComponentWithClass(@component, 'attachments-area') + expect(attachments).toBeDefined() - it "should replace cid:// with the FileDownloadStore's path for the file", -> - body = @component._formatBody() - expect(body.indexOf('alt="A" src="/fake/path-inline.png"')).toEqual(@message.body.indexOf('alt="A"')) - - it "should not replace cid:// with the FileDownloadStore's path if the download is in progress", -> - body = @component._formatBody() - expect(body.indexOf('/fake/path-downloading.png')).toEqual(-1) - - it "should not include them in the attachments area", -> + it "should render the registered AttachmentComponent for each attachment", -> attachments = ReactTestUtils.scryRenderedComponentsWithType(@component, AttachmentStub) - expect(attachments.length).toEqual(1) expect(attachments[0].props.file).toBe(file) + it "should list attachments that are not mentioned in the body via cid", -> + attachments = ReactTestUtils.scryRenderedComponentsWithType(@component, AttachmentStub) + expect(attachments.length).toEqual(3) + expect(attachments[0].props.file).toBe(file) + expect(attachments[1].props.file).toBe(file_not_downloaded) + expect(attachments[2].props.file).toBe(file_cid_but_not_referenced) + + it "should provide file download state to each AttachmentComponent", -> + attachments = ReactTestUtils.scryRenderedComponentsWithType(@component, AttachmentStub) + expect(attachments[0].props.download).toBe(download) + expect(attachments[1].props.download).toBe(undefined) + + describe "inline", -> + it "should never leave src=cid:// in the message body", -> + body = @component._formatBody() + expect(body.indexOf('cid')).toEqual(-1) + + it "should replace cid:// with the FileDownloadStore's path for the file", -> + body = @component._formatBody() + expect(body.indexOf('alt="A" src="/fake/path-inline.png"')).toEqual(@message.body.indexOf('alt="A"')) + + it "should not replace cid:// with the FileDownloadStore's path if the download is in progress", -> + body = @component._formatBody() + expect(body.indexOf('/fake/path-downloading.png')).toEqual(-1) + + describe "showQuotedText", -> it "should be initialized to false", -> @createComponent()