From 295e67cb6faef501c2c845e02ef43ed5c11175c9 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 29 Jan 2019 15:04:47 +0100 Subject: [PATCH] Fixes #2454 - Missing validation for trigger.notification.* at Model/API/REST level. --- .../_ui_element/ticket_perform_action.coffee | 83 ++++- .../ticket_zoom/article_view.coffee | 2 +- .../ticket_perform_action/article.jst.eco | 5 + ...ion_email.jst.eco => notification.jst.eco} | 0 .../generic/ticket_perform_action/row.jst.eco | 1 + .../concerns/checks_perform_validation.rb | 30 ++ app/models/job.rb | 1 + app/models/ticket.rb | 28 +- app/models/trigger.rb | 1 + ...ble_domain_links_in_trigger_emails_spec.rb | 2 +- test/unit/job_test.rb | 99 +++++- test/unit/ticket_trigger_test.rb | 314 +++++++++++++----- 12 files changed, 475 insertions(+), 91 deletions(-) create mode 100644 app/assets/javascripts/app/views/generic/ticket_perform_action/article.jst.eco rename app/assets/javascripts/app/views/generic/ticket_perform_action/{notification_email.jst.eco => notification.jst.eco} (100%) create mode 100644 app/models/concerns/checks_perform_validation.rb diff --git a/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee b/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee index 7d30630b0..ae86a59e2 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee +++ b/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] - elements["#{groupKey}.email"] = { name: 'email', display: 'Email' } - elements["#{groupKey}.sms"] = { name: 'sms', display: 'SMS' } + 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.') diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee index 7c1f39ca3..33f1b2406 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee +++ b/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 diff --git a/app/assets/javascripts/app/views/generic/ticket_perform_action/article.jst.eco b/app/assets/javascripts/app/views/generic/ticket_perform_action/article.jst.eco new file mode 100644 index 000000000..5501980d2 --- /dev/null +++ b/app/assets/javascripts/app/views/generic/ticket_perform_action/article.jst.eco @@ -0,0 +1,5 @@ +
+
+
+
<%- @meta.body %>
+
diff --git a/app/assets/javascripts/app/views/generic/ticket_perform_action/notification_email.jst.eco b/app/assets/javascripts/app/views/generic/ticket_perform_action/notification.jst.eco similarity index 100% rename from app/assets/javascripts/app/views/generic/ticket_perform_action/notification_email.jst.eco rename to app/assets/javascripts/app/views/generic/ticket_perform_action/notification.jst.eco diff --git a/app/assets/javascripts/app/views/generic/ticket_perform_action/row.jst.eco b/app/assets/javascripts/app/views/generic/ticket_perform_action/row.jst.eco index 661b772f8..13fd03350 100644 --- a/app/assets/javascripts/app/views/generic/ticket_perform_action/row.jst.eco +++ b/app/assets/javascripts/app/views/generic/ticket_perform_action/row.jst.eco @@ -7,6 +7,7 @@
+
diff --git a/app/models/concerns/checks_perform_validation.rb b/app/models/concerns/checks_perform_validation.rb new file mode 100644 index 000000000..7aedd4a85 --- /dev/null +++ b/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 diff --git a/app/models/job.rb b/app/models/job.rb index e0c4dac9e..dc75f1ad0 100644 --- a/app/models/job.rb +++ b/app/models/job.rb @@ -3,6 +3,7 @@ class Job < ApplicationModel include ChecksClientNotification include ChecksConditionValidation + include ChecksPerformValidation include Job::Assets diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 5a8eeda77..fd74bc376 100644 --- a/app/models/ticket.rb +++ b/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 diff --git a/app/models/trigger.rb b/app/models/trigger.rb index d76144db7..837fd3325 100644 --- a/app/models/trigger.rb +++ b/app/models/trigger.rb @@ -2,6 +2,7 @@ class Trigger < ApplicationModel include ChecksConditionValidation + include ChecksPerformValidation include CanSeed include Trigger::Assets diff --git a/spec/db/migrate/issue_2019_fix_double_domain_links_in_trigger_emails_spec.rb b/spec/db/migrate/issue_2019_fix_double_domain_links_in_trigger_emails_spec.rb index 4bbb95c68..9aa9bc8c1 100644 --- a/spec/db/migrate/issue_2019_fix_double_domain_links_in_trigger_emails_spec.rb +++ b/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 '' \ diff --git a/test/unit/job_test.rb b/test/unit/job_test.rb index 2d10389de..c8deb5e5c 100644 --- a/test/unit/job_test.rb +++ b/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 diff --git a/test/unit/ticket_trigger_test.rb b/test/unit/ticket_trigger_test.rb index 26b6ae24f..1475d7569 100644 --- a/test/unit/ticket_trigger_test.rb +++ b/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', @@ -1643,7 +1643,7 @@ class TicketTriggerTest < ActiveSupport::TestCase assert_equal(1, ticket1.articles.count, 'ticket1.articles verify') assert_equal([], ticket1.tag_list) - ticket1.update!(customer: customer ) + ticket1.update!(customer: customer) UserInfo.current_user_id = agent.id Ticket::Article.create!( @@ -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 note\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 title\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 note\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 title äöüß', 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 title äöüß', 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 note\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 ', article_auto_responder1.from) + assert_match('nicole.braun@zammad.org', article_auto_responder1.to) + assert_match('Thanks for your inquiry - loop check (some title äöüß)!', 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