Fixes #2982 - Trigger with current_user.id as precondition is executed correctly

This commit is contained in:
Mantas Masalskis 2020-03-24 16:40:53 +01:00 committed by Martin Edenhofer
parent eabc924230
commit 92f7d9307c
3 changed files with 125 additions and 8 deletions

View file

@ -423,8 +423,22 @@ returns
get count of tickets and tickets which match on selector get count of tickets and tickets which match on selector
@param [Hash] selectors hash with conditions
@oparam [Hash] options
@option options [String] :access can be 'full', 'read', 'create' or 'ignore' (ignore means a selector over all tickets), defaults to 'full'
@option options [Integer] :limit of tickets to return
@option options [User] :user is a current user
@option options [Integer] :execution_time is a current user
@return [Integer, [<Ticket>]]
@example
ticket_count, tickets = Ticket.selectors(params[:condition], limit: limit, current_user: current_user, access: 'full') ticket_count, tickets = Ticket.selectors(params[:condition], limit: limit, current_user: current_user, access: 'full')
ticket_count # count of found tickets
tickets # tickets
=end =end
def self.selectors(selectors, options) def self.selectors(selectors, options)
@ -438,7 +452,7 @@ get count of tickets and tickets which match on selector
ActiveRecord::Base.transaction(requires_new: true) do ActiveRecord::Base.transaction(requires_new: true) do
if !current_user if !current_user || access == 'ignore'
ticket_count = Ticket.distinct.where(query, *bind_params).joins(tables).count ticket_count = Ticket.distinct.where(query, *bind_params).joins(tables).count
tickets = Ticket.distinct.where(query, *bind_params).joins(tables).limit(limit) tickets = Ticket.distinct.where(query, *bind_params).joins(tables).limit(limit)
return [ticket_count, tickets] return [ticket_count, tickets]
@ -1118,18 +1132,22 @@ perform active triggers on ticket
} }
end end
# verify is condition is matching
ticket_count, tickets = Ticket.selectors(condition, limit: 1, execution_time: true)
next if ticket_count.blank?
next if ticket_count.zero?
next if tickets.first.id != ticket.id
user_id = ticket.updated_by_id user_id = ticket.updated_by_id
if article if article
user_id = article.updated_by_id user_id = article.updated_by_id
end end
user = if user_id != 1
User.lookup(id: user_id)
end
# verify is condition is matching
ticket_count, tickets = Ticket.selectors(condition, limit: 1, execution_time: true, current_user: user, access: 'ignore')
next if ticket_count.blank?
next if ticket_count.zero?
next if tickets.first.id != ticket.id
if recursive == false && local_options[:loop_count] > 1 if recursive == false && local_options[:loop_count] > 1
message = "Do not execute recursive triggers per default until Zammad 3.0. With Zammad 3.0 and higher the following trigger is executed '#{trigger.name}' on Ticket:#{ticket.id}. Please review your current triggers and change them if needed." message = "Do not execute recursive triggers per default until Zammad 3.0. With Zammad 3.0 and higher the following trigger is executed '#{trigger.name}' on Ticket:#{ticket.id}. Please review your current triggers and change them if needed."
logger.info { message } logger.info { message }

View file

@ -294,4 +294,40 @@ RSpec.describe Trigger, type: :model do
end end
end end
end end
context 'with pre condition current_user.id' do
let(:perform) do
{ 'ticket.title'=>{ 'value'=>'triggered' } }
end
let(:user) do
user = create(:agent_user)
user.roles.first.groups << group
user
end
let(:group) { Group.first }
let(:ticket) do
create(:ticket,
title: 'Test Ticket', group: group,
owner_id: user.id, created_by_id: user.id, updated_by_id: user.id)
end
shared_examples 'successful trigger' do |attribute:|
let(:attribute) { attribute }
let(:condition) do
{ attribute => { operator: 'is', pre_condition: 'current_user.id', value: '', value_completion: '' } }
end
it "for #{attribute}" do
ticket && trigger
expect { Observer::Transaction.commit }.to change { ticket.reload.title }.to('triggered')
end
end
it_behaves_like 'successful trigger', attribute: 'ticket.updated_by_id'
it_behaves_like 'successful trigger', attribute: 'ticket.owner_id'
end
end end

View file

@ -1199,4 +1199,67 @@ class TicketSelectorTest < ActiveSupport::TestCase
end end
test 'access: "ignore"' do
Ticket.destroy_all
Ticket.create!(
title: 'some title1',
group: @group,
customer_id: @customer1.id,
owner_id: @agent1.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
created_at: '2015-02-05 16:37:00',
updated_by_id: 1,
created_by_id: 1,
)
Ticket.create!(
title: 'some title2',
group: @group,
customer_id: @customer1.id,
owner_id: @agent1.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
created_at: '2015-02-05 16:37:00',
updated_by_id: @agent2.id,
created_by_id: 1,
)
condition = {
'ticket.title' => {
operator: 'contains',
value: 'some',
},
}
# visible by owner
ticket_count, _tickets = Ticket.selectors(condition, limit: 10, current_user: @agent1)
assert_equal(2, ticket_count)
# not visible by another agent
ticket_count, _tickets = Ticket.selectors(condition, limit: 10, current_user: @agent2)
assert_equal(0, ticket_count)
# visible by another user when access: "ignore". For example, when tickets are performed after action of another user
ticket_count, _tickets = Ticket.selectors(condition, limit: 10, current_user: @agent2, access: 'ignore')
assert_equal(2, ticket_count)
condition2 = {
'ticket.updated_by_id' => {
operator: 'is',
pre_condition: 'current_user.id',
value: '',
value_completion: ''
}
}
# not visible by another agent even if matches current user precondition
ticket_count, _tickets = Ticket.selectors(condition2, limit: 10, current_user: @agent2)
assert_equal(0, ticket_count)
# visible by another user when access: "ignore" if matches current user precondition
ticket_count, _tickets = Ticket.selectors(condition2, limit: 10, current_user: @agent2, access: 'ignore')
assert_equal(1, ticket_count)
end
end end