diff --git a/app/models/concerns/validates_condition.rb b/app/models/concerns/validates_condition.rb new file mode 100644 index 000000000..77f5beca5 --- /dev/null +++ b/app/models/concerns/validates_condition.rb @@ -0,0 +1,34 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +module ValidatesCondition + extend ActiveSupport::Concern + + included do + before_create :validate_condition + before_update :validate_condition + end + + def validate_condition + # use Marshal to do a deep copy of the condition hash + validate_condition = Marshal.load(Marshal.dump(condition)) + + # check if a valid condition got inserted. + validate_condition.delete('ticket.action') + validate_condition.each do |key, value| + next if !value + next if !value['operator'] + next if !value['operator']['has changed'] + + validate_condition.delete(key) + end + + validate_condition['ticket.id'] = { + operator: 'is', + value: 1, + } + + ticket_count, tickets = Ticket.selectors(validate_condition, 1, User.find(1)) + return if ticket_count.present? + + raise Exceptions::UnprocessableEntity, 'Invalid ticket selector conditions' + end +end diff --git a/app/models/job.rb b/app/models/job.rb index 7d0a010fa..e954008f8 100644 --- a/app/models/job.rb +++ b/app/models/job.rb @@ -2,6 +2,7 @@ class Job < ApplicationModel include NotifiesClients + include ValidatesCondition load 'job/assets.rb' include Job::Assets diff --git a/app/models/overview.rb b/app/models/overview.rb index 8b4c284ce..259a49354 100644 --- a/app/models/overview.rb +++ b/app/models/overview.rb @@ -3,6 +3,7 @@ class Overview < ApplicationModel include NotifiesClients include LatestChangeObserved + include ValidatesCondition load 'overview/assets.rb' include Overview::Assets diff --git a/app/models/sla.rb b/app/models/sla.rb index 0c31f48b0..29fe06c5a 100644 --- a/app/models/sla.rb +++ b/app/models/sla.rb @@ -2,6 +2,7 @@ class Sla < ApplicationModel include NotifiesClients + include ValidatesCondition load 'sla/assets.rb' include Sla::Assets diff --git a/app/models/transaction/trigger.rb b/app/models/transaction/trigger.rb index 33a504b72..a2be8c673 100644 --- a/app/models/transaction/trigger.rb +++ b/app/models/transaction/trigger.rb @@ -130,6 +130,7 @@ class Transaction::Trigger # verify is condition is matching ticket_count, tickets = Ticket.selectors(condition, 1) + next if ticket_count.blank? next if ticket_count.zero? next if tickets.first.id != ticket.id user_id = ticket.updated_by_id diff --git a/app/models/trigger.rb b/app/models/trigger.rb index 2cd3940ff..8fee22905 100644 --- a/app/models/trigger.rb +++ b/app/models/trigger.rb @@ -1,6 +1,8 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Trigger < ApplicationModel + include ValidatesCondition + store :condition store :perform validates :name, presence: true diff --git a/test/unit/job_test.rb b/test/unit/job_test.rb index 71533282b..7e41e925d 100644 --- a/test/unit/job_test.rb +++ b/test/unit/job_test.rb @@ -330,7 +330,7 @@ class JobTest < ActiveSupport::TestCase }, }, condition: { - 'ticket.state_id' => { 'operator' => 'is', 'value' => '' }, + 'ticket.state_id' => { 'operator' => 'is', 'value' => '9999' }, 'ticket.created_at' => { 'operator' => 'before (relative)', 'value' => '2', 'range' => 'day' }, }, perform: { @@ -355,81 +355,6 @@ class JobTest < ActiveSupport::TestCase ticket2_later = Ticket.find(ticket2.id) assert_equal('new', ticket2_later.state.name) assert_equal(ticket2.updated_at.to_s, ticket2_later.updated_at.to_s) - - job1 = Job.create_or_update( - name: 'Test Job1', - timeplan: { - days: { - Mon: true, - Tue: true, - Wed: true, - Thu: true, - Fri: true, - Sat: true, - Sun: true, - }, - hours: { - 0 => true, - 1 => true, - 2 => true, - 3 => true, - 4 => true, - 5 => true, - 6 => true, - 7 => true, - 8 => true, - 9 => true, - 10 => true, - 11 => true, - 12 => true, - 13 => true, - 14 => true, - 15 => true, - 16 => true, - 17 => true, - 18 => true, - 19 => true, - 20 => true, - 21 => true, - 22 => true, - 23 => true, - }, - minutes: { - 0 => true, - 10 => true, - 20 => true, - 30 => true, - 40 => true, - 50 => true, - }, - }, - condition: { - 'ticket.state_id' => { 'operator' => 'is' }, - 'ticket.created_at' => { 'operator' => 'before (relative)', 'value' => '2', 'range' => 'day' }, - }, - perform: { - 'ticket.state_id' => { 'value' => Ticket::State.lookup(name: 'closed').id.to_s } - }, - disable_notification: true, - last_run_at: nil, - updated_at: Time.zone.now - 15.minutes, - active: true, - updated_by_id: 1, - created_by_id: 1, - ) - assert(job1.executable?) - assert(job1.in_timeplan?) - Job.run - - # verify changes on tickets - ticket1_later = Ticket.find(ticket1.id) - assert_equal('new', ticket1_later.state.name) - assert_equal(ticket1.updated_at.to_s, ticket1_later.updated_at.to_s) - - ticket2_later = Ticket.find(ticket2.id) - assert_equal('new', ticket2_later.state.name) - assert_equal(ticket2.updated_at.to_s, ticket2_later.updated_at.to_s) - end test 'case 3' do diff --git a/test/unit/ticket_trigger_test.rb b/test/unit/ticket_trigger_test.rb index f5d7e6fc7..3bf16da4e 100644 --- a/test/unit/ticket_trigger_test.rb +++ b/test/unit/ticket_trigger_test.rb @@ -2446,4 +2446,29 @@ class TicketTriggerTest < ActiveSupport::TestCase end + test '1 empty condition should not create errors' do + assert_raises(Exception) { + trigger_empty = Trigger.create_or_update( + name: 'aaa loop check', + condition: { + 'ticket.number' => { + 'operator' => 'contains', + 'value' => '', + }, + }, + perform: { + '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, + ) + } + end + end