Fixed issue#441 - Unable to execute more then one trigger (e. g. adding 2 tags with 2 triggers) at one ticket at same update cycle.

app/models/observer/transaction.rb

@@ -29,6 +29,7 @@ class Observer::Transaction < ActiveRecord::Observer
     sync_backends = []
     Setting.where(area: 'Transaction::Backend::Sync').order(:name).each { |setting|
       backend = Setting.get(setting.name)
+      next if params[:disable] && params[:disable].include?(backend)
       sync_backends.push Kernel.const_get(backend)
     }
 

app/models/ticket.rb

@@ -844,6 +844,22 @@ result
     references
   end
 
+=begin
+
+get all articles of a ticket in correct order (overwrite active record default method)
+
+  artilces = ticket.articles
+
+result
+
+  [article1, articl2]
+
+=end
+
+  def articles
+    Ticket::Article.where(ticket_id: id).order(:created_at, :id)
+  end
+
   private
 
   def check_generate

app/models/transaction.rb

@@ -13,7 +13,10 @@ class Transaction
       if options[:interface_handle]
         ApplicationHandleInfo.current = original_interface_handle
       end
-      Observer::Transaction.commit(disable_notification: options[:disable_notification])
+      Observer::Transaction.commit(
+        disable_notification: options[:disable_notification],
+        disable: options[:disable],
+      )
       PushMessages.finish
     end
   end

app/models/transaction/background_job.rb

@@ -24,7 +24,9 @@ class Transaction::BackgroundJob
 
   def perform
     Setting.where(area: 'Transaction::Backend::Async').order(:name).each { |setting|
-      backend = Kernel.const_get(Setting.get(setting.name))
+      backend = Setting.get(setting.name)
+      next if @params[:disable] && @params[:disable].include?(backend)
+      backend = Kernel.const_get(backend)
       Observer::Transaction.execute_singel_backend(backend, @item, @params)
     }
   end

app/models/transaction/trigger.rb

@@ -29,7 +29,7 @@ class Transaction::Trigger
 
     return if @item[:object] != 'Ticket'
 
-    triggers = Trigger.where(active: true)
+    triggers = Trigger.where(active: true).order('LOWER(name)')
     return if triggers.empty?
 
     ticket = Ticket.lookup(id: @item[:object_id])
@@ -39,86 +39,94 @@ class Transaction::Trigger
     end
 
     original_user_id = UserInfo.current_user_id
-    UserInfo.current_user_id = 1
-
-    triggers.each { |trigger|
-      condition = trigger.condition
-
-      # check action
-      if condition['ticket.action']
-        next if condition['ticket.action']['operator'] == 'is' && condition['ticket.action']['value'] != @item[:type]
-        next if condition['ticket.action']['operator'] != 'is' && condition['ticket.action']['value'] == @item[:type]
-        condition.delete('ticket.action')
-      end
-
-      # check "has changed" options
-      has_changed_condition_exists = false
-      has_changed = false
-      condition.each do |key, value|
-        next if !value
-        next if !value['operator']
-        next if !value['operator']['has changed']
-        has_changed_condition_exists = true
-
-        # next if has changed? && !@item[:changes][attribute]
-        (object_name, attribute) = key.split('.', 2)
-
-        # remove condition item, because it has changed
-        if @item[:changes][attribute]
-          has_changed = true
-          condition.delete(key)
-          next
+
+    Transaction.execute(reset_user_id: true, disable: ['Transaction::Trigger']) do
+      triggers.each { |trigger|
+        condition = trigger.condition
+
+        # check action
+        if condition['ticket.action']
+          next if condition['ticket.action']['operator'] == 'is' && condition['ticket.action']['value'] != @item[:type]
+          next if condition['ticket.action']['operator'] != 'is' && condition['ticket.action']['value'] == @item[:type]
+          condition.delete('ticket.action')
         end
-        break
-      end
 
-      next if has_changed_condition_exists && !has_changed
+        # check action
+        if condition['article.action']
+          next if !article
+          condition.delete('article.action')
+        end
 
-      # check if selector is matching
-      condition['ticket.id'] = {
-        operator: 'is',
-        value: ticket.id,
-      }
-      if article
-        condition['article.id'] = {
+        # check "has changed" options
+        has_changed_condition_exists = false
+        has_changed = false
+        condition.each do |key, value|
+          next if !value
+          next if !value['operator']
+          next if !value['operator']['has changed']
+          has_changed_condition_exists = true
+
+          # next if has changed? && !@item[:changes][attribute]
+          (object_name, attribute) = key.split('.', 2)
+
+          # remove condition item, because it has changed
+          if @item[:changes][attribute]
+            has_changed = true
+            condition.delete(key)
+            next
+          end
+          break
+        end
+
+        next if has_changed_condition_exists && !has_changed
+
+        # check if selector is matching
+        condition['ticket.id'] = {
           operator: 'is',
-          value: article.id,
+          value: ticket.id,
         }
-      end
-
-      ticket_count, tickets = Ticket.selectors(condition, 1)
-      next if ticket_count.zero?
-      next if tickets.first.id != ticket.id
-
-      # check if min one article attribute is used
-      article_selector = false
-      trigger.condition.each do |key, _value|
-        (object_name, attribute) = key.split('.', 2)
-        next if object_name != 'article'
-        next if attribute == 'id'
-        article_selector = true
-      end
-
-      # check in min one attribute has changed
-      if @item[:type] == 'update' && !article_selector
-        match = false
-        if has_changed_condition_exists && has_changed
-          match = true
-        else
-          trigger.condition.each do |key, _value|
-            (object_name, attribute) = key.split('.', 2)
-            next if object_name != 'ticket'
-            next if !@item[:changes][attribute]
+
+        # check if min one article attribute is used
+        article_selector = false
+        trigger.condition.each do |key, _value|
+          (object_name, attribute) = key.split('.', 2)
+          next if object_name != 'article'
+          next if attribute == 'id'
+          article_selector = true
+        end
+
+        next if article_selector && !article
+        if article_selector
+          condition['article.id'] = {
+            operator: 'is',
+            value: article.id,
+          }
+        end
+
+        ticket_count, tickets = Ticket.selectors(condition, 1)
+        next if ticket_count.zero?
+        next if tickets.first.id != ticket.id
+
+        # check in min one attribute has changed
+        if @item[:type] == 'update' && !article_selector
+          match = false
+          if has_changed_condition_exists && has_changed
             match = true
-            break
+          else
+            trigger.condition.each do |key, _value|
+              (object_name, attribute) = key.split('.', 2)
+              next if object_name != 'ticket'
+              next if !@item[:changes][attribute]
+              match = true
+              break
+            end
           end
+          next if !match
         end
-        next if !match
-      end
-      Transaction.execute do
+
         ticket.perform_changes(trigger.perform, 'trigger', @item)
-      end
-    }
+      }
+    end
     UserInfo.current_user_id = original_user_id
   end
 

test/unit/ticket_trigger_test.rb

@@ -4,8 +4,37 @@ require 'test_helper'
 class TicketTriggerTest < ActiveSupport::TestCase
   test '1 basic' do
     trigger1 = Trigger.create_or_update(
+      name: 'aaa loop check',
+      condition: {
+        'article.subject' => {
+          'operator' => 'contains',
+          'value' => 'Thanks for your inquiry',
+        },
+      },
+      perform: {
+        'ticket.tags' => {
+          'operator' => 'add',
+          'value' => 'should_not_loop',
+        },
+        'notification.email' => {
+          'body' => 'some lala',
+          'recipient' => 'ticket_customer',
+          'subject' => 'Thanks for your inquiry - loop check (#{ticket.title})!',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    trigger2 = Trigger.create_or_update(
       name: 'auto reply',
       condition: {
+        'ticket.action' => {
+          'operator' => 'is',
+          'value' => 'create',
+        },
         'ticket.state_id' => {
           'operator' => 'is',
           'value' => Ticket::State.lookup(name: 'new').id.to_s,
@@ -31,7 +60,54 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
-    trigger2 = Trigger.create_or_update(
+    trigger3 = Trigger.create_or_update(
+      name: 'auto tag 1',
+      condition: {
+        'ticket.action' => {
+          'operator' => 'is',
+          'value' => 'update',
+        },
+        'ticket.state_id' => {
+          'operator' => 'is',
+          'value' => Ticket::State.lookup(name: 'new').id.to_s,
+        }
+      },
+      perform: {
+        'ticket.priority_id' => {
+          'value' => Ticket::Priority.lookup(name: '3 high').id.to_s,
+        },
+        'ticket.tags' => {
+          'operator' => 'remove',
+          'value' => 'kk',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    trigger4 = Trigger.create_or_update(
+      name: 'auto tag 2',
+      condition: {
+        'ticket.state_id' => {
+          'operator' => 'is',
+          'value' => Ticket::State.lookup(name: 'new').id.to_s,
+        }
+      },
+      perform: {
+        'ticket.tags' => {
+          'operator' => 'add',
+          'value' => 'abc',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    trigger5 = Trigger.create_or_update(
       name: 'not matching',
       condition: {
         'ticket.state_id' => {
@@ -50,6 +126,31 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
+    trigger6 = Trigger.create_or_update(
+      name: 'zzz last',
+      condition: {
+        'article.subject' => {
+          'operator' => 'contains',
+          'value' => 'some subject 1234',
+        },
+      },
+      perform: {
+        'ticket.tags' => {
+          'operator' => 'add',
+          'value' => 'article_create_trigger',
+        },
+        'notification.email' => {
+          'body' => 'some lala',
+          'recipient' => 'ticket_customer',
+          'subject' => 'Thanks for your inquiry - 1234 check (#{ticket.title})!',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
     ticket1 = Ticket.create(
       title: "some <b>title</b>\n äöüß",
       group: Group.lookup(name: 'Users'),
@@ -89,7 +190,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     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), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
+    assert_equal(%w(aa kk abc), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
     article1 = ticket1.articles.last
     assert_match('Zammad <zammad@localhost>', article1.from)
     assert_match('nicole.braun@zammad.org', article1.to)
@@ -108,7 +209,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal('new', ticket1.state.name, 'ticket1.state verify')
     assert_equal('2 normal', ticket1.priority.name, 'ticket1.priority verify')
     assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
-    assert_equal(%w(aa kk), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
+    assert_equal(%w(aa kk abc), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
 
     ticket1.state = Ticket::State.lookup(name: 'open')
     ticket1.save
@@ -120,7 +221,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal('open', ticket1.state.name, 'ticket1.state verify')
     assert_equal('2 normal', ticket1.priority.name, 'ticket1.priority verify')
     assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
-    assert_equal(%w(aa kk), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
+    assert_equal(%w(aa kk abc), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
 
     ticket1.state = Ticket::State.lookup(name: 'new')
     ticket1.save
@@ -132,14 +233,8 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal('Users', 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(3, ticket1.articles.count, 'ticket1.articles verify')
-    assert_equal(%w(aa kk), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
-    article1 = ticket1.articles.last
-    assert_match('Zammad <zammad@localhost>', article1.from)
-    assert_match('nicole.braun@zammad.org', article1.to)
-    assert_match('Thanks for your inquiry (some <b>title</b>  äöüß)!', article1.subject)
-    assert_match('Braun<br>some &lt;b&gt;title&lt;/b&gt;', article1.body)
-    assert_equal('text/html', article1.content_type)
+    assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
+    assert_equal(%w(aa abc), Tag.tag_list(object: 'Ticket', o_id: ticket1.id))
 
     ticket2 = Ticket.create(
       title: "some title\n äöüß",
@@ -179,11 +274,12 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     assert(ticket3, 'ticket3 created')
+
     Ticket::Article.create(
       ticket_id: ticket3.id,
       from: 'some_sender@example.com',
       to: 'some_recipient@example.com',
-      subject: 'some subject',
+      subject: 'some subject 1234',
       message_id: 'some@id',
       content_type: 'text/html',
       body: 'some message <b>note</b><br>new line',
@@ -208,9 +304,9 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal('Users', ticket3.group.name, 'ticket3.group verify')
     assert_equal('new', ticket3.state.name, 'ticket3.state verify')
     assert_equal('3 high', ticket3.priority.name, 'ticket3.priority verify')
-    assert_equal(2, ticket3.articles.count, 'ticket3.articles verify')
-    assert_equal(%w(aa kk), Tag.tag_list(object: 'Ticket', o_id: ticket3.id))
-    article3 = ticket3.articles.last
+    assert_equal(3, ticket3.articles.count, 'ticket3.articles verify')
+    assert_equal(%w(aa kk abc article_create_trigger), Tag.tag_list(object: 'Ticket', o_id: ticket3.id))
+    article3 = ticket3.articles[1]
     assert_match('Zammad <zammad@localhost>', article3.from)
     assert_match('nicole.braun@zammad.org', article3.to)
     assert_match('Thanks for your inquiry (some <b>title</b>  äöüß3)!', article3.subject)
@@ -218,6 +314,61 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_match('&gt; some message note<br>&gt; new line', article3.body)
     assert_no_match('&gt; some message &lt;b&gt;note&lt;/b&gt;<br>&gt; new line', article3.body)
     assert_equal('text/html', article3.content_type)
+    article3 = ticket3.articles[2]
+    assert_match('Zammad <zammad@localhost>', article3.from)
+    assert_match('nicole.braun@zammad.org', article3.to)
+    assert_match('Thanks for your inquiry - 1234 check (some <b>title</b>  äöüß3)!', article3.subject)
+    assert_equal('text/html', article3.content_type)
+
+    Ticket::Article.create(
+      ticket_id: ticket3.id,
+      from: 'some_sender@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'some subject - not 1234',
+      message_id: 'some@id',
+      content_type: 'text/html',
+      body: 'some message <b>note</b><br>new line',
+      internal: false,
+      sender: Ticket::Article::Sender.find_by(name: 'Agent'),
+      type: Ticket::Article::Type.find_by(name: 'note'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    Observer::Transaction.commit
+
+    ticket3 = Ticket.lookup(id: ticket3.id)
+    assert_equal('some <b>title</b>  äöüß3', ticket3.title, 'ticket3.title verify')
+    assert_equal('Users', ticket3.group.name, 'ticket3.group verify')
+    assert_equal('new', ticket3.state.name, 'ticket3.state verify')
+    assert_equal('3 high', ticket3.priority.name, 'ticket3.priority verify')
+    assert_equal(4, ticket3.articles.count, 'ticket3.articles verify')
+    assert_equal(%w(aa kk abc article_create_trigger), Tag.tag_list(object: 'Ticket', o_id: ticket3.id))
+
+    Ticket::Article.create(
+      ticket_id: ticket3.id,
+      from: 'some_sender@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'some subject 1234',
+      message_id: 'some@id',
+      content_type: 'text/html',
+      body: 'some message <b>note</b><br>new line',
+      internal: false,
+      sender: Ticket::Article::Sender.find_by(name: 'Agent'),
+      type: Ticket::Article::Type.find_by(name: 'note'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    Observer::Transaction.commit
+
+    ticket3 = Ticket.lookup(id: ticket3.id)
+    assert_equal('some <b>title</b>  äöüß3', ticket3.title, 'ticket3.title verify')
+    assert_equal('Users', ticket3.group.name, 'ticket3.group verify')
+    assert_equal('new', ticket3.state.name, 'ticket3.state verify')
+    assert_equal('3 high', ticket3.priority.name, 'ticket3.priority verify')
+    assert_equal(5, ticket3.articles.count, 'ticket3.articles verify')
+    assert_equal(%w(aa kk abc article_create_trigger), Tag.tag_list(object: 'Ticket', o_id: ticket3.id))
 
     Trigger.destroy_all
   end