Sutty/trabajo-afectivo
5
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fixed issue #2016 - Scheduler will stop after removing own ticket attribute but overview with condition which is including attribute git dropped. Merge pull request #2033.

Browse source
This commit is contained in:
Martin Edenhofer 2018-06-20 13:39:41 +02:00
parent 577776c324
commit 2ec4c965de
8 changed files with 719 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
app/assets/javascripts/app/controllers/object_manager.coffee
View file

@ -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) ->

6
app/assets/javascripts/app/views/object_manager/index.jst.eco
View file

@ -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: %>
<% else if item.editable: %>
<% if item.deletable: %>
<a href="#" class="js-delete" title="<%- @Ti('Delete') %>"><%- @Icon('trash') %></a>
<% else: %>
<span class="is-disabled" title="<%= item.not_deletable_reason %>"><%- @Icon('trash') %></span>
<% end %>
<% end %>
</td>
</tr>

5
app/assets/stylesheets/zammad.scss
View file

@ -9610,3 +9610,8 @@ body.fit {
.flex-spacer {
flex: 1;
}
span.is-disabled {
cursor: not-allowed;
opacity: 0.5;
}

3
app/controllers/object_manager_attributes_controller.rb
View file

@ -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

126
app/models/object_manager/attribute.rb
View file

@ -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

95
test/browser/admin_object_manager_test.rb
View file

@ -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

9
test/browser_test_helper.rb
View file

@ -1659,6 +1659,14 @@ wait untill text in selector disabppears
mute_log: true,
)
sleep 0.5
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',
@ -1667,6 +1675,7 @@ wait untill text in selector disabppears
mute_log: true,
)
end
end
if data['order::direction']
select(

464
test/integration/object_manager_attributes_controller_test.rb
View file

@ -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