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.

app/models/ticket/article.rb

@@ -118,7 +118,8 @@ def attachments_inline
 
       # 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,19 +168,24 @@ def clone_attachments(object_type, object_id, options = {})
 
       # 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
 
       # only_inline_attachments mode is used when quoting HTML mail with #{article.body_as_html}
       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

spec/models/ticket/article_spec.rb

@@ -160,13 +160,26 @@
             },
             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
@@ -204,6 +217,21 @@
             },
             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
 

spec/models/trigger_spec.rb

@@ -0,0 +1,165 @@
+require 'rails_helper'
+require 'models/application_model_examples'
+
+RSpec.describe Trigger, type: :model do
+  it_behaves_like 'ApplicationModel', can_assets: { selectors: %i[condition perform] }
+
+  before { Trigger.destroy_all }  # Default DB state includes three sample triggers
+  subject!(:trigger) { create(:trigger, condition: condition, perform: perform) }
+
+  describe '#assets (for supplying model data to front-end framework)' do
+    let(:condition) { { 'ticket.state_id' => { operator: 'is', value: 1 } } }
+    let(:perform) { { 'ticket.priority_id' => { value: 1 } } }
+
+    it 'returns a hash with asset attributes for objects referenced in #condition and #perform' do
+      expect(trigger.assets({}))
+        .to include(Ticket::State.first.assets({}))
+        .and include(Ticket::Priority.first.assets({}))
+    end
+  end
+
+  describe 'Send-email triggers' do
+    let(:perform) do
+      {
+        'notification.email' => {
+          'recipient' => 'ticket_customer',
+          'subject'   => 'foo',
+          'body'      => 'some body with >snip<#{article.body_as_html}>/snip<', # rubocop:disable Lint/InterpolationCheck
+        }
+      }
+    end
+
+    context 'for condition "ticket created"' do
+      let(:condition) do
+        { 'ticket.action' => { 'operator' => 'is', 'value' => 'create' } }
+      end
+
+      context 'when ticket is created directly' do
+        let!(:ticket) { create(:ticket) }
+
+        it 'fires (without altering ticket state)' do
+          expect { Observer::Transaction.commit }
+            .to change { Ticket::Article.count }.by(1)
+            .and not_change { ticket.reload.state.name }.from('new')
+        end
+      end
+
+      context 'when ticket is created via Channel::EmailParser.process' do
+        before { create(:email_address, groups: [Group.first]) }
+        let(:raw_email) { File.read(Rails.root.join('test', 'data', 'mail', 'mail001.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')
+        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
+      let(:condition) do
+        { 'ticket.action' => { 'operator' => 'is', 'value' => 'update' } }
+      end
+
+      let!(:ticket) { create(:ticket).tap { Observer::Transaction.commit } }
+
+      context 'when new article is created directly' do
+        context 'with empty #preferences hash' do
+          let!(:article) { create(:ticket_article, ticket: ticket)  }
+
+          it 'fires (without altering ticket state)' do
+            expect { Observer::Transaction.commit }
+              .to change { ticket.reload.articles.count }.by(1)
+              .and not_change { ticket.reload.state.name }.from('new')
+          end
+        end
+
+        context 'with #preferences { "send-auto-response" => false }' do
+          let!(:article) do
+            create(:ticket_article,
+                   ticket:      ticket,
+                   preferences: { 'send-auto-response' => false })
+          end
+
+          it 'does not fire' do
+            expect { Observer::Transaction.commit }
+              .not_to change { ticket.reload.articles.count }
+          end
+        end
+      end
+
+      context 'when new article is created via Channel::EmailParser.process' do
+        context 'with a regular message' do
+          let!(:article) do
+            create(:ticket_article,
+                   ticket:     ticket,
+                   message_id: raw_email[/(?<=^References: )\S*/],
+                   subject:    raw_email[/(?<=^Subject: Re: ).*$/])
+          end
+
+          let(:raw_email) { File.read(Rails.root.join('test', 'data', 'mail', 'mail005.box')) }
+
+          it 'fires (without altering ticket state)' do
+            expect { Channel::EmailParser.new.process({}, raw_email) }
+              .to not_change { Ticket.count }
+              .and change { ticket.reload.articles.count }.by(2)
+              .and not_change { ticket.reload.state.name }.from('new')
+          end
+        end
+
+        context 'with delivery-failed "bounce message"' do
+          let!(:article) do
+            create(:ticket_article,
+                   ticket:     ticket,
+                   message_id: raw_email[/(?<=^Message-ID: )\S*/])
+          end
+
+          let(:raw_email) { File.read(Rails.root.join('test', 'data', 'mail', 'mail055.box')) }
+
+          it 'does not fire' do
+            expect { Channel::EmailParser.new.process({}, raw_email) }
+              .to change { ticket.reload.articles.count }.by(1)
+          end
+        end
+      end
+    end
+  end
+end