diff --git a/app/listeners/action_cable_listener.rb b/app/listeners/action_cable_listener.rb index 94a0828c4..8ce2461a8 100644 --- a/app/listeners/action_cable_listener.rb +++ b/app/listeners/action_cable_listener.rb @@ -90,6 +90,13 @@ class ActionCableListener < BaseListener broadcast(account, tokens, ASSIGNEE_CHANGED, conversation.push_event_data) end + def team_changed(event) + conversation, account = extract_conversation_and_account(event) + tokens = user_tokens(account, conversation.inbox.members) + + broadcast(account, tokens, TEAM_CHANGED, conversation.push_event_data) + end + def conversation_contact_changed(event) conversation, account = extract_conversation_and_account(event) tokens = user_tokens(account, conversation.inbox.members) diff --git a/app/models/concerns/assignment_handler.rb b/app/models/concerns/assignment_handler.rb new file mode 100644 index 000000000..c206289d4 --- /dev/null +++ b/app/models/concerns/assignment_handler.rb @@ -0,0 +1,77 @@ +module AssignmentHandler + extend ActiveSupport::Concern + include Events::Types + + included do + before_save :ensure_assignee_is_from_team + after_update :notify_assignment_change, :process_assignment_activities + end + + private + + def ensure_assignee_is_from_team + return unless team_id_changed? + + ensure_current_assignee_team + self.assignee_id ||= find_team_assignee_id_for_inbox if team&.allow_auto_assign.present? + end + + def ensure_current_assignee_team + self.assignee_id = nil if team&.members&.exclude?(assignee) + end + + def find_team_assignee_id_for_inbox + members = inbox.members.ids & team.members.ids + # TODO: User round robin to determine the next agent instead of using sample + members.sample + end + + def notify_assignment_change + { + ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? }, + TEAM_CHANGED => -> { saved_change_to_team_id? } + }.each do |event, condition| + condition.call && dispatcher_dispatch(event) + end + end + + def process_assignment_activities + user_name = Current.user.name if Current.user.present? + if saved_change_to_team_id? + create_team_change_activity(user_name) + elsif saved_change_to_assignee_id? + create_assignee_change_activity(user_name) + end + end + + def generate_team_change_activity_key + key = team_id ? 'assigned' : 'removed' + key += '_with_assignee' if key == 'assigned' && saved_change_to_assignee_id? && assignee + key + end + + def create_team_change_activity(user_name) + return unless user_name + + key = generate_team_change_activity_key + params = { assignee_name: assignee&.name, team_name: team&.name, user_name: user_name } + if key == 'removed' + previous_team_id = previous_changes[:team_id][0] + params[:team_name] = Team.find_by(id: previous_team_id)&.name if previous_team_id.present? + end + content = I18n.t("conversations.activity.team.#{key}", **params) + + messages.create(activity_message_params(content)) + end + + def create_assignee_change_activity(user_name) + return unless user_name + + params = { assignee_name: assignee&.name, user_name: user_name }.compact + key = assignee_id ? 'assigned' : 'removed' + key = 'self_assigned' if self_assign? assignee_id + content = I18n.t("conversations.activity.assignee.#{key}", **params) + + messages.create(activity_message_params(content)) + end +end diff --git a/app/models/concerns/round_robin_handler.rb b/app/models/concerns/round_robin_handler.rb new file mode 100644 index 000000000..80865acd7 --- /dev/null +++ b/app/models/concerns/round_robin_handler.rb @@ -0,0 +1,26 @@ +module RoundRobinHandler + extend ActiveSupport::Concern + include Events::Types + + included do + after_save :run_round_robin + end + + private + + def run_round_robin + # Round robin kicks in on conversation create & update + # run it only when conversation status changes to open + return unless conversation_status_changed_to_open? + return unless should_round_robin? + + ::RoundRobin::AssignmentService.new(conversation: self).perform + end + + def should_round_robin? + return false unless inbox.enable_auto_assignment? + + # run only if assignee is blank or doesn't have access to inbox + assignee.blank? || inbox.members.exclude?(assignee) + end +end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index e51291a34..bb210f6bc 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -36,6 +36,8 @@ class Conversation < ApplicationRecord include Labelable + include AssignmentHandler + include RoundRobinHandler validates :account_id, presence: true validates :inbox_id, presence: true @@ -62,7 +64,6 @@ class Conversation < ApplicationRecord # reinvestigate in future and identity the implications after_update :notify_status_change, :create_activity after_create_commit :notify_conversation_creation, :queue_conversation_auto_resolution_job - after_save :run_round_robin after_commit :set_display_id, unless: :display_id? delegate :auto_resolve_duration, to: :account @@ -170,7 +171,6 @@ class Conversation < ApplicationRecord def create_activity user_name = Current.user.name if Current.user.present? status_change_activity(user_name) if saved_change_to_status? - create_assignee_change(user_name) if saved_change_to_assignee_id? create_label_change(user_name) if saved_change_to_label_list? end @@ -189,7 +189,6 @@ class Conversation < ApplicationRecord CONVERSATION_RESOLVED => -> { saved_change_to_status? && resolved? }, CONVERSATION_READ => -> { saved_change_to_contact_last_seen_at? }, CONVERSATION_LOCK_TOGGLE => -> { saved_change_to_locked? }, - ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? }, CONVERSATION_CONTACT_CHANGED => -> { saved_change_to_contact_id? } }.each do |event, condition| condition.call && dispatcher_dispatch(event) @@ -200,28 +199,12 @@ class Conversation < ApplicationRecord Rails.configuration.dispatcher.dispatch(event_name, Time.zone.now, conversation: self) end - def should_round_robin? - return false unless inbox.enable_auto_assignment? - - # run only if assignee is blank or doesn't have access to inbox - assignee.blank? || inbox.members.exclude?(assignee) - end - def conversation_status_changed_to_open? return false unless open? # saved_change_to_status? method only works in case of update return true if previous_changes.key?(:id) || saved_change_to_status? end - def run_round_robin - # Round robin kicks in on conversation create & update - # run it only when conversation status changes to open - return unless conversation_status_changed_to_open? - return unless should_round_robin? - - ::RoundRobin::AssignmentService.new(conversation: self).perform - end - def create_status_change_message(user_name) content = if user_name I18n.t("conversations.activity.status.#{status}", user_name: user_name) @@ -232,17 +215,6 @@ class Conversation < ApplicationRecord messages.create(activity_message_params(content)) if content end - def create_assignee_change(user_name) - return unless user_name - - params = { assignee_name: assignee&.name, user_name: user_name }.compact - key = assignee_id ? 'assigned' : 'removed' - key = 'self_assigned' if self_assign? assignee_id - content = I18n.t("conversations.activity.assignee.#{key}", **params) - - messages.create(activity_message_params(content)) - end - def create_label_change(user_name) return unless user_name diff --git a/config/locales/en.yml b/config/locales/en.yml index b8e0952d9..a344118a0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -63,6 +63,10 @@ en: self_assigned: "%{user_name} self-assigned this conversation" assigned: "Assigned to %{assignee_name} by %{user_name}" removed: "Conversation unassigned by %{user_name}" + team: + assigned: "Assigned to %{team_name} by %{user_name}" + assigned_with_assignee: "Assigned to %{assignee_name} via %{team_name} by %{user_name}" + removed: "Unassigned from %{team_name} by %{user_name}" labels: added: "%{user_name} added %{labels}" removed: "%{user_name} removed %{labels}" diff --git a/lib/events/types.rb b/lib/events/types.rb index d1bc0885b..d63255379 100644 --- a/lib/events/types.rb +++ b/lib/events/types.rb @@ -17,6 +17,7 @@ module Events::Types CONVERSATION_LOCK_TOGGLE = 'conversation.lock_toggle' CONVERSATION_CONTACT_CHANGED = 'conversation.contact_changed' ASSIGNEE_CHANGED = 'assignee.changed' + TEAM_CHANGED = 'team.changed' CONVERSATION_TYPING_ON = 'conversation.typing_on' CONVERSATION_TYPING_OFF = 'conversation.typing_off' diff --git a/spec/models/concerns/assignment_handler_spec.rb b/spec/models/concerns/assignment_handler_spec.rb new file mode 100644 index 000000000..64267a7b4 --- /dev/null +++ b/spec/models/concerns/assignment_handler_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'rails_helper' + +shared_examples_for 'assignment_handler' do + describe '#update_team' do + let(:conversation) { create(:conversation, assignee: create(:user)) } + let(:agent) do + create(:user, email: 'agent@example.com', account: conversation.account, role: :agent) + end + let(:team) do + create(:team, account: conversation.account, allow_auto_assign: false) + end + + context 'when agent is current user' do + before do + Current.user = agent + create(:team_member, team: team, user: agent) + create(:inbox_member, user: agent, inbox: conversation.inbox) + conversation.inbox.reload + end + + it 'creates team assigned and unassigned message activity' do + expect(conversation.update(team: team)).to eq true + expect(conversation.messages.pluck(:content)).to include("Assigned to #{team.name} by #{agent.name}") + expect(conversation.update(team: nil)).to eq true + expect(conversation.messages.pluck(:content)).to include("Unassigned from #{team.name} by #{agent.name}") + end + + it 'changes assignee to nil if they doesnt belong to the team and allow_auto_assign is false' do + expect(team.allow_auto_assign).to eq false + + conversation.update(team: team) + + expect(conversation.reload.assignee).to eq nil + end + + it 'changes assignee to a team member if allow_auto_assign is enabled' do + team.update!(allow_auto_assign: true) + + conversation.update(team: team) + + expect(conversation.reload.assignee).to eq agent + expect(conversation.messages.pluck(:content)).to include("Assigned to #{conversation.assignee.name} via #{team.name} by #{agent.name}") + end + + it 'wont change assignee if he is already a team member' do + team.update!(allow_auto_assign: true) + assignee = create(:user, account: conversation.account, role: :agent) + create(:inbox_member, user: assignee, inbox: conversation.inbox) + create(:team_member, team: team, user: assignee) + conversation.update(assignee: assignee) + + conversation.update(team: team) + + expect(conversation.reload.assignee).to eq assignee + end + end + end + + describe '#update_assignee' do + subject(:update_assignee) { conversation.update_assignee(agent) } + + let(:conversation) { create(:conversation, assignee: nil) } + let(:agent) do + create(:user, email: 'agent@example.com', account: conversation.account, role: :agent) + end + let(:assignment_mailer) { instance_double(AgentNotifications::ConversationNotificationsMailer, deliver: true) } + + it 'assigns the agent to conversation' do + expect(update_assignee).to eq(true) + expect(conversation.reload.assignee).to eq(agent) + end + + it 'creates a new notification for the agent' do + expect(update_assignee).to eq(true) + expect(agent.notifications.count).to eq(1) + end + + it 'does not create assignment notification if notification setting is turned off' do + notification_setting = agent.notification_settings.first + notification_setting.unselect_all_email_flags + notification_setting.unselect_all_push_flags + notification_setting.save! + + expect(update_assignee).to eq(true) + expect(agent.notifications.count).to eq(0) + end + + context 'when agent is current user' do + before do + Current.user = agent + end + + it 'creates self-assigned message activity' do + expect(update_assignee).to eq(true) + expect(conversation.messages.pluck(:content)).to include("#{agent.name} self-assigned this conversation") + end + end + end +end diff --git a/spec/models/concerns/round_robin_handler_spec.rb b/spec/models/concerns/round_robin_handler_spec.rb new file mode 100644 index 000000000..4c4c654cc --- /dev/null +++ b/spec/models/concerns/round_robin_handler_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rails_helper' + +shared_examples_for 'round_robin_handler' do + describe '#round robin' do + let(:account) { create(:account) } + let(:agent) { create(:user, email: 'agent1@example.com', account: account) } + let(:inbox) { create(:inbox, account: account) } + let(:conversation) do + create( + :conversation, + account: account, + contact: create(:contact, account: account), + inbox: inbox, + assignee: nil + ) + end + + before do + create(:inbox_member, inbox: inbox, user: agent) + allow(Redis::Alfred).to receive(:rpoplpush).and_return(agent.id) + end + + it 'runs round robin on after_save callbacks' do + # run_round_robin + expect(conversation.reload.assignee).to eq(agent) + end + + it 'will not auto assign agent if enable_auto_assignment is false' do + inbox.update(enable_auto_assignment: false) + + # run_round_robin + expect(conversation.reload.assignee).to eq(nil) + end + + it 'will not auto assign agent if its a bot conversation' do + conversation = create( + :conversation, + account: account, + contact: create(:contact, account: account), + inbox: inbox, + status: 'bot', + assignee: nil + ) + + # run_round_robin + expect(conversation.reload.assignee).to eq(nil) + end + + it 'gets triggered on update only when status changes to open' do + conversation.status = 'resolved' + conversation.save! + expect(conversation.reload.assignee).to eq(agent) + inbox.inbox_members.where(user_id: agent.id).first.destroy! + + # round robin changes assignee in this case since agent doesn't have access to inbox + agent2 = create(:user, email: 'agent2@example.com', account: account) + create(:inbox_member, inbox: inbox, user: agent2) + allow(Redis::Alfred).to receive(:rpoplpush).and_return(agent2.id) + conversation.status = 'open' + conversation.save! + expect(conversation.reload.assignee).to eq(agent2) + end + end +end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 806b159a4..f6c6f8f63 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -1,12 +1,19 @@ # frozen_string_literal: true require 'rails_helper' +require Rails.root.join 'spec/models/concerns/assignment_handler_spec.rb' +require Rails.root.join 'spec/models/concerns/round_robin_handler_spec.rb' RSpec.describe Conversation, type: :model do describe 'associations' do it { is_expected.to belong_to(:account) } end + describe 'concerns' do + it_behaves_like 'assignment_handler' + it_behaves_like 'round_robin_handler' + end + describe '.before_create' do let(:conversation) { build(:conversation, display_id: nil) } @@ -133,108 +140,6 @@ RSpec.describe Conversation, type: :model do end end - describe '#round robin' do - let(:account) { create(:account) } - let(:agent) { create(:user, email: 'agent1@example.com', account: account) } - let(:inbox) { create(:inbox, account: account) } - let(:conversation) do - create( - :conversation, - account: account, - contact: create(:contact, account: account), - inbox: inbox, - assignee: nil - ) - end - - before do - create(:inbox_member, inbox: inbox, user: agent) - allow(Redis::Alfred).to receive(:rpoplpush).and_return(agent.id) - end - - it 'runs round robin on after_save callbacks' do - # run_round_robin - expect(conversation.reload.assignee).to eq(agent) - end - - it 'will not auto assign agent if enable_auto_assignment is false' do - inbox.update(enable_auto_assignment: false) - - # run_round_robin - expect(conversation.reload.assignee).to eq(nil) - end - - it 'will not auto assign agent if its a bot conversation' do - conversation = create( - :conversation, - account: account, - contact: create(:contact, account: account), - inbox: inbox, - status: 'bot', - assignee: nil - ) - - # run_round_robin - expect(conversation.reload.assignee).to eq(nil) - end - - it 'gets triggered on update only when status changes to open' do - conversation.status = 'resolved' - conversation.save! - expect(conversation.reload.assignee).to eq(agent) - inbox.inbox_members.where(user_id: agent.id).first.destroy! - - # round robin changes assignee in this case since agent doesn't have access to inbox - agent2 = create(:user, email: 'agent2@example.com', account: account) - create(:inbox_member, inbox: inbox, user: agent2) - allow(Redis::Alfred).to receive(:rpoplpush).and_return(agent2.id) - conversation.status = 'open' - conversation.save! - expect(conversation.reload.assignee).to eq(agent2) - end - end - - describe '#update_assignee' do - subject(:update_assignee) { conversation.update_assignee(agent) } - - let(:conversation) { create(:conversation, assignee: nil) } - let(:agent) do - create(:user, email: 'agent@example.com', account: conversation.account, role: :agent) - end - let(:assignment_mailer) { double(deliver: true) } - - it 'assigns the agent to conversation' do - expect(update_assignee).to eq(true) - expect(conversation.reload.assignee).to eq(agent) - end - - it 'creates a new notification for the agent' do - expect(update_assignee).to eq(true) - expect(agent.notifications.count).to eq(1) - end - - it 'does not create assignment notification if notification setting is turned off' do - notification_setting = agent.notification_settings.first - notification_setting.unselect_all_email_flags - notification_setting.unselect_all_push_flags - notification_setting.save! - - expect(update_assignee).to eq(true) - expect(agent.notifications.count).to eq(0) - end - - context 'when agent is current user' do - before do - Current.user = agent - end - - it 'creates self-assigned message activity' do - expect(update_assignee).to eq(true) - expect(conversation.messages.pluck(:content)).to include("#{agent.name} self-assigned this conversation") - end - end - end - describe '#update_labels' do let(:account) { create(:account) } let(:conversation) { create(:conversation, account: account) }