From 923b4637db3d255da9607b0b60206c23d4cee02c Mon Sep 17 00:00:00 2001 From: Tejaswini Chile Date: Tue, 12 Apr 2022 20:23:34 +0530 Subject: [PATCH] chore: Automation bug fix (#4442) --- .../settings/automation/AddAutomationRule.vue | 2 +- .../automation/EditAutomationRule.vue | 2 +- .../settings/automation/constants.js | 8 +- app/listeners/automation_rule_listener.rb | 22 ++-- .../automation_notification_mailer.rb | 12 +- app/models/conversation.rb | 5 +- app/models/message.rb | 10 +- .../automation_rules/action_service.rb | 2 +- .../conversation_creation.liquid | 3 +- .../automation_rules_controller_spec.rb | 16 ++- .../automation_rule_listener_spec.rb | 119 ------------------ spec/models/conversation_spec.rb | 10 +- 12 files changed, 47 insertions(+), 164 deletions(-) diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/AddAutomationRule.vue b/app/javascript/dashboard/routes/dashboard/settings/automation/AddAutomationRule.vue index 68d2c687c..73a02fd1a 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/AddAutomationRule.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/AddAutomationRule.vue @@ -194,7 +194,7 @@ export default { required: requiredIf(prop => { return !( prop.action_name === 'mute_conversation' || - prop.action_name === 'snooze_convresation' || + prop.action_name === 'snooze_conversation' || prop.action_name === 'resolve_convresation' ); }), diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/EditAutomationRule.vue b/app/javascript/dashboard/routes/dashboard/settings/automation/EditAutomationRule.vue index cf70a35bb..14a8e84bb 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/EditAutomationRule.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/EditAutomationRule.vue @@ -198,7 +198,7 @@ export default { required: requiredIf(prop => { return !( prop.action_name === 'mute_conversation' || - prop.action_name === 'snooze_convresation' || + prop.action_name === 'snooze_conversation' || prop.action_name === 'resolve_convresation' ); }), diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js index dd11700bd..cd306007a 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js @@ -92,7 +92,7 @@ export const AUTOMATIONS = { attributeI18nKey: 'MUTE_CONVERSATION', }, { - key: 'snooze_convresation', + key: 'snooze_conversation', name: 'Snooze conversation', attributeI18nKey: 'MUTE_CONVERSATION', }, @@ -166,7 +166,7 @@ export const AUTOMATIONS = { attributeI18nKey: 'MUTE_CONVERSATION', }, { - key: 'snooze_convresation', + key: 'snooze_conversation', name: 'Snooze conversation', attributeI18nKey: 'MUTE_CONVERSATION', }, @@ -254,7 +254,7 @@ export const AUTOMATIONS = { attributeI18nKey: 'MUTE_CONVERSATION', }, { - key: 'snooze_convresation', + key: 'snooze_conversation', name: 'Snooze conversation', attributeI18nKey: 'MUTE_CONVERSATION', }, @@ -314,7 +314,7 @@ export const AUTOMATION_ACTION_TYPES = [ inputType: null, }, { - key: 'snooze_convresation', + key: 'snooze_conversation', label: 'Snooze conversation', inputType: null, }, diff --git a/app/listeners/automation_rule_listener.rb b/app/listeners/automation_rule_listener.rb index 19fb56287..c8c021a4e 100644 --- a/app/listeners/automation_rule_listener.rb +++ b/app/listeners/automation_rule_listener.rb @@ -1,5 +1,7 @@ class AutomationRuleListener < BaseListener def conversation_updated(event_obj) + return if performed_by_automation?(event_obj) + conversation = event_obj.data[:conversation] account = conversation.account @@ -11,19 +13,9 @@ class AutomationRuleListener < BaseListener end end - def conversation_status_changed(event_obj) - conversation = event_obj.data[:conversation] - account = conversation.account - - return unless rule_present?('conversation_status_changed', account) - - @rules.each do |rule| - conditions_match = ::AutomationRules::ConditionsFilterService.new(rule, conversation).perform - AutomationRules::ActionService.new(rule, account, conversation).perform if conditions_match.present? - end - end - def conversation_created(event_obj) + return if performed_by_automation?(event_obj) + conversation = event_obj.data[:conversation] account = conversation.account @@ -36,6 +28,8 @@ class AutomationRuleListener < BaseListener end def message_created(event_obj) + return if performed_by_automation?(event_obj) + message = event_obj.data[:message] account = message.try(:account) @@ -57,4 +51,8 @@ class AutomationRuleListener < BaseListener ) @rules.any? end + + def performed_by_automation?(event_obj) + event_obj.data[:performed_by].present? && event_obj.data[:performed_by].instance_of?(AutomationRule) + end end diff --git a/app/mailers/team_notifications/automation_notification_mailer.rb b/app/mailers/team_notifications/automation_notification_mailer.rb index fd96cdd1d..d6ca47f81 100644 --- a/app/mailers/team_notifications/automation_notification_mailer.rb +++ b/app/mailers/team_notifications/automation_notification_mailer.rb @@ -35,18 +35,14 @@ class TeamNotifications::AutomationNotificationMailer < ApplicationMailer private def send_an_email_to_team - @agents.each do |agent| - subject = "#{agent.user.available_name}, This email has been sent via automation rule actions." - @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) - @agent = agent - - send_mail_with_liquid(to: @agent.user.email, subject: subject) - end + subject = 'This email has been sent via automation rule actions.' + @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) + @agent_emails = @agents.collect(&:user).pluck(:email) + send_mail_with_liquid(to: @agent_emails, subject: subject) and return end def liquid_droppables super.merge!({ - user: @agent.user, conversation: @conversation, inbox: @conversation.inbox }) diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 75433f7fe..409efab6d 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -214,10 +214,9 @@ class Conversation < ApplicationRecord end def dispatcher_dispatch(event_name, changed_attributes = nil) - return if Current.executed_by.present? && Current.executed_by.instance_of?(AutomationRule) - Rails.configuration.dispatcher.dispatch(event_name, Time.zone.now, conversation: self, notifiable_assignee_change: notifiable_assignee_change?, - changed_attributes: changed_attributes) + changed_attributes: changed_attributes, + performed_by: Current.executed_by) end def conversation_status_changed_to_open? diff --git a/app/models/message.rb b/app/models/message.rb index adfc914b6..13d58db6b 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -166,19 +166,15 @@ class Message < ApplicationRecord end def dispatch_create_events - return if Current.executed_by.present? && Current.executed_by.instance_of?(AutomationRule) - - Rails.configuration.dispatcher.dispatch(MESSAGE_CREATED, Time.zone.now, message: self) + Rails.configuration.dispatcher.dispatch(MESSAGE_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by) if outgoing? && conversation.messages.outgoing.count == 1 - Rails.configuration.dispatcher.dispatch(FIRST_REPLY_CREATED, Time.zone.now, message: self) + Rails.configuration.dispatcher.dispatch(FIRST_REPLY_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by) end end def dispatch_update_event - return if Current.executed_by.present? && Current.executed_by.instance_of?(AutomationRule) - - Rails.configuration.dispatcher.dispatch(MESSAGE_UPDATED, Time.zone.now, message: self) + Rails.configuration.dispatcher.dispatch(MESSAGE_UPDATED, Time.zone.now, message: self, performed_by: Current.executed_by) end def send_reply diff --git a/app/services/automation_rules/action_service.rb b/app/services/automation_rules/action_service.rb index 75b1f0474..1249bd9c5 100644 --- a/app/services/automation_rules/action_service.rb +++ b/app/services/automation_rules/action_service.rb @@ -41,7 +41,7 @@ class AutomationRules::ActionService end def snooze_conversation(_params) - @conversation.ensure_snooze_until_reset + @conversation.snoozed! end def change_status(status) diff --git a/app/views/mailers/team_notifications/automation_notification_mailer/conversation_creation.liquid b/app/views/mailers/team_notifications/automation_notification_mailer/conversation_creation.liquid index 8edefcc07..036e96603 100644 --- a/app/views/mailers/team_notifications/automation_notification_mailer/conversation_creation.liquid +++ b/app/views/mailers/team_notifications/automation_notification_mailer/conversation_creation.liquid @@ -1,5 +1,4 @@ -

Hi {{user.available_name}}

- +

This is the mail from Automation System

{{ custom_message }}

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 0d3b3e784..0b921bc07 100644 --- a/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb @@ -259,7 +259,7 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do end context 'when it is an authenticated user' do - it 'returns for cloned automation_rule for account' do + it 'returns for updated automation_rule for account' do params = { name: 'Update name' } expect(account.automation_rules.count).to eq(1) @@ -271,6 +271,20 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do body = JSON.parse(response.body, symbolize_names: true) expect(body[:payload][:name]).to eq('Update name') end + + it 'returns for updated active flag for automation_rule' do + expect(automation_rule.active).to eq(true) + params = { active: false } + + patch "/api/v1/accounts/#{account.id}/automation_rules/#{automation_rule.id}", + headers: administrator.create_new_auth_token, + params: params + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body, symbolize_names: true) + expect(body[:payload][:active]).to eq(false) + expect(automation_rule.reload.active).to eq(false) + end end end diff --git a/spec/listeners/automation_rule_listener_spec.rb b/spec/listeners/automation_rule_listener_spec.rb index 6c4fc0c2a..b0ed4d0bd 100644 --- a/spec/listeners/automation_rule_listener_spec.rb +++ b/spec/listeners/automation_rule_listener_spec.rb @@ -50,125 +50,6 @@ describe AutomationRuleListener do automation_rule.save end - describe '#conversation_status_changed' do - context 'when rule matches' do - it 'triggers automation rule send webhook events' do - payload = conversation.webhook_data.merge(event: "automation_event: #{automation_rule.event_name}") - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - expect(WebhookJob).to receive(:perform_later).with('https://www.example.com', payload).once - - listener.conversation_status_changed(event) - end - - it 'triggers automation rule to assign team' do - expect(conversation.team_id).not_to eq(team.id) - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - expect(conversation.team_id).to eq(team.id) - end - - it 'triggers automation rule to add label' do - expect(conversation.labels).to eq([]) - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - - expect(conversation.labels.pluck(:name)).to contain_exactly('support', 'priority_customer') - end - - it 'triggers automation rule to assign best agents' do - expect(conversation.assignee).to be_nil - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - - expect(conversation.assignee).to eq(user_1) - end - - it 'triggers automation rule send message to the contacts' do - expect(conversation.messages).to be_empty - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - - expect(conversation.messages.first.content).to eq('Send this message.') - end - - it 'triggers automation rule changes status to snoozed' do - expect(conversation.status).to eq('resolved') - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - - expect(conversation.status).to eq('snoozed') - end - - it 'triggers automation rule send email transcript to the mentioned email' do - mailer = double - - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - - allow(mailer).to receive(:conversation_transcript) - end - - it 'triggers automation rule send email to the team' do - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - end - - it 'triggers automation rule send attachments in messages' do - automation_rule - - expect(TeamNotifications::AutomationNotificationMailer).to receive(:conversation_creation) - - listener.conversation_status_changed(event) - - conversation.reload - - expect(conversation.messages.last.attachments.count).to eq(1) - end - end - end - describe '#conversation_updated with contacts attributes' do before do conversation.contact.update!(custom_attributes: { customer_type: 'platinum', signed_in_at: '2022-01-19' }, diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 6a9002795..d5295e340 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -55,7 +55,7 @@ RSpec.describe Conversation, type: :model do # send_events expect(Rails.configuration.dispatcher).to have_received(:dispatch) .with(described_class::CONVERSATION_CREATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: false, - changed_attributes: nil) + changed_attributes: nil, performed_by: nil) end end @@ -121,16 +121,16 @@ RSpec.describe Conversation, type: :model do expect(Rails.configuration.dispatcher).to have_received(:dispatch) .with(described_class::CONVERSATION_RESOLVED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true, - changed_attributes: status_change) + changed_attributes: status_change, performed_by: nil) expect(Rails.configuration.dispatcher).to have_received(:dispatch) .with(described_class::CONVERSATION_READ, kind_of(Time), conversation: conversation, notifiable_assignee_change: true, - changed_attributes: nil) + changed_attributes: nil, performed_by: nil) expect(Rails.configuration.dispatcher).to have_received(:dispatch) .with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true, - changed_attributes: nil) + changed_attributes: nil, performed_by: nil) expect(Rails.configuration.dispatcher).to have_received(:dispatch) .with(described_class::CONVERSATION_UPDATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true, - changed_attributes: changed_attributes) + changed_attributes: changed_attributes, performed_by: nil) end it 'will not run conversation_updated event for empty updates' do