feat: Activity messages for team assignments (#1893)

- assignment messages when the conversation team is changed
- change assignee based on team
- move round-robin and assignee logic to concerns

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)

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

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

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
 

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}"

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'
 

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

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

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) }