fix: Avoid joining tables to fix the distinct value query (#7965)
DISTINCT query with custom attributes return an error. To avoid the error, this PR refactors the query to include tags only when it is required. Fixes #7931 Fixes #7836 Co-authored-by: Sojan Jose <sojan@pepalo.com>
This commit is contained in:
@@ -35,7 +35,7 @@ class Contacts::FilterService < FilterService
|
||||
" (contacts.#{attribute_key})::#{current_filter['data_type']} #{filter_operator_value}#{current_filter['data_type']} #{query_operator} "
|
||||
when 'standard'
|
||||
if attribute_key == 'labels'
|
||||
" tags.id #{filter_operator_value} #{query_operator} "
|
||||
tag_filter_query('Contact', 'contacts', query_hash, current_index)
|
||||
else
|
||||
" LOWER(contacts.#{attribute_key}) #{filter_operator_value} #{query_operator} "
|
||||
end
|
||||
@@ -54,7 +54,7 @@ class Contacts::FilterService < FilterService
|
||||
end
|
||||
|
||||
def base_relation
|
||||
Current.account.contacts.distinct(:id).left_outer_joins(:labels)
|
||||
Current.account.contacts
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -46,7 +46,7 @@ class Conversations::FilterService < FilterService
|
||||
" (conversations.#{attribute_key})::#{current_filter['data_type']} #{filter_operator_value}#{current_filter['data_type']} #{query_operator} "
|
||||
when 'standard'
|
||||
if attribute_key == 'labels'
|
||||
" tags.name #{filter_operator_value} #{query_operator} "
|
||||
tag_filter_query('Conversation', 'conversations', query_hash, current_index)
|
||||
else
|
||||
" conversations.#{attribute_key} #{filter_operator_value} #{query_operator} "
|
||||
end
|
||||
@@ -54,7 +54,9 @@ class Conversations::FilterService < FilterService
|
||||
end
|
||||
|
||||
def base_relation
|
||||
@account.conversations.left_outer_joins(:labels)
|
||||
@account.conversations.includes(
|
||||
:taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team, :messages, :contact_inbox
|
||||
)
|
||||
end
|
||||
|
||||
def current_page
|
||||
@@ -62,10 +64,6 @@ class Conversations::FilterService < FilterService
|
||||
end
|
||||
|
||||
def conversations
|
||||
@conversations = @conversations.includes(
|
||||
:taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team, :messages, :contact_inbox
|
||||
)
|
||||
|
||||
@conversations.latest.page(current_page)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -106,6 +106,27 @@ class FilterService
|
||||
]
|
||||
end
|
||||
|
||||
def tag_filter_query(model_name, table_name, query_hash, current_index)
|
||||
query_operator = query_hash[:query_operator]
|
||||
@filter_values["value_#{current_index}"] = filter_values(query_hash)
|
||||
|
||||
tag_model_relation_query =
|
||||
"SELECT * FROM taggings WHERE taggings.taggable_id = #{table_name}.id AND taggings.taggable_type = '#{model_name}'"
|
||||
tag_query =
|
||||
"AND taggings.tag_id IN (SELECT tags.id FROM tags WHERE tags.name IN (:value_#{current_index}))"
|
||||
|
||||
case query_hash[:filter_operator]
|
||||
when 'equal_to'
|
||||
"EXISTS (#{tag_model_relation_query} #{tag_query}) #{query_operator}"
|
||||
when 'not_equal_to'
|
||||
"NOT EXISTS (#{tag_model_relation_query} #{tag_query}) #{query_operator}"
|
||||
when 'is_present'
|
||||
"EXISTS (#{tag_model_relation_query}) #{query_operator}"
|
||||
when 'is_not_present'
|
||||
"NOT EXISTS (#{tag_model_relation_query}) #{query_operator}"
|
||||
end
|
||||
end
|
||||
|
||||
def custom_attribute_query(query_hash, custom_attribute_type, current_index)
|
||||
@attribute_key = query_hash[:attribute_key]
|
||||
@custom_attribute_type = custom_attribute_type
|
||||
|
||||
@@ -15,7 +15,7 @@ describe Contacts::FilterService do
|
||||
create(:inbox_member, user: first_user, inbox: inbox)
|
||||
create(:inbox_member, user: second_user, inbox: inbox)
|
||||
create(:conversation, account: account, inbox: inbox, assignee: first_user, contact: en_contact)
|
||||
create(:conversation, account: account, inbox: inbox)
|
||||
create(:conversation, account: account, inbox: inbox, contact: el_contact)
|
||||
Current.account = account
|
||||
|
||||
create(:custom_attribute_definition,
|
||||
@@ -38,8 +38,7 @@ describe Contacts::FilterService do
|
||||
|
||||
describe '#perform' do
|
||||
before do
|
||||
en_contact.add_labels(%w[random_label support])
|
||||
el_contact.update_labels('random_label')
|
||||
en_contact.update_labels(%w[random_label support])
|
||||
cs_contact.update_labels('support')
|
||||
|
||||
en_contact.update!(custom_attributes: { contact_additional_information: 'test custom data' })
|
||||
@@ -60,6 +59,70 @@ describe Contacts::FilterService do
|
||||
]
|
||||
end
|
||||
|
||||
context 'with label filter' do
|
||||
it 'returns equal_to filter results properly' do
|
||||
params[:payload] = [
|
||||
{
|
||||
attribute_key: 'labels',
|
||||
filter_operator: 'equal_to',
|
||||
values: ['support'],
|
||||
query_operator: nil
|
||||
}.with_indifferent_access
|
||||
]
|
||||
|
||||
result = filter_service.new(params, first_user).perform
|
||||
expect(result[:contacts].length).to be 2
|
||||
expect(result[:contacts].first.label_list).to include('support')
|
||||
expect(result[:contacts].last.label_list).to include('support')
|
||||
end
|
||||
|
||||
it 'returns not_equal_to filter results properly' do
|
||||
params[:payload] = [
|
||||
{
|
||||
attribute_key: 'labels',
|
||||
filter_operator: 'not_equal_to',
|
||||
values: ['support'],
|
||||
query_operator: nil
|
||||
}.with_indifferent_access
|
||||
]
|
||||
|
||||
result = filter_service.new(params, first_user).perform
|
||||
expect(result[:contacts].length).to be 1
|
||||
expect(result[:contacts].first.id).to eq el_contact.id
|
||||
end
|
||||
|
||||
it 'returns is_present filter results properly' do
|
||||
params[:payload] = [
|
||||
{
|
||||
attribute_key: 'labels',
|
||||
filter_operator: 'is_present',
|
||||
values: [],
|
||||
query_operator: nil
|
||||
}.with_indifferent_access
|
||||
]
|
||||
|
||||
result = filter_service.new(params, first_user).perform
|
||||
expect(result[:contacts].length).to be 2
|
||||
expect(result[:contacts].first.label_list).to include('support')
|
||||
expect(result[:contacts].last.label_list).to include('support')
|
||||
end
|
||||
|
||||
it 'returns is_not_present filter results properly' do
|
||||
params[:payload] = [
|
||||
{
|
||||
attribute_key: 'labels',
|
||||
filter_operator: 'is_not_present',
|
||||
values: [],
|
||||
query_operator: nil
|
||||
}.with_indifferent_access
|
||||
]
|
||||
|
||||
result = filter_service.new(params, first_user).perform
|
||||
expect(result[:contacts].length).to be 1
|
||||
expect(result[:contacts].first.id).to eq el_contact.id
|
||||
end
|
||||
end
|
||||
|
||||
it 'filter contacts by additional_attributes' do
|
||||
params[:payload] = payload
|
||||
result = filter_service.new(params, first_user).perform
|
||||
@@ -83,25 +146,7 @@ describe Contacts::FilterService do
|
||||
expect(result[:contacts].first.name).to eq(en_contact.name)
|
||||
end
|
||||
|
||||
it 'filter contact by tags' do
|
||||
label_id = cs_contact.labels.last.id
|
||||
params[:payload] = [
|
||||
{
|
||||
attribute_key: 'labels',
|
||||
filter_operator: 'equal_to',
|
||||
values: [label_id],
|
||||
query_operator: nil
|
||||
}.with_indifferent_access
|
||||
]
|
||||
|
||||
result = filter_service.new(params, first_user).perform
|
||||
expect(result[:contacts].length).to be 2
|
||||
expect(result[:contacts].first.label_list).to include('support')
|
||||
expect(result[:contacts].last.label_list).to include('support')
|
||||
end
|
||||
|
||||
it 'filter by custom_attributes and labels' do
|
||||
label_id = cs_contact.labels.last.id
|
||||
params[:payload] = [
|
||||
{
|
||||
attribute_key: 'customer_type',
|
||||
@@ -112,7 +157,7 @@ describe Contacts::FilterService do
|
||||
{
|
||||
attribute_key: 'labels',
|
||||
filter_operator: 'equal_to',
|
||||
values: [label_id],
|
||||
values: ['support'],
|
||||
query_operator: 'AND'
|
||||
}.with_indifferent_access,
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user