From c80ab26ad971d0c573bed799740951a54159c9c6 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Tue, 14 Apr 2026 14:56:17 +0530 Subject: [PATCH] fix(macros): add explicit remove assignment actions --- .../composables/spec/useMacros.spec.js | 9 ++-- .../dashboard/composables/useMacros.js | 14 +++--- .../helper/specs/macrosHelper.spec.js | 8 +++ .../dashboard/helper/validations.js | 1 + .../dashboard/i18n/locale/en/automation.json | 2 + .../dashboard/i18n/locale/en/macros.json | 1 + .../conversation/Macros/MacroPreview.vue | 1 + .../settings/automation/constants.js | 50 +++++++++++++++++++ .../dashboard/settings/macros/MacroEditor.vue | 4 +- .../dashboard/settings/macros/constants.js | 7 ++- app/models/automation_rule.rb | 6 +-- app/models/macro.rb | 6 +-- app/services/action_service.rb | 22 ++++++-- .../automation_rules_controller_spec.rb | 6 +++ .../api/v1/accounts/macros_controller_spec.rb | 19 +++++++ spec/models/automation_rule_spec.rb | 6 +++ spec/services/action_service_spec.rb | 42 ++++++++++++++++ .../automation_rules/action_service_spec.rb | 32 +++++++++++- 18 files changed, 211 insertions(+), 25 deletions(-) diff --git a/app/javascript/dashboard/composables/spec/useMacros.spec.js b/app/javascript/dashboard/composables/spec/useMacros.spec.js index ed6a7bc87..58ea78c15 100644 --- a/app/javascript/dashboard/composables/spec/useMacros.spec.js +++ b/app/javascript/dashboard/composables/spec/useMacros.spec.js @@ -111,7 +111,7 @@ describe('useMacros', () => { useStoreGetters.mockReturnValue({ 'labels/getLabels': { value: mockLabels }, 'teams/getTeams': { value: mockTeams }, - 'agents/getAgents': { value: mockAgents }, + 'agents/getVerifiedAgents': { value: mockAgents }, }); }); @@ -129,7 +129,10 @@ describe('useMacros', () => { it('returns teams with "None" option for assign_team and teams only for send_email_to_team', () => { const { getMacroDropdownValues } = useMacros(); const assignTeamResult = getMacroDropdownValues('assign_team'); - expect(assignTeamResult[0]).toEqual({ id: 'nil', name: 'AUTOMATION.NONE_OPTION' }); + expect(assignTeamResult[0]).toEqual({ + id: 'nil', + name: 'AUTOMATION.NONE_OPTION', + }); expect(assignTeamResult.slice(1)).toEqual(mockTeams); expect(getMacroDropdownValues('send_email_to_team')).toEqual(mockTeams); }); @@ -170,7 +173,7 @@ describe('useMacros', () => { useStoreGetters.mockReturnValue({ 'labels/getLabels': { value: [] }, 'teams/getTeams': { value: [] }, - 'agents/getAgents': { value: [] }, + 'agents/getVerifiedAgents': { value: [] }, }); const { getMacroDropdownValues } = useMacros(); diff --git a/app/javascript/dashboard/composables/useMacros.js b/app/javascript/dashboard/composables/useMacros.js index 11ed5b424..52aacd73f 100644 --- a/app/javascript/dashboard/composables/useMacros.js +++ b/app/javascript/dashboard/composables/useMacros.js @@ -13,7 +13,12 @@ export const useMacros = () => { const labels = computed(() => getters['labels/getLabels'].value); const teams = computed(() => getters['teams/getTeams'].value); - const agents = computed(() => getters['agents/getAgents'].value); + const agents = computed(() => getters['agents/getVerifiedAgents'].value); + + const withNoneOption = options => [ + { id: 'nil', name: t('AUTOMATION.NONE_OPTION') }, + ...(options || []), + ]; /** * Get dropdown values based on the specified type @@ -23,15 +28,12 @@ export const useMacros = () => { const getMacroDropdownValues = type => { switch (type) { case 'assign_team': - return [ - { id: 'nil', name: t('AUTOMATION.NONE_OPTION') }, - ...teams.value, - ]; + return withNoneOption(teams.value); case 'send_email_to_team': return teams.value; case 'assign_agent': return [ - { id: 'nil', name: t('AUTOMATION.NONE_OPTION') }, + ...withNoneOption(), { id: 'self', name: 'Self' }, ...agents.value, ]; diff --git a/app/javascript/dashboard/helper/specs/macrosHelper.spec.js b/app/javascript/dashboard/helper/specs/macrosHelper.spec.js index d4c8dc5fb..fb5b62b38 100644 --- a/app/javascript/dashboard/helper/specs/macrosHelper.spec.js +++ b/app/javascript/dashboard/helper/specs/macrosHelper.spec.js @@ -45,6 +45,10 @@ describe('#resolveTeamIds', () => { const resolvedTeams = '⚙️ sales team, 🤷‍♂️ fayaz'; expect(resolveTeamIds(teams, [1, 2])).toEqual(resolvedTeams); }); + + it('resolves nil as None', () => { + expect(resolveTeamIds(teams, ['nil'])).toEqual('None'); + }); }); describe('#resolveLabels', () => { @@ -59,6 +63,10 @@ describe('#resolveAgents', () => { const resolvedAgents = 'John Doe'; expect(resolveAgents(agents, [1])).toEqual(resolvedAgents); }); + + it('resolves nil and self values', () => { + expect(resolveAgents(agents, ['nil', 'self'])).toEqual('None, Self'); + }); }); describe('#getFileName', () => { diff --git a/app/javascript/dashboard/helper/validations.js b/app/javascript/dashboard/helper/validations.js index edebc4656..c1b9a9d99 100644 --- a/app/javascript/dashboard/helper/validations.js +++ b/app/javascript/dashboard/helper/validations.js @@ -125,6 +125,7 @@ const validateSingleAction = action => { 'mute_conversation', 'snooze_conversation', 'resolve_conversation', + 'remove_assigned_agent', 'remove_assigned_team', 'open_conversation', ]; diff --git a/app/javascript/dashboard/i18n/locale/en/automation.json b/app/javascript/dashboard/i18n/locale/en/automation.json index 80274f488..b6d484079 100644 --- a/app/javascript/dashboard/i18n/locale/en/automation.json +++ b/app/javascript/dashboard/i18n/locale/en/automation.json @@ -137,6 +137,8 @@ "ACTIONS": { "ASSIGN_AGENT": "Assign to Agent", "ASSIGN_TEAM": "Assign a Team", + "REMOVE_ASSIGNED_AGENT": "Remove Assigned Agent", + "REMOVE_ASSIGNED_TEAM": "Remove Assigned Team", "ADD_LABEL": "Add a Label", "REMOVE_LABEL": "Remove a Label", "SEND_EMAIL_TO_TEAM": "Send an Email to Team", diff --git a/app/javascript/dashboard/i18n/locale/en/macros.json b/app/javascript/dashboard/i18n/locale/en/macros.json index fcb409f34..8cd6cc374 100644 --- a/app/javascript/dashboard/i18n/locale/en/macros.json +++ b/app/javascript/dashboard/i18n/locale/en/macros.json @@ -89,6 +89,7 @@ "ASSIGN_AGENT": "Assign an Agent", "ADD_LABEL": "Add a Label", "REMOVE_LABEL": "Remove a Label", + "REMOVE_ASSIGNED_AGENT": "Remove Assigned Agent", "REMOVE_ASSIGNED_TEAM": "Remove Assigned Team", "SEND_EMAIL_TRANSCRIPT": "Send an Email Transcript", "MUTE_CONVERSATION": "Mute Conversation", diff --git a/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroPreview.vue b/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroPreview.vue index d5a2ae0b8..9e57c34c5 100644 --- a/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroPreview.vue +++ b/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroPreview.vue @@ -25,6 +25,7 @@ const getActionValue = (key, params) => { add_label: resolveLabels(labels.value, params), remove_label: resolveLabels(labels.value, params), assign_agent: resolveAgents(agents.value, params), + remove_assigned_agent: null, mute_conversation: null, snooze_conversation: null, resolve_conversation: null, diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js index 0a6905039..7be9c2dd6 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js @@ -78,6 +78,14 @@ export const AUTOMATIONS = { key: 'assign_team', name: 'ASSIGN_TEAM', }, + { + key: 'remove_assigned_agent', + name: 'REMOVE_ASSIGNED_AGENT', + }, + { + key: 'remove_assigned_team', + name: 'REMOVE_ASSIGNED_TEAM', + }, { key: 'add_label', name: 'ADD_LABEL', @@ -196,6 +204,14 @@ export const AUTOMATIONS = { key: 'assign_team', name: 'ASSIGN_TEAM', }, + { + key: 'remove_assigned_agent', + name: 'REMOVE_ASSIGNED_AGENT', + }, + { + key: 'remove_assigned_team', + name: 'REMOVE_ASSIGNED_TEAM', + }, { key: 'assign_agent', name: 'ASSIGN_AGENT', @@ -318,6 +334,14 @@ export const AUTOMATIONS = { key: 'assign_team', name: 'ASSIGN_TEAM', }, + { + key: 'remove_assigned_agent', + name: 'REMOVE_ASSIGNED_AGENT', + }, + { + key: 'remove_assigned_team', + name: 'REMOVE_ASSIGNED_TEAM', + }, { key: 'assign_agent', name: 'ASSIGN_AGENT', @@ -434,6 +458,14 @@ export const AUTOMATIONS = { key: 'assign_team', name: 'ASSIGN_TEAM', }, + { + key: 'remove_assigned_agent', + name: 'REMOVE_ASSIGNED_AGENT', + }, + { + key: 'remove_assigned_team', + name: 'REMOVE_ASSIGNED_TEAM', + }, { key: 'assign_agent', name: 'ASSIGN_AGENT', @@ -546,6 +578,14 @@ export const AUTOMATIONS = { key: 'assign_team', name: 'ASSIGN_TEAM', }, + { + key: 'remove_assigned_agent', + name: 'REMOVE_ASSIGNED_AGENT', + }, + { + key: 'remove_assigned_team', + name: 'REMOVE_ASSIGNED_TEAM', + }, { key: 'send_email_to_team', name: 'SEND_EMAIL_TO_TEAM', @@ -604,6 +644,16 @@ export const AUTOMATION_ACTION_TYPES = [ label: 'ASSIGN_TEAM', inputType: 'search_select', }, + { + key: 'remove_assigned_agent', + label: 'REMOVE_ASSIGNED_AGENT', + inputType: null, + }, + { + key: 'remove_assigned_team', + label: 'REMOVE_ASSIGNED_TEAM', + inputType: null, + }, { key: 'add_label', label: 'ADD_LABEL', diff --git a/app/javascript/dashboard/routes/dashboard/settings/macros/MacroEditor.vue b/app/javascript/dashboard/routes/dashboard/settings/macros/MacroEditor.vue index c974ef4d6..60772af06 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/macros/MacroEditor.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/macros/MacroEditor.vue @@ -58,7 +58,9 @@ const formatMacro = macroData => { ), message: action.action_params[0].message, }; - } else actionParams = [...action.action_params]; + } else { + actionParams = [...action.action_params]; + } } return { ...action, diff --git a/app/javascript/dashboard/routes/dashboard/settings/macros/constants.js b/app/javascript/dashboard/routes/dashboard/settings/macros/constants.js index e510a5304..8a0088d06 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/macros/constants.js +++ b/app/javascript/dashboard/routes/dashboard/settings/macros/constants.js @@ -19,8 +19,11 @@ export const MACRO_ACTION_TYPES = [ label: 'REMOVE_LABEL', inputType: 'multi_select', }, - // @deprecated: Use assign_team with 'None' option instead - // Kept for backward compatibility + { + key: 'remove_assigned_agent', + label: 'REMOVE_ASSIGNED_AGENT', + inputType: null, + }, { key: 'remove_assigned_team', label: 'REMOVE_ASSIGNED_TEAM', diff --git a/app/models/automation_rule.rb b/app/models/automation_rule.rb index 6f3f47d9c..f1868f98f 100644 --- a/app/models/automation_rule.rb +++ b/app/models/automation_rule.rb @@ -40,9 +40,9 @@ class AutomationRule < ApplicationRecord end def actions_attributes - %w[send_message add_label remove_label send_email_to_team assign_team assign_agent send_webhook_event mute_conversation - send_attachment change_status resolve_conversation open_conversation snooze_conversation change_priority send_email_transcript - add_private_note].freeze + %w[send_message add_label remove_label send_email_to_team assign_team assign_agent remove_assigned_agent + remove_assigned_team send_webhook_event mute_conversation send_attachment change_status resolve_conversation + open_conversation snooze_conversation change_priority send_email_transcript add_private_note].freeze end def file_base_data diff --git a/app/models/macro.rb b/app/models/macro.rb index 1fe8192dc..fc25beeac 100644 --- a/app/models/macro.rb +++ b/app/models/macro.rb @@ -30,9 +30,9 @@ class Macro < ApplicationRecord validate :json_actions_format - ACTIONS_ATTRS = %w[send_message add_label assign_team assign_agent mute_conversation change_status remove_label remove_assigned_team - resolve_conversation snooze_conversation change_priority send_email_transcript send_attachment - add_private_note send_webhook_event].freeze + ACTIONS_ATTRS = %w[send_message add_label assign_team assign_agent mute_conversation change_status remove_label remove_assigned_agent + remove_assigned_team resolve_conversation snooze_conversation change_priority send_email_transcript + send_attachment add_private_note send_webhook_event].freeze def set_visibility(user, params) self.visibility = params[:visibility] diff --git a/app/services/action_service.rb b/app/services/action_service.rb index 2266f0ea5..fa0d1a7fc 100644 --- a/app/services/action_service.rb +++ b/app/services/action_service.rb @@ -22,6 +22,10 @@ class ActionService @conversation.open! end + def pending_conversation(_params) + @conversation.pending! + end + def change_status(status) @conversation.update!(status: status[0]) end @@ -43,7 +47,9 @@ class ActionService @agent = @account.users.find_by(id: agent_ids) - @conversation.update!(assignee_id: @agent.id) if @agent.present? + return unless @agent.present? && @agent.confirmed? + + @conversation.update!(assignee_id: @agent.id) end def remove_label(labels) @@ -54,8 +60,7 @@ class ActionService end def assign_team(team_ids = []) - # FIXME: The explicit checks for zero or nil (string) is bad. Move - # this to a separate unassign action. + # Keep nil/0 handling for existing automation and macro payloads. should_unassign = team_ids.blank? || %w[nil 0].include?(team_ids[0].to_s) return @conversation.update!(team_id: nil) if should_unassign @@ -66,18 +71,25 @@ class ActionService @conversation.update!(team_id: team_ids[0]) end - # @deprecated Use assign_team with 'nil' value instead - # This method is kept for backward compatibility + def remove_assigned_agent(_params) + @conversation.update!(assignee_id: nil) + end + def remove_assigned_team(_params) @conversation.update!(team_id: nil) end def send_email_transcript(emails) + return unless @account.email_transcript_enabled? + emails = emails[0].gsub(/\s+/, '').split(',') emails.each do |email| + break unless @account.within_email_rate_limit? + email = parse_email_variables(@conversation, email) ConversationReplyMailer.with(account: @conversation.account).conversation_transcript(@conversation, email)&.deliver_later + @account.increment_email_sent_count end end diff --git a/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb b/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb index 35ff53b12..727a53eca 100644 --- a/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb @@ -68,6 +68,12 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do 'action_name': :assign_team, 'action_params': [1] }, + { + 'action_name': :remove_assigned_agent + }, + { + 'action_name': :remove_assigned_team + }, { 'action_name': :add_label, 'action_params': %w[support priority_customer] diff --git a/spec/controllers/api/v1/accounts/macros_controller_spec.rb b/spec/controllers/api/v1/accounts/macros_controller_spec.rb index 5fb8fb2b4..5027e7651 100644 --- a/spec/controllers/api/v1/accounts/macros_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/macros_controller_spec.rb @@ -78,6 +78,9 @@ RSpec.describe 'Api::V1::Accounts::MacrosController', type: :request do 'action_name': :add_label, 'action_params': %w[support priority_customer] }, + { + 'action_name': :remove_assigned_agent + }, { 'action_name': :remove_assigned_team }, @@ -410,6 +413,22 @@ RSpec.describe 'Api::V1::Accounts::MacrosController', type: :request do expect(conversation.reload.team_id).to be_nil end + + it 'Unassign the agent' do + macro.update!(actions: [ + { 'action_name' => 'remove_assigned_agent' } + ]) + conversation.update!(assignee: user_1) + expect(conversation.reload.assignee).to be_present + + perform_enqueued_jobs do + post "/api/v1/accounts/#{account.id}/macros/#{macro.id}/execute", + params: { conversation_ids: [conversation.display_id] }, + headers: administrator.create_new_auth_token + end + + expect(conversation.reload.assignee).to be_nil + end end end end diff --git a/spec/models/automation_rule_spec.rb b/spec/models/automation_rule_spec.rb index 53ebfa0c7..dbeaa99ff 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -37,6 +37,12 @@ RSpec.describe AutomationRule do action_name: :assign_team, action_params: [1] }, + { + action_name: :remove_assigned_agent + }, + { + action_name: :remove_assigned_team + }, { action_name: :add_label, action_params: %w[support priority_customer] diff --git a/spec/services/action_service_spec.rb b/spec/services/action_service_spec.rb index 95b1e6ecf..b994afe0a 100644 --- a/spec/services/action_service_spec.rb +++ b/spec/services/action_service_spec.rb @@ -50,6 +50,26 @@ describe ActionService do action_service.assign_agent(['nil']) expect(conversation.reload.assignee).to be_nil end + + context 'when agent is confirmed' do + it 'assigns the agent to the conversation' do + inbox_member + action_service.assign_agent([agent.id]) + expect(conversation.reload.assignee).to eq(agent) + end + end + + context 'when agent is unconfirmed' do + let(:unconfirmed_agent) { create(:user, account: account, role: :agent, skip_confirmation: false) } + let(:unconfirmed_inbox_member) { create(:inbox_member, inbox: conversation.inbox, user: unconfirmed_agent) } + + it 'does not assign unconfirmed agent to the conversation' do + unconfirmed_inbox_member + original_assignee = conversation.assignee + action_service.assign_agent([unconfirmed_agent.id]) + expect(conversation.reload.assignee).to eq(original_assignee) + end + end end describe '#assign_team' do @@ -92,4 +112,26 @@ describe ActionService do end end end + + describe '#remove_assigned_agent' do + let(:conversation) { create(:conversation, :with_assignee, account: account) } + let(:action_service) { described_class.new(conversation) } + + it 'unassigns the conversation' do + expect(conversation.reload.assignee).to be_present + action_service.remove_assigned_agent(nil) + expect(conversation.reload.assignee).to be_nil + end + end + + describe '#remove_assigned_team' do + let(:conversation) { create(:conversation, :with_team, account: account) } + let(:action_service) { described_class.new(conversation) } + + it 'unassigns the team' do + expect(conversation.reload.team).to be_present + action_service.remove_assigned_team(nil) + expect(conversation.reload.team).to be_nil + end + end end diff --git a/spec/services/automation_rules/action_service_spec.rb b/spec/services/automation_rules/action_service_spec.rb index e63fd7545..7e30f0c31 100644 --- a/spec/services/automation_rules/action_service_spec.rb +++ b/spec/services/automation_rules/action_service_spec.rb @@ -88,7 +88,35 @@ RSpec.describe AutomationRules::ActionService do end end + describe '#perform with remove assignment actions' do + let!(:team) { create(:team, account: account) } + + before do + conversation.update!(assignee: agent, team: team) + rule.actions = [ + { action_name: 'remove_assigned_agent', action_params: [] }, + { action_name: 'remove_assigned_team', action_params: [] } + ] + rule.save! + end + + it 'removes assignee and team from the conversation' do + described_class.new(rule, account, conversation).perform + + expect(conversation.reload.assignee).to be_nil + expect(conversation.team).to be_nil + end + end + describe '#perform with send_email_transcript action' do + let(:account_double) do + double( + email_transcript_enabled?: true, + within_email_rate_limit?: true, + increment_email_sent_count: true + ) + end + before do rule.actions << { action_name: 'send_email_transcript', action_params: ['contact@example.com, agent@example.com,agent1@example.com'] } rule.save @@ -101,7 +129,7 @@ RSpec.describe AutomationRules::ActionService do allow(mailer).to receive(:conversation_transcript).with(conversation, 'agent@example.com') allow(mailer).to receive(:conversation_transcript).with(conversation, 'agent1@example.com') - described_class.new(rule, account, conversation).perform + described_class.new(rule, account_double, conversation).perform expect(mailer).to have_received(:conversation_transcript).exactly(3).times end @@ -113,7 +141,7 @@ RSpec.describe AutomationRules::ActionService do allow(ConversationReplyMailer).to receive(:with).and_return(mailer) allow(mailer).to receive(:conversation_transcript).with(conversation, conversation.contact.email) - described_class.new(rule.reload, account, conversation).perform + described_class.new(rule.reload, account_double, conversation).perform expect(mailer).to have_received(:conversation_transcript).exactly(1).times end end