diff --git a/app/controllers/concerns/clones_ticket_article_attachments.rb b/app/controllers/concerns/clones_ticket_article_attachments.rb
index 5a47d57ae..a91a6bce9 100644
--- a/app/controllers/concerns/clones_ticket_article_attachments.rb
+++ b/app/controllers/concerns/clones_ticket_article_attachments.rb
@@ -6,41 +6,7 @@ module ClonesTicketArticleAttachments
def article_attachments_clone(article)
raise Exceptions::UnprocessableEntity, 'Need form_id to attach attachments to new form.' if params[:form_id].blank?
- existing_attachments = Store.list(
- object: 'UploadCache',
- o_id: params[:form_id],
- )
- attachments = []
- article.attachments.each do |new_attachment|
- next if new_attachment.preferences['content-alternative'] == true
-
- if article.content_type.present? && article.content_type =~ %r{text/html}i
- next if new_attachment.preferences['content_disposition'].present? && new_attachment.preferences['content_disposition'] !~ /inline/
-
- if new_attachment.preferences['Content-ID'].present? && article.body.present?
- next if article.body.match?(/#{Regexp.quote(new_attachment.preferences['Content-ID'])}/i)
- end
- end
- already_added = false
- existing_attachments.each do |existing_attachment|
- next if existing_attachment.filename != new_attachment.filename || existing_attachment.size != new_attachment.size
-
- already_added = true
- break
- end
- next if already_added == true
-
- file = Store.add(
- object: 'UploadCache',
- o_id: params[:form_id],
- data: new_attachment.content,
- filename: new_attachment.filename,
- preferences: new_attachment.preferences,
- )
- attachments.push file
- end
-
- attachments
+ article.clone_attachments('UploadCache', params[:form_id], only_attached_attachments: true)
end
end
diff --git a/app/models/ticket.rb b/app/models/ticket.rb
index fd74bc376..28dd42579 100644
--- a/app/models/ticket.rb
+++ b/app/models/ticket.rb
@@ -1486,6 +1486,12 @@ result
preferences: attachment[:preferences],
)
end
+
+ original_article = objects[:article]
+ if original_article&.should_clone_inline_attachments? # rubocop:disable Style/GuardClause
+ original_article.clone_attachments('Ticket::Article', message.id, only_inline_attachments: true)
+ original_article.should_clone_inline_attachments = false # cancel the temporary flag after cloning
+ end
end
def sms_recipients_by_type(recipient_type, article)
diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb
index 7fc8b6771..e5c1c56f7 100644
--- a/app/models/ticket/article.rb
+++ b/app/models/ticket/article.rb
@@ -44,6 +44,9 @@ class Ticket::Article < ApplicationModel
:to,
:cc
+ attr_accessor :should_clone_inline_attachments
+ alias should_clone_inline_attachments? should_clone_inline_attachments
+
# fillup md5 of message id to search easier on very long message ids
def check_message_id_md5
return true if message_id.blank?
@@ -130,6 +133,77 @@ returns
new_attachments
end
+=begin
+
+clone existing attachments of article to the target object
+
+ article_parent = Ticket::Article.find(123)
+ article_new = Ticket::Article.find(456)
+
+ attached_attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_attached_attachments: true)
+
+ inline_attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_inline_attachments: true)
+
+returns
+
+ [attachment1, attachment2, ...]
+
+=end
+
+ def clone_attachments(object_type, object_id, options = {})
+ existing_attachments = Store.list(
+ object: object_type,
+ o_id: object_id,
+ )
+
+ is_html_content = false
+ if content_type.present? && content_type =~ %r{text/html}i
+ is_html_content = true
+ end
+
+ new_attachments = []
+ attachments.each do |new_attachment|
+ next if new_attachment.preferences['content-alternative'] == true
+
+ # 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)
+ 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)
+ end
+
+ already_added = false
+ existing_attachments.each do |existing_attachment|
+ next if existing_attachment.filename != new_attachment.filename || existing_attachment.size != new_attachment.size
+
+ already_added = true
+ break
+ end
+ next if already_added == true
+
+ file = Store.add(
+ object: object_type,
+ o_id: object_id,
+ data: new_attachment.content,
+ filename: new_attachment.filename,
+ preferences: new_attachment.preferences,
+ )
+ new_attachments.push file
+ end
+
+ new_attachments
+ end
+
def self.last_customer_agent_article(ticket_id)
sender = Ticket::Article::Sender.lookup(name: 'System')
Ticket::Article.where('ticket_id = ? AND sender_id NOT IN (?)', ticket_id, sender.id).order('created_at DESC').first
diff --git a/lib/notification_factory/renderer.rb b/lib/notification_factory/renderer.rb
index a03876026..bc7f5dfa8 100644
--- a/lib/notification_factory/renderer.rb
+++ b/lib/notification_factory/renderer.rb
@@ -126,6 +126,11 @@ examples how to use
begin
previous_object_refs = object_refs
object_refs = object_refs.send(method.to_sym, *arguments)
+
+ # body_as_html should trigger the cloning of all inline attachments from the parent article (issue #2399)
+ if method.to_sym == :body_as_html && previous_object_refs.respond_to?(:should_clone_inline_attachments)
+ previous_object_refs.should_clone_inline_attachments = true
+ end
rescue => e
value = "\#{#{object_name}.#{object_methods_s} / #{e.message}}"
break
diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb
index 5b2a87dbd..88303271a 100644
--- a/spec/models/ticket/article_spec.rb
+++ b/spec/models/ticket/article_spec.rb
@@ -124,4 +124,95 @@ RSpec.describe Ticket::Article, type: :model do
end
end
end
+
+ describe 'clone attachments' do
+ context 'of forwarded article' do
+ context 'via email' do
+
+ it 'only need to clone attached attachments' do
+ article_parent = create(:ticket_article,
+ type: Ticket::Article::Type.find_by(name: 'email'),
+ content_type: 'text/html',
+ body: '
some text',)
+ Store.add(
+ object: 'Ticket::Article',
+ o_id: article_parent.id,
+ data: 'content_file1_normally_should_be_an_image',
+ filename: 'some_file1.jpg',
+ preferences: {
+ 'Content-Type' => 'image/jpeg',
+ 'Mime-Type' => 'image/jpeg',
+ 'Content-ID' => '15.274327094.140938@zammad.example.com',
+ 'Content-Disposition' => 'inline',
+ },
+ created_by_id: 1,
+ )
+ Store.add(
+ object: 'Ticket::Article',
+ o_id: article_parent.id,
+ data: 'content_file2_normally_should_be_an_image',
+ filename: 'some_file2.jpg',
+ preferences: {
+ 'Content-Type' => 'image/jpeg',
+ 'Mime-Type' => 'image/jpeg',
+ 'Content-ID' => '15.274327094.140938_not_reffered@zammad.example.com',
+ 'Content-Disposition' => 'inline',
+ },
+ 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[0].filename).to eq('some_file2.jpg')
+ end
+ end
+ end
+
+ context 'of trigger' do
+ context 'via email notifications' do
+ it 'only need to clone inline attachments used in body' do
+ article_parent = create(:ticket_article,
+ type: Ticket::Article::Type.find_by(name: 'email'),
+ content_type: 'text/html',
+ body: ' some text',)
+ Store.add(
+ object: 'Ticket::Article',
+ o_id: article_parent.id,
+ data: 'content_file1_normally_should_be_an_image',
+ filename: 'some_file1.jpg',
+ preferences: {
+ 'Content-Type' => 'image/jpeg',
+ 'Mime-Type' => 'image/jpeg',
+ 'Content-ID' => '15.274327094.140938@zammad.example.com',
+ 'Content-Disposition' => 'inline',
+ },
+ created_by_id: 1,
+ )
+ Store.add(
+ object: 'Ticket::Article',
+ o_id: article_parent.id,
+ data: 'content_file2_normally_should_be_an_image',
+ filename: 'some_file2.jpg',
+ preferences: {
+ 'Content-Type' => 'image/jpeg',
+ 'Mime-Type' => 'image/jpeg',
+ 'Content-ID' => '15.274327094.140938_not_reffered@zammad.example.com',
+ 'Content-Disposition' => 'inline',
+ },
+ 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_inline_attachments: true )
+
+ expect(attachments.count).to eq(1)
+ expect(attachments[0].filename).to eq('some_file1.jpg')
+ end
+ end
+ end
+ end
end
diff --git a/test/unit/ticket_trigger_test.rb b/test/unit/ticket_trigger_test.rb
index 1475d7569..975953622 100644
--- a/test/unit/ticket_trigger_test.rb
+++ b/test/unit/ticket_trigger_test.rb
@@ -4596,4 +4596,91 @@ class TicketTriggerTest < ActiveSupport::TestCase
end
end
+ #2399 - Attached images are broken on trigger reply with #{article.body_as_html}
+ test 'make sure auto reply using #{article.body_as_html} copies all articles image attachments as well' do
+ # make sure that this auto reply trigger only reacts to this particular test in order not to interfer with other auto reply tests
+ trigger1 = Trigger.create!(
+ name: 'auto reply with HTML quote',
+ condition: {
+ 'ticket.action' => {
+ 'operator' => 'is',
+ 'value' => 'create',
+ },
+ 'ticket.state_id' => {
+ 'operator' => 'is',
+ 'value' => Ticket::State.lookup(name: 'new').id.to_s,
+ },
+ 'ticket.title' => {
+ 'operator' => 'contains',
+ 'value' => 'AW: OTRS / Anfrage OTRS Einführung/Präsentation [Ticket#11545]',
+ },
+ },
+ perform: {
+ 'notification.email' => {
+ 'body' => '#{article.body_as_html}',
+ 'recipient' => 'article_last_sender',
+ 'subject' => 'Thanks for your inquiry (#{ticket.title})!',
+ },
+ },
+ disable_notification: true,
+ active: true,
+ created_by_id: 1,
+ updated_by_id: 1,
+ )
+
+ ticket1, article1, user, mail = Channel::EmailParser.new.process({}, File.read(Rails.root.join('test', 'data', 'mail', 'mail048.box')))
+
+ assert_equal('AW: OTRS / Anfrage OTRS Einführung/Präsentation [Ticket#11545]', ticket1.title, 'ticket1.title verify')
+ assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
+ assert_equal(2, ticket1.articles.first.attachments.count)
+
+ article1 = ticket1.articles.last
+ assert_match('Thanks for your inquiry (AW: OTRS / Anfrage OTRS Einführung/Präsentation [Ticket#11545])!', article1.subject)
+ assert_equal(1, article1.attachments.count)
+ assert_equal('50606', article1.attachments[0].size)
+ assert_equal('CPG-Reklamationsmitteilung bezügl.01234567895 an Voda-28.03.2017.jpg', article1.attachments[0].filename)
+ end
+
+ #2399 - Attached images are broken on trigger reply with #{article.body_as_html}
+ test 'make sure auto reply using #{article.body_as_html} does not copy any non-image attachments' do
+ # make sure that this auto reply trigger only reacts to this particular test in order not to interfer with other auto reply tests
+ trigger1 = Trigger.create!(
+ name: 'auto reply with HTML quote',
+ condition: {
+ 'ticket.action' => {
+ 'operator' => 'is',
+ 'value' => 'create',
+ },
+ 'ticket.state_id' => {
+ 'operator' => 'is',
+ 'value' => Ticket::State.lookup(name: 'new').id.to_s,
+ },
+ 'ticket.title' => {
+ 'operator' => 'contains',
+ 'value' => 'Online-apotheke. Günstigster Preis. Ohne Rezepte',
+ },
+ },
+ perform: {
+ 'notification.email' => {
+ 'body' => '#{article.body_as_html}',
+ 'recipient' => 'article_last_sender',
+ 'subject' => 'Thanks for your inquiry (#{ticket.title})!',
+ },
+ },
+ disable_notification: true,
+ active: true,
+ created_by_id: 1,
+ updated_by_id: 1,
+ )
+
+ ticket1, article1, user, mail = Channel::EmailParser.new.process({}, File.read(Rails.root.join('test', 'data', 'mail', 'mail069.box')))
+
+ assert_equal('Online-apotheke. Günstigster Preis. Ohne Rezepte', ticket1.title, 'ticket1.title verify')
+ assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
+ assert_equal(1, ticket1.articles.first.attachments.count)
+
+ article1 = ticket1.articles.last
+ assert_match('Thanks for your inquiry (Online-apotheke. Günstigster Preis. Ohne Rezepte)!', article1.subject)
+ assert_equal(0, article1.attachments.count)
+ end
end