fix: SQL error when rules with missing attributes is triggered (#8673)

app/services/automation_rules/condition_validation_service.rb

@@ -0,0 +1,56 @@
+class AutomationRules::ConditionValidationService
+  ATTRIBUTE_MODEL = 'conversation_attribute'.freeze
+
+  def initialize(rule)
+    @rule = rule
+    @account = rule.account
+
+    file = File.read('./lib/filters/filter_keys.json')
+    @filters = JSON.parse(file)
+
+    @conversation_filters = @filters['conversations']
+    @contact_filters = @filters['contacts']
+    @message_filters = @filters['messages']
+  end
+
+  def perform
+    @rule.conditions.each do |condition|
+      return false unless valid_condition?(condition)
+    end
+
+    true
+  end
+
+  private
+
+  def valid_condition?(condition)
+    key = condition['attribute_key']
+
+    conversation_filter = @conversation_filters[key]
+    contact_filter = @contact_filters[key]
+    message_filter = @message_filters[key]
+
+    if conversation_filter || contact_filter || message_filter
+      operation_valid?(condition, conversation_filter || contact_filter || message_filter)
+    else
+      custom_attribute_present?(key, condition['custom_attribute_type'])
+    end
+  end
+
+  def operation_valid?(condition, filter)
+    filter_operator = condition['filter_operator']
+
+    # attribute changed is a special case
+    return true if filter_operator == 'attribute_changed'
+
+    filter['filter_operators'].include?(filter_operator)
+  end
+
+  def custom_attribute_present?(attribute_key, attribute_model)
+    attribute_model = attribute_model.presence || self.class::ATTRIBUTE_MODEL
+
+    @account.custom_attribute_definitions.where(
+      attribute_model: attribute_model
+    ).find_by(attribute_key: attribute_key).present?
+  end
+end

app/services/automation_rules/conditions_filter_service.rb

@@ -5,19 +5,25 @@ class AutomationRules::ConditionsFilterService < FilterService
 
   def initialize(rule, conversation = nil, options = {})
     super([], nil)
+    # assign rule, conversation and account to instance variables
     @rule = rule
     @conversation = conversation
     @account = conversation.account
+
+    # setup filters from json file
     file = File.read('./lib/filters/filter_keys.json')
     @filters = JSON.parse(file)
+    @conversation_filters = @filters['conversations']
+    @contact_filters = @filters['contacts']
+    @message_filters = @filters['messages']
+
     @options = options
     @changed_attributes = options[:changed_attributes]
   end
 
   def perform
-    @conversation_filters = @filters['conversations']
-    @contact_filters = @filters['contacts']
-    @message_filters = @filters['messages']
+    return false unless rule_valid?
+
     @attribute_changed_query_filter = []
 
     @rule.conditions.each_with_index do |query_hash, current_index|
@@ -36,6 +42,14 @@ class AutomationRules::ConditionsFilterService < FilterService
     false
   end
 
+  def rule_valid?
+    is_valid = AutomationRules::ConditionValidationService.new(@rule).perform
+
+    Rails.logger.info "Automation rule condition validation failed for rule id: #{@rule.id}" unless is_valid
+
+    is_valid
+  end
+
   def filter_operation(query_hash, current_index)
     if query_hash[:filter_operator] == 'starts_with'
       @filter_values["value_#{current_index}"] = "#{string_filter_values(query_hash)}%"

lib/filters/filter_keys.json

@@ -118,7 +118,7 @@
       "attribute_name": "Phone Number",
       "input_type": "text",
       "data_type": "text",
-      "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain" ],
+      "filter_operators": [ "equal_to", "not_equal_to", "contains", "does_not_contain", "starts_with"],
       "attribute_type": "standard"
     },
     "email": {

spec/listeners/automation_rule_listener_old_spec.rb

@@ -83,6 +83,7 @@ describe AutomationRuleListener do
           {
             attribute_key: 'customer_type',
             filter_operator: 'equal_to',
+            custom_attribute_type: 'contact_attribute',
             values: ['platinum'],
             query_operator: 'AND'
           }.with_indifferent_access,
@@ -154,6 +155,7 @@ describe AutomationRuleListener do
             {
               attribute_key: 'customer_type',
               filter_operator: 'equal_to',
+              custom_attribute_type: 'contact_attribute',
               values: ['platinum'],
               query_operator: nil
             }.with_indifferent_access

spec/services/automation_rules/condition_validation_service_spec.rb

@@ -0,0 +1,105 @@
+require 'rails_helper'
+
+RSpec.describe AutomationRules::ConditionValidationService do
+  let(:account) { create(:account) }
+  let(:rule) { create(:automation_rule, account: account) }
+
+  describe '#perform' do
+    context 'with standard attributes' do
+      before do
+        rule.conditions = [
+          { 'values': ['open'], 'attribute_key': 'status', 'query_operator': nil, 'filter_operator': 'equal_to' },
+          { 'values': ['+918484'], 'attribute_key': 'phone_number', 'query_operator': 'OR', 'filter_operator': 'contains' },
+          { 'values': ['test'], 'attribute_key': 'email', 'query_operator': nil, 'filter_operator': 'contains' }
+        ]
+        rule.save
+      end
+
+      it 'returns true' do
+        expect(described_class.new(rule).perform).to be(true)
+      end
+    end
+
+    context 'with wrong attribute' do
+      before do
+        rule.conditions = [
+          { 'values': ['open'], 'attribute_key': 'not-a-standard-attribute-for-sure', 'query_operator': nil, 'filter_operator': 'equal_to' }
+        ]
+        rule.save
+      end
+
+      it 'returns false' do
+        expect(described_class.new(rule).perform).to be(false)
+      end
+    end
+
+    context 'with wrong filter operator' do
+      before do
+        rule.conditions = [
+          { 'values': ['open'], 'attribute_key': 'status', 'query_operator': nil, 'filter_operator': 'not-a-filter-operator' }
+        ]
+        rule.save
+      end
+
+      it 'returns false' do
+        expect(described_class.new(rule).perform).to be(false)
+      end
+    end
+
+    context 'with "attribute_changed" filter operator' do
+      before do
+        rule.conditions = [
+          { 'values': ['open'], 'attribute_key': 'status', 'query_operator': nil, 'filter_operator': 'attribute_changed' }
+        ]
+        rule.save
+      end
+
+      it 'returns true' do
+        expect(described_class.new(rule).perform).to be(true)
+      end
+    end
+
+    context 'with correct custom attribute' do
+      before do
+        create(:custom_attribute_definition,
+               attribute_key: 'custom_attr_priority',
+               account: account,
+               attribute_model: 'conversation_attribute',
+               attribute_display_type: 'list',
+               attribute_values: %w[P0 P1 P2])
+
+        rule.conditions = [
+          {
+            'values': ['true'],
+            'attribute_key': 'custom_attr_priority',
+            'filter_operator': 'equal_to',
+            'custom_attribute_type': 'conversation_attribute'
+          }
+        ]
+        rule.save
+      end
+
+      it 'returns true' do
+        expect(described_class.new(rule).perform).to be(true)
+      end
+    end
+
+    context 'with missing custom attribute' do
+      before do
+        rule.conditions = [
+          {
+            'values': ['true'],
+            'attribute_key': 'attribute_is_not_present', # the attribute is not present
+            'filter_operator': 'equal_to',
+            'custom_attribute_type': 'conversation_attribute'
+          }
+        ]
+        rule.save
+      end
+
+      it 'returns false for missing custom attribute' do
+        expect(described_class.new(rule).perform).to be(false)
+      end
+    end
+  end
+end