Fixes #2454 - Missing validation for trigger.notification.* at Model/API/REST level.

app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee

@@ -7,6 +7,9 @@ class App.UiElement.ticket_perform_action
       ticket:
         name: 'Ticket'
         model: 'Ticket'
+      article:
+        name: 'Article'
+        model: 'Article'
 
     if attribute.notification
       groups.notification =
@@ -17,8 +20,11 @@ class App.UiElement.ticket_perform_action
     elements = {}
     for groupKey, groupMeta of groups
       if !groupMeta.model || !App[groupMeta.model]
+        if groupKey is 'notification'
           elements["#{groupKey}.email"] = { name: 'email', display: 'Email' }
           elements["#{groupKey}.sms"] = { name: 'sms', display: 'SMS' }
+        else if groupKey is 'article'
+          elements["#{groupKey}.note"] = { name: 'note', display: 'Notiz' }
       else
 
         for row in App[groupMeta.model].configure_attributes
@@ -163,19 +169,26 @@ class App.UiElement.ticket_perform_action
       elementRow.find('.js-attributeSelector select').val(groupAndAttribute)
 
     notificationTypeMatch = groupAndAttribute.match(/^notification.([\w]+)$/)
+    articleTypeMatch = groupAndAttribute.match(/^article.([\w]+)$/)
 
     if _.isArray(notificationTypeMatch) && notificationType = notificationTypeMatch[1]
-      elementRow.find('.js-setAttribute').html('')
-      @buildRecipientList(notificationType, elementFull, elementRow, groupAndAttribute, elements, meta, attribute)
+      elementRow.find('.js-setAttribute').html('').addClass('hide')
+      elementRow.find('.js-setArticle').html('').addClass('hide')
+      @buildNotificationArea(notificationType, elementFull, elementRow, groupAndAttribute, elements, meta, attribute)
+    else if _.isArray(articleTypeMatch) && articleType = articleTypeMatch[1]
+      elementRow.find('.js-setAttribute').html('').addClass('hide')
+      elementRow.find('.js-setNotification').html('').addClass('hide')
+      @buildArticleArea(articleType, elementFull, elementRow, groupAndAttribute, elements, meta, attribute)
     else
-      elementRow.find('.js-setNotification').html('')
+      elementRow.find('.js-setNotification').html('').addClass('hide')
+      elementRow.find('.js-setArticle').html('').addClass('hide')
       if !elementRow.find('.js-setAttribute div').get(0)
         attributeSelectorElement = $( App.view('generic/ticket_perform_action/attribute_selector')(
           attribute: attribute
           name: name
           meta: meta || {}
         ))
-        elementRow.find('.js-setAttribute').html(attributeSelectorElement)
+        elementRow.find('.js-setAttribute').html(attributeSelectorElement).removeClass('hide')
       @buildOperator(elementFull, elementRow, groupAndAttribute, elements, meta, attribute)
 
   @buildOperator: (elementFull, elementRow, groupAndAttribute, elements, meta, attribute) ->
@@ -307,7 +320,7 @@ class App.UiElement.ticket_perform_action
 
     elementRow.find('.js-value').removeClass('hide').html(item)
 
-  @buildRecipientList: (notificationType, elementFull, elementRow, groupAndAttribute, elements, meta, attribute) ->
+  @buildNotificationArea: (notificationType, elementFull, elementRow, groupAndAttribute, elements, meta, attribute) ->
 
     return if elementRow.find(".js-setNotification .js-body-#{notificationType}").get(0)
 
@@ -344,7 +357,7 @@ class App.UiElement.ticket_perform_action
 
     selection = column_select.element()
 
-    notificationElement = $( App.view('generic/ticket_perform_action/notification_email')(
+    notificationElement = $( App.view('generic/ticket_perform_action/notification')(
       attribute: attribute
       name: name
       notificationType: notificationType
@@ -377,7 +390,59 @@ class App.UiElement.ticket_perform_action
       ]
     )
 
-    elementRow.find('.js-setNotification').html(notificationElement)
+    elementRow.find('.js-setNotification').html(notificationElement).removeClass('hide')
+
+  @buildArticleArea: (articleType, elementFull, elementRow, groupAndAttribute, elements, meta, attribute) ->
+
+    return if elementRow.find(".js-setArticle .js-body-#{articleType}").get(0)
+
+    elementRow.find('.js-setArticle').empty()
+
+    name = "#{attribute.name}::article.#{articleType}"
+    console.log('meta', meta)
+    selection = App.UiElement.select.render(
+      name: "#{name}::internal"
+      multiple: false
+      null: false
+      options: { true: 'internal', false: 'public' }
+      value: meta.internal
+      class: 'form-control--small'
+      translate: true
+    )
+    articleElement = $( App.view('generic/ticket_perform_action/article')(
+      attribute: attribute
+      name: name
+      articleType: articleType
+      meta: meta || {}
+    ))
+    articleElement.find('.js-internal').html(selection)
+    articleElement.find('.js-body div[contenteditable="true"]').ce(
+      mode: 'richtext'
+      placeholder: 'message'
+      maxlength: 200000
+    )
+    new App.WidgetPlaceholder(
+      el: articleElement.find('.js-body div[contenteditable="true"]').parent()
+      objects: [
+        {
+          prefix: 'ticket'
+          object: 'Ticket'
+          display: 'Ticket'
+        },
+        {
+          prefix: 'article'
+          object: 'TicketArticle'
+          display: 'Article'
+        },
+        {
+          prefix: 'user'
+          object: 'User'
+          display: 'Current User'
+        },
+      ]
+    )
+
+    elementRow.find('.js-setArticle').html(articleElement).removeClass('hide')
 
   @humanText: (condition) ->
     none = App.i18n.translateContent('No filter.')

app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee

@@ -180,7 +180,7 @@ class ArticleViewItem extends App.ObserverController
         links:       links
       )
       return
-    if article.sender.name is 'System'
+    if article.sender.name is 'System' && article.type.name isnt 'note'
     #if article.sender.name is 'System' && article.preferences.perform_origin is 'trigger'
       @html App.view('ticket_zoom/article_view_system')(
         ticket:      @ticket

app/assets/javascripts/app/views/generic/ticket_perform_action/article.jst.eco

@@ -0,0 +1,5 @@
+<div class="flex">
+  <div class="js-internal"></div>
+  <div class="controls js-subject"><input type="text" name="<%= @name %>::subject" value="<%= @meta.subject %>" style="width: 100%;" placeholder="<%- @T('Subject') %>"></div>
+  <div class="controls js-body js-body-<%- @articleType %>"><div class="richtext form-control"><div contenteditable="true" data-name="<%= @name %>::body"><%- @meta.body %></div></div></div>
+</div>

app/assets/javascripts/app/views/generic/ticket_perform_action/row.jst.eco

@@ -7,6 +7,7 @@
     </div>
     <div class="js-setAttribute"></div>
     <div class="js-setNotification flex"></div>
+    <div class="js-setArticle flex"></div>
   </div>
   <div class="filter-controls">
     <div class="filter-control filter-control-remove js-remove" title="<%- @Ti('Remote') %>">

app/models/concerns/checks_perform_validation.rb

@@ -0,0 +1,30 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module ChecksPerformValidation
+  extend ActiveSupport::Concern
+
+  included do
+    before_create :validate_perform
+    before_update :validate_perform
+  end
+
+  def validate_perform
+    # use Marshal to do a deep copy of the perform hash
+    validate_perform = Marshal.load(Marshal.dump(perform))
+
+    check_present = {
+      'article.note'       => %w[body subject internal],
+      'notification.email' => %w[body recipient subject],
+      'notification.sms'   => %w[body recipient],
+    }
+
+    check_present.each do |key, values|
+      next if validate_perform[key].blank?
+
+      values.each do |value|
+        raise Exceptions::UnprocessableEntity, "Invalid perform #{key}, #{value} is missing!" if validate_perform[key][value].blank?
+      end
+    end
+
+    true
+  end
+end

app/models/job.rb

@@ -3,6 +3,7 @@
 class Job < ApplicationModel
   include ChecksClientNotification
   include ChecksConditionValidation
+  include ChecksPerformValidation
 
   include Job::Assets
 

app/models/ticket.rb

@@ -846,12 +846,17 @@ perform changes on ticket
     end
 
     perform_notification = {}
+    perform_article = {}
     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'
+      raise "Unable to update object #{object_name}.#{attribute}, only can update tickets, send notifications and create articles!" if object_name != 'ticket' && object_name != 'article' && object_name != 'notification'
 
-      # send notification (after changes are done)
+      # send notification/create article (after changes are done)
+      if object_name == 'article'
+        perform_article[key] = value
+        next
+      end
       if object_name == 'notification'
         perform_notification[key] = value
         next
@@ -910,6 +915,25 @@ perform changes on ticket
       save!
     end
 
+    perform_article.each do |key, value|
+      raise 'Unable to create article, we only support article.note' if key != 'article.note'
+
+      Ticket::Article.create!(
+        ticket_id:     id,
+        subject:       value[:subject],
+        content_type:  'text/html',
+        body:          value[:body],
+        internal:      value[:internal],
+        sender:        Ticket::Article::Sender.find_by(name: 'System'),
+        type:          Ticket::Article::Type.find_by(name: 'note'),
+        preferences:   {
+          perform_origin: perform_origin,
+        },
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+    end
+
     perform_notification.each do |key, value|
 
       # send notification

app/models/trigger.rb

@@ -2,6 +2,7 @@
 
 class Trigger < ApplicationModel
   include ChecksConditionValidation
+  include ChecksPerformValidation
   include CanSeed
 
   include Trigger::Assets

spec/db/migrate/issue_2019_fix_double_domain_links_in_trigger_emails_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe Issue2019FixDoubleDomainLinksInTriggerEmails, type: :db_migration do
-  subject { create(:trigger, perform: { 'notification.email' => { 'body' => faulty_link } }) }
+  subject { create(:trigger, perform: { 'notification.email' => { 'body' => faulty_link, 'recipient' => 'customer', 'subject' => 'some subject' } }) }
 
   let(:faulty_link) do
     '<a href="https://example.com/#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}">' \

test/unit/job_test.rb

@@ -1064,6 +1064,103 @@ class JobTest < ActiveSupport::TestCase
 
     assert_not(Ticket.find_by(id: ticket1.id))
     assert(Ticket.find_by(id: ticket2.id))
+  end
+
+  test 'validates perform with article.note - should fail because of missing body' do
+    assert_raises(Exception) do
+      Job.create!(
+        name:                 'some job',
+        timeplan:             {
+          days:    {
+            Mon: true,
+          },
+          hours:   {
+            '0' => false,
+          },
+          minutes: {
+            '0' => true,
+          }
+        },
+        condition:            {
+          'ticket.state_id' => { 'operator' => 'is', 'value' => Ticket::State.find_by(name: 'closed').id }
+        },
+        perform:              {
+          'article.note' => {
+            'subject'  => 'some subject!',
+            'internal' => 'true',
+          },
+        },
+        disable_notification: true,
+        active:               true,
+        updated_by_id:        1,
+        created_by_id:        1,
+      )
+    end
+  end
+
+  test 'validates perform with notification.email - should fail because of missing recipient' do
+    assert_raises(Exception) do
+      Job.create!(
+        name:                 'some job',
+        timeplan:             {
+          days:    {
+            Mon: true,
+          },
+          hours:   {
+            '0' => false,
+          },
+          minutes: {
+            '0' => true,
+          }
+        },
+        condition:            {
+          'ticket.state_id' => { 'operator' => 'is', 'value' => Ticket::State.find_by(name: 'closed').id }
+        },
+        perform:              {
+          'notification.email' => {
+            'body'      => 'some lala',
+            'recipient' => '',
+            'subject'   => 'Thanks for your inquiry!',
+          },
+        },
+        disable_notification: true,
+        active:               true,
+        updated_by_id:        1,
+        created_by_id:        1,
+      )
+    end
+  end
+
+  test 'validates perform with notification.sms - should fail because of missing recipient' do
+    assert_raises(Exception) do
+      Job.create!(
+        name:                 'some job',
+        timeplan:             {
+          days:    {
+            Mon: true,
+          },
+          hours:   {
+            '0' => false,
+          },
+          minutes: {
+            '0' => true,
+          }
+        },
+        condition:            {
+          'ticket.state_id' => { 'operator' => 'is', 'value' => Ticket::State.find_by(name: 'closed').id }
+        },
+        perform:              {
+          'notification.sms' => {
+            'body'      => 'some lala',
+            'recipient' => '',
+          },
+        },
+        disable_notification: true,
+        active:               true,
+        updated_by_id:        1,
+        created_by_id:        1,
+      )
+    end
+  end
 
 end
-end

test/unit/ticket_trigger_test.rb

@@ -7,7 +7,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '1 basic' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa loop check',
       condition:            {
         'article.subject' => {
@@ -32,7 +32,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger2 = Trigger.create_or_update(
+    trigger2 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -64,7 +64,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger3 = Trigger.create_or_update(
+    trigger3 = Trigger.create!(
       name:                 'auto tag 1',
       condition:            {
         'ticket.action'   => {
@@ -91,7 +91,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger4 = Trigger.create_or_update(
+    trigger4 = Trigger.create!(
       name:                 'auto tag 2',
       condition:            {
         'ticket.state_id' => {
@@ -111,7 +111,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger5 = Trigger.create_or_update(
+    trigger5 = Trigger.create!(
       name:                 'not matching',
       condition:            {
         'ticket.state_id' => {
@@ -130,7 +130,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger6 = Trigger.create_or_update(
+    trigger6 = Trigger.create!(
       name:                 'zzz last',
       condition:            {
         'article.subject' => {
@@ -411,7 +411,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '2 actions - create' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -504,7 +504,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '2 actions - update' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -592,7 +592,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
   test '3 auto replys' do
     roles = Role.where(name: 'Customer')
-    customer1 = User.create_or_update(
+    customer1 = User.create!(
       login:         'postmaster@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -604,7 +604,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    customer2 = User.create_or_update(
+    customer2 = User.create!(
       login:           'ticket-auto-reply-customer2@example.com',
       firstname:       'Trigger',
       lastname:        'Customer2',
@@ -618,7 +618,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id:   1,
     )
 
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply - new ticket',
       condition:            {
         'ticket.action'   => {
@@ -657,7 +657,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger2 = Trigger.create_or_update(
+    trigger2 = Trigger.create!(
       name:          'auto reply (on follow up of tickets)',
       condition:     {
         'ticket.action'     => {
@@ -695,7 +695,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
-    trigger3 = Trigger.create_or_update(
+    trigger3 = Trigger.create!(
       name:                 'not matching',
       condition:            {
         'ticket.action'   => {
@@ -881,7 +881,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
   test '4 has changed' do
     roles = Role.where(name: 'Customer')
-    customer1 = User.create_or_update(
+    customer1 = User.create!(
       login:         'postmaster@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -893,7 +893,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    customer2 = User.create_or_update(
+    customer2 = User.create!(
       login:           'ticket-has-changed-customer2@example.com',
       firstname:       'Trigger',
       lastname:        'Customer2',
@@ -908,7 +908,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent1 = User.create_or_update(
+    agent1 = User.create!(
       login:         'agent-has-changed@example.com',
       firstname:     'Has Changed',
       lastname:      'Agent1',
@@ -921,7 +921,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'owner update - to customer',
       condition:            {
         'ticket.owner_id' => {
@@ -1197,7 +1197,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '5 notify owner' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa notify mail',
       condition:            {
         'ticket.state_id' => {
@@ -1223,7 +1223,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -1287,7 +1287,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
     assert_equal(3, ticket1.articles.count)
 
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa notify mail 2',
       condition:            {
         'ticket.state_id' => {
@@ -1332,7 +1332,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '6 owner auto assignment' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa auto assignment',
       condition:            {
         'ticket.owner_id' => {
@@ -1360,7 +1360,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -1446,7 +1446,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '6.1 owner auto assignment based on organization' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa auto assignment',
       condition:            {
         'ticket.organization_id' => {
@@ -1474,7 +1474,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     roles = Role.where(name: 'Agent')
     groups = Group.where(name: 'Users')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -1487,7 +1487,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     roles = Role.where(name: 'Customer')
-    customer = User.create_or_update(
+    customer = User.create!(
       login:         'customer@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -1559,7 +1559,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '6.2 owner auto assignment based on organization' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa auto assignment',
       condition:            {
         'ticket.organization_id' => {
@@ -1587,7 +1587,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -1600,7 +1600,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     roles = Role.where(name: 'Customer')
-    customer = User.create_or_update(
+    customer = User.create!(
       login:         'customer@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -1672,7 +1672,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '7 owner auto assignment' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa auto assignment',
       condition:            {
         'ticket.owner_id'   => {
@@ -1704,7 +1704,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent1 = User.create_or_update(
+    agent1 = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -1716,11 +1716,11 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    agent2 = User.create_or_update(
-      login:         'agent@example.com',
+    agent2 = User.create!(
+      login:         'agent2@example.com',
       firstname:     'Trigger',
       lastname:      'Agent2',
-      email:         'agent@example.com',
+      email:         'agent2@example.com',
       password:      'agentpw',
       active:        true,
       roles:         roles,
@@ -1857,7 +1857,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     ticket1.reload
     assert_equal('test 123', ticket1.title, 'ticket1.title verify')
     assert_equal('Users', ticket1.group.name, 'ticket1.group verify')
-    assert_equal(agent1.id, ticket1.owner_id, 'ticket1.owner_id verify')
+    assert_equal(agent2.id, ticket1.owner_id, 'ticket1.owner_id verify')
     assert_equal('new', ticket1.state.name, 'ticket1.state verify')
     assert_equal('2 normal', ticket1.priority.name, 'ticket1.priority verify')
     assert_equal(4, ticket1.articles.count, 'ticket1.articles verify')
@@ -1865,7 +1865,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '8 owner auto assignment' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa auto assignment',
       condition:            {
         'ticket.owner_id'    => {
@@ -1899,7 +1899,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -2015,7 +2015,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '9 vip priority set' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa vip priority',
       condition:            {
         'customer.vip' => {
@@ -2035,7 +2035,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -2048,7 +2048,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     roles = Role.where(name: 'Customer')
-    customer = User.create_or_update(
+    customer = User.create!(
       login:         'customer@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -2153,7 +2153,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '10 owner auto assignment notify to customer' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa auto assignment',
       condition:            {
         'ticket.owner_id' => {
@@ -2177,7 +2177,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent1 = User.create_or_update(
+    agent1 = User.create!(
       login:         'agent1@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -2189,7 +2189,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    agent2 = User.create_or_update(
+    agent2 = User.create!(
       login:         'agent2@example.com',
       firstname:     'Trigger',
       lastname:      'Agent2',
@@ -2280,7 +2280,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '11 notify to customer on public note' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa notify to customer on public note',
       condition:            {
         'article.internal'  => {
@@ -2312,7 +2312,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -2325,7 +2325,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     roles = Role.where(name: 'Customer')
-    customer = User.create_or_update(
+    customer = User.create!(
       login:         'customer@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -2490,7 +2490,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '12 notify on owner change' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa notify to customer on public note',
       condition:            {
         'ticket.owner_id' => {
@@ -2512,7 +2512,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id:        1,
       updated_by_id:        1,
     )
-    Trigger.create_or_update(
+    Trigger.create!(
       name:          'auto reply (on new tickets)',
       condition:     {
         'ticket.action'     => {
@@ -2555,7 +2555,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
-    Trigger.create_or_update(
+    Trigger.create!(
       name:          'auto reply (on follow up of tickets)',
       condition:     {
         'ticket.action'     => {
@@ -2597,7 +2597,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
     groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
-    agent = User.create_or_update(
+    agent = User.create!(
       login:         'agent@example.com',
       firstname:     'Trigger',
       lastname:      'Agent1',
@@ -2610,7 +2610,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     roles = Role.where(name: 'Customer')
-    customer = User.create_or_update(
+    customer = User.create!(
       login:         'customer@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -2719,7 +2719,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
   test '1 empty condition should not create errors' do
     assert_raises(Exception) do
-      trigger_empty = Trigger.create_or_update(
+      trigger_empty = Trigger.create!(
         name:                 'aaa loop check',
         condition:            {
           'ticket.number' => {
@@ -2743,7 +2743,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'article_last_sender trigger -> reply_to' do
-    trigger = Trigger.create_or_update(
+    trigger = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -2800,7 +2800,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'article_last_sender trigger -> from' do
-    trigger = Trigger.create_or_update(
+    trigger = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -2856,7 +2856,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'article_last_sender trigger -> origin_by_id' do
-    trigger = Trigger.create_or_update(
+    trigger = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -2881,7 +2881,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
     roles = Role.where(name: 'Customer')
-    customer1 = User.create_or_update(
+    customer1 = User.create!(
       login:         'customer+origin_by_id@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -2925,7 +2925,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'article_last_sender trigger -> created_by_id' do
-    trigger = Trigger.create_or_update(
+    trigger = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -2950,7 +2950,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
     roles = Role.where(name: 'Customer')
-    customer1 = User.create_or_update(
+    customer1 = User.create!(
       login:         'customer+created_by_id@example.com',
       firstname:     'Trigger',
       lastname:      'Customer1',
@@ -2993,7 +2993,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'multiple recipients owner_id, article_last_sender(reply_to) trigger' do
-    trigger = Trigger.create_or_update(
+    trigger = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -3017,7 +3017,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id:        1,
       updated_by_id:        1,
     )
-    admin = User.create_or_update(
+    admin = User.create!(
       login:         'admin+owner_recipient@example.com',
       firstname:     'Role',
       lastname:      "Admin#{name}",
@@ -3064,7 +3064,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'article_last_sender trigger -> invalid reply_to' do
-    trigger = Trigger.create_or_update(
+    trigger = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -3118,7 +3118,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '2 loop check' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa loop check',
       condition:            {
         'ticket.state_id'   => {
@@ -3417,7 +3417,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '3 invalid condition' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'aaa loop check',
       condition:            {
         'ticket.action' => {
@@ -3448,7 +3448,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
                            })
     assert_equal('invalid invalid 4', trigger1.condition['ticket.first_response_at']['value'])
 
-    trigger2 = Trigger.create_or_update(
+    trigger2 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -3529,7 +3529,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test '4 tag based auto response' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 '100 add tag if sender 1',
       condition:            {
         'ticket.action' => {
@@ -3553,7 +3553,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger2 = Trigger.create_or_update(
+    trigger2 = Trigger.create!(
       name:                 '200 add tag if sender 2',
       condition:            {
         'ticket.action' => {
@@ -3577,7 +3577,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id:        1,
     )
 
-    trigger3 = Trigger.create_or_update(
+    trigger3 = Trigger.create!(
       name:                 '300 auto reply',
       condition:            {
         'ticket.action'   => {
@@ -3725,7 +3725,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'article.body' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -3962,7 +3962,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   test 'change owner' do
     roles = Role.where(name: 'Agent')
     groups = Group.where(name: 'Users')
-    agent1 = User.create_or_update(
+    agent1 = User.create!(
       login:         'agent-has-changed@example.com',
       firstname:     'Has Changed',
       lastname:      'Agent1',
@@ -3976,7 +3976,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
 
-    agent2 = User.create_or_update(
+    agent2 = User.create!(
       login:         'agent-has-changed2@example.com',
       firstname:     'Has Changed',
       lastname:      'Agent2',
@@ -3991,7 +3991,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     )
 
     # multi tag trigger with changed owner
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'change owner',
       condition:            {
         'ticket.owner_id' => {
@@ -4160,7 +4160,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'trigger auto reply with umlaut in form' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -4234,7 +4234,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'trigger auto reply with 2 sender addresses in form' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -4275,7 +4275,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
   end
 
   test 'make sure attachments should be attached with content id' do
-    trigger1 = Trigger.create_or_update(
+    trigger1 = Trigger.create!(
       name:                 'auto reply',
       condition:            {
         'ticket.action'   => {
@@ -4324,7 +4324,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
-    Trigger.create_or_update(
+    Trigger.create!(
       name:          'auto reply (condition: organization-is-not)',
       condition:     {
         'ticket.organization_id' => {
@@ -4367,8 +4367,8 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal('text/html', autoreply.content_type)
   end
 
-  test 'trigger when there is an article body contains matched values' do
-    trigger1 = Trigger.create_or_update(
+  test 'trigger tags and auto responder when there is an article body contains matched values' do
+    trigger1 = Trigger.create!(
       name:                 'detect message body',
       condition:            {
         'article.body' => {
@@ -4436,4 +4436,164 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_match("some message <b>note</b>\nnew line", article1.body)
     assert_equal('text/plain', article1.content_type)
   end
+
+  test 'trigger note and auto responder (correct order) when there is an article body contains matched values' do
+    trigger1 = Trigger.create!(
+      name:                 'detect message body',
+      condition:            {
+        'article.body' => {
+          'operator' => 'contains',
+          'value'    => 'some message',
+        },
+      },
+      perform:              {
+        'article.note'       => {
+          'body'     => 'some note',
+          'subject'  => 'some subject!',
+          'internal' => 'true',
+        },
+        '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,
+    )
+    ticket1 = Ticket.create!(
+      title:         "some <b>title</b>\n äöüß",
+      group:         Group.lookup(name: 'Users'),
+      customer:      User.lookup(email: 'nicole.braun@zammad.org'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    article1 = 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 <b>note</b>\nnew 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,
+    )
+    ticket1.reload
+    assert_equal('some <b>title</b>  äöüß', ticket1.title, 'ticket1.title verify')
+    assert_equal('Users', 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 <b>title</b>  äöüß', ticket1.title, 'ticket1.title verify')
+    assert_equal('Users', 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(3, ticket1.articles.count, 'ticket1.articles verify')
+    assert_equal([], ticket1.tag_list)
+
+    assert_match('- ', article1.from)
+    assert_match('some_recipient@example.com', article1.to)
+    assert_match('some subject', article1.subject)
+    assert_match("some message <b>note</b>\nnew line", article1.body)
+    assert_equal('text/plain', article1.content_type)
+
+    article_note1 = ticket1.articles[1]
+    assert_match('- ', article_note1.from)
+    assert_nil(article_note1.to)
+    assert_match('some subject!', article_note1.subject)
+    assert_match('some note', article_note1.body)
+    assert_equal('text/html', article_note1.content_type)
+    assert_equal(true, article_note1.internal)
+
+    article_auto_responder1 = ticket1.articles[2]
+    assert_match('Zammad <zammad@localhost>', article_auto_responder1.from)
+    assert_match('nicole.braun@zammad.org', article_auto_responder1.to)
+    assert_match('Thanks for your inquiry - loop check (some <b>title</b>  äöüß)!', article_auto_responder1.subject)
+    assert_match('some lala', article_auto_responder1.body)
+    assert_equal('text/html', article_auto_responder1.content_type)
+  end
+
+  test 'validates perform with article.note - should fail because of missing body' do
+    assert_raises(Exception) do
+      Trigger.create!(
+        name:                 'some trigger',
+        condition:            {
+          'article.body' => {
+            'operator' => 'contains',
+            'value'    => 'some message',
+          },
+        },
+        perform:              {
+          'article.note' => {
+            'subject'  => 'some subject!',
+            'internal' => 'true',
+          },
+        },
+        disable_notification: true,
+        active:               true,
+        created_by_id:        1,
+        updated_by_id:        1,
+      )
+    end
+  end
+
+  test 'validates perform with notification.email - should fail because of missing recipient' do
+    assert_raises(Exception) do
+      Trigger.create!(
+        name:                 'some trigger',
+        condition:            {
+          'article.body' => {
+            'operator' => 'contains',
+            'value'    => 'some message',
+          },
+        },
+        perform:              {
+          'notification.email' => {
+            'body'      => 'some lala',
+            'recipient' => '',
+            'subject'   => 'Thanks for your inquiry - loop check (#{ticket.title})!',
+          },
+        },
+        disable_notification: true,
+        active:               true,
+        created_by_id:        1,
+        updated_by_id:        1,
+      )
+    end
+  end
+
+  test 'validates perform with notification.sms - should fail because of missing recipient' do
+    assert_raises(Exception) do
+      Trigger.create!(
+        name:                 'some trigger',
+        condition:            {
+          'article.body' => {
+            'operator' => 'contains',
+            'value'    => 'some message',
+          },
+        },
+        perform:              {
+          'notification.sms' => {
+            'body'      => 'some lala',
+            'recipient' => '',
+          },
+        },
+        disable_notification: true,
+        active:               true,
+        created_by_id:        1,
+        updated_by_id:        1,
+      )
+    end
+  end
+
 end