Skip to content

Commit

Permalink
Fixes #4547, #4557 - Invalid results in complex OR structures joining…
Browse files Browse the repository at this point in the history 
… tables.
  • Loading branch information
@rolfschmidt
rolfschmidt committed Apr 13, 2023
1 parent a436d76 commit 1344c0f
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 30 deletions.
43 changes: 13 additions & 30 deletions app/models/ticket/selector/sql.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@ class Ticket::Selector::Sql < Ticket::Selector::Base
'is not in working time'
].freeze

attr_accessor :final_query, :final_bind_params, :final_tables, :final_tables_query, :changed_attributes
attr_accessor :final_query, :final_bind_params, :final_tables, :changed_attributes

def get
@final_query = []
@final_bind_params = []
@final_tables = []
@final_tables_query = []
@final_query = run(selector, 0)
[query_sql, final_bind_params, tables_sql]
rescue InvalidCondition => e
Expand All @@ -34,26 +33,22 @@ def get
end

def query_sql
[
final_tables_query.join(' AND ').presence,
final_query.presence,
].compact.join(' AND ')
Array(final_query).join(' AND ')
end

def tables_sql
return '' if final_tables.blank?

", #{final_tables.join(', ')}"
" #{final_tables.join(' ')}"
end

def run(block, level)
if block.key?(:conditions)
run_block(block, level)
else
query, bind_params, tables, tables_query = condition_sql(block)
query, bind_params, tables = condition_sql(block)
@final_bind_params += bind_params
@final_tables |= tables
@final_tables_query |= tables_query
query
end
end
Expand Down Expand Up @@ -85,7 +80,6 @@ def condition_sql(block_condition)
# remember query and bind params
query = []
tables = []
tables_query = []
bind_params = []
like = Rails.application.config.db_like
attribute_table, attribute_name = block_condition[:name].split('.')
Expand All @@ -97,24 +91,18 @@ def condition_sql(block_condition)
if attribute_table && attribute_table != 'execution_time' && tables.exclude?(attribute_table) && !(attribute_table == 'ticket' && attribute_name != 'mention_user_ids') && !(attribute_table == 'ticket' && attribute_name == 'mention_user_ids' && block_condition[:pre_condition] == 'not_set')
case attribute_table
when 'customer'
tables |= ['users customers']
tables_query |= ['tickets.customer_id = customers.id']
tables |= ['INNER JOIN users customers ON tickets.customer_id = customers.id']
when 'organization'
tables |= ['organizations']
tables_query |= ['tickets.organization_id = organizations.id']
tables |= ['LEFT JOIN organizations ON tickets.organization_id = organizations.id']
when 'owner'
tables |= ['users owners']
tables_query |= ['tickets.owner_id = owners.id']
tables |= ['INNER JOIN users owners ON tickets.owner_id = owners.id']
when 'article'
tables |= ['ticket_articles articles']
tables_query |= ['tickets.id = articles.ticket_id']
tables |= ['INNER JOIN ticket_articles articles ON tickets.id = articles.ticket_id']
when 'ticket_state'
tables |= ['ticket_states']
tables_query |= ['tickets.state_id = ticket_states.id']
tables |= ['INNER JOIN ticket_states ON tickets.state_id = ticket_states.id']
when 'ticket'
if attribute_name == 'mention_user_ids'
tables |= ['mentions']
tables_query |= ["tickets.id = mentions.mentionable_id AND mentions.mentionable_type = 'Ticket'"]
tables |= ["LEFT JOIN mentions ON tickets.id = mentions.mentionable_id AND mentions.mentionable_type = 'Ticket'"]
end
else
raise "invalid selector #{attribute_table}, #{attribute_name}"
Expand Down Expand Up @@ -324,13 +312,8 @@ def condition_sql(block_condition)
end
elsif block_condition[:operator] == 'contains one' && attribute_table == 'ticket'
if attribute_name == 'tags'
tables |= %w[tag_objects tag_items tags]
query << "
tickets.id = tags.o_id AND
tag_objects.id = tags.tag_object_id AND
tag_objects.name = 'Ticket' AND
tag_items.id = tags.tag_item_id AND
tag_items.name IN (?)"
tables |= ["LEFT JOIN tags ON tickets.id = tags.o_id LEFT JOIN tag_objects ON tag_objects.id = tags.tag_object_id AND tag_objects.name = 'Ticket' LEFT JOIN tag_items ON tag_items.id = tags.tag_item_id"]
query << 'tag_items.name IN (?)'

bind_params.push block_condition[:value]
elsif Ticket.column_names.include?(attribute_name)
Expand Down Expand Up @@ -421,7 +404,7 @@ def condition_sql(block_condition)
raise "Invalid operator '#{block_condition[:operator]}' for '#{block_condition[:value].inspect}'"
end

[query, bind_params, tables, tables_query]
[query, bind_params, tables]
end

def range(selector)
Expand Down
183 changes: 183 additions & 0 deletions spec/models/ticket/selector/sql_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,187 @@ def get_condition(operator, range)
end
end
end

describe 'Expert mode overview not working when using "owner is me" OR "subscribe is me #4547' do
let(:agent) { create(:agent, groups: [Group.first]) }
let(:ticket_1) { create(:ticket, owner: agent, group: Group.first) }
let(:ticket_2) { create(:ticket, group: Group.first) }
let(:ticket_3) { create(:ticket, owner: agent, group: Group.first) }

before do
Ticket.destroy_all

ticket_1 && ticket_2 && ticket_3
create(:mention, mentionable: ticket_2, user: agent)
create(:mention, mentionable: ticket_3, user: agent)
end

it 'does return 1 mentioned ticket' do
condition = {
operator: 'AND',
conditions: [
{
name: 'ticket.mention_user_ids',
operator: 'is',
pre_condition: 'specific',
value: agent.id,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(2)
end

it 'does return 1 owned ticket' do
condition = {
operator: 'AND',
conditions: [
{
name: 'ticket.owner_id',
operator: 'is',
pre_condition: 'specific',
value: agent.id,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(2)
end

it 'does return 1 owned & subscribed ticket' do
condition = {
operator: 'AND',
conditions: [
{
name: 'ticket.mention_user_ids',
operator: 'is',
pre_condition: 'specific',
value: agent.id,
},
{
name: 'ticket.owner_id',
operator: 'is',
pre_condition: 'specific',
value: agent.id,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(1)
end

it 'does return 3 owned or subscribed tickets' do
condition = {
operator: 'OR',
conditions: [
{
name: 'ticket.mention_user_ids',
operator: 'is',
pre_condition: 'specific',
value: agent.id,
},
{
name: 'ticket.owner_id',
operator: 'is',
pre_condition: 'specific',
value: agent.id,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(3)
end
end

describe 'Overviews: "Organization" does not work as a pre-condition in the expert mode #4557' do
let(:agent) { create(:agent, groups: [Group.first]) }
let(:organization) { create(:organization) }
let(:customer_1) { create(:customer) }
let(:customer_2) { create(:customer, organization: organization) }
let(:ticket_1) { create(:ticket, customer: customer_1, group: Group.first) }
let(:ticket_2) { create(:ticket, customer: customer_2, group: Group.first) }

before do
Ticket.destroy_all
ticket_1 && ticket_2
end

it 'does return 1 customer ticket without organization' do
condition = {
operator: 'AND',
conditions: [
{
name: 'ticket.organization_id',
operator: 'is',
pre_condition: 'not_set',
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(1)
end

it 'does return 1 ticket with organization title' do
condition = {
operator: 'AND',
conditions: [
{
name: 'organization.name',
operator: 'is',
value: organization.name,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(1)
end

it 'does return 1 ticket with organization and name' do
condition = {
operator: 'AND',
conditions: [
{
name: 'ticket.organization_id',
operator: 'is not',
pre_condition: 'not_set',
},
{
name: 'organization.name',
operator: 'is',
value: organization.name,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(1)
end

it 'does return 1 ticket without organization OR NO name' do
condition = {
operator: 'OR',
conditions: [
{
name: 'ticket.organization_id',
operator: 'is',
pre_condition: 'not_set',
},
{
name: 'organization.name',
operator: 'is not',
value: organization.name,
}
]
}

count, = Ticket.selectors(condition, { current_user: agent })
expect(count).to eq(1)
end
end
end

0 comments on commit 1344c0f

Please sign in to comment.