From 9527ff62699d9b0868070810a4b1a612d150939b Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Thu, 18 Sep 2025 14:17:54 +0530 Subject: [PATCH] feat: Add support for labels in automations (#11658) - Add support for using labels as an action event for automation - Fix duplicated conversation_updated event dispatch for labels Fixes https://github.com/chatwoot/chatwoot/issues/8539 and multiple issues around duplication related to label change events. --------- Co-authored-by: Muhsin Keloth --- .../composables/useAutomationValues.js | 1 + .../dashboard/helper/automationHelper.js | 2 + .../dashboard/i18n/locale/en/automation.json | 3 +- .../settings/automation/constants.js | 24 ++ app/models/automation_rule.rb | 2 +- app/models/concerns/labelable.rb | 2 + app/models/conversation.rb | 2 - .../conditions_filter_service.rb | 39 ++- .../automation_rule_listener_labels_spec.rb | 244 ++++++++++++++++++ spec/models/automation_rule_spec.rb | 26 ++ spec/models/conversation_spec.rb | 2 +- .../automation_rules/action_service_spec.rb | 39 +++ .../conditions_filter_service_spec.rb | 81 ++++++ 13 files changed, 461 insertions(+), 6 deletions(-) create mode 100644 spec/listeners/automation_rule_listener_labels_spec.rb diff --git a/app/javascript/dashboard/composables/useAutomationValues.js b/app/javascript/dashboard/composables/useAutomationValues.js index abc44f66b..5279f15e4 100644 --- a/app/javascript/dashboard/composables/useAutomationValues.js +++ b/app/javascript/dashboard/composables/useAutomationValues.js @@ -104,6 +104,7 @@ export default function useAutomationValues() { contacts: contacts.value, customAttributes: getters['attributes/getAttributes'].value, inboxes: inboxes.value, + labels: labels.value, statusFilterOptions: statusFilterOptions.value, priorityOptions: priorityOptions.value, messageTypeOptions: messageTypeOptions.value, diff --git a/app/javascript/dashboard/helper/automationHelper.js b/app/javascript/dashboard/helper/automationHelper.js index 3723fd4d5..3e5f46f90 100644 --- a/app/javascript/dashboard/helper/automationHelper.js +++ b/app/javascript/dashboard/helper/automationHelper.js @@ -124,6 +124,7 @@ export const getConditionOptions = ({ customAttributes, inboxes, languages, + labels, statusFilterOptions, teams, type, @@ -150,6 +151,7 @@ export const getConditionOptions = ({ country_code: countries, message_type: messageTypeOptions, priority: priorityOptions, + labels: generateConditionOptions(labels, 'title'), }; return conditionFilterMaps[type]; diff --git a/app/javascript/dashboard/i18n/locale/en/automation.json b/app/javascript/dashboard/i18n/locale/en/automation.json index 80274f488..43245a1d5 100644 --- a/app/javascript/dashboard/i18n/locale/en/automation.json +++ b/app/javascript/dashboard/i18n/locale/en/automation.json @@ -177,7 +177,8 @@ "REFERER_LINK": "Referrer Link", "ASSIGNEE_NAME": "Assignee", "TEAM_NAME": "Team", - "PRIORITY": "Priority" + "PRIORITY": "Priority", + "LABELS": "Labels" } } } diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js index 0a6905039..bc767040b 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js @@ -68,6 +68,12 @@ export const AUTOMATIONS = { inputType: 'plain_text', filterOperators: OPERATOR_TYPES_6, }, + { + key: 'labels', + name: 'LABELS', + inputType: 'multi_select', + filterOperators: OPERATOR_TYPES_3, + }, ], actions: [ { @@ -186,6 +192,12 @@ export const AUTOMATIONS = { inputType: 'multi_select', filterOperators: OPERATOR_TYPES_1, }, + { + key: 'labels', + name: 'LABELS', + inputType: 'multi_select', + filterOperators: OPERATOR_TYPES_3, + }, ], actions: [ { @@ -308,6 +320,12 @@ export const AUTOMATIONS = { inputType: 'multi_select', filterOperators: OPERATOR_TYPES_1, }, + { + key: 'labels', + name: 'LABELS', + inputType: 'multi_select', + filterOperators: OPERATOR_TYPES_3, + }, ], actions: [ { @@ -424,6 +442,12 @@ export const AUTOMATIONS = { inputType: 'multi_select', filterOperators: OPERATOR_TYPES_1, }, + { + key: 'labels', + name: 'LABELS', + inputType: 'multi_select', + filterOperators: OPERATOR_TYPES_3, + }, ], actions: [ { diff --git a/app/models/automation_rule.rb b/app/models/automation_rule.rb index 6f3f47d9c..9dc4d97eb 100644 --- a/app/models/automation_rule.rb +++ b/app/models/automation_rule.rb @@ -36,7 +36,7 @@ class AutomationRule < ApplicationRecord def conditions_attributes %w[content email country_code status message_type browser_language assignee_id team_id referer city company inbox_id - mail_subject phone_number priority conversation_language] + mail_subject phone_number priority conversation_language labels] end def actions_attributes diff --git a/app/models/concerns/labelable.rb b/app/models/concerns/labelable.rb index e710e97e9..bf8778921 100644 --- a/app/models/concerns/labelable.rb +++ b/app/models/concerns/labelable.rb @@ -10,6 +10,8 @@ module Labelable end def add_labels(new_labels = nil) + return if new_labels.blank? + new_labels = Array(new_labels) # Make sure new_labels is an array combined_labels = labels + new_labels update!(label_list: combined_labels) diff --git a/app/models/conversation.rb b/app/models/conversation.rb index d6c5d0e4a..4ec63acc2 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -297,8 +297,6 @@ class Conversation < ApplicationRecord previous_labels, current_labels = previous_changes[:label_list] return unless (previous_labels.is_a? Array) && (current_labels.is_a? Array) - dispatcher_dispatch(CONVERSATION_UPDATED, previous_changes) - create_label_added(user_name, current_labels - previous_labels) create_label_removed(user_name, previous_labels - current_labels) end diff --git a/app/services/automation_rules/conditions_filter_service.rb b/app/services/automation_rules/conditions_filter_service.rb index 23873371d..993ed21c9 100644 --- a/app/services/automation_rules/conditions_filter_service.rb +++ b/app/services/automation_rules/conditions_filter_service.rb @@ -151,13 +151,36 @@ class AutomationRules::ConditionsFilterService < FilterService " #{table_name}.additional_attributes ->> '#{attribute_key}' #{filter_operator_value} #{query_operator} " when 'standard' if attribute_key == 'labels' - " tags.id #{filter_operator_value} #{query_operator} " + build_label_query_string(query_hash, current_index, query_operator) else " #{table_name}.#{attribute_key} #{filter_operator_value} #{query_operator} " end end end + def build_label_query_string(query_hash, current_index, query_operator) + case query_hash['filter_operator'] + when 'equal_to' + return " 1=0 #{query_operator} " if query_hash['values'].blank? + + value_placeholder = "value_#{current_index}" + @filter_values[value_placeholder] = query_hash['values'].first + " tags.name = :#{value_placeholder} #{query_operator} " + when 'not_equal_to' + return " 1=0 #{query_operator} " if query_hash['values'].blank? + + value_placeholder = "value_#{current_index}" + @filter_values[value_placeholder] = query_hash['values'].first + " tags.name != :#{value_placeholder} #{query_operator} " + when 'is_present' + " tags.id IS NOT NULL #{query_operator} " + when 'is_not_present' + " tags.id IS NULL #{query_operator} " + else + " tags.id #{filter_operation(query_hash, current_index)} #{query_operator} " + end + end + private def base_relation @@ -166,7 +189,21 @@ class AutomationRules::ConditionsFilterService < FilterService ).joins( 'LEFT OUTER JOIN messages on messages.conversation_id = conversations.id' ) + + # Only add label joins when label conditions exist + if label_conditions? + records = records.joins( + 'LEFT OUTER JOIN taggings ON taggings.taggable_id = conversations.id AND taggings.taggable_type = \'Conversation\'' + ).joins( + 'LEFT OUTER JOIN tags ON taggings.tag_id = tags.id' + ) + end + records = records.where(messages: { id: @options[:message].id }) if @options[:message].present? records end + + def label_conditions? + @rule.conditions.any? { |condition| condition['attribute_key'] == 'labels' } + end end diff --git a/spec/listeners/automation_rule_listener_labels_spec.rb b/spec/listeners/automation_rule_listener_labels_spec.rb new file mode 100644 index 000000000..36002f7f3 --- /dev/null +++ b/spec/listeners/automation_rule_listener_labels_spec.rb @@ -0,0 +1,244 @@ +require 'rails_helper' + +describe AutomationRuleListener do + let(:listener) { described_class.instance } + let!(:account) { create(:account) } + let!(:user) { create(:user, account: account) } + let!(:inbox) { create(:inbox, account: account) } + let!(:contact) { create(:contact, account: account) } + let!(:conversation) { create(:conversation, account: account, inbox: inbox, contact: contact) } + let(:label1) { create(:label, account: account, title: 'bug') } + let(:label2) { create(:label, account: account, title: 'feature') } + let(:label3) { create(:label, account: account, title: 'urgent') } + + before do + Current.user = user + end + + describe 'conversation_updated with label conditions and actions' do + context 'when label is added and automation rule has label condition' do + let(:automation_rule) do + create(:automation_rule, + event_name: 'conversation_updated', + account: account, + conditions: [ + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: ['bug'], + query_operator: nil + } + ], + actions: [ + { + action_name: 'add_label', + action_params: ['urgent'] + }, + { + action_name: 'send_message', + action_params: ['Bug report received. We will investigate this issue.'] + } + ]) + end + + it 'triggers automation when the specified label is added' do + automation_rule # Create the automation rule + expect(Messages::MessageBuilder).to receive(:new).and_call_original + + # Add the 'bug' label to trigger the automation + conversation.add_labels(['bug']) + + # Dispatch the event + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [[], ['bug']] } + }) + + listener.conversation_updated(event) + + # Verify the label was added by automation + expect(conversation.reload.label_list).to include('urgent') + + # Verify a message was sent + expect(conversation.messages.last.content).to eq('Bug report received. We will investigate this issue.') + end + + it 'does not trigger automation when a different label is added' do + automation_rule # Create the automation rule + expect(Messages::MessageBuilder).not_to receive(:new) + + # Add a different label + conversation.add_labels(['feature']) + + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [[], ['feature']] } + }) + + listener.conversation_updated(event) + + # Verify the automation did not run + expect(conversation.reload.label_list).not_to include('urgent') + end + end + + context 'when automation rule has is_present label condition' do + let(:automation_rule) do + create(:automation_rule, + event_name: 'conversation_updated', + account: account, + conditions: [ + { + attribute_key: 'labels', + filter_operator: 'is_present', + values: [], + query_operator: nil + } + ], + actions: [ + { + action_name: 'send_message', + action_params: ['Thank you for adding a label to categorize this conversation.'] + } + ]) + end + + it 'triggers automation when any label is added to an unlabeled conversation' do + automation_rule # Create the automation rule + expect(Messages::MessageBuilder).to receive(:new).and_call_original + + # Add any label to trigger the automation + conversation.add_labels(['feature']) + + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [[], ['feature']] } + }) + + listener.conversation_updated(event) + + # Verify a message was sent + expect(conversation.messages.last.content).to eq('Thank you for adding a label to categorize this conversation.') + end + + it 'still triggers when labels are removed but conversation still has labels' do + automation_rule # Create the automation rule + # Start with multiple labels + conversation.add_labels(%w[bug feature]) + conversation.reload + + expect(Messages::MessageBuilder).to receive(:new).and_call_original + + # Remove one label but conversation still has labels + conversation.update_labels(['bug']) + + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [%w[bug feature], ['bug']] } + }) + + listener.conversation_updated(event) + + # Should still trigger because conversation has labels (is_present condition) + expect(conversation.messages.last.content).to eq('Thank you for adding a label to categorize this conversation.') + end + + it 'does not trigger when all labels are removed' do + automation_rule # Create the automation rule + # Start with labels + conversation.add_labels(['bug']) + conversation.reload + + expect(Messages::MessageBuilder).not_to receive(:new) + + # Remove all labels + conversation.update_labels([]) + + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [['bug'], []] } + }) + + listener.conversation_updated(event) + end + end + + context 'when automation rule has remove_label action' do + let!(:automation_rule) do + create(:automation_rule, + event_name: 'conversation_updated', + account: account, + conditions: [ + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: ['urgent'], + query_operator: nil + } + ], + actions: [ + { + action_name: 'remove_label', + action_params: ['bug'] + } + ]) + end + + it 'removes specified labels when condition is met' do + automation_rule # Create the automation rule + # Start with both labels + conversation.add_labels(%w[bug urgent]) + + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [['bug'], %w[bug urgent]] } + }) + + listener.conversation_updated(event) + + # Verify the bug label was removed but urgent remains + expect(conversation.reload.label_list).to include('urgent') + expect(conversation.reload.label_list).not_to include('bug') + end + end + end + + describe 'preventing infinite loops' do + let!(:automation_rule) do + create(:automation_rule, + event_name: 'conversation_updated', + account: account, + conditions: [ + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: ['bug'], + query_operator: nil + } + ], + actions: [ + { + action_name: 'add_label', + action_params: ['processed'] + } + ]) + end + + it 'does not trigger automation when performed by automation rule' do + automation_rule # Create the automation rule + conversation.add_labels(['bug']) + + # Simulate event performed by automation rule + event = Events::Base.new('conversation_updated', Time.zone.now, { + conversation: conversation, + changed_attributes: { label_list: [[], ['bug']] }, + performed_by: automation_rule + }) + + # Should not process the event since it was performed by automation + expect(AutomationRules::ActionService).not_to receive(:new) + + listener.conversation_updated(event) + end + end +end diff --git a/spec/models/automation_rule_spec.rb b/spec/models/automation_rule_spec.rb index 53ebfa0c7..91452b8a4 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -60,6 +60,32 @@ RSpec.describe AutomationRule do expect(rule.valid?).to be false expect(rule.errors.messages[:conditions]).to eq(['Automation conditions should have query operator.']) end + + it 'allows labels as a valid condition attribute' do + params[:conditions] = [ + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: ['bug'], + query_operator: nil + } + ] + rule = FactoryBot.build(:automation_rule, params) + expect(rule.valid?).to be true + end + + it 'validates label condition operators' do + params[:conditions] = [ + { + attribute_key: 'labels', + filter_operator: 'is_present', + values: [], + query_operator: nil + } + ] + rule = FactoryBot.build(:automation_rule, params) + expect(rule.valid?).to be true + end end describe 'reauthorizable' do diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index a29359528..007ce987f 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -136,7 +136,7 @@ RSpec.describe Conversation do notifiable_assignee_change: false, changed_attributes: changed_attributes, performed_by: nil - ).exactly(2).times + ) end it 'runs after_update callbacks' do diff --git a/spec/services/automation_rules/action_service_spec.rb b/spec/services/automation_rules/action_service_spec.rb index e63fd7545..b4eaa5dd0 100644 --- a/spec/services/automation_rules/action_service_spec.rb +++ b/spec/services/automation_rules/action_service_spec.rb @@ -118,6 +118,45 @@ RSpec.describe AutomationRules::ActionService do end end + describe '#perform with add_label action' do + before do + rule.actions << { action_name: 'add_label', action_params: %w[bug feature] } + rule.save + end + + it 'will add labels to conversation' do + described_class.new(rule, account, conversation).perform + expect(conversation.reload.label_list).to include('bug', 'feature') + end + + it 'will not duplicate existing labels' do + conversation.add_labels(['bug']) + described_class.new(rule, account, conversation).perform + expect(conversation.reload.label_list.count('bug')).to eq(1) + expect(conversation.reload.label_list).to include('feature') + end + end + + describe '#perform with remove_label action' do + before do + conversation.add_labels(%w[bug feature support]) + rule.actions << { action_name: 'remove_label', action_params: %w[bug feature] } + rule.save + end + + it 'will remove specified labels from conversation' do + described_class.new(rule, account, conversation).perform + expect(conversation.reload.label_list).not_to include('bug', 'feature') + expect(conversation.reload.label_list).to include('support') + end + + it 'will not fail if labels do not exist on conversation' do + conversation.update_labels(['support']) # Remove bug and feature first + expect { described_class.new(rule, account, conversation).perform }.not_to raise_error + expect(conversation.reload.label_list).to include('support') + end + end + describe '#perform with add_private_note action' do let(:message_builder) { double } diff --git a/spec/services/automation_rules/conditions_filter_service_spec.rb b/spec/services/automation_rules/conditions_filter_service_spec.rb index 7082d31b1..5efd26341 100644 --- a/spec/services/automation_rules/conditions_filter_service_spec.rb +++ b/spec/services/automation_rules/conditions_filter_service_spec.rb @@ -134,5 +134,86 @@ RSpec.describe AutomationRules::ConditionsFilterService do end end end + + context 'when conditions based on labels' do + before do + conversation.add_labels(['bug']) + end + + context 'when filter_operator is equal_to' do + before do + rule.conditions = [ + { 'values': ['bug'], 'attribute_key': 'labels', 'query_operator': nil, 'filter_operator': 'equal_to' } + ] + rule.save + end + + it 'will return true when conversation has the label' do + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(true) + end + + it 'will return false when conversation does not have the label' do + rule.conditions = [ + { 'values': ['feature'], 'attribute_key': 'labels', 'query_operator': nil, 'filter_operator': 'equal_to' } + ] + rule.save + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(false) + end + end + + context 'when filter_operator is not_equal_to' do + before do + rule.conditions = [ + { 'values': ['feature'], 'attribute_key': 'labels', 'query_operator': nil, 'filter_operator': 'not_equal_to' } + ] + rule.save + end + + it 'will return true when conversation does not have the label' do + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(true) + end + + it 'will return false when conversation has the label' do + conversation.add_labels(['feature']) + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(false) + end + end + + context 'when filter_operator is is_present' do + before do + rule.conditions = [ + { 'values': [], 'attribute_key': 'labels', 'query_operator': nil, 'filter_operator': 'is_present' } + ] + rule.save + end + + it 'will return true when conversation has any labels' do + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(true) + end + + it 'will return false when conversation has no labels' do + conversation.update_labels([]) + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(false) + end + end + + context 'when filter_operator is is_not_present' do + before do + rule.conditions = [ + { 'values': [], 'attribute_key': 'labels', 'query_operator': nil, 'filter_operator': 'is_not_present' } + ] + rule.save + end + + it 'will return false when conversation has any labels' do + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(false) + end + + it 'will return true when conversation has no labels' do + conversation.update_labels([]) + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(true) + end + end + end end end