From f78f278e2fd2f30f829c9cb6df631661787de020 Mon Sep 17 00:00:00 2001 From: Pranav Date: Wed, 20 Mar 2024 03:59:36 -0700 Subject: [PATCH] fix: Update validations for filter service (#8239) - Refactor filter service for better readability and maintenance - Add validations for the following: - If an invalid attribute is passed, a custom exception InvalidAttribute will be thrown. - If an invalid operator is passed, a custom exception InvalidOperator will be thrown. - If an invalid value (currently checking only null check), a custom exception InvalidValue will be thrown. Fixes: https://linear.app/chatwoot/issue/CW-2702/activerecordstatementinvalid-pginvalidtextrepresentation-error-invalid Fixes: https://linear.app/chatwoot/issue/CW-2703/activerecordstatementinvalid-pginvaliddatetimeformat-error-invalid Fixes: https://linear.app/chatwoot/issue/CW-2700/activerecordstatementinvalid-pgsyntaxerror-error-syntax-error-at-or Co-authored-by: Sojan --- .../api/v1/accounts/contacts_controller.rb | 4 + .../v1/accounts/conversations_controller.rb | 4 + app/helpers/filter_helper.rb | 86 +++++++ .../condition_validation_service.rb | 4 +- .../conditions_filter_service.rb | 5 +- app/services/contacts/filter_service.rb | 41 +--- app/services/conversations/filter_service.rb | 40 +--- app/services/filter_service.rb | 26 ++- config/locales/en.yml | 4 +- lib/automation_rules/conditions.json | 195 ---------------- lib/custom_exceptions/custom_filter.rb | 19 ++ lib/filters/conversation_filters.json | 92 -------- lib/filters/filter_keys.json | 204 ---------------- lib/filters/filter_keys.yml | 220 ++++++++++++++++++ .../v1/accounts/contacts_controller_spec.rb | 42 +++- .../accounts/conversations_controller_spec.rb | 45 +++- spec/services/contacts/filter_service_spec.rb | 14 +- .../conversations/filter_service_spec.rb | 13 +- 18 files changed, 470 insertions(+), 588 deletions(-) create mode 100644 app/helpers/filter_helper.rb delete mode 100644 lib/automation_rules/conditions.json create mode 100644 lib/custom_exceptions/custom_filter.rb delete mode 100644 lib/filters/conversation_filters.json delete mode 100644 lib/filters/filter_keys.json create mode 100644 lib/filters/filter_keys.yml diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb index 71e9100e7..729db34b5 100644 --- a/app/controllers/api/v1/accounts/contacts_controller.rb +++ b/app/controllers/api/v1/accounts/contacts_controller.rb @@ -65,6 +65,10 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController contacts = result[:contacts] @contacts_count = result[:count] @contacts = fetch_contacts(contacts) + rescue CustomExceptions::CustomFilter::InvalidAttribute, + CustomExceptions::CustomFilter::InvalidOperator, + CustomExceptions::CustomFilter::InvalidValue => e + render_could_not_create_error(e.message) end def contactable_inboxes diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index d0d8f6d5b..2aedf1928 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -44,6 +44,10 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro result = ::Conversations::FilterService.new(params.permit!, current_user).perform @conversations = result[:conversations] @conversations_count = result[:count] + rescue CustomExceptions::CustomFilter::InvalidAttribute, + CustomExceptions::CustomFilter::InvalidOperator, + CustomExceptions::CustomFilter::InvalidValue => e + render_could_not_create_error(e.message) end def mute diff --git a/app/helpers/filter_helper.rb b/app/helpers/filter_helper.rb new file mode 100644 index 000000000..b3efc393a --- /dev/null +++ b/app/helpers/filter_helper.rb @@ -0,0 +1,86 @@ +module FilterHelper + def build_condition_query(model_filters, query_hash, current_index) + current_filter = model_filters[query_hash['attribute_key']] + + # Throw InvalidOperator Error if the attribute is a standard attribute + # and the operator is not allowed in the config + if current_filter.present? && current_filter['filter_operators'].exclude?(query_hash[:filter_operator]) + raise CustomExceptions::CustomFilter::InvalidOperator.new( + attribute_name: query_hash['attribute_key'], + allowed_keys: current_filter['filter_operators'] + ) + end + + # Every other filter expects a value to be present + if %w[is_present is_not_present].exclude?(query_hash[:filter_operator]) && query_hash['values'].blank? + raise CustomExceptions::CustomFilter::InvalidValue.new(attribute_name: query_hash['attribute_key']) + end + + condition_query = build_condition_query_string(current_filter, query_hash, current_index) + # The query becomes empty only when it doesn't match to any supported + # standard attribute or custom attribute defined in the account. + if condition_query.empty? + raise CustomExceptions::CustomFilter::InvalidAttribute.new(key: query_hash['attribute_key'], + allowed_keys: model_filters.keys) + end + + condition_query + end + + def build_condition_query_string(current_filter, query_hash, current_index) + filter_operator_value = filter_operation(query_hash, current_index) + + return handle_nil_filter(query_hash, current_index) if current_filter.nil? + + case current_filter['attribute_type'] + when 'additional_attributes' + handle_additional_attributes(query_hash, filter_operator_value) + else + handle_standard_attributes(current_filter, query_hash, current_index, filter_operator_value) + end + end + + def handle_nil_filter(query_hash, current_index) + attribute_type = "#{filter_config[:entity].downcase}_attribute" + custom_attribute_query(query_hash, attribute_type, current_index) + end + + # TODO: Change the reliance on entity instead introduce datatype text_case_insensive + # Then we can remove the condition for Contact + def handle_additional_attributes(query_hash, filter_operator_value) + if filter_config[:entity] == 'Contact' + "LOWER(#{filter_config[:table_name]}.additional_attributes ->> '#{query_hash[:attribute_key]}') " \ + "#{filter_operator_value} #{query_hash[:query_operator]}" + else + "#{filter_config[:table_name]}.additional_attributes ->> '#{query_hash[:attribute_key]}' " \ + "#{filter_operator_value} #{query_hash[:query_operator]} " + end + end + + def handle_standard_attributes(current_filter, query_hash, current_index, filter_operator_value) + case current_filter['data_type'] + when 'date' + date_filter(current_filter, query_hash, filter_operator_value) + when 'labels' + tag_filter_query(query_hash, current_index) + else + default_filter(query_hash, filter_operator_value) + end + end + + def date_filter(current_filter, query_hash, filter_operator_value) + "(#{filter_config[:table_name]}.#{query_hash[:attribute_key]})::#{current_filter['data_type']} " \ + "#{filter_operator_value}#{current_filter['data_type']} #{query_hash[:query_operator]}" + end + + # TODO: Change the reliance on entity instead introduce datatype text_case_insensive + # Then we can remove the condition for Contact + def default_filter(query_hash, filter_operator_value) + if filter_config[:entity] == 'Contact' + "LOWER(#{filter_config[:table_name]}.#{query_hash[:attribute_key]}) " \ + "#{filter_operator_value} #{query_hash[:query_operator]}" + else + "#{filter_config[:table_name]}.#{query_hash[:attribute_key]} #{filter_operator_value} #{query_hash[:query_operator]}" + end + end +end diff --git a/app/services/automation_rules/condition_validation_service.rb b/app/services/automation_rules/condition_validation_service.rb index 6f9f9a7c3..63e58b5bc 100644 --- a/app/services/automation_rules/condition_validation_service.rb +++ b/app/services/automation_rules/condition_validation_service.rb @@ -5,8 +5,8 @@ class AutomationRules::ConditionValidationService @rule = rule @account = rule.account - file = File.read('./lib/filters/filter_keys.json') - @filters = JSON.parse(file) + file = File.read('./lib/filters/filter_keys.yml') + @filters = YAML.safe_load(file) @conversation_filters = @filters['conversations'] @contact_filters = @filters['contacts'] diff --git a/app/services/automation_rules/conditions_filter_service.rb b/app/services/automation_rules/conditions_filter_service.rb index 44b72db9e..daf9cb7e2 100644 --- a/app/services/automation_rules/conditions_filter_service.rb +++ b/app/services/automation_rules/conditions_filter_service.rb @@ -11,8 +11,9 @@ class AutomationRules::ConditionsFilterService < FilterService @account = conversation.account # setup filters from json file - file = File.read('./lib/filters/filter_keys.json') - @filters = JSON.parse(file) + file = File.read('./lib/filters/filter_keys.yml') + @filters = YAML.safe_load(file) + @conversation_filters = @filters['conversations'] @contact_filters = @filters['contacts'] @message_filters = @filters['messages'] diff --git a/app/services/contacts/filter_service.rb b/app/services/contacts/filter_service.rb index 70062cad0..d3b24e4e2 100644 --- a/app/services/contacts/filter_service.rb +++ b/app/services/contacts/filter_service.rb @@ -2,7 +2,7 @@ class Contacts::FilterService < FilterService ATTRIBUTE_MODEL = 'contact_attribute'.freeze def perform - @contacts = contact_query_builder + @contacts = query_builder(@filters['contacts']) { contacts: @contacts, @@ -10,38 +10,6 @@ class Contacts::FilterService < FilterService } end - def contact_query_builder - contact_filters = @filters['contacts'] - - @params[:payload].each_with_index do |query_hash, current_index| - current_filter = contact_filters[query_hash['attribute_key']] - @query_string += contact_query_string(current_filter, query_hash, current_index) - end - - base_relation.where(@query_string, @filter_values.with_indifferent_access) - end - - def contact_query_string(current_filter, query_hash, current_index) - attribute_key = query_hash[:attribute_key] - query_operator = query_hash[:query_operator] - filter_operator_value = filter_operation(query_hash, current_index) - - return custom_attribute_query(query_hash, 'contact_attribute', 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} " - when 'date_attributes' - " (contacts.#{attribute_key})::#{current_filter['data_type']} #{filter_operator_value}#{current_filter['data_type']} #{query_operator} " - when 'standard' - if attribute_key == 'labels' - " #{tag_filter_query('Contact', 'contacts', query_hash, current_index)} " - else - " LOWER(contacts.#{attribute_key}) #{filter_operator_value} #{query_operator} " - end - end - end - def filter_values(query_hash) current_val = query_hash['values'][0] if query_hash['attribute_key'] == 'phone_number' @@ -57,6 +25,13 @@ class Contacts::FilterService < FilterService Current.account.contacts end + def filter_config + { + entity: 'Contact', + table_name: 'contacts' + } + end + private def equals_to_filter_string(filter_operator, current_index) diff --git a/app/services/conversations/filter_service.rb b/app/services/conversations/filter_service.rb index 87384017c..3a06959b8 100644 --- a/app/services/conversations/filter_service.rb +++ b/app/services/conversations/filter_service.rb @@ -7,7 +7,7 @@ class Conversations::FilterService < FilterService end def perform - @conversations = conversation_query_builder + @conversations = query_builder(@filters['conversations']) mine_count, unassigned_count, all_count, = set_count_for_all_conversations assigned_count = all_count - unassigned_count @@ -22,37 +22,6 @@ class Conversations::FilterService < FilterService } end - def conversation_query_builder - conversation_filters = @filters['conversations'] - @params[:payload].each_with_index do |query_hash, current_index| - current_filter = conversation_filters[query_hash['attribute_key']] - @query_string += conversation_query_string(current_filter, query_hash, current_index) - end - - base_relation.where(@query_string, @filter_values.with_indifferent_access) - end - - def conversation_query_string(current_filter, query_hash, current_index) - attribute_key = query_hash[:attribute_key] - query_operator = query_hash[:query_operator] - filter_operator_value = filter_operation(query_hash, current_index) - - return custom_attribute_query(query_hash, 'conversation_attribute', current_index) if current_filter.nil? - - case current_filter['attribute_type'] - when 'additional_attributes' - " conversations.additional_attributes ->> '#{attribute_key}' #{filter_operator_value} #{query_operator} " - when 'date_attributes' - " (conversations.#{attribute_key})::#{current_filter['data_type']} #{filter_operator_value}#{current_filter['data_type']} #{query_operator} " - when 'standard' - if attribute_key == 'labels' - " #{tag_filter_query('Conversation', 'conversations', query_hash, current_index)} " - else - " conversations.#{attribute_key} #{filter_operator_value} #{query_operator} " - end - end - end - def base_relation @account.conversations.includes( :taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team, :messages, :contact_inbox @@ -63,6 +32,13 @@ class Conversations::FilterService < FilterService @params[:page] || 1 end + def filter_config + { + entity: 'Conversation', + table_name: 'conversations' + } + end + def conversations @conversations.sort_on_last_activity_at.page(current_page) end diff --git a/app/services/filter_service.rb b/app/services/filter_service.rb index f1a699811..c545ac7d8 100644 --- a/app/services/filter_service.rb +++ b/app/services/filter_service.rb @@ -1,6 +1,9 @@ require 'json' class FilterService + include FilterHelper + include CustomExceptions::CustomFilter + ATTRIBUTE_MODEL = 'conversation_attribute'.freeze ATTRIBUTE_TYPES = { date: 'date', text: 'text', number: 'numeric', link: 'text', list: 'text', checkbox: 'boolean' @@ -9,8 +12,8 @@ class FilterService def initialize(params, user) @params = params @user = user - file = File.read('./lib/filters/filter_keys.json') - @filters = JSON.parse(file) + file = File.read('./lib/filters/filter_keys.yml') + @filters = YAML.safe_load(file) @query_string = '' @filter_values = {} end @@ -106,7 +109,9 @@ class FilterService ] end - def tag_filter_query(model_name, table_name, query_hash, current_index) + def tag_filter_query(query_hash, current_index) + model_name = filter_config[:entity] + table_name = filter_config[:table_name] query_operator = query_hash[:query_operator] @filter_values["value_#{current_index}"] = filter_values(query_hash) @@ -130,10 +135,8 @@ class FilterService def custom_attribute_query(query_hash, custom_attribute_type, current_index) @attribute_key = query_hash[:attribute_key] @custom_attribute_type = custom_attribute_type - attribute_data_type - - return ' ' if @custom_attribute.blank? + return '' if @custom_attribute.blank? build_custom_attr_query(query_hash, current_index) end @@ -155,9 +158,9 @@ class FilterService table_name = attribute_model == 'conversation_attribute' ? 'conversations' : 'contacts' query = if attribute_data_type == 'text' - " LOWER(#{table_name}.custom_attributes ->> '#{@attribute_key}')::#{attribute_data_type} #{filter_operator_value} #{query_operator} " + "LOWER(#{table_name}.custom_attributes ->> '#{@attribute_key}')::#{attribute_data_type} #{filter_operator_value} #{query_operator} " else - " (#{table_name}.custom_attributes ->> '#{@attribute_key}')::#{attribute_data_type} #{filter_operator_value} #{query_operator} " + "(#{table_name}.custom_attributes ->> '#{@attribute_key}')::#{attribute_data_type} #{filter_operator_value} #{query_operator} " end query + not_in_custom_attr_query(table_name, query_hash, attribute_data_type) @@ -194,4 +197,11 @@ class FilterService "NOT LIKE :value_#{current_index}" end + + def query_builder(model_filters) + @params[:payload].each_with_index do |query_hash, current_index| + @query_string += " #{build_condition_query(model_filters, query_hash, current_index).strip}" + end + base_relation.where(@query_string, @filter_values.with_indifferent_access) + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index c19499fa5..4c4f0483c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -77,7 +77,9 @@ en: name: should not start or end with symbols, and it should not have < > / \ @ characters. custom_filters: number_of_records: Limit reached. The maximum number of allowed custom filters for a user per account is 50. - + invalid_attribute: Invalid attribute key - [%{key}]. The key should be one of [%{allowed_keys}] or a custom attribute defined in the account. + invalid_operator: Invalid operator. The allowed operators for %{attribute_name} are [%{allowed_keys}]. + invalid_value: Invalid value. The values provided for %{attribute_name} are invalid reports: period: Reporting period %{since} to %{until} utc_warning: The report generated is in UTC timezone diff --git a/lib/automation_rules/conditions.json b/lib/automation_rules/conditions.json deleted file mode 100644 index ba4bd4b5b..000000000 --- a/lib/automation_rules/conditions.json +++ /dev/null @@ -1,195 +0,0 @@ -{ - "conversations": { - "status": { - "attribute_name": "Status", - "input_type": "multi_select", - "table_name": "conversations", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "standard" - }, - "assignee_id": { - "attribute_name": "Assignee Name", - "input_type": "search_box with name tags/plain text", - "table_name": "conversations", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "contact_id": { - "attribute_name": "Contact Name", - "input_type": "plain_text", - "table_name": "conversations", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "inbox_id": { - "attribute_name": "Inbox Name", - "input_type": "search_box", - "table_name": "conversations", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "team_id": { - "attribute_name": "Team Name", - "input_type": "search_box", - "table_name": "conversations", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "id": { - "attribute_name": "Conversation Identifier", - "input_type": "textbox", - "table_name": "conversations", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "campaign_id": { - "attribute_name": "Campaign Name", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "labels": { - "attribute_name": "Labels", - "input_type": "tags", - "data_type": "text", - "filter_operators": ["exactly_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "browser_language": { - "attribute_name": "Browser Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "conversation_language": { - "attribute_name": "Conversation Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "additional_attributes" - }, - "mail_subject": { - "attribute_name": "Email Subject", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "country_code": { - "attribute_name": "Country Name", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - }, - "referer": { - "attribute_name": "Referer link", - "input_type": "textbox", - "data_type": "link", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - }, - "plan": { - "attribute_name": "Plan", - "input_type": "multi_select", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - } - }, - "contacts": { - "assignee_id": { - "attribute_name": "Assignee Name", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "phone_number": { - "attribute_name": "Phone Number", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "starts_with" ], - "attribute_type": "standard" - }, - "contact_id": { - "attribute_name": "Contact Name", - "input_type": "plain_text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "inbox_id": { - "attribute_name": "Inbox Name", - "input_type": "search_box", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "team_id": { - "attribute_name": "Team Name", - "input_type": "search_box", - "data_type": "number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "id": { - "attribute_name": "Conversation Identifier", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "campaign_id": { - "attribute_name": "Campaign Name", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "labels": { - "attribute_name": "Labels", - "input_type": "tags", - "data_type": "text", - "filter_operators": ["exactly_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "browser_language": { - "attribute_name": "Browser Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "mail_subject": { - "attribute_name": "Email Subject", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "email": { - "attribute_name": "Email", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "country_code": { - "attribute_name": "Country Name", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - }, - "referer": { - "attribute_name": "Referer link", - "input_type": "textbox", - "data_type": "link", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - } - } -} diff --git a/lib/custom_exceptions/custom_filter.rb b/lib/custom_exceptions/custom_filter.rb new file mode 100644 index 000000000..03ff9ec7a --- /dev/null +++ b/lib/custom_exceptions/custom_filter.rb @@ -0,0 +1,19 @@ +module CustomExceptions::CustomFilter + class InvalidAttribute < CustomExceptions::Base + def message + I18n.t('errors.custom_filters.invalid_attribute', key: @data[:key], allowed_keys: @data[:allowed_keys].join(',')) + end + end + + class InvalidOperator < CustomExceptions::Base + def message + I18n.t('errors.custom_filters.invalid_operator', attribute_name: @data[:attribute_name], allowed_keys: @data[:allowed_keys].join(',')) + end + end + + class InvalidValue < CustomExceptions::Base + def message + I18n.t('errors.custom_filters.invalid_value', attribute_name: @data[:attribute_name]) + end + end +end diff --git a/lib/filters/conversation_filters.json b/lib/filters/conversation_filters.json deleted file mode 100644 index 39f58f5c6..000000000 --- a/lib/filters/conversation_filters.json +++ /dev/null @@ -1,92 +0,0 @@ -{ - "conversations": [ - { - "attribute_key": "status", - "attribute_name": "Status", - "input_type": "multi_select", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "standard" - }, - { - "attribute_key": "assigne", - "attribute_name": "Assignee Name", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - { - "attribute_key": "contact", - "attribute_name": "Contact Name", - "input_type": "plain_text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - { - "attribute_key": "inbox", - "attribute_name": "Inbox Name", - "input_type": "search_box", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - { - "attribute_key": "team_id", - "attribute_name": "Team Name", - "input_type": "search_box", - "data_type": "number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - { - "attribute_key": "id", - "attribute_name": "Conversation Identifier", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - { - "attribute_key": "campaign_id", - "attribute_name": "Campaign Name", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - { - "attribute_key": "labels", - "attribute_name": "Labels", - "input_type": "tags", - "data_type": "text", - "filter_operators": ["exactly_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - { - "attribute_key": "browser_language", - "attribute_name": "Browser Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - { - "attribute_key": "country_code", - "attribute_name": "Country Name", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - }, - { - "attribute_key": "referer", - "attribute_name": "Referer link", - "input_type": "textbox", - "data_type": "link", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - } - ] -} diff --git a/lib/filters/filter_keys.json b/lib/filters/filter_keys.json deleted file mode 100644 index 9266d9bea..000000000 --- a/lib/filters/filter_keys.json +++ /dev/null @@ -1,204 +0,0 @@ -{ - "conversations": { - "status": { - "attribute_name": "Status", - "input_type": "multi_select", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "standard" - }, - "assignee_id": { - "attribute_name": "Assignee Name", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "contact_id": { - "attribute_name": "Contact Name", - "input_type": "plain_text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "inbox_id": { - "attribute_name": "Inbox Name", - "input_type": "search_box", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "team_id": { - "attribute_name": "Team Name", - "input_type": "search_box", - "data_type": "number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "display_id": { - "attribute_name": "Conversation Identifier", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "campaign_id": { - "attribute_name": "Campaign Name", - "input_type": "textbox", - "data_type": "Number", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present" ], - "attribute_type": "standard" - }, - "labels": { - "attribute_name": "Labels", - "input_type": "tags", - "data_type": "text", - "filter_operators": ["exactly_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "browser_language": { - "attribute_name": "Browser Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "conversation_language": { - "attribute_name": "Conversation Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "additional_attributes" - }, - "country_code": { - "attribute_name": "Country Name", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - }, - "referer": { - "attribute_name": "Referer link", - "input_type": "textbox", - "data_type": "link", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "present", "is_not_present" ], - "attribute_type": "additional_attributes" - }, - "created_at": { - "attribute_name": "Created At", - "input_type": "date", - "data_type": "date", - "filter_operators": [ "is_greater_than", "is_less_than", "days_before" ], - "attribute_type": "date_attributes" - }, - "last_activity_at": { - "attribute_name": "Created At", - "input_type": "date", - "data_type": "date", - "filter_operators": [ "is_greater_than", "is_less_than", "days_before" ], - "attribute_type": "date_attributes" - }, - "mail_subject": { - "attribute_name": "Email Subject", - "input_type": "text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain"], - "attribute_type": "additional_attributes" - } - }, - "contacts": { - "name": { - "attribute_name": "Name", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "phone_number": { - "attribute_name": "Phone Number", - "input_type": "text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "starts_with"], - "attribute_type": "standard" - }, - "email": { - "attribute_name": "Email", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "identifier": { - "attribute_name": "Contact Identifier", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "standard" - }, - "country_code": { - "attribute_name": "Country", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "additional_attributes" - }, - "city": { - "attribute_name": "City", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "browser_language": { - "attribute_name": "Browser Language", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "company": { - "attribute_name": "Company", - "input_type": "textbox", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "additional_attributes" - }, - "labels": { - "attribute_name": "Labels", - "input_type": "tags", - "data_type": "text", - "filter_operators": ["exactly_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - }, - "created_at": { - "attribute_name": "Created At", - "input_type": "date", - "data_type": "date", - "filter_operators": [ "is_greater_than", "is_less_than", "days_before" ], - "attribute_type": "date_attributes" - }, - "last_activity_at": { - "attribute_name": "Created At", - "input_type": "date", - "data_type": "date", - "filter_operators": [ "is_greater_than", "is_less_than", "days_before" ], - "attribute_type": "date_attributes" - } - }, - "messages": { - "message_type": { - "attribute_name": "Message Type", - "input_type": "search_box with name tags/plain text", - "data_type": "numeric", - "filter_operators": [ "equal_to", "not_equal_to" ], - "attribute_type": "standard" - }, - "content": { - "attribute_name": "Message Content", - "input_type": "search_box with name tags/plain text", - "data_type": "text", - "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ], - "attribute_type": "standard" - } - } -} diff --git a/lib/filters/filter_keys.yml b/lib/filters/filter_keys.yml new file mode 100644 index 000000000..afdcc6be7 --- /dev/null +++ b/lib/filters/filter_keys.yml @@ -0,0 +1,220 @@ +## This file contains the filter configurations which we use for the following +# 1. Conversation Filters (app/services/filter_service.rb) +# 2. Contact Filters (app/services/filter_service.rb) +# 3. Automation Filters (app/services/automation_rules/conditions_filter_service.rb), (app/services/automation_rules/condition_validation_service.rb) + + +# Format +# - Parent Key (conversation, contact, messages) +# - Key (attribute_name) +# - attribute_type: "standard" : supported ["standard", "additional_attributes (only for conversations and messages)"] +# - data_type: "text" : supported ["text", "number", "labels", "date", "link"] +# - filter_operators: ["equal_to", "not_equal_to", "contains", "does_not_contain", "is_present", "is_not_present", "is_greater_than", "is_less_than", "days_before", "starts_with"] + +### ----- Conversation Filters ----- ### + +conversations: + status: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + assignee_id: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "is_present" + - "is_not_present" + inbox_id: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "is_present" + - "is_not_present" + team_id: + attribute_type: "standard" + data_type: "number" + filter_operators: + - "equal_to" + - "not_equal_to" + - "is_present" + - "is_not_present" + display_id: + attribute_type: "standard" + data_type: "Number" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + campaign_id: + attribute_type: "standard" + data_type: "Number" + filter_operators: + - "equal_to" + - "not_equal_to" + - "is_present" + - "is_not_present" + labels: + attribute_type: "standard" + data_type: "labels" + filter_operators: + - "equal_to" + - "not_equal_to" + - "is_present" + - "is_not_present" + browser_language: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + conversation_language: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + country_code: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + referer: + attribute_type: "additional_attributes" + data_type: "link" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + created_at: + attribute_type: "standard" + data_type: "date" + filter_operators: + - "is_greater_than" + - "is_less_than" + - "days_before" + last_activity_at: + attribute_type: "standard" + data_type: "date" + filter_operators: + - "is_greater_than" + - "is_less_than" + - "days_before" + mail_subject: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + +### ----- End of Conversation Filters ----- ### + + +### ----- Contact Filters ----- ### +contacts: + name: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + phone_number: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + - "starts_with" + email: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + identifier: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + country_code: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + city: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + company: + attribute_type: "additional_attributes" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + labels: + attribute_type: "standard" + data_type: "labels" + filter_operators: + - "equal_to" + - "not_equal_to" + - "is_present" + - "is_not_present" + created_at: + attribute_type: "standard" + data_type: "date" + filter_operators: + - "is_greater_than" + - "is_less_than" + - "days_before" + last_activity_at: + attribute_type: "standard" + data_type: "date" + filter_operators: + - "is_greater_than" + - "is_less_than" + - "days_before" + +### ----- End of Contact Filters ----- ### + +### ----- Message Filters ----- ### +messages: + message_type: + attribute_type: "standard" + data_type: "numeric" + filter_operators: + - "equal_to" + - "not_equal_to" + content: + attribute_type: "standard" + data_type: "text" + filter_operators: + - "equal_to" + - "not_equal_to" + - "contains" + - "does_not_contain" + +### ----- End of Message Filters ----- ### diff --git a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb index d2527e6a9..37e64357f 100644 --- a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb @@ -338,14 +338,18 @@ RSpec.describe 'Contacts API', type: :request do context 'when it is an authenticated user' do let(:admin) { create(:user, account: account, role: :administrator) } - let!(:contact1) { create(:contact, :with_email, account: account) } - let!(:contact2) { create(:contact, :with_email, name: 'testcontact', account: account, email: 'test@test.com') } + let!(:contact1) { create(:contact, :with_email, account: account, additional_attributes: { country_code: 'US' }) } + let!(:contact2) do + create(:contact, :with_email, name: 'testcontact', account: account, email: 'test@test.com', additional_attributes: { country_code: 'US' }) + end it 'returns all contacts when query is empty' do post "/api/v1/accounts/#{account.id}/contacts/filter", - params: { - payload: [] - }, + params: { payload: [ + attribute_key: 'country_code', + filter_operator: 'equal_to', + values: ['US'] + ] }, headers: admin.create_new_auth_token, as: :json @@ -353,6 +357,34 @@ RSpec.describe 'Contacts API', type: :request do expect(response.body).to include(contact2.email) expect(response.body).to include(contact1.email) end + + it 'returns error the query operator is invalid' do + post "/api/v1/accounts/#{account.id}/contacts/filter", + params: { payload: [ + attribute_key: 'country_code', + filter_operator: 'eq', + values: ['US'] + ] }, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.body).to include('Invalid operator. The allowed operators for country_code are [equal_to,not_equal_to]') + end + + it 'returns error the query value is invalid' do + post "/api/v1/accounts/#{account.id}/contacts/filter", + params: { payload: [ + attribute_key: 'country_code', + filter_operator: 'equal_to', + values: [] + ] }, + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.body).to include('Invalid value. The values provided for country_code are invalid"') + end end end diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index 70f5b82ee..d93886fc3 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -152,17 +152,56 @@ RSpec.describe 'Conversations API', type: :request do create(:inbox_member, user: agent, inbox: conversation.inbox) end - it 'returns all conversations with empty query' do + it 'returns all conversations matching the query' do post "/api/v1/accounts/#{account.id}/conversations/filter", headers: agent.create_new_auth_token, - params: { payload: [] }, + params: { + payload: [{ + attribute_key: 'status', + filter_operator: 'equal_to', + values: ['open'] + }] + }, as: :json expect(response).to have_http_status(:success) response_data = JSON.parse(response.body, symbolize_names: true) - expect(response_data.count).to eq(2) end + + it 'returns error if the filters contain invalid attributes' do + post "/api/v1/accounts/#{account.id}/conversations/filter", + headers: agent.create_new_auth_token, + params: { + payload: [{ + attribute_key: 'phone_number', + filter_operator: 'equal_to', + values: ['open'] + }] + }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + response_data = JSON.parse(response.body, symbolize_names: true) + expect(response_data[:error]).to include('Invalid attribute key - [phone_number]') + end + + it 'returns error if the filters contain invalid operator' do + post "/api/v1/accounts/#{account.id}/conversations/filter", + headers: agent.create_new_auth_token, + params: { + payload: [{ + attribute_key: 'status', + filter_operator: 'eq', + values: ['open'] + }] + }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + response_data = JSON.parse(response.body, symbolize_names: true) + expect(response_data[:error]).to eq('Invalid operator. The allowed operators for status are [equal_to,not_equal_to].') + end end end diff --git a/spec/services/contacts/filter_service_spec.rb b/spec/services/contacts/filter_service_spec.rb index 572d2b205..e8c0ff184 100644 --- a/spec/services/contacts/filter_service_spec.rb +++ b/spec/services/contacts/filter_service_spec.rb @@ -7,9 +7,9 @@ describe Contacts::FilterService do let!(:first_user) { create(:user, account: account) } let!(:second_user) { create(:user, account: account) } let!(:inbox) { create(:inbox, account: account, enable_auto_assignment: false) } - 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' }) } + let!(:en_contact) { create(:contact, account: account, additional_attributes: { 'country_code': 'uk' }) } + let!(:el_contact) { create(:contact, account: account, additional_attributes: { 'country_code': 'gr' }) } + let!(:cs_contact) { create(:contact, account: account, additional_attributes: { 'country_code': 'cz' }) } before do create(:inbox_member, user: first_user, inbox: inbox) @@ -51,9 +51,9 @@ describe Contacts::FilterService do let(:payload) do [ { - attribute_key: 'browser_language', + attribute_key: 'country_code', filter_operator: 'equal_to', - values: ['en'], + values: ['uk'], query_operator: nil }.with_indifferent_access ] @@ -181,9 +181,9 @@ describe Contacts::FilterService do query_operator: 'AND' }.with_indifferent_access, { - attribute_key: 'browser_language', + attribute_key: 'country_code', filter_operator: 'equal_to', - values: ['el'], + values: ['GR'], query_operator: 'AND' }.with_indifferent_access, { diff --git a/spec/services/conversations/filter_service_spec.rb b/spec/services/conversations/filter_service_spec.rb index cb2279acf..2fbaf5f61 100644 --- a/spec/services/conversations/filter_service_spec.rb +++ b/spec/services/conversations/filter_service_spec.rb @@ -55,7 +55,7 @@ describe Conversations::FilterService do [ { attribute_key: 'browser_language', - filter_operator: 'contains', + filter_operator: 'equal_to', values: 'en', query_operator: 'AND', custom_attribute_type: '' @@ -88,7 +88,7 @@ describe Conversations::FilterService do it 'filters items with contains filter_operator with values being an array' do params[:payload] = [{ attribute_key: 'browser_language', - filter_operator: 'contains', + filter_operator: 'equal_to', values: %w[tr fr], query_operator: '', custom_attribute_type: '' @@ -106,7 +106,7 @@ describe Conversations::FilterService do it 'filters items with does not contain filter operator with values being an array' do params[:payload] = [{ attribute_key: 'browser_language', - filter_operator: 'does_not_contain', + filter_operator: 'not_equal_to', values: %w[tr en], query_operator: '', custom_attribute_type: '' @@ -291,6 +291,11 @@ describe Conversations::FilterService do end it 'filter by custom_attributes and additional_attributes' do + conversations = user_1.conversations + conversations[0].update!(additional_attributes: { 'browser_language': 'en' }, custom_attributes: { conversation_type: 'silver' }) + conversations[1].update!(additional_attributes: { 'browser_language': 'en' }, custom_attributes: { conversation_type: 'platinum' }) + conversations[2].update!(additional_attributes: { 'browser_language': 'tr' }, custom_attributes: { conversation_type: 'platinum' }) + params[:payload] = [ { attribute_key: 'conversation_type', @@ -301,7 +306,7 @@ describe Conversations::FilterService do }.with_indifferent_access, { attribute_key: 'browser_language', - filter_operator: 'is_equal_to', + filter_operator: 'not_equal_to', values: 'en', query_operator: nil, custom_attribute_type: ''