From 80bf2ed43a35cc20f92c502f2f7094f4a8adcf37 Mon Sep 17 00:00:00 2001
From: Ben Gotow <bengotow@gmail.com>
Date: Thu, 28 May 2015 18:15:47 -0700
Subject: [PATCH] fix(T1744): Display attachments when message has no body,
 additional specs

Summary:
This fixes T1744 and adds specs for the logic in _attachmentComponents.
 - Attachments should display even when the body is an empty string or null
 - Attachments should not include files without names (mime parts of the message)

Test Plan: Run 2 new tests

Reviewers: evan

Reviewed By: evan

Maniphest Tasks: T1744

Differential Revision: https://phab.nylas.com/D1566
---
 .../message-list/lib/message-item.cjsx        |  9 ++++---
 .../message-list/spec/message-item-spec.cjsx  | 27 ++++++++++++++++++-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/internal_packages/message-list/lib/message-item.cjsx b/internal_packages/message-list/lib/message-item.cjsx
index 99bd66f86..a99ac576e 100644
--- a/internal_packages/message-list/lib/message-item.cjsx
+++ b/internal_packages/message-list/lib/message-item.cjsx
@@ -285,11 +285,12 @@ class MessageItem extends React.Component
   _formatContacts: (contacts=[]) =>
 
   _attachmentComponents: =>
-    return [] unless @props.message.body
-
     attachments = _.filter @props.message.files, (f) =>
-      inBody = f.contentId? and @props.message.body.indexOf(f.contentId) > 0
-      not inBody and f.filename.length > 0
+      # We ignore files with no name because they're actually mime-parts of the
+      # message being served by the API as files.
+      hasName = f.filename and f.filename.length > 0
+      hasCIDInBody = f.contentId? and @props.message.body?.indexOf(f.contentId) > 0
+      hasName and not hasCIDInBody
 
     attachments.map (file) =>
       <InjectedComponent
diff --git a/internal_packages/message-list/spec/message-item-spec.cjsx b/internal_packages/message-list/spec/message-item-spec.cjsx
index 1ea12920b..c559f1eef 100644
--- a/internal_packages/message-list/spec/message-item-spec.cjsx
+++ b/internal_packages/message-list/spec/message-item-spec.cjsx
@@ -46,6 +46,17 @@ file_cid_but_not_referenced = new File
   contentId: 'file_cid_but_not_referenced'
   contentType: 'image/png'
   size: 10
+file_cid_but_not_referenced_or_image = new File
+  id: 'file_cid_but_not_referenced_or_image'
+  filename: 'ansible notes.txt'
+  contentId: 'file_cid_but_not_referenced_or_image'
+  contentType: 'text/plain'
+  size: 300
+file_actually_a_mime_part = new File
+  id: 'file_actually_a_mime_part'
+  contentId: 'file_actually_a_mime_part'
+  contentType: 'text/html'
+  size: 300
 
 download =
   fileId: 'file_1_id'
@@ -161,10 +172,12 @@ describe "MessageItem", ->
         file,
         file_not_downloaded,
         file_cid_but_not_referenced,
+        file_cid_but_not_referenced_or_image,
 
         file_inline,
         file_inline_downloading,
         file_inline_not_downloaded,
+        file_actually_a_mime_part
       ]
       @message.body = """
         <img alt=\"A\" src=\"cid:#{file_inline.contentId}\"/>
@@ -184,16 +197,28 @@ describe "MessageItem", ->
 
     it "should list attachments that are not mentioned in the body via cid", ->
       attachments = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@component, InjectedComponent, matching: {role: 'Attachment'})
-      expect(attachments.length).toEqual(3)
+      expect(attachments.length).toEqual(4)
       expect(attachments[0].props.exposedProps.file).toBe(file)
       expect(attachments[1].props.exposedProps.file).toBe(file_not_downloaded)
       expect(attachments[2].props.exposedProps.file).toBe(file_cid_but_not_referenced)
+      expect(attachments[3].props.exposedProps.file).toBe(file_cid_but_not_referenced_or_image)
+
+    it "should list attachments without a filename", ->
+      attachments = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@component, InjectedComponent, matching: {role: 'Attachment'})
+      for attachment in attachments
+        expect(attachment.props.exposedProps.file).not.toBe(file_actually_a_mime_part)
 
     it "should provide file download state to each InjectedComponent", ->
       attachments = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@component, InjectedComponent, matching: {role: 'Attachment'})
       expect(attachments[0].props.exposedProps.download).toBe(download)
       expect(attachments[1].props.exposedProps.download).toBe(undefined)
 
+    it "should still list attachments when the message has no body", ->
+      @message.body = ""
+      @createComponent()
+      attachments = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@component, InjectedComponent, matching: {role: 'Attachment'})
+      expect(attachments.length).toEqual(7)
+
     describe "inline", ->
       it "should never leave src=cid:// in the message body", ->
         body = @component._formatBody()