From ec23915bd01aed5196de0c05ddaa5210c00d720d Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Thu, 20 Feb 2020 13:34:16 +0100 Subject: [PATCH] Fixes #1553: Calendar as filter condition for Trigger/Automatization. --- .rubocop_todo.yml | 4 +- .../_ui_element/ticket_selector.coffee | 48 ++++++++++++------- app/assets/javascripts/app/models/job.coffee | 2 +- .../javascripts/app/models/trigger.coffee | 2 +- app/controllers/tickets_controller.rb | 2 +- app/models/application_model/can_assets.rb | 1 + app/models/calendar.rb | 15 ++++++ .../concerns/checks_condition_validation.rb | 1 + app/models/job.rb | 4 +- app/models/job/assets.rb | 13 ++--- app/models/ticket.rb | 21 ++++++-- app/models/ticket/escalation.rb | 13 +---- app/models/trigger/assets.rb | 14 +++--- spec/models/trigger_spec.rb | 42 ++++++++++++++++ 14 files changed, 132 insertions(+), 50 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f4be71eb8..1431a0e63 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -37,7 +37,7 @@ Metrics/BlockNesting: # Offense count: 340 Metrics/CyclomaticComplexity: - Max: 97 + Max: 107 # Offense count: 27 # Configuration parameters: CountComments. @@ -46,7 +46,7 @@ Metrics/ModuleLength: # Offense count: 274 Metrics/PerceivedComplexity: - Max: 115 + Max: 125 # Offense count: 2 # Cop supports --auto-correct. diff --git a/app/assets/javascripts/app/controllers/_ui_element/ticket_selector.coffee b/app/assets/javascripts/app/controllers/_ui_element/ticket_selector.coffee index a6ca32c52..90ed255f4 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/ticket_selector.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/ticket_selector.coffee @@ -17,6 +17,10 @@ class App.UiElement.ticket_selector name: 'Organization' model: 'Organization' + if attribute.executionTime + groups.execution_time = + name: 'Execution Time' + operators_type = '^datetime$': ['before (absolute)', 'after (absolute)', 'before (relative)', 'after (relative)', 'within next (relative)', 'within last (relative)'] '^timestamp$': ['before (absolute)', 'after (absolute)', 'before (relative)', 'after (relative)', 'within next (relative)', 'within last (relative)'] @@ -71,24 +75,36 @@ class App.UiElement.ticket_selector operator: ['is', 'is not'] for groupKey, groupMeta of groups - for row in App[groupMeta.model].configure_attributes + if groupKey is 'execution_time' + if attribute.executionTime + elements['execution_time.calendar_id'] = + name: 'calendar_id' + display: 'Calendar' + tag: 'select' + relation: 'Calendar' + null: false + translate: false + operator: ['is in working time', 'is not in working time'] - # ignore passwords and relations - if row.type isnt 'password' && row.name.substr(row.name.length-4,4) isnt '_ids' && row.searchable isnt false - config = _.clone(row) - for operatorRegEx, operator of operators_type - myRegExp = new RegExp(operatorRegEx, 'i') - if config.tag && config.tag.match(myRegExp) - config.operator = operator - elements["#{groupKey}.#{config.name}"] = config - for operatorRegEx, operator of operators_name - myRegExp = new RegExp(operatorRegEx, 'i') - if config.name && config.name.match(myRegExp) - config.operator = operator - elements["#{groupKey}.#{config.name}"] = config + else + for row in App[groupMeta.model].configure_attributes - if config.tag == 'select' - config.multiple = true + # ignore passwords and relations + if row.type isnt 'password' && row.name.substr(row.name.length-4,4) isnt '_ids' && row.searchable isnt false + config = _.clone(row) + for operatorRegEx, operator of operators_type + myRegExp = new RegExp(operatorRegEx, 'i') + if config.tag && config.tag.match(myRegExp) + config.operator = operator + elements["#{groupKey}.#{config.name}"] = config + for operatorRegEx, operator of operators_name + myRegExp = new RegExp(operatorRegEx, 'i') + if config.name && config.name.match(myRegExp) + config.operator = operator + elements["#{groupKey}.#{config.name}"] = config + + if config.tag == 'select' + config.multiple = true if attribute.out_of_office elements['ticket.out_of_office_replacement_id'] = diff --git a/app/assets/javascripts/app/models/job.coffee b/app/assets/javascripts/app/models/job.coffee index 6107a1073..abdee338f 100644 --- a/app/assets/javascripts/app/models/job.coffee +++ b/app/assets/javascripts/app/models/job.coffee @@ -5,7 +5,7 @@ class App.Job extends App.Model @configure_attributes = [ { name: 'name', display: 'Name', tag: 'input', type: 'text', limit: 100, null: false }, { name: 'timeplan', display: 'When should the job run?', tag: 'timer', null: true }, - { name: 'condition', display: 'Conditions for effected objects', tag: 'ticket_selector', null: true }, + { name: 'condition', display: 'Conditions for effected objects', tag: 'ticket_selector', null: true, executionTime: true }, { name: 'perform', display: 'Execute changes on objects', tag: 'ticket_perform_action', null: true, notification: true, ticket_delete: true }, { name: 'disable_notification', display: 'Disable Notifications', tag: 'boolean', default: true }, { name: 'note', display: 'Note', tag: 'textarea', note: 'Notes are visible to agents only, never to customers.', limit: 250, null: true }, diff --git a/app/assets/javascripts/app/models/trigger.coffee b/app/assets/javascripts/app/models/trigger.coffee index d270db004..728a51fba 100644 --- a/app/assets/javascripts/app/models/trigger.coffee +++ b/app/assets/javascripts/app/models/trigger.coffee @@ -4,7 +4,7 @@ class App.Trigger extends App.Model @url: @apiPath + '/triggers' @configure_attributes = [ { name: 'name', display: 'Name', tag: 'input', type: 'text', limit: 100, null: false }, - { name: 'condition', display: 'Conditions for effected objects', tag: 'ticket_selector', null: false, preview: false, action: true, hasChanged: true }, + { name: 'condition', display: 'Conditions for effected objects', tag: 'ticket_selector', null: false, preview: false, action: true, hasChanged: true, executionTime: true }, { name: 'perform', display: 'Execute changes on objects', tag: 'ticket_perform_action', null: true, notification: true, trigger: true }, { name: 'active', display: 'Active', tag: 'active', default: true }, { name: 'updated_at', display: 'Updated', tag: 'datetime', readonly: 1 }, diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 7abc93768..b39092f7d 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -498,7 +498,7 @@ class TicketsController < ApplicationController def selector permission_check('admin.*') - ticket_count, tickets = Ticket.selectors(params[:condition], limit: 6) + ticket_count, tickets = Ticket.selectors(params[:condition], limit: 6, execution_time: true) assets = {} ticket_ids = [] diff --git a/app/models/application_model/can_assets.rb b/app/models/application_model/can_assets.rb index c9ca50a17..b5dee446a 100644 --- a/app/models/application_model/can_assets.rb +++ b/app/models/application_model/can_assets.rb @@ -68,6 +68,7 @@ get assets and record_ids of selector attribute_class = attribute[0].to_classname.constantize rescue => e next if attribute[0] == 'article' + next if attribute[0] == 'execution_time' logger.error "Unable to get asset for '#{attribute[0]}': #{e.inspect}" next diff --git a/app/models/calendar.rb b/app/models/calendar.rb index da82df7b7..31f130060 100644 --- a/app/models/calendar.rb +++ b/app/models/calendar.rb @@ -327,6 +327,21 @@ returns holidays end + def biz + Biz::Schedule.new do |config| + + # get business hours + hours = business_hours_to_hash + raise "No configured hours found in calendar #{inspect}" if hours.blank? + + config.hours = hours + + # get holidays + config.holidays = public_holidays_to_array + config.time_zone = timezone + end + end + private # if changed calendar is default, set all others default to false diff --git a/app/models/concerns/checks_condition_validation.rb b/app/models/concerns/checks_condition_validation.rb index 4b6ccde16..d88844166 100644 --- a/app/models/concerns/checks_condition_validation.rb +++ b/app/models/concerns/checks_condition_validation.rb @@ -13,6 +13,7 @@ module ChecksConditionValidation # check if a valid condition got inserted. validate_condition.delete('ticket.action') + validate_condition.delete('execution_time.calendar_id') validate_condition.each do |key, value| next if !value next if !value['operator'] diff --git a/app/models/job.rb b/app/models/job.rb index 08553c85a..c4dd51812 100644 --- a/app/models/job.rb +++ b/app/models/job.rb @@ -74,7 +74,7 @@ job.run(true) return end - ticket_count, tickets = Ticket.selectors(condition, limit: 2_000) + ticket_count, tickets = Ticket.selectors(condition, limit: 2_000, execution_time: true) logger.debug { "Job #{name} with #{ticket_count} tickets" } @@ -140,7 +140,7 @@ job.run(true) end def matching_count - ticket_count, _tickets = Ticket.selectors(condition, limit: 1) + ticket_count, _tickets = Ticket.selectors(condition, limit: 1, execution_time: true) ticket_count || 0 end diff --git a/app/models/job/assets.rb b/app/models/job/assets.rb index 9852f8eca..121e30720 100644 --- a/app/models/job/assets.rb +++ b/app/models/job/assets.rb @@ -23,21 +23,22 @@ returns =end def assets(data) - app_model = Job.to_app_model - if !data[ app_model ] - data[ app_model ] = {} - end + data[ app_model ] ||= {} return data if data[ app_model ][ id ] data[ app_model ][ id ] = attributes_with_association_ids data = assets_of_selector('condition', data) data = assets_of_selector('perform', data) - if !data[ User.to_app_model ] - data[ User.to_app_model ] = {} + app_model_calendar = Calendar.to_app_model + data[ app_model_calendar ] ||= {} + Calendar.find_each do |calendar| + data = calendar.assets(data) end + + data[ User.to_app_model ] ||= {} %w[created_by_id updated_by_id].each do |local_user_id| next if !self[ local_user_id ] next if data[ User.to_app_model ][ self[ local_user_id ] ] diff --git a/app/models/ticket.rb b/app/models/ticket.rb index fda7cfc83..5beb9cc4d 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -434,7 +434,7 @@ get count of tickets and tickets which match on selector access = options[:access] || 'full' raise 'no selectors given' if !selectors - query, bind_params, tables = selector2sql(selectors, current_user: current_user) + query, bind_params, tables = selector2sql(selectors, current_user: current_user, execution_time: options[:execution_time]) return [] if !query ActiveRecord::Base.transaction(requires_new: true) do @@ -528,6 +528,7 @@ condition example selector = attribute.split(/\./) next if !selector[1] next if selector[0] == 'ticket' + next if selector[0] == 'execution_time' next if tables.include?(selector[0]) if query != '' @@ -554,6 +555,7 @@ condition example end # add conditions + no_result = false selectors.each do |attribute, selector_raw| # validation @@ -562,7 +564,7 @@ condition example selector = selector_raw.stringify_keys raise "Invalid selector, operator missing #{selector.inspect}" if !selector['operator'] - raise "Invalid selector, operator #{selector['operator']} is invalid #{selector.inspect}" if !selector['operator'].match?(/^(is|is\snot|contains|contains\s(not|all|one|all\snot|one\snot)|(after|before)\s\(absolute\)|(within\snext|within\slast|after|before)\s\(relative\))$/) + raise "Invalid selector, operator #{selector['operator']} is invalid #{selector.inspect}" if !selector['operator'].match?(/^(is|is\snot|contains|contains\s(not|all|one|all\snot|one\snot)|(after|before)\s\(absolute\)|(within\snext|within\slast|after|before)\s\(relative\))|(is\sin\sworking\stime|is\snot\sin\sworking\stime)$/) # validate value / allow blank but only if pre_condition exists and is not specific if !selector.key?('value') || @@ -826,11 +828,24 @@ condition example raise "Unknown selector attributes '#{selector.inspect}'" end bind_params.push time + elsif selector['operator'].include?('in working time') + next if attributes[1] != 'calendar_id' + raise 'Please enable execution_time feature to use it (currently only allowed for triggers and schedulers)' if !options[:execution_time] + + biz = Calendar.lookup(id: selector['value'])&.biz + next if biz.blank? + + if ( selector['operator'] == 'is in working time' && !biz.in_hours?(Time.zone.now) ) || ( selector['operator'] == 'is not in working time' && biz.in_hours?(Time.zone.now) ) + no_result = true + break + end else raise "Invalid operator '#{selector['operator']}' for '#{selector['value'].inspect}'" end end + return if no_result + [query, bind_params, tables] end @@ -1106,7 +1121,7 @@ perform active triggers on ticket end # verify is condition is matching - ticket_count, tickets = Ticket.selectors(condition, limit: 1) + ticket_count, tickets = Ticket.selectors(condition, limit: 1, execution_time: true) next if ticket_count.blank? next if ticket_count.zero? diff --git a/app/models/ticket/escalation.rb b/app/models/ticket/escalation.rb index 3a834fd20..454b4f2f3 100644 --- a/app/models/ticket/escalation.rb +++ b/app/models/ticket/escalation.rb @@ -154,18 +154,7 @@ returns self.update_escalation_at = nil self.close_escalation_at = nil end - biz = Biz::Schedule.new do |config| - - # get business hours - hours = calendar.business_hours_to_hash - raise "No configured hours found in calendar #{calendar.inspect}" if hours.blank? - - config.hours = hours - - # get holidays - config.holidays = calendar.public_holidays_to_array - config.time_zone = calendar.timezone - end + biz = calendar.biz # get history data history_data = nil diff --git a/app/models/trigger/assets.rb b/app/models/trigger/assets.rb index 389f9aebc..4b9a21fd9 100644 --- a/app/models/trigger/assets.rb +++ b/app/models/trigger/assets.rb @@ -25,21 +25,23 @@ returns def assets(data) app_model_trigger = Trigger.to_app_model + data[ app_model_trigger ] ||= {} - if !data[ app_model_trigger ] - data[ app_model_trigger ] = {} - end return data if data[ app_model_trigger ][ id ] data[ app_model_trigger ][ id ] = attributes_with_association_ids data = assets_of_selector('condition', data) data = assets_of_selector('perform', data) - app_model_user = User.to_app_model - if !data[ app_model_user ] - data[ app_model_user ] = {} + app_model_calendar = Calendar.to_app_model + data[ app_model_calendar ] ||= {} + Calendar.find_each do |calendar| + data = calendar.assets(data) end + app_model_user = User.to_app_model + data[ app_model_user ] ||= {} + %w[created_by_id updated_by_id].each do |local_user_id| next if !self[ local_user_id ] next if data[ app_model_user ][ self[ local_user_id ] ] diff --git a/spec/models/trigger_spec.rb b/spec/models/trigger_spec.rb index 0bf0368ec..1d08dbf7e 100644 --- a/spec/models/trigger_spec.rb +++ b/spec/models/trigger_spec.rb @@ -156,5 +156,47 @@ RSpec.describe Trigger, type: :model do end end end + + context 'with condition execution_time.calendar_id' do + let(:calendar) { create(:calendar) } + let(:perform) do + { 'ticket.title'=>{ 'value'=>'triggered' } } + end + let!(:ticket) { create(:ticket, title: 'Test Ticket') } + + context 'is in working time' do + let(:condition) do + { 'execution_time.calendar_id' => { 'operator' => 'is in working time', 'value' => calendar.id } } + end + + it 'does trigger only in working time' do + travel_to Time.zone.parse('2020-02-12T12:00:00Z0') + expect { Observer::Transaction.commit }.to change { ticket.reload.title }.to('triggered') + end + + it 'does not trigger out of working time' do + travel_to Time.zone.parse('2020-02-12T02:00:00Z0') + Observer::Transaction.commit + expect(ticket.reload.title).to eq('Test Ticket') + end + end + + context 'is not in working time' do + let(:condition) do + { 'execution_time.calendar_id' => { 'operator' => 'is not in working time', 'value' => calendar.id } } + end + + it 'does not trigger in working time' do + travel_to Time.zone.parse('2020-02-12T12:00:00Z0') + Observer::Transaction.commit + expect(ticket.reload.title).to eq('Test Ticket') + end + + it 'does trigger out of working time' do + travel_to Time.zone.parse('2020-02-12T02:00:00Z0') + expect { Observer::Transaction.commit }.to change { ticket.reload.title }.to('triggered') + end + end + end end end