From a95d249ec13e8bdd574a80f17ae6ff0e37e19a41 Mon Sep 17 00:00:00 2001 From: Tejaswini Chile Date: Mon, 31 Jan 2022 13:36:44 +0530 Subject: [PATCH] Feat: Custom attribute advanced filter (#3818) --- Gemfile | 2 +- Gemfile.lock | 6 +- app/services/contacts/filter_service.rb | 20 +++- app/services/conversations/filter_service.rb | 20 ++++ app/services/filter_service.rb | 27 +++++ spec/services/contacts/filter_service_spec.rb | 98 +++++++++++++++++-- .../conversations/filter_service_spec.rb | 88 ++++++++++++++--- 7 files changed, 238 insertions(+), 23 deletions(-) diff --git a/Gemfile b/Gemfile index f3fbce89d..0664e5aef 100644 --- a/Gemfile +++ b/Gemfile @@ -102,7 +102,7 @@ gem 'sentry-ruby' gem 'sentry-sidekiq' ##-- background job processing --## -gem 'sidekiq' +gem 'sidekiq', '~> 6.4.0' # We want cron jobs gem 'sidekiq-cron' diff --git a/Gemfile.lock b/Gemfile.lock index 079dd0ee6..4f1e056bc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -447,7 +447,7 @@ GEM rb-fsevent (0.11.0) rb-inotify (0.10.1) ffi (~> 1.0) - redis (4.4.0) + redis (4.5.1) redis-namespace (1.8.1) redis (>= 3.0.4) regexp_parser (2.1.1) @@ -546,7 +546,7 @@ GEM sexp_processor (4.15.3) shoulda-matchers (5.0.0) activesupport (>= 5.2.0) - sidekiq (6.2.2) + sidekiq (6.4.0) connection_pool (>= 2.2.2) rack (~> 2.0) redis (>= 4.2.0) @@ -726,7 +726,7 @@ DEPENDENCIES sentry-ruby sentry-sidekiq shoulda-matchers - sidekiq + sidekiq (~> 6.4.0) sidekiq-cron simplecov (= 0.17.1) slack-ruby-client diff --git a/app/services/contacts/filter_service.rb b/app/services/contacts/filter_service.rb index 0b0ee57e9..cc90b8502 100644 --- a/app/services/contacts/filter_service.rb +++ b/app/services/contacts/filter_service.rb @@ -1,4 +1,6 @@ class Contacts::FilterService < FilterService + ATTRIBUTE_MODEL = 'contact_attribute'.freeze + def perform @contacts = contact_query_builder @@ -16,7 +18,7 @@ class Contacts::FilterService < FilterService @query_string += contact_query_string(current_filter, query_hash, current_index) end - base_relation.where(@query_string, @filter_values.with_indifferent_access) + base_relation.select('distinct contacts.id').where(@query_string, @filter_values.with_indifferent_access) end def contact_query_string(current_filter, query_hash, current_index) @@ -24,6 +26,8 @@ class Contacts::FilterService < FilterService query_operator = query_hash[:query_operator] filter_operator_value = filter_operation(query_hash, current_index) + return custom_attribute_query(query_hash, current_index) if current_filter.nil? + case current_filter['attribute_type'] when 'additional_attributes' " LOWER(contacts.additional_attributes ->> '#{attribute_key}') #{filter_operator_value} #{query_operator} " @@ -58,4 +62,18 @@ class Contacts::FilterService < FilterService "!= :value_#{current_index}" end + + def custom_attribute_query(query_hash, current_index) + attribute_key = query_hash[:attribute_key] + query_operator = query_hash[:query_operator] + attribute_type = custom_attribute(attribute_key).try(:attribute_display_type) + filter_operator_value = filter_operation(query_hash, current_index) + attribute_data_type = self.class::ATTRIBUTE_TYPES[attribute_type] + + if custom_attribute(attribute_key) + " LOWER(contacts.custom_attributes ->> '#{attribute_key}')::#{attribute_data_type} #{filter_operator_value} #{query_operator} " + else + ' ' + end + end end diff --git a/app/services/conversations/filter_service.rb b/app/services/conversations/filter_service.rb index d0ee60ca7..6e73ae9b8 100644 --- a/app/services/conversations/filter_service.rb +++ b/app/services/conversations/filter_service.rb @@ -1,4 +1,6 @@ class Conversations::FilterService < FilterService + ATTRIBUTE_MODEL = 'conversation_attribute'.freeze + def perform @conversations = conversation_query_builder mine_count, unassigned_count, all_count, = set_count_for_all_conversations @@ -30,6 +32,8 @@ class Conversations::FilterService < FilterService query_operator = query_hash[:query_operator] filter_operator_value = filter_operation(query_hash, current_index) + return custom_attribute_query(query_hash, current_index) if current_filter.nil? + case current_filter['attribute_type'] when 'additional_attributes' " conversations.additional_attributes ->> '#{attribute_key}' #{filter_operator_value} #{query_operator} " @@ -56,4 +60,20 @@ class Conversations::FilterService < FilterService ) @conversations.latest.page(current_page) end + + private + + def custom_attribute_query(query_hash, current_index) + attribute_key = query_hash[:attribute_key] + query_operator = query_hash[:query_operator] + attribute_type = custom_attribute(attribute_key).try(:attribute_display_type) + filter_operator_value = filter_operation(query_hash, current_index) + attribute_data_type = self.class::ATTRIBUTE_TYPES[attribute_type] + + if custom_attribute(attribute_key) + " LOWER(conversations.custom_attributes ->> '#{attribute_key}')::#{attribute_data_type} #{filter_operator_value} #{query_operator} " + else + ' ' + end + end end diff --git a/app/services/filter_service.rb b/app/services/filter_service.rb index 054060742..a08fb947a 100644 --- a/app/services/filter_service.rb +++ b/app/services/filter_service.rb @@ -1,6 +1,16 @@ require 'json' class FilterService + ATTRIBUTE_MODEL = 'conversation_attribute'.freeze + ATTRIBUTE_TYPES = { + date: 'date', + text: 'text', + number: 'numeric', + link: 'text', + list: 'text', + checkbox: 'boolean' + }.with_indifferent_access + def initialize(params, user) @params = params @user = user @@ -24,6 +34,8 @@ class FilterService @filter_values["value_#{current_index}"] = 'IS NOT NULL' when 'is_not_present' @filter_values["value_#{current_index}"] = 'IS NULL' + when 'is_greater_than', 'is_less_than' + @filter_values["value_#{current_index}"] = lt_gt_filter_values(query_hash) else @filter_values["value_#{current_index}"] = filter_values(query_hash).to_s "= :value_#{current_index}" @@ -43,6 +55,15 @@ class FilterService end end + def lt_gt_filter_values(query_hash) + attribute_key = query_hash[:attribute_key] + attribute_type = custom_attribute(attribute_key).try(:attribute_display_type) + attribute_data_type = self.class::ATTRIBUTE_TYPES[attribute_type] + value = query_hash['values'][0] + operator = query_hash['filter_operator'] == 'is_less_than' ? '<' : '>' + "#{operator} '#{value}'::#{attribute_data_type}" + end + def set_count_for_all_conversations [ @conversations.assigned_to(@user).count, @@ -53,6 +74,12 @@ class FilterService private + def custom_attribute(attribute_key) + @custom_attribute = Current.account.custom_attribute_definitions.where( + attribute_model: self.class::ATTRIBUTE_MODEL + ).find_by(attribute_key: attribute_key) + end + def equals_to_filter_string(filter_operator, current_index) return "IN (:value_#{current_index})" if filter_operator == 'equal_to' diff --git a/spec/services/contacts/filter_service_spec.rb b/spec/services/contacts/filter_service_spec.rb index 8275d0d52..14f4c1d27 100644 --- a/spec/services/contacts/filter_service_spec.rb +++ b/spec/services/contacts/filter_service_spec.rb @@ -7,17 +7,46 @@ describe ::Contacts::FilterService do let!(:user_1) { create(:user, account: account) } let!(:user_2) { create(:user, account: account) } let!(:inbox) { create(:inbox, account: account, enable_auto_assignment: false) } - let!(:contact) { create(:contact, account: account, additional_attributes: { 'browser_language': 'en' }) } + let(:en_contact) { create(:contact, account: account, additional_attributes: { 'browser_language': 'en' }) } + let(:el_contact) { create(:contact, account: account, additional_attributes: { 'browser_language': 'el' }) } + let(:cs_contact) { create(:contact, account: account, additional_attributes: { 'browser_language': 'cs' }) } before do create(:inbox_member, user: user_1, inbox: inbox) create(:inbox_member, user: user_2, inbox: inbox) - create(:conversation, account: account, inbox: inbox, assignee: user_1, contact: contact) + create(:conversation, account: account, inbox: inbox, assignee: user_1, contact: en_contact) create(:conversation, account: account, inbox: inbox) Current.account = account + + create(:custom_attribute_definition, + attribute_key: 'contact_additional_information', + account: account, + attribute_model: 'contact_attribute', + attribute_display_type: 'text') + create(:custom_attribute_definition, + attribute_key: 'customer_type', + account: account, + attribute_model: 'contact_attribute', + attribute_display_type: 'list', + attribute_values: %w[regular platinum gold]) + create(:custom_attribute_definition, + attribute_key: 'signed_in_at', + account: account, + attribute_model: 'contact_attribute', + attribute_display_type: 'date') end describe '#perform' do + before do + en_contact.add_labels(%w[random_label support]) + el_contact.update_labels('random_label') + cs_contact.update_labels('support') + + en_contact.update!(custom_attributes: { contact_additional_information: 'test custom data' }) + el_contact.update!(custom_attributes: { contact_additional_information: 'test custom data', customer_type: 'platinum' }) + cs_contact.update!(custom_attributes: { customer_type: 'platinum', signed_in_at: '2022-01-19' }) + end + context 'with query present' do let!(:params) { { payload: [], page: 1 } } let(:payload) do @@ -31,7 +60,7 @@ describe ::Contacts::FilterService do { attribute_key: 'name', filter_operator: 'equal_to', - values: [contact.name], + values: [en_contact.name], query_operator: nil }.with_indifferent_access ] @@ -40,22 +69,77 @@ describe ::Contacts::FilterService do it 'filter contacts by additional_attributes' do params[:payload] = payload result = filter_service.new(params, user_1).perform - expect(result.length).to be 2 + expect(result[:contacts].length).to be 1 end it 'filter contact by tags' do - Contact.last.update_labels('support') + label_id = cs_contact.labels.last.id params[:payload] = [ { attribute_key: 'labels', filter_operator: 'equal_to', - values: [1], + values: [label_id], query_operator: nil }.with_indifferent_access ] result = filter_service.new(params, user_1).perform - expect(result.length).to be 2 + 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', + filter_operator: 'equal_to', + values: ['platinum'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: [label_id], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'signed_in_at', + filter_operator: 'is_less_than', + values: ['2022-01-20'], + query_operator: nil + }.with_indifferent_access + ] + result = filter_service.new(params, user_1).perform + expect(result[:contacts].length).to be 1 + expect(result[:contacts].first.id).to eq(cs_contact.id) + end + + it 'filter by custom_attributes and additional_attributes' do + params[:payload] = [ + { + attribute_key: 'customer_type', + filter_operator: 'equal_to', + values: ['platinum'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'browser_language', + filter_operator: 'equal_to', + values: ['el'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'contact_additional_information', + filter_operator: 'equal_to', + values: ['test custom data'], + query_operator: nil + }.with_indifferent_access + ] + result = filter_service.new(params, user_1).perform + expect(result[:contacts].length).to be 1 + expect(result[:contacts].first.id).to eq(el_contact.id) end end end diff --git a/spec/services/conversations/filter_service_spec.rb b/spec/services/conversations/filter_service_spec.rb index e226e96d5..c199cd3e2 100644 --- a/spec/services/conversations/filter_service_spec.rb +++ b/spec/services/conversations/filter_service_spec.rb @@ -10,18 +10,43 @@ describe ::Conversations::FilterService do let!(:campaign_2) { create(:campaign, title: 'Campaign', account: account) } let!(:inbox) { create(:inbox, account: account, enable_auto_assignment: false) } + let!(:unassigned_conversation) { create(:conversation, account: account, inbox: inbox) } + let!(:user_2_assigned_conversation) { create(:conversation, account: account, inbox: inbox, assignee: user_2) } + let!(:en_conversation_1) do + create(:conversation, account: account, inbox: inbox, assignee: user_1, campaign_id: campaign_1.id, + status: 'pending', additional_attributes: { 'browser_language': 'en' }) + end + let!(:en_conversation_2) do + create(:conversation, account: account, inbox: inbox, assignee: user_1, campaign_id: campaign_2.id, + status: 'pending', additional_attributes: { 'browser_language': 'en' }) + end + before do create(:inbox_member, user: user_1, inbox: inbox) create(:inbox_member, user: user_2, inbox: inbox) - create(:conversation, account: account, inbox: inbox, assignee: user_1) - create(:conversation, account: account, inbox: inbox, assignee: user_1, campaign_id: campaign_1.id, - status: 'pending', additional_attributes: { 'browser_language': 'en' }) - create(:conversation, account: account, inbox: inbox, assignee: user_1, campaign_id: campaign_2.id, - status: 'pending', additional_attributes: { 'browser_language': 'en' }) - create(:conversation, account: account, inbox: inbox, assignee: user_2) - # unassigned conversation - create(:conversation, account: account, inbox: inbox) Current.account = account + + en_conversation_1.update!(custom_attributes: { conversation_additional_information: 'test custom data' }) + en_conversation_2.update!(custom_attributes: { conversation_additional_information: 'test custom data', conversation_type: 'platinum' }) + user_2_assigned_conversation.update!(custom_attributes: { conversation_type: 'platinum', conversation_created: '2022-01-19' }) + create(:conversation, account: account, inbox: inbox, assignee: user_1) + + create(:custom_attribute_definition, + attribute_key: 'conversation_type', + account: account, + attribute_model: 'conversation_attribute', + attribute_display_type: 'list', + attribute_values: %w[regular platinum gold]) + create(:custom_attribute_definition, + attribute_key: 'conversation_created', + account: account, + attribute_model: 'conversation_attribute', + attribute_display_type: 'date') + create(:custom_attribute_definition, + attribute_key: 'conversation_additional_information', + account: account, + attribute_model: 'conversation_attribute', + attribute_display_type: 'text') end describe '#perform' do @@ -44,14 +69,14 @@ describe ::Conversations::FilterService do ] end - it 'filter conversations by custom_attributes and status' do + it 'filter conversations by additional_attributes and status' do params[:payload] = payload result = filter_service.new(params, user_1).perform conversations = Conversation.where("additional_attributes ->> 'browser_language' IN (?) AND status IN (?)", ['en'], [1, 2]) expect(result.length).to be conversations.count end - it 'filter conversations by custom_attributes and status with pagination' do + it 'filter conversations by additional_attributes and status with pagination' do params[:payload] = payload params[:page] = 2 result = filter_service.new(params, user_1).perform @@ -60,7 +85,7 @@ describe ::Conversations::FilterService do end it 'filter conversations by tags' do - Conversation.last.update_labels('support') + unassigned_conversation.update_labels('support') params[:payload] = [ { attribute_key: 'assignee_id', @@ -107,4 +132,45 @@ describe ::Conversations::FilterService do end end end + + describe '#perform on custom attribute' do + context 'with query present' do + let!(:params) { { payload: [], page: 1 } } + let(:payload) do + [ + { + attribute_key: 'browser_language', + filter_operator: 'contains', + values: 'en', + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'status', + filter_operator: 'not_equal_to', + values: %w[resolved], + query_operator: nil + }.with_indifferent_access + ] + end + + it 'filter by custom_attributes and labels' do + params[:payload] = [ + { + attribute_key: 'conversation_type', + filter_operator: 'equal_to', + values: ['platinum'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'conversation_created', + filter_operator: 'is_less_than', + values: ['2022-01-20'], + query_operator: nil + }.with_indifferent_access + ] + result = filter_service.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + end + end end