Kimirock/zammad
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes #4547, #4557 - Invalid results in complex OR structures joining tables.

Browse Source
This commit is contained in:
Rolf Schmidt
2023-04-13 13:09:35 +02:00
parent d4dddcd02f
commit be48ff8983
2 changed files with 196 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
app/models/ticket/selector/sql.rb
View File

@@ -1,13 +1,12 @@
# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
class Ticket::Selector::Sql < Ticket::Selector::Base
attr_accessor :final_query, :final_bind_params, :final_tables, :final_tables_query
attr_accessor :final_query, :final_bind_params, :final_tables
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
@@ -19,26 +18,22 @@ class Ticket::Selector::Sql < Ticket::Selector::Base
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
@@ -70,7 +65,6 @@ class Ticket::Selector::Sql < Ticket::Selector::Base
# 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('.')
@@ -82,24 +76,18 @@ class Ticket::Selector::Sql < Ticket::Selector::Base
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}"
@@ -305,13 +293,8 @@ class Ticket::Selector::Sql < Ticket::Selector::Base
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)
@@ -402,7 +385,7 @@ class Ticket::Selector::Sql < Ticket::Selector::Base
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)

183
spec/models/ticket/selector/sql_spec.rb
View File

@@ -82,4 +82,187 @@ RSpec.describe Ticket::Selector::Sql do
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