Browse Source

Fixed #2483 - #{article.body_as_html} now includes attachments (e.g. PDFs). Fixes #2487 - Forwarding: Attachment is missing if original HTML Mail contains an image.

Billy Zhou 6 years ago
parent
commit
f080a144e0
3 changed files with 79 additions and 9 deletions
  1. 13 7
      app/models/ticket/article.rb
  2. 29 1
      spec/models/ticket/article_spec.rb
  3. 37 1
      spec/models/trigger_spec.rb

+ 13 - 7
app/models/ticket/article.rb

@@ -118,7 +118,8 @@ returns
 
       # look for attachment
       attachments.each do |file|
-        next if !file.preferences['Content-ID'] || (file.preferences['Content-ID'] != cid && file.preferences['Content-ID'] != "<#{cid}>" )
+        content_id = file.preferences['Content-ID'] || file.preferences['content_id']
+        next if content_id.blank? || (content_id != cid && content_id != "<#{cid}>" )
 
         inline_attachments[file.id] = true
         break
@@ -167,10 +168,10 @@ returns
 
       # only_attached_attachments mode is used by apply attached attachments to forwared article
       if options[:only_attached_attachments] == true
-        if is_html_content
-          next if new_attachment.preferences['content_disposition'].present? && new_attachment.preferences['content_disposition'] =~ /inline/
-          next if new_attachment.preferences['Content-ID'].blank?
-          next if body.present? && body.match?(/#{Regexp.quote(new_attachment.preferences['Content-ID'])}/i)
+        if is_html_content == true
+
+          content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
+          next if content_id.present? && body.present? && body.match?(/#{Regexp.quote(content_id)}/i)
         end
       end
 
@@ -178,8 +179,13 @@ returns
       if options[:only_inline_attachments] == true
         next if is_html_content == false
         next if body.blank?
-        next if new_attachment.preferences['content_disposition'].present? && new_attachment.preferences['content_disposition'] !~ /inline/
-        next if new_attachment.preferences['Content-ID'].present? && !body.match?(/#{Regexp.quote(new_attachment.preferences['Content-ID'])}/i)
+
+        content_disposition = new_attachment.preferences['Content-Disposition'] || new_attachment.preferences['content_disposition']
+        next if content_disposition.present? && content_disposition !~ /inline/
+
+        content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
+        next if content_id.blank?
+        next if !body.match?(/#{Regexp.quote(content_id)}/i)
       end
 
       already_added = false

+ 29 - 1
spec/models/ticket/article_spec.rb

@@ -184,13 +184,26 @@ RSpec.describe Ticket::Article, type: :model do
             },
             created_by_id: 1,
           )
+          Store.add(
+            object:        'Ticket::Article',
+            o_id:          article_parent.id,
+            data:          'content_file3_normally_should_be_an_image',
+            filename:      'some_file3.jpg',
+            preferences:   {
+              'Content-Type'        => 'image/jpeg',
+              'Mime-Type'           => 'image/jpeg',
+              'Content-Disposition' => 'attached',
+            },
+            created_by_id: 1,
+          )
           article_new = create(:ticket_article)
           UserInfo.current_user_id = 1
 
           attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_attached_attachments: true)
 
-          expect(attachments.count).to eq(1)
+          expect(attachments.count).to eq(2)
           expect(attachments[0].filename).to eq('some_file2.jpg')
+          expect(attachments[1].filename).to eq('some_file3.jpg')
         end
       end
     end
@@ -228,6 +241,21 @@ RSpec.describe Ticket::Article, type: :model do
             },
             created_by_id: 1,
           )
+
+          # #2483 - #{article.body_as_html} now includes attachments (e.g. PDFs)
+          # Regular attachments do not get assigned a Content-ID, and should not be copied in this use case
+          Store.add(
+            object:        'Ticket::Article',
+            o_id:          article_parent.id,
+            data:          'content_file3_with_no_content_id',
+            filename:      'some_file3.jpg',
+            preferences:   {
+              'Content-Type' => 'image/jpeg',
+              'Mime-Type'    => 'image/jpeg',
+            },
+            created_by_id: 1,
+          )
+
           article_new = create(:ticket_article)
           UserInfo.current_user_id = 1
 

+ 37 - 1
spec/models/trigger_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Trigger, type: :model do
         'notification.email' => {
           'recipient' => 'ticket_customer',
           'subject'   => 'foo',
-          'body'      => 'bar'
+          'body'      => 'some body with &gt;snip&lt;#{article.body_as_html}&gt;/snip&lt;', # rubocop:disable Lint/InterpolationCheck
         }
       }
     end
@@ -56,6 +56,42 @@ RSpec.describe Trigger, type: :model do
           expect(Ticket.last.state.name).to eq('new')
         end
       end
+
+      context 'when ticket is created via Channel::EmailParser.process with inline image' do
+        before { create(:email_address, groups: [Group.first]) }
+        let(:raw_email) { File.read(Rails.root.join('test', 'data', 'mail', 'mail010.box')) }
+
+        it 'fires (without altering ticket state)' do
+          expect { Channel::EmailParser.new.process({}, raw_email) }
+            .to change { Ticket.count }.by(1)
+            .and change { Ticket::Article.count }.by(2)
+
+          expect(Ticket.last.state.name).to eq('new')
+
+          article = Ticket::Article.last
+          expect(article.type.name).to eq('email')
+          expect(article.sender.name).to eq('System')
+          expect(article.attachments.count).to eq(1)
+          expect(article.attachments[0].filename).to eq('image001.jpg')
+          expect(article.attachments[0].preferences['Content-ID']).to eq('image001.jpg@01CDB132.D8A510F0')
+
+          expect(article.body).to eq(<<~RAW.chomp
+            some body with &gt;snip&lt;<div>
+            <p>Herzliche Grüße aus Oberalteich sendet Herrn Smith</p>
+            <p> </p>
+            <p>Sepp Smith - Dipl.Ing. agr. (FH)</p>
+            <p>Geschäftsführer der example Straubing-Bogen</p>
+            <p>Klosterhof 1 | 94327 Bogen-Oberalteich</p>
+            <p>Tel: 09422-505601 | Fax: 09422-505620</p>
+            <p>Internet: <a href="http://example-straubing-bogen.de/" rel="nofollow noreferrer noopener" target="_blank">http://example-straubing-bogen.de</a></p>
+            <p>Facebook: <a href="http://facebook.de/examplesrbog" rel="nofollow noreferrer noopener" target="_blank">http://facebook.de/examplesrbog</a></p>
+            <p><b><img border="0" src="cid:image001.jpg@01CDB132.D8A510F0" alt="Beschreibung: Beschreibung: efqmLogo" style="width:60px;height:19px;"></b><b> - European Foundation für Quality Management</b></p>
+            <p> </p>
+            </div>&gt;/snip&lt;
+          RAW
+                                    )
+        end
+      end
     end
 
     context 'for condition "ticket updated"' do