From 9f559276e4f2610b00286d628b1ef6acc80965eb Mon Sep 17 00:00:00 2001 From: Muhammad Nuzaihan Date: Wed, 20 Jun 2018 20:13:35 +0800 Subject: [PATCH] improve email filters by adding a tag option - closes #1991 --- Gemfile.lock | 4 +- .../_ui_element/postmaster_set.coffee | 55 ++- .../app/controllers/object_manager.coffee | 5 + .../app/views/generic/postmaster_set.jst.eco | 8 +- .../app/views/object_manager/index.jst.eco | 8 +- app/assets/stylesheets/zammad.scss | 5 + .../object_manager_attributes_controller.rb | 3 + app/models/channel/email_parser.rb | 10 + app/models/channel/filter/database.rb | 23 + app/models/object_manager/attribute.rb | 126 +++++ app/models/ticket.rb | 4 +- public/assets/tests/form.js | 49 ++ test/browser/admin_object_manager_test.rb | 95 ++++ test/browser_test_helper.rb | 23 +- ...ject_manager_attributes_controller_test.rb | 464 +++++++++++++++++- test/integration/object_manager_test.rb | 74 +++ test/unit/email_postmaster_test.rb | 102 ++++ test/unit/ticket_escalation_test.rb | 18 +- 18 files changed, 1050 insertions(+), 26 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 34e57db2f..2a389b00c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -306,7 +306,7 @@ GEM slop (~> 3.0) public_suffix (3.0.1) puma (3.11.0) - rack (2.0.4) + rack (2.0.5) rack-livereload (0.3.16) rack rack-test (1.0.0) @@ -405,7 +405,7 @@ GEM simplecov (>= 0.4.1) slack-notifier (2.3.1) slop (3.6.0) - sprockets (3.7.1) + sprockets (3.7.2) concurrent-ruby (~> 1.0) rack (> 1, < 3) sprockets-rails (3.2.1) diff --git a/app/assets/javascripts/app/controllers/_ui_element/postmaster_set.coffee b/app/assets/javascripts/app/controllers/_ui_element/postmaster_set.coffee index 049bc2735..25e29568a 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/postmaster_set.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/postmaster_set.coffee @@ -4,6 +4,7 @@ class App.UiElement.postmaster_set groups = ticket: name: 'Ticket' + model: 'Ticket' options: [ { value: 'priority_id' @@ -15,6 +16,11 @@ class App.UiElement.postmaster_set name: 'State' relation: 'TicketState' } + { + value: 'tags' + name: 'Tag' + tag: 'tag' + } { value: 'customer_id' name: 'Customer' @@ -64,6 +70,24 @@ class App.UiElement.postmaster_set } ] + elements = {} + for groupKey, groupMeta of groups + if !App[groupMeta.model] + elements["#{groupKey}.email"] = { name: 'email', display: 'Email' } + else + + for row in App[groupMeta.model].configure_attributes + + # ignore passwords and relations + if row.type isnt 'password' && row.name.substr(row.name.length-4,4) isnt '_ids' + + # ignore readonly attributes + if !row.readonly + config = _.clone(row) + if config.tag is 'tag' + config.operator = ['add', 'remove'] + elements["x-zammad-ticket-#{config.name}"] = config + # add additional ticket attributes for row in App.Ticket.configure_attributes exists = false @@ -91,11 +115,11 @@ class App.UiElement.postmaster_set for item in groups.ticket.options item.value = "x-zammad-ticket-#{item.value}" - groups + [elements, groups] @render: (attribute, params = {}) -> - groups = @defaults() + [elements, groups] = @defaults() selector = @buildAttributeSelector(groups, attribute) @@ -121,7 +145,9 @@ class App.UiElement.postmaster_set item.find('.js-attributeSelector select').bind('change', (e) => key = $(e.target).find('option:selected').attr('value') elementRow = $(e.target).closest('.js-filterElement') + groupAndAttribute = elementRow.find('.js-attributeSelector option:selected').attr('value') @rebuildAttributeSelectors(item, elementRow, key, attribute) + @buildOperator(item, elementRow, groupAndAttribute, elements, {}, attribute) @buildValue(item, elementRow, key, groups, undefined, undefined, attribute) ) @@ -203,3 +229,28 @@ class App.UiElement.postmaster_set if key elementRow.find('.js-attributeSelector select').val(key) + @buildOperator: (elementFull, elementRow, groupAndAttribute, elements, meta, attribute) -> + currentOperator = elementRow.find('.js-operator option:selected').attr('value') + + if !meta.operator + meta.operator = currentOperator + + name = "#{attribute.name}::#{groupAndAttribute}::operator" + + selection = $("") + attributeConfig = elements[groupAndAttribute] + + if !attributeConfig.operator + elementRow.find('.js-operator').addClass('hide') + else + elementRow.find('.js-operator').removeClass('hide') + if attributeConfig.operator + for operator in attributeConfig.operator + operatorName = App.i18n.translateInline(operator) + selected = '' + if meta.operator is operator + selected = 'selected="selected"' + selection.append("") + selection + + elementRow.find('.js-operator select').replaceWith(selection) diff --git a/app/assets/javascripts/app/controllers/object_manager.coffee b/app/assets/javascripts/app/controllers/object_manager.coffee index e63bf8830..f489f3ffc 100644 --- a/app/assets/javascripts/app/controllers/object_manager.coffee +++ b/app/assets/javascripts/app/controllers/object_manager.coffee @@ -143,12 +143,17 @@ class Items extends App.ControllerSubContent e.preventDefault() id = $(e.target).closest('tr').data('id') item = App.ObjectManagerAttribute.find(id) + ui = @ @ajax( id: "object_manager_attributes/#{id}" type: 'DELETE' url: "#{@apiPath}/object_manager_attributes/#{id}" success: (data) => @render() + error: (jqXHR, textStatus, errorThrown) -> + ui.log 'errors' + # this code is unreachable so alert will do fine + alert(jqXHR.responseJSON.error) ) discard: (e) -> diff --git a/app/assets/javascripts/app/views/generic/postmaster_set.jst.eco b/app/assets/javascripts/app/views/generic/postmaster_set.jst.eco index 17e5c88e0..32f35019b 100644 --- a/app/assets/javascripts/app/views/generic/postmaster_set.jst.eco +++ b/app/assets/javascripts/app/views/generic/postmaster_set.jst.eco @@ -6,6 +6,12 @@ <%- @Icon('arrow-down', 'dropdown-arrow') %> +
+
+ + <%- @Icon('arrow-down') %> +
+
@@ -17,4 +23,4 @@
- \ No newline at end of file + diff --git a/app/assets/javascripts/app/views/object_manager/index.jst.eco b/app/assets/javascripts/app/views/object_manager/index.jst.eco index 957782dd0..b984dc0c0 100644 --- a/app/assets/javascripts/app/views/object_manager/index.jst.eco +++ b/app/assets/javascripts/app/views/object_manager/index.jst.eco @@ -71,8 +71,12 @@ <%- @T('will be deleted') %> <% else if item.to_migrate is true || item.to_config is true: %> <%- @T('has changed') %> - <% else if item.editable isnt false: %> - <%- @Icon('trash') %> + <% else if item.editable: %> + <% if item.deletable: %> + <%- @Icon('trash') %> + <% else: %> + <%- @Icon('trash') %> + <% end %> <% end %> diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 013a61106..8e1cd1a4c 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -9610,3 +9610,8 @@ body.fit { .flex-spacer { flex: 1; } + +span.is-disabled { + cursor: not-allowed; + opacity: 0.5; +} diff --git a/app/controllers/object_manager_attributes_controller.rb b/app/controllers/object_manager_attributes_controller.rb index 02f702129..62c8346ab 100644 --- a/app/controllers/object_manager_attributes_controller.rb +++ b/app/controllers/object_manager_attributes_controller.rb @@ -77,6 +77,9 @@ class ObjectManagerAttributesController < ApplicationController name: object_manager_attribute.name, ) model_destroy_render_item + rescue => e + logger.error e + raise Exceptions::UnprocessableEntity, e end # POST /object_manager_attributes_discard_changes diff --git a/app/models/channel/email_parser.rb b/app/models/channel/email_parser.rb index 73590e1a9..ad91a04ee 100644 --- a/app/models/channel/email_parser.rb +++ b/app/models/channel/email_parser.rb @@ -238,6 +238,14 @@ returns # create ticket ticket.save! + + end + + # apply tags to ticket + if mail['x-zammad-ticket-tags'.to_sym].present? + mail['x-zammad-ticket-tags'.to_sym].each do |tag| + ticket.tag_add(tag) + end end # set attributes @@ -309,6 +317,8 @@ returns def self.check_attributes_by_x_headers(header_name, value) class_name = nil attribute = nil + # skip check attributes if it is tags + return true if header_name == 'x-zammad-ticket-tags' if header_name =~ /^x-zammad-(.+?)-(followup-|)(.*)$/i class_name = $1 attribute = $3 diff --git a/app/models/channel/filter/database.rb b/app/models/channel/filter/database.rb index 8161da2ee..3dba8f2c3 100644 --- a/app/models/channel/filter/database.rb +++ b/app/models/channel/filter/database.rb @@ -45,6 +45,29 @@ module Channel::Filter::Database filter[:perform].each do |key, meta| next if !Channel::EmailParser.check_attributes_by_x_headers(key, meta['value']) Rails.logger.info " perform '#{key.downcase}' = '#{meta.inspect}'" + + if key.downcase == 'x-zammad-ticket-tags' && meta['value'].present? && meta['operator'].present? + mail[ 'x-zammad-ticket-tags'.downcase.to_sym ] ||= [] + tags = meta['value'].split(',') + + case meta['operator'] + when 'add' + tags.each do |tag| + next if tag.blank? + tag.strip! + next if mail[ 'x-zammad-ticket-tags'.downcase.to_sym ].include?(tag) + mail[ 'x-zammad-ticket-tags'.downcase.to_sym ].push tag + end + when 'remove' + tags.each do |tag| + next if tag.blank? + tag.strip! + mail[ 'x-zammad-ticket-tags'.downcase.to_sym ] -= [tag] + end + end + next + end + mail[ key.downcase.to_sym ] = meta['value'] end end diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 5ed09f906..b9fec4173 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -29,12 +29,25 @@ list of all attributes def self.list_full result = ObjectManager::Attribute.all.order('position ASC, name ASC') + references = ObjectManager::Attribute.attribute_to_references_hash attributes = [] assets = {} result.each do |item| attribute = item.attributes attribute[:object] = ObjectLookup.by_id(item.object_lookup_id) attribute.delete('object_lookup_id') + + # an attribute is deletable if it is both editable and not referenced by other Objects (Triggers, Overviews, Schedulers) + deletable = true + not_deletable_reason = '' + if ObjectManager::Attribute.attribute_used_by_references?(attribute[:object], attribute['name'], references) + deletable = false + not_deletable_reason = ObjectManager::Attribute.attribute_used_by_references_humaniced(attribute[:object], attribute['name'], references) + end + attribute[:deletable] = attribute['editable'] && deletable == true + if not_deletable_reason.present? + attribute[:not_deletable_reason] = "This attribute is referenced by #{not_deletable_reason} and thus cannot be deleted!" + end attributes.push attribute end attributes @@ -354,6 +367,10 @@ use "force: true" to delete also not editable fields # lookups if data[:object] data[:object_lookup_id] = ObjectLookup.by_name(data[:object]) + elsif data[:object_lookup_id] + data[:object] = ObjectLookup.by_id(data[:object_lookup_id]) + else + raise 'ERROR: need object or object_lookup_id param!' end data[:name].downcase! @@ -371,6 +388,12 @@ use "force: true" to delete also not editable fields raise "ERROR: #{data[:object]}.#{data[:name]} can't be removed!" end + # check to make sure that no triggers, overviews, or schedulers references this attribute + if ObjectManager::Attribute.attribute_used_by_references?(data[:object], data[:name]) + text = ObjectManager::Attribute.attribute_used_by_references_humaniced(data[:object], data[:name]) + raise "ERROR: #{data[:object]}.#{data[:name]} is referenced by #{text} and thus cannot be deleted!" + end + # if record is to create, just destroy it if record.to_create record.destroy @@ -721,6 +744,109 @@ to send no browser reload event, pass false true end +=begin + +where attributes are used by triggers, overviews or schedulers + + result = ObjectManager::Attribute.attribute_to_references_hash + + result = { + ticket.category: { + Trigger: ['abc', 'xyz'], + Overview: ['abc1', 'abc2'], + }, + ticket.field_b: { + Trigger: ['abc'], + Overview: ['abc1', 'abc2'], + }, + }, + +=end + + def self.attribute_to_references_hash + objects = Trigger.select(:name, :condition) + Overview.select(:name, :condition) + Job.select(:name, :condition) + attribute_list = {} + objects.each do |item| + item.condition.each do |condition_key, _condition_attributes| + attribute_list[condition_key] ||= {} + attribute_list[condition_key][item.class.name] ||= [] + next if attribute_list[condition_key][item.class.name].include?(item.name) + attribute_list[condition_key][item.class.name].push item.name + end + end + attribute_list + end + +=begin + +is certain attribute used by triggers, overviews or schedulers + + ObjectManager::Attribute.attribute_used_by_references?('Ticket', 'attribute_name') + +=end + + def self.attribute_used_by_references?(object_name, attribute_name, references = attribute_to_references_hash) + references.each do |reference_key, _relations| + local_object, local_attribute = reference_key.split('.') + next if local_object != object_name.downcase + next if local_attribute != attribute_name + return true + end + false + end + +=begin + +is certain attribute used by triggers, overviews or schedulers + + result = ObjectManager::Attribute.attribute_used_by_references('Ticket', 'attribute_name') + + result = { + Trigger: ['abc', 'xyz'], + Overview: ['abc1', 'abc2'], + } + +=end + + def self.attribute_used_by_references(object_name, attribute_name, references = attribute_to_references_hash) + result = {} + references.each do |reference_key, relations| + local_object, local_attribute = reference_key.split('.') + next if local_object != object_name.downcase + next if local_attribute != attribute_name + relations.each do |relation, relation_names| + result[relation] ||= [] + result[relation].push relation_names.sort + end + break + end + result + end + +=begin + +is certain attribute used by triggers, overviews or schedulers + + text = ObjectManager::Attribute.attribute_used_by_references_humaniced('Ticket', 'attribute_name', references) + +=end + + def self.attribute_used_by_references_humaniced(object_name, attribute_name, references = nil) + result = if references.present? + ObjectManager::Attribute.attribute_used_by_references(object_name, attribute_name, references) + else + ObjectManager::Attribute.attribute_used_by_references(object_name, attribute_name) + end + not_deletable_reason = '' + result.each do |relation, relation_names| + if not_deletable_reason.present? + not_deletable_reason += '; ' + end + not_deletable_reason += "#{relation}: #{relation_names.sort.join(',')}" + end + not_deletable_reason + end + def self.reset_database_info(model) model.connection.schema_cache.clear! model.reset_column_information diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 49c1179a9..66a7fad0d 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -557,11 +557,11 @@ condition example # get attributes attributes = attribute.split(/\./) - attribute = "#{attributes[0]}s.#{attributes[1]}" + attribute = "#{ActiveRecord::Base.connection.quote_table_name("#{attributes[0]}s")}.#{ActiveRecord::Base.connection.quote_column_name(attributes[1])}" # magic selectors if attributes[0] == 'ticket' && attributes[1] == 'out_of_office_replacement_id' - attribute = "#{attributes[0]}s.owner_id" + attribute = "#{ActiveRecord::Base.connection.quote_table_name("#{attributes[0]}s")}.#{ActiveRecord::Base.connection.quote_column_name('owner_id')}" end if attributes[0] == 'ticket' && attributes[1] == 'tags' diff --git a/public/assets/tests/form.js b/public/assets/tests/form.js index 4271c4e6f..45b392af2 100644 --- a/public/assets/tests/form.js +++ b/public/assets/tests/form.js @@ -769,6 +769,10 @@ test("form postmaster filter", function() { }, 'x-zammad-ticket-priority_id': { value: '1' + }, + 'x-zammad-ticket-tags': { + operator: 'add', + value: 'test, test1' } }, } @@ -812,6 +816,10 @@ test("form postmaster filter", function() { }, 'x-zammad-ticket-priority_id': { value: '1' + }, + 'x-zammad-ticket-tags': { + operator: 'add', + value: 'test, test1' } }, }; @@ -842,6 +850,10 @@ test("form postmaster filter", function() { 'x-zammad-ticket-group_id': { value: '1' }, + 'x-zammad-ticket-tags': { + operator: 'add', + value: "test, test1" + }, }, }; deepEqual(params, test_params, 'form param check') @@ -849,6 +861,43 @@ test("form postmaster filter", function() { }, 1000 ); + App.Delay.set(function() { + el.find('.postmaster_set .js-filterElement').last().find('.filter-controls .js-add').click() + test("form param check click add after tag option", function() { + params = App.ControllerForm.params(el) + test_params = { + input1: 'some not used default', + input2: 'some name', + match: { + from: { + operator: 'contains', + value: 'some@address' + }, + subject: { + operator: 'contains', + value: 'some subject' + } + }, + set: { + 'x-zammad-ticket-owner_id': { + value: 'owner', + value_completion: '' + }, + 'x-zammad-ticket-group_id': { + value: '1' + }, + 'x-zammad-ticket-tags': { + operator: 'add', + value: "test, test1" + }, + }, + }; + deepEqual(params, test_params, 'form param check') + }); + }, + 1000 + ); + }); diff --git a/test/browser/admin_object_manager_test.rb b/test/browser/admin_object_manager_test.rb index d54222884..2f8c7ddcc 100644 --- a/test/browser/admin_object_manager_test.rb +++ b/test/browser/admin_object_manager_test.rb @@ -499,4 +499,99 @@ class AdminObjectManagerTest < TestCase end + def test_that_attributes_with_references_should_have_a_disabled_delete_button + @browser = instance = browser_instance + login( + username: 'master@example.com', + password: 'test', + url: browser_url, + ) + + tasks_close_all() + + # create two new attributes + object_manager_attribute_create( + data: { + name: 'deletable_attribute', + display: 'Deletable Attribute', + data_type: 'Text', + }, + ) + + object_manager_attribute_create( + data: { + name: 'undeletable_attribute', + display: 'Undeletable Attribute', + data_type: 'Text', + }, + ) + + watch_for( + css: '.content.active', + value: 'Database Update required', + ) + click(css: '.content.active .tab-pane.active div.js-execute') + watch_for( + css: '.modal', + value: 'restart', + ) + watch_for_disappear( + css: '.modal', + timeout: 120, + ) + sleep 5 + watch_for( + css: '.content.active', + ) + match_not( + css: '.content.active', + value: 'Database Update required', + ) + + # create a new overview that references the undeletable_attribute + overview_create( + browser: instance, + data: { + name: 'test_overview', + roles: ['Agent'], + selector: { + 'Undeletable Attribute' => 'DUMMY', + }, + 'order::direction' => 'down', + 'text_input' => true, + } + ) + click( + browser: instance, + css: 'a[href="#manage"]', + mute_log: true, + ) + click( + browser: instance, + css: '.content.active a[href="#system/object_manager"]', + mute_log: true, + ) + + 30.times do + deletable_attribute = instance.find_elements(xpath: '//td[text()="deletable_attribute"]/following-sibling::*[2]')[0] + break if deletable_attribute + sleep 1 + end + + sleep 1 + deletable_attribute = instance.find_elements(xpath: '//td[text()="deletable_attribute"]/following-sibling::*[2]')[0] + assert_not_nil(deletable_attribute) + deletable_attribute_html = deletable_attribute.attribute('innerHTML') + assert(deletable_attribute_html.include?('title="Delete"')) + assert(deletable_attribute_html.include?('href="#"')) + assert(deletable_attribute_html.exclude?('cannot be deleted')) + + undeletable_attribute = instance.find_elements(xpath: '//td[text()="undeletable_attribute"]/following-sibling::*[2]')[0] + assert_not_nil(undeletable_attribute) + undeletable_attribute_html = undeletable_attribute.attribute('innerHTML') + assert(undeletable_attribute_html.include?('Overview')) + assert(undeletable_attribute_html.include?('test_overview')) + assert(undeletable_attribute_html.include?('cannot be deleted')) + assert(undeletable_attribute_html.exclude?('href="#"')) + end end diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index 9b271e77b..e531688f0 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -1659,13 +1659,22 @@ wait untill text in selector disabppears mute_log: true, ) sleep 0.5 - select( - browser: instance, - css: '.modal .ticket_selector .js-value select', - value: value, - deselect_all: true, - mute_log: true, - ) + if data.key?('text_input') + set( + browser: instance, + css: '.modal .ticket_selector .js-value input', + value: value, + mute_log: true, + ) + else + select( + browser: instance, + css: '.modal .ticket_selector .js-value select', + value: value, + deselect_all: true, + mute_log: true, + ) + end end if data['order::direction'] diff --git a/test/integration/object_manager_attributes_controller_test.rb b/test/integration/object_manager_attributes_controller_test.rb index f209c6684..5f137b113 100644 --- a/test/integration/object_manager_attributes_controller_test.rb +++ b/test/integration/object_manager_attributes_controller_test.rb @@ -536,7 +536,6 @@ class ObjectManagerAttributesControllerTest < ActionDispatch::IntegrationTest } } }, - 'id': 'c-202' } post '/api/v1/object_manager_attributes', params: params.to_json, headers: @headers.merge('Authorization' => credentials) @@ -553,4 +552,467 @@ class ObjectManagerAttributesControllerTest < ActionDispatch::IntegrationTest assert_equal(result['data_type'], 'boolean') end + test '03 ticket attributes cannot be removed when it is referenced by an overview' do + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin@example.com', 'adminpw') + + # 1. create a new ticket attribute and execute migration + migration = ObjectManager::Attribute.migration_execute + + params = { + 'name': 'test_attribute_referenced_by_an_overview', + 'object': 'Ticket', + 'display': 'Test Attribute', + 'active': true, + 'data_type': 'input', + 'data_option': { + 'default': '', + 'type': 'text', + 'maxlength': 120, + 'null': true, + 'options': {}, + 'relation': '' + }, + 'screens': { + 'create_middle': { + 'ticket.customer': { + 'shown': true, + 'item_class': 'column' + }, + 'ticket.agent': { + 'shown': true, + 'item_class': 'column' + } + }, + 'edit': { + 'ticket.customer': { + 'shown': true + }, + 'ticket.agent': { + 'shown': true + } + } + }, + } + + post '/api/v1/object_manager_attributes', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + + migration = ObjectManager::Attribute.migration_execute + assert_equal(migration, true) + + # 2. create an overview that uses the attribute + params = { + name: 'test_overview', + roles: Role.where(name: 'Agent').pluck(:name), + condition: { + 'ticket.state_id': { + 'operator': 'is', + 'value': Ticket::State.all.pluck(:id), + }, + 'ticket.test_attribute_referenced_by_an_overview': { + 'operator': 'contains', + 'value': 'DUMMY' + }, + }, + order: { + by: 'created_at', + direction: 'DESC', + }, + view: { + d: %w[title customer state created_at], + s: %w[number title customer state created_at], + m: %w[number title customer state created_at], + view_mode_default: 's', + }, + user_ids: [ '1' ], + } + + if Overview.where('name like ?', '%test%').empty? + post '/api/v1/overviews', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + assert_response(201) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('test_overview', result['name']) + end + + # 3. attempt to delete the ticket attribute + get '/api/v1/object_manager_attributes', headers: @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + target_attribute = result.select { |x| x['name'] == 'test_attribute_referenced_by_an_overview' && x['object'] == 'Ticket' } + assert_equal target_attribute.size, 1 + target_id = target_attribute[0]['id'] + + delete "/api/v1/object_manager_attributes/#{target_id}", headers: @headers.merge('Authorization' => credentials) + assert_response(422) + assert @response.body.include?('Overview') + assert @response.body.include?('test_overview') + assert @response.body.include?('cannot be deleted!') + end + + test '04 ticket attributes cannot be removed when it is referenced by a trigger' do + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin@example.com', 'adminpw') + + # 1. create a new ticket attribute and execute migration + migration = ObjectManager::Attribute.migration_execute + + params = { + 'name': 'test_attribute_referenced_by_a_trigger', + 'object': 'Ticket', + 'display': 'Test Attribute', + 'active': true, + 'data_type': 'input', + 'data_option': { + 'default': '', + 'type': 'text', + 'maxlength': 120, + 'null': true, + 'options': {}, + 'relation': '' + }, + 'screens': { + 'create_middle': { + 'ticket.customer': { + 'shown': true, + 'item_class': 'column' + }, + 'ticket.agent': { + 'shown': true, + 'item_class': 'column' + } + }, + 'edit': { + 'ticket.customer': { + 'shown': true + }, + 'ticket.agent': { + 'shown': true + } + } + }, + } + + post '/api/v1/object_manager_attributes', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + + migration = ObjectManager::Attribute.migration_execute + assert_equal(migration, true) + + # 2. create an trigger that uses the attribute + params = { + name: 'test_trigger', + condition: { + 'ticket.test_attribute_referenced_by_a_trigger': { + 'operator': 'contains', + 'value': 'DUMMY' + } + }, + 'perform': { + 'ticket.state_id': { + 'value': '2' + } + }, + 'active': true, + 'id': 'c-3' + } + + if Trigger.where('name like ?', '%test%').empty? + post '/api/v1/triggers', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + assert_response(201) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('test_trigger', result['name']) + end + + # 3. attempt to delete the ticket attribute + get '/api/v1/object_manager_attributes', headers: @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + target_attribute = result.select { |x| x['name'] == 'test_attribute_referenced_by_a_trigger' && x['object'] == 'Ticket' } + assert_equal target_attribute.size, 1 + target_id = target_attribute[0]['id'] + + delete "/api/v1/object_manager_attributes/#{target_id}", headers: @headers.merge('Authorization' => credentials) + assert_response(422) + assert @response.body.include?('Trigger') + assert @response.body.include?('test_trigger') + assert @response.body.include?('cannot be deleted!') + end + + test '05 ticket attributes cannot be removed when it is referenced by a scheduler' do + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin@example.com', 'adminpw') + + # 1. create a new ticket attribute and execute migration + migration = ObjectManager::Attribute.migration_execute + + params = { + 'name': 'test_attribute_referenced_by_a_scheduler', + 'object': 'Ticket', + 'display': 'Test Attribute', + 'active': true, + 'data_type': 'input', + 'data_option': { + 'default': '', + 'type': 'text', + 'maxlength': 120, + 'null': true, + 'options': {}, + 'relation': '' + }, + 'screens': { + 'create_middle': { + 'ticket.customer': { + 'shown': true, + 'item_class': 'column' + }, + 'ticket.agent': { + 'shown': true, + 'item_class': 'column' + } + }, + 'edit': { + 'ticket.customer': { + 'shown': true + }, + 'ticket.agent': { + 'shown': true + } + } + }, + } + + post '/api/v1/object_manager_attributes', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + + migration = ObjectManager::Attribute.migration_execute + assert_equal(migration, true) + + # 2. create a scheduler that uses the attribute + params = { + name: 'test_scheduler', + 'timeplan': { + 'days': { + 'Mon': true, + 'Tue': false, + 'Wed': false, + 'Thu': false, + 'Fri': false, + 'Sat': false, + 'Sun': false + }, + 'hours': { + '0': true, + '1': false, + '2': false, + '3': false, + '4': false, + '5': false, + '6': false, + '7': false, + '8': false, + '9': false, + '10': false, + '11': false, + '12': false, + '13': false, + '14': false, + '15': false, + '16': false, + '17': false, + '18': false, + '19': false, + '20': false, + '21': false, + '22': false, + '23': false + }, + 'minutes': { + '0': true, + '10': false, + '20': false, + '30': false, + '40': false, + '50': false + } + }, + 'condition': { + 'ticket.test_attribute_referenced_by_a_scheduler': { + 'operator': 'contains', + 'value': 'DUMMY' + } + }, + 'perform': { + 'ticket.state_id': { + 'value': '2' + } + }, + 'disable_notification': true, + 'note': '', + 'active': true, + 'id': 'c-0' + } + + if Job.where('name like ?', '%test%').empty? + post '/api/v1/jobs', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + assert_response(201) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('test_scheduler', result['name']) + end + + # 3. attempt to delete the ticket attribute + get '/api/v1/object_manager_attributes', headers: @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + target_attribute = result.select { |x| x['name'] == 'test_attribute_referenced_by_a_scheduler' && x['object'] == 'Ticket' } + assert_equal target_attribute.size, 1 + target_id = target_attribute[0]['id'] + + delete "/api/v1/object_manager_attributes/#{target_id}", headers: @headers.merge('Authorization' => credentials) + assert_response(422) + assert @response.body.include?('Job') + assert @response.body.include?('test_scheduler') + assert @response.body.include?('cannot be deleted!') + end + + test '06 ticket attributes can be removed when it is referenced by an overview but by user object' do + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin@example.com', 'adminpw') + + # 1. create a new ticket attribute and execute migration + migration = ObjectManager::Attribute.migration_execute + + params = { + 'name': 'test_attribute_referenced_by_an_overview', + 'object': 'Ticket', + 'display': 'Test Attribute', + 'active': true, + 'data_type': 'input', + 'data_option': { + 'default': '', + 'type': 'text', + 'maxlength': 120, + 'null': true, + 'options': {}, + 'relation': '' + }, + 'screens': { + 'create_middle': { + 'ticket.customer': { + 'shown': true, + 'item_class': 'column' + }, + 'ticket.agent': { + 'shown': true, + 'item_class': 'column' + } + }, + 'edit': { + 'ticket.customer': { + 'shown': true + }, + 'ticket.agent': { + 'shown': true + } + } + }, + } + + post '/api/v1/object_manager_attributes', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + + params = { + 'name': 'test_attribute_referenced_by_an_overview', + 'object': 'User', + 'display': 'Test Attribute', + 'active': true, + 'data_type': 'input', + 'data_option': { + 'default': '', + 'type': 'text', + 'maxlength': 120, + 'null': true, + 'options': {}, + 'relation': '' + }, + 'screens': { + 'create_middle': { + 'ticket.customer': { + 'shown': true, + 'item_class': 'column' + }, + 'ticket.agent': { + 'shown': true, + 'item_class': 'column' + } + }, + 'edit': { + 'ticket.customer': { + 'shown': true + }, + 'ticket.agent': { + 'shown': true + } + } + }, + } + + post '/api/v1/object_manager_attributes', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + + migration = ObjectManager::Attribute.migration_execute + assert_equal(migration, true) + + # 2. create an overview that uses the attribute + params = { + name: 'test_overview', + roles: Role.where(name: 'Agent').pluck(:name), + condition: { + 'ticket.state_id': { + 'operator': 'is', + 'value': Ticket::State.all.pluck(:id), + }, + 'ticket.test_attribute_referenced_by_an_overview': { + 'operator': 'contains', + 'value': 'DUMMY' + }, + }, + order: { + by: 'created_at', + direction: 'DESC', + }, + view: { + d: %w[title customer state created_at], + s: %w[number title customer state created_at], + m: %w[number title customer state created_at], + view_mode_default: 's', + }, + user_ids: [ '1' ], + } + + if Overview.where('name like ?', '%test%').empty? + post '/api/v1/overviews', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + assert_response(201) + result = JSON.parse(@response.body) + assert_equal(Hash, result.class) + assert_equal('test_overview', result['name']) + end + + # 3. attempt to delete the ticket attribute + get '/api/v1/object_manager_attributes', headers: @headers.merge('Authorization' => credentials) + assert_response(200) + result = JSON.parse(@response.body) + + target_attribute = result.select { |x| x['name'] == 'test_attribute_referenced_by_an_overview' && x['object'] == 'User' } + assert_equal target_attribute.size, 1 + target_id = target_attribute[0]['id'] + + delete "/api/v1/object_manager_attributes/#{target_id}", headers: @headers.merge('Authorization' => credentials) + assert_response(200) + + target_attribute = result.select { |x| x['name'] == 'test_attribute_referenced_by_an_overview' && x['object'] == 'Ticket' } + assert_equal target_attribute.size, 1 + target_id = target_attribute[0]['id'] + + delete "/api/v1/object_manager_attributes/#{target_id}", headers: @headers.merge('Authorization' => credentials) + assert_response(422) + assert @response.body.include?('Overview') + assert @response.body.include?('test_overview') + assert @response.body.include?('cannot be deleted!') + end + end diff --git a/test/integration/object_manager_test.rb b/test/integration/object_manager_test.rb index 75d53be6d..9a6064668 100644 --- a/test/integration/object_manager_test.rb +++ b/test/integration/object_manager_test.rb @@ -663,4 +663,78 @@ class ObjectManagerTest < ActiveSupport::TestCase end + test 'c object manager attribute - certain names' do + + assert_equal(false, ObjectManager::Attribute.pending_migration?) + assert_equal(0, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(0, ObjectManager::Attribute.migrations.count) + + attribute1 = ObjectManager::Attribute.add( + object: 'Ticket', + name: '1_a_anfrage_status', + display: '1_a_anfrage_status', + data_type: 'input', + data_option: { + maxlength: 200, + type: 'text', + null: true, + }, + active: true, + screens: {}, + position: 20, + created_by_id: 1, + updated_by_id: 1, + ) + assert(attribute1) + + assert_equal(true, ObjectManager::Attribute.pending_migration?) + assert_equal(1, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(1, ObjectManager::Attribute.migrations.count) + + # execute migrations + assert(ObjectManager::Attribute.migration_execute) + + assert_equal(false, ObjectManager::Attribute.pending_migration?) + assert_equal(0, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(0, ObjectManager::Attribute.migrations.count) + + # create example ticket + ticket1 = Ticket.create( + title: 'some attribute test3', + group: Group.lookup(name: 'Users'), + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + '1_a_anfrage_status': 'some attribute text', + updated_by_id: 1, + created_by_id: 1, + ) + assert('ticket1 created', ticket1) + + assert_equal('some attribute test3', ticket1.title) + assert_equal('Users', ticket1.group.name) + assert_equal('new', ticket1.state.name) + assert_equal('some attribute text', ticket1['1_a_anfrage_status']) + + condition = { + 'ticket.title' => { + operator: 'is', + value: 'some attribute test3', + }, + } + ticket_count, tickets = Ticket.selectors(condition, 10) + assert_equal(ticket_count, 1) + assert_equal(tickets[0].id, ticket1.id) + + condition = { + 'ticket.1_a_anfrage_status' => { + operator: 'is', + value: 'some attribute text', + }, + } + ticket_count, tickets = Ticket.selectors(condition, 10) + assert_equal(ticket_count, 1) + assert_equal(tickets[0].id, ticket1.id) + + end end diff --git a/test/unit/email_postmaster_test.rb b/test/unit/email_postmaster_test.rb index 00336e314..194c9e7cb 100644 --- a/test/unit/email_postmaster_test.rb +++ b/test/unit/email_postmaster_test.rb @@ -961,4 +961,106 @@ Some Text' assert_equal(false, article.internal) end + test 'tags in postmaster filter' do + group_default = Group.lookup(name: 'Users') + + PostmasterFilter.create!( + name: '01 set tag for email', + match: { + from: { + operator: 'contains', + value: 'nobody@example.com', + }, + }, + perform: { + 'x-zammad-ticket-tags' => { + operator: 'add', + value: 'test1, test2, test3', + }, + }, + channel: 'email', + active: true, + created_by_id: 1, + updated_by_id: 1, + ) + + PostmasterFilter.create!( + name: '02 set tag for email', + match: { + from: { + operator: 'contains', + value: 'nobody@example.com', + }, + }, + perform: { + 'x-zammad-ticket-tags' => { + operator: 'remove', + value: 'test2, test3', + }, + }, + channel: 'email', + active: true, + created_by_id: 1, + updated_by_id: 1, + ) + + PostmasterFilter.create!( + name: '03 set tag for email', + match: { + from: { + operator: 'contains', + value: 'nobody@example.com', + }, + }, + perform: { + 'x-zammad-ticket-tags' => { + operator: 'add', + value: 'test3', + }, + }, + channel: 'email', + active: true, + created_by_id: 1, + updated_by_id: 1, + ) + + PostmasterFilter.create!( + name: '04 set tag for email', + match: { + from: { + operator: 'contains', + value: 'nobody@example.com', + }, + }, + perform: { + 'x-zammad-ticket-tags' => { + operator: 'add', + value: 'abc1 , abc2 ', + }, + }, + channel: 'email', + active: true, + created_by_id: 1, + updated_by_id: 1, + ) + + data = 'From: ME Bob +To: customer@example.com +Subject: some subject + +Some Text' + + parser = Channel::EmailParser.new + ticket, article, user = parser.process({ group_id: group_default.id, trusted: false }, data) + tags = Tag.tag_list(object: 'Ticket', o_id: ticket.id) + assert_equal('Users', ticket.group.name) + assert_equal('2 normal', ticket.priority.name) + assert_equal('some subject', ticket.title) + assert_equal('nobody@example.com', ticket.customer.email) + assert_equal(4, tags.count) + assert(tags.include?('test1')) + assert(tags.include?('test3')) + assert(tags.include?('abc1')) + assert(tags.include?('abc2')) + end end diff --git a/test/unit/ticket_escalation_test.rb b/test/unit/ticket_escalation_test.rb index 89045bb3c..f4f4ebc8a 100644 --- a/test/unit/ticket_escalation_test.rb +++ b/test/unit/ticket_escalation_test.rb @@ -115,7 +115,7 @@ class TicketEscalationTest < ActiveSupport::TestCase ticket.save! assert_not(ticket.has_changes_to_save?) assert(ticket.escalation_at) - assert_equal((ticket_escalation_at - 30.minutes).to_s, ticket.escalation_at.to_s) + assert_in_delta((ticket_escalation_at - 30.minutes).to_i, ticket.escalation_at.to_i, 90) sla.destroy! calendar.destroy! @@ -192,10 +192,10 @@ Some Text" ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email) ticket_p.reload assert(ticket_p.escalation_at) - assert_in_delta(ticket_p.first_response_escalation_at.to_i, (ticket_p.created_at + 1.hour).to_i, 120) - assert_in_delta(ticket_p.update_escalation_at.to_i, (ticket_p.created_at + 3.hours).to_i, 120) - assert_in_delta(ticket_p.close_escalation_at.to_i, (ticket_p.created_at + 4.hours).to_i, 120) - assert_in_delta(ticket_p.escalation_at.to_i, (ticket_p.created_at + 1.hour).to_i, 120) + assert_in_delta(ticket_p.first_response_escalation_at.to_i, (ticket_p.created_at + 1.hour).to_i, 90) + assert_in_delta(ticket_p.update_escalation_at.to_i, (ticket_p.created_at + 3.hours).to_i, 90) + assert_in_delta(ticket_p.close_escalation_at.to_i, (ticket_p.created_at + 4.hours).to_i, 90) + assert_in_delta(ticket_p.escalation_at.to_i, (ticket_p.created_at + 1.hour).to_i, 90) travel 3.hours article = nil @@ -217,10 +217,10 @@ Some Text" end ticket_p.reload - assert_in_delta(ticket_p.first_response_escalation_at.to_i, (ticket_p.created_at + 1.hour).to_i, 120) - assert_in_delta(ticket_p.update_escalation_at.to_i, (ticket_p.last_contact_agent_at + 3.hours).to_i, 120) - assert_in_delta(ticket_p.close_escalation_at.to_i, (ticket_p.created_at + 4.hours).to_i, 120) - assert_in_delta(ticket_p.escalation_at.to_i, (ticket_p.created_at + 4.hours).to_i, 120) + assert_in_delta(ticket_p.first_response_escalation_at.to_i, (ticket_p.created_at + 1.hour).to_i, 90) + assert_in_delta(ticket_p.update_escalation_at.to_i, (ticket_p.last_contact_agent_at + 3.hours).to_i, 90) + assert_in_delta(ticket_p.close_escalation_at.to_i, (ticket_p.created_at + 4.hours).to_i, 90) + assert_in_delta(ticket_p.escalation_at.to_i, (ticket_p.created_at + 4.hours).to_i, 90) end end