diff --git a/app/models/observer/ticket/article/fillup_from_email.rb b/app/models/observer/ticket/article/fillup_from_email.rb
index c44c7fcd9..1a59bb69d 100644
--- a/app/models/observer/ticket/article/fillup_from_email.rb
+++ b/app/models/observer/ticket/article/fillup_from_email.rb
@@ -24,7 +24,7 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
return true if type['name'] != 'email'
# set subject if empty
- ticket = Ticket.lookup(id: record.ticket_id)
+ ticket = record.ticket
if !record.subject || record.subject == ''
record.subject = ticket.title
end
@@ -44,7 +44,7 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
# set sender
email_address = ticket.group.email_address
if !email_address
- raise "No email address found for group '#{ticket.group.name}'"
+ raise "No email address found for group '#{ticket.group.name}' (#{ticket.group_id})"
end
# remember email address for background job
diff --git a/app/models/ticket.rb b/app/models/ticket.rb
index 54bcb9506..8c8db6eee 100644
--- a/app/models/ticket.rb
+++ b/app/models/ticket.rb
@@ -824,219 +824,15 @@ perform changes on ticket
end
end
+ perform_notification = {}
changed = false
perform.each do |key, value|
(object_name, attribute) = key.split('.', 2)
raise "Unable to update object #{object_name}.#{attribute}, only can update tickets and send notifications!" if object_name != 'ticket' && object_name != 'notification'
- # send notification
+ # send notification (after changes are done)
if object_name == 'notification'
-
- # value['recipient'] was a string in the past (single-select) so we convert it to array if needed
- value_recipient = value['recipient']
- if !value_recipient.is_a?(Array)
- value_recipient = [value_recipient]
- end
-
- recipients_raw = []
- value_recipient.each do |recipient|
- if recipient == 'article_last_sender'
- if article.present?
- if article.reply_to.present?
- recipients_raw.push(article.reply_to)
- elsif article.from.present?
- recipients_raw.push(article.from)
- elsif article.origin_by_id
- email = User.find_by(id: article.origin_by_id).email
- recipients_raw.push(email)
- elsif article.created_by_id
- email = User.find_by(id: article.created_by_id).email
- recipients_raw.push(email)
- end
- end
- elsif recipient == 'ticket_customer'
- email = User.find_by(id: customer_id).email
- recipients_raw.push(email)
- elsif recipient == 'ticket_owner'
- email = User.find_by(id: owner_id).email
- recipients_raw.push(email)
- elsif recipient == 'ticket_agents'
- User.group_access(group_id, 'full').sort_by(&:login).each do |user|
- recipients_raw.push(user.email)
- end
- else
- logger.error "Unknown email notification recipient '#{recipient}'"
- next
- end
- end
-
- recipients_checked = []
- recipients_raw.each do |recipient_email|
-
- skip_user = false
- users = User.where(email: recipient_email)
- users.each do |user|
- next if user.preferences[:mail_delivery_failed] != true
- next if !user.preferences[:mail_delivery_failed_data]
- till_blocked = ((user.preferences[:mail_delivery_failed_data] - Time.zone.now - 60.days) / 60 / 60 / 24).round
- next if till_blocked.positive?
- logger.info "Send no trigger based notification to #{recipient_email} because email is marked as mail_delivery_failed for #{till_blocked} days"
- skip_user = true
- break
- end
- next if skip_user
-
- # send notifications only to email adresses
- next if recipient_email.blank?
- next if recipient_email !~ /@/
-
- # check if address is valid
- begin
- Mail::AddressList.new(recipient_email).addresses.each do |address|
- recipient_email = address.address
- break if recipient_email.present? && recipient_email =~ /@/ && !recipient_email.match?(/\s/)
- end
- rescue
- if recipient_email.present?
- if recipient_email !~ /^(.+?)<(.+?)@(.+?)>$/
- next # no usable format found
- end
- recipient_email = "#{$2}@#{$3}"
- end
- next if recipient_email.blank?
- next if recipient_email !~ /@/
- next if recipient_email.match?(/\s/)
- end
-
- # do not sent notifications to this recipients
- send_no_auto_response_reg_exp = Setting.get('send_no_auto_response_reg_exp')
- begin
- next if recipient_email.match?(/#{send_no_auto_response_reg_exp}/i)
- rescue => e
- logger.error "ERROR: Invalid regex '#{send_no_auto_response_reg_exp}' in setting send_no_auto_response_reg_exp"
- logger.error 'ERROR: ' + e.inspect
- next if recipient_email.match?(/(mailer-daemon|postmaster|abuse|root|noreply|noreply.+?|no-reply|no-reply.+?)@.+?/i)
- end
-
- # check if notification should be send because of customer emails
- if article.present? && article.preferences.fetch('is-auto-response', false) == true && article.from && article.from =~ /#{Regexp.quote(recipient_email)}/i
- logger.info "Send no trigger based notification to #{recipient_email} because of auto response tagged incoming email"
- next
- end
-
- # loop protection / check if maximal count of trigger mail has reached
- map = {
- 10 => 10,
- 30 => 15,
- 60 => 25,
- 180 => 50,
- 600 => 100,
- }
- skip = false
- map.each do |minutes, count|
- already_sent = Ticket::Article.where(
- ticket_id: id,
- sender: Ticket::Article::Sender.find_by(name: 'System'),
- type: Ticket::Article::Type.find_by(name: 'email'),
- ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
- next if already_sent < count
- logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} for this ticket within last #{minutes} minutes (loop protection)"
- skip = true
- break
- end
- next if skip
- map = {
- 10 => 30,
- 30 => 60,
- 60 => 120,
- 180 => 240,
- 600 => 360,
- }
- skip = false
- map.each do |minutes, count|
- already_sent = Ticket::Article.where(
- sender: Ticket::Article::Sender.find_by(name: 'System'),
- type: Ticket::Article::Type.find_by(name: 'email'),
- ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
- next if already_sent < count
- logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} in total within last #{minutes} minutes (loop protection)"
- skip = true
- break
- end
- next if skip
-
- email = recipient_email.downcase.strip
- next if recipients_checked.include?(email)
- recipients_checked.push(email)
- end
-
- next if recipients_checked.blank?
- recipient_string = recipients_checked.join(', ')
-
- group = self.group
- next if !group
- email_address = group.email_address
- if !email_address
- logger.info "Unable to send trigger based notification to #{recipient_string} because no email address is set for group '#{group.name}'"
- next
- end
-
- if !email_address.channel_id
- logger.info "Unable to send trigger based notification to #{recipient_string} because no channel is set for email address '#{email_address.email}' (id: #{email_address.id})"
- next
- end
-
- # articles.last breaks (returns the wrong article)
- # if another email notification trigger preceded this one
- # (see https://github.com/zammad/zammad/issues/1543)
- objects = {
- ticket: self,
- article: article || articles.last
- }
-
- # get subject
- subject = NotificationFactory::Mailer.template(
- templateInline: value['subject'],
- locale: 'en-en',
- objects: objects,
- quote: false,
- )
- subject = subject_build(subject)
-
- body = NotificationFactory::Mailer.template(
- templateInline: value['body'],
- locale: 'en-en',
- objects: objects,
- quote: true,
- )
-
- (body, attachments_inline) = HtmlSanitizer.replace_inline_images(body, id)
-
- message = Ticket::Article.create(
- ticket_id: id,
- to: recipient_string,
- subject: subject,
- content_type: 'text/html',
- body: body,
- internal: false,
- sender: Ticket::Article::Sender.find_by(name: 'System'),
- type: Ticket::Article::Type.find_by(name: 'email'),
- preferences: {
- perform_origin: perform_origin,
- },
- updated_by_id: 1,
- created_by_id: 1,
- )
-
- attachments_inline.each do |attachment|
- Store.add(
- object: 'Ticket::Article',
- o_id: message.id,
- data: attachment[:data],
- filename: attachment[:filename],
- preferences: attachment[:preferences],
- )
- end
+ perform_notification[key] = value
next
end
@@ -1081,10 +877,227 @@ perform changes on ticket
changed = true
self[attribute] = value['value']
- logger.debug { "set #{object_name}.#{attribute} = #{value['value'].inspect}" }
+ logger.debug { "set #{object_name}.#{attribute} = #{value['value'].inspect} for ticket_id #{id}" }
+ end
+
+ if changed
+ save!
+ end
+
+ perform_notification.each do |key, value|
+ perform_changes_notification(key, value, perform_origin, article)
+ end
+
+ true
+ end
+
+ def perform_changes_notification(_key, value, perform_origin, article)
+
+ # value['recipient'] was a string in the past (single-select) so we convert it to array if needed
+ value_recipient = value['recipient']
+ if !value_recipient.is_a?(Array)
+ value_recipient = [value_recipient]
+ end
+
+ recipients_raw = []
+ value_recipient.each do |recipient|
+ if recipient == 'article_last_sender'
+ if article.present?
+ if article.reply_to.present?
+ recipients_raw.push(article.reply_to)
+ elsif article.from.present?
+ recipients_raw.push(article.from)
+ elsif article.origin_by_id
+ email = User.find_by(id: article.origin_by_id).email
+ recipients_raw.push(email)
+ elsif article.created_by_id
+ email = User.find_by(id: article.created_by_id).email
+ recipients_raw.push(email)
+ end
+ end
+ elsif recipient == 'ticket_customer'
+ email = User.find_by(id: customer_id).email
+ recipients_raw.push(email)
+ elsif recipient == 'ticket_owner'
+ email = User.find_by(id: owner_id).email
+ recipients_raw.push(email)
+ elsif recipient == 'ticket_agents'
+ User.group_access(group_id, 'full').sort_by(&:login).each do |user|
+ recipients_raw.push(user.email)
+ end
+ else
+ logger.error "Unknown email notification recipient '#{recipient}'"
+ next
+ end
+ end
+
+ recipients_checked = []
+ recipients_raw.each do |recipient_email|
+
+ skip_user = false
+ users = User.where(email: recipient_email)
+ users.each do |user|
+ next if user.preferences[:mail_delivery_failed] != true
+ next if !user.preferences[:mail_delivery_failed_data]
+ till_blocked = ((user.preferences[:mail_delivery_failed_data] - Time.zone.now - 60.days) / 60 / 60 / 24).round
+ next if till_blocked.positive?
+ logger.info "Send no trigger based notification to #{recipient_email} because email is marked as mail_delivery_failed for #{till_blocked} days"
+ skip_user = true
+ break
+ end
+ next if skip_user
+
+ # send notifications only to email adresses
+ next if recipient_email.blank?
+ next if recipient_email !~ /@/
+
+ # check if address is valid
+ begin
+ Mail::AddressList.new(recipient_email).addresses.each do |address|
+ recipient_email = address.address
+ break if recipient_email.present? && recipient_email =~ /@/ && !recipient_email.match?(/\s/)
+ end
+ rescue
+ if recipient_email.present?
+ if recipient_email !~ /^(.+?)<(.+?)@(.+?)>$/
+ next # no usable format found
+ end
+ recipient_email = "#{$2}@#{$3}"
+ end
+ next if recipient_email.blank?
+ next if recipient_email !~ /@/
+ next if recipient_email.match?(/\s/)
+ end
+
+ # do not sent notifications to this recipients
+ send_no_auto_response_reg_exp = Setting.get('send_no_auto_response_reg_exp')
+ begin
+ next if recipient_email.match?(/#{send_no_auto_response_reg_exp}/i)
+ rescue => e
+ logger.error "ERROR: Invalid regex '#{send_no_auto_response_reg_exp}' in setting send_no_auto_response_reg_exp"
+ logger.error 'ERROR: ' + e.inspect
+ next if recipient_email.match?(/(mailer-daemon|postmaster|abuse|root|noreply|noreply.+?|no-reply|no-reply.+?)@.+?/i)
+ end
+
+ # check if notification should be send because of customer emails
+ if article.present? && article.preferences.fetch('is-auto-response', false) == true && article.from && article.from =~ /#{Regexp.quote(recipient_email)}/i
+ logger.info "Send no trigger based notification to #{recipient_email} because of auto response tagged incoming email"
+ next
+ end
+
+ # loop protection / check if maximal count of trigger mail has reached
+ map = {
+ 10 => 10,
+ 30 => 15,
+ 60 => 25,
+ 180 => 50,
+ 600 => 100,
+ }
+ skip = false
+ map.each do |minutes, count|
+ already_sent = Ticket::Article.where(
+ ticket_id: id,
+ sender: Ticket::Article::Sender.find_by(name: 'System'),
+ type: Ticket::Article::Type.find_by(name: 'email'),
+ ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
+ next if already_sent < count
+ logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} for this ticket within last #{minutes} minutes (loop protection)"
+ skip = true
+ break
+ end
+ next if skip
+ map = {
+ 10 => 30,
+ 30 => 60,
+ 60 => 120,
+ 180 => 240,
+ 600 => 360,
+ }
+ skip = false
+ map.each do |minutes, count|
+ already_sent = Ticket::Article.where(
+ sender: Ticket::Article::Sender.find_by(name: 'System'),
+ type: Ticket::Article::Type.find_by(name: 'email'),
+ ).where('ticket_articles.created_at > ? AND ticket_articles.to LIKE ?', Time.zone.now - minutes.minutes, "%#{recipient_email.strip}%").count
+ next if already_sent < count
+ logger.info "Send no trigger based notification to #{recipient_email} because already sent #{count} in total within last #{minutes} minutes (loop protection)"
+ skip = true
+ break
+ end
+ next if skip
+
+ email = recipient_email.downcase.strip
+ next if recipients_checked.include?(email)
+ recipients_checked.push(email)
+ end
+
+ return if recipients_checked.blank?
+ recipient_string = recipients_checked.join(', ')
+
+ group_id = self.group_id
+ return if !group_id
+ email_address = Group.find(group_id).email_address
+ if !email_address
+ logger.info "Unable to send trigger based notification to #{recipient_string} because no email address is set for group '#{group.name}'"
+ return
+ end
+
+ if !email_address.channel_id
+ logger.info "Unable to send trigger based notification to #{recipient_string} because no channel is set for email address '#{email_address.email}' (id: #{email_address.id})"
+ return
+ end
+
+ # articles.last breaks (returns the wrong article)
+ # if another email notification trigger preceded this one
+ # (see https://github.com/zammad/zammad/issues/1543)
+ objects = {
+ ticket: self,
+ article: article || articles.last
+ }
+
+ # get subject
+ subject = NotificationFactory::Mailer.template(
+ templateInline: value['subject'],
+ locale: 'en-en',
+ objects: objects,
+ quote: false,
+ )
+ subject = subject_build(subject)
+
+ body = NotificationFactory::Mailer.template(
+ templateInline: value['body'],
+ locale: 'en-en',
+ objects: objects,
+ quote: true,
+ )
+
+ (body, attachments_inline) = HtmlSanitizer.replace_inline_images(body, id)
+
+ message = Ticket::Article.create(
+ ticket_id: id,
+ to: recipient_string,
+ subject: subject,
+ content_type: 'text/html',
+ body: body,
+ internal: false,
+ sender: Ticket::Article::Sender.find_by(name: 'System'),
+ type: Ticket::Article::Type.find_by(name: 'email'),
+ preferences: {
+ perform_origin: perform_origin,
+ },
+ updated_by_id: 1,
+ created_by_id: 1,
+ )
+
+ attachments_inline.each do |attachment|
+ Store.add(
+ object: 'Ticket::Article',
+ o_id: message.id,
+ data: attachment[:data],
+ filename: attachment[:filename],
+ preferences: attachment[:preferences],
+ )
end
- return if !changed
- save
true
end
@@ -1136,6 +1149,8 @@ perform active triggers on ticket
Transaction.execute(local_options) do
triggers.each do |trigger|
+ logger.debug { "Probe trigger (#{trigger.name}/#{trigger.id}) for this object (Ticket:#{ticket.id}/Loop:#{local_options[:loop_count]})" }
+
condition = trigger.condition
# check if one article attribute is used
diff --git a/test/unit/ticket_trigger_extended_test.rb b/test/unit/ticket_trigger_extended_test.rb
index a8c99ab1a..8322567f2 100644
--- a/test/unit/ticket_trigger_extended_test.rb
+++ b/test/unit/ticket_trigger_extended_test.rb
@@ -715,4 +715,141 @@ Some Text'
end
+ test 'recursive trigger with auto responder' do
+
+ group1 = Group.create!(
+ name: 'Group dispatch',
+ active: true,
+ created_by_id: 1,
+ updated_by_id: 1,
+ )
+ group2 = Group.create!(
+ name: 'Group with auto responder',
+ active: true,
+ email_address: EmailAddress.first,
+ created_by_id: 1,
+ updated_by_id: 1,
+ )
+
+ trigger1 = Trigger.create!(
+ name: "002 - move ticket to #{group2.name}",
+ condition: {
+ 'ticket.action' => {
+ 'operator' => 'is',
+ 'value' => 'create',
+ },
+ 'ticket.group_id' => {
+ 'operator' => 'is',
+ 'value' => group1.id.to_s,
+ },
+ 'ticket.organization_id' => {
+ 'operator' => 'is',
+ 'pre_condition' => 'specific',
+ 'value' => User.lookup(email: 'nicole.braun@zammad.org').organization_id.to_s,
+ }
+ },
+ perform: {
+ 'ticket.group_id' => {
+ 'value' => group2.id.to_s,
+ },
+ },
+ disable_notification: true,
+ active: true,
+ created_by_id: 1,
+ updated_by_id: 1,
+ )
+
+ trigger2 = Trigger.create_or_update(
+ name: "001 auto reply for tickets in group #{group1.name}",
+ condition: {
+ 'ticket.action' => {
+ 'operator' => 'is',
+ 'value' => 'create',
+ },
+ 'ticket.state_id' => {
+ 'operator' => 'is',
+ 'value' => Ticket::State.lookup(name: 'new').id.to_s,
+ },
+ 'ticket.group_id' => {
+ 'operator' => 'is not',
+ 'value' => group1.id.to_s,
+ },
+ },
+ perform: {
+ 'notification.email' => {
+ 'body' => "some text
\#{ticket.customer.lastname}
\#{ticket.title}
\#{article.body}",
+ 'recipient' => 'ticket_customer',
+ 'subject' => "Thanks for your inquiry (\#{ticket.title})!",
+ },
+ 'ticket.priority_id' => {
+ 'value' => Ticket::Priority.lookup(name: '3 high').id.to_s,
+ },
+ 'ticket.tags' => {
+ 'operator' => 'add',
+ 'value' => 'aa, kk',
+ },
+ },
+ disable_notification: true,
+ active: true,
+ created_by_id: 1,
+ updated_by_id: 1,
+ )
+
+ ticket1 = Ticket.create!(
+ title: "some title\n äöüß",
+ group: group1,
+ customer: User.lookup(email: 'nicole.braun@zammad.org'),
+ updated_by_id: 1,
+ created_by_id: 1,
+ )
+ Ticket::Article.create!(
+ ticket_id: ticket1.id,
+ from: 'some_sender@example.com',
+ to: 'some_recipient@example.com',
+ subject: 'some subject',
+ message_id: 'some@id',
+ body: "some message note\nnew line",
+ internal: false,
+ sender: Ticket::Article::Sender.find_by(name: 'Customer'),
+ type: Ticket::Article::Type.find_by(name: 'web'),
+ updated_by_id: 1,
+ created_by_id: 1,
+ )
+
+ ticket1.reload
+ assert_equal('some title äöüß', ticket1.title, 'ticket1.title verify')
+ assert_equal('Group dispatch', ticket1.group.name, 'ticket1.group verify')
+ assert_equal('new', ticket1.state.name, 'ticket1.state verify')
+ assert_equal('2 normal', ticket1.priority.name, 'ticket1.priority verify')
+ assert_equal(1, ticket1.articles.count, 'ticket1.articles verify')
+ assert_equal([], ticket1.tag_list)
+
+ Observer::Transaction.commit
+
+ ticket1.reload
+ assert_equal('some title äöüß', ticket1.title, 'ticket1.title verify')
+ assert_equal('Group with auto responder', ticket1.group.name, 'ticket1.group verify')
+ assert_equal('new', ticket1.state.name, 'ticket1.state verify')
+ assert_equal('3 high', ticket1.priority.name, 'ticket1.priority verify')
+ assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
+ assert_equal(%w[aa kk], ticket1.tag_list)
+
+ email_raw = "From: nicole.braun@zammad.org
+To: zammad@example.com
+Subject: test 1
+X-Zammad-Ticket-Group: #{group1.name}
+
+test 1"
+
+ ticket2, article2, user2 = Channel::EmailParser.new.process({ trusted: true }, email_raw)
+
+ assert_equal('test 1', ticket2.title, 'ticket2.title verify')
+ assert_equal('Group with auto responder', ticket2.group.name, 'ticket2.group verify')
+ assert_equal('new', ticket2.state.name, 'ticket2.state verify')
+ assert_equal('3 high', ticket2.priority.name, 'ticket2.priority verify')
+ assert_equal(2, ticket2.articles.count, 'ticket2.articles verify')
+ assert_equal(%w[aa kk], ticket2.tag_list)
+
+ end
+
end