Conversations could get reassigned repeatedly in quick succession — bouncing between agents with several "Assigned to X" activity messages in a row — whenever an assignment write raced against another assignment write on the same conversation (an automation rule vs. Assignment V2's auto-assign, two manual assignment clicks, two concurrent status transitions triggering legacy round-robin, etc). This was reproducible with or without Assignment V2 enabled; it wasn't a V2-specific issue, just more visible under message bursts. Assignment V2's own job-vs-job race was already fixed separately (#14495) — this PR covers the other writers that don't check the conversation's current state before overwriting it. ## How to reproduce Configure an automation rule that assigns an agent/team on `message_created`, then send a burst of messages on one conversation from a channel/inbox with concurrent message delivery (or race it against another assignment source — a manual assignment click, or a status change that triggers legacy auto-assignment). The conversation's assignee flips between agents multiple times, each producing its own activity message, even though later writes were often redundant (setting the assignee to a value it already had, from the writer's stale point of view). ## What changed Three assignment write paths now take a row lock (`with_lock`) before writing, so a concurrent writer re-reads the conversation's true current state before deciding whether a write is actually needed: - `ActionService#assign_agent`/`#assign_team` (and the corresponding unassign methods) — used by automation rules, macros, and delayed automations. - `Conversations::AssignmentService#assign_agent`/`#assign_agent_bot` — the dashboard's assignee dropdown (human agent or AgentBot). - `AutoAssignment::AgentAssignmentService#perform` — the legacy (non-V2) round-robin auto-assignment path. A write that would just reapply an already-current value becomes a genuine no-op instead of producing a redundant `UPDATE` and duplicate activity message. Legitimate reassignment (a real change in target, or reassignment away from an agent who lost inbox access) is unaffected.
123 lines
3.5 KiB
Ruby
123 lines
3.5 KiB
Ruby
class ActionService
|
|
include EmailHelper
|
|
|
|
def initialize(conversation)
|
|
@conversation = conversation.reload
|
|
@account = @conversation.account
|
|
end
|
|
|
|
def mute_conversation(_params)
|
|
@conversation.mute!
|
|
end
|
|
|
|
def snooze_conversation(_params)
|
|
@conversation.snoozed!
|
|
end
|
|
|
|
def resolve_conversation(_params)
|
|
@conversation.resolved!
|
|
end
|
|
|
|
def open_conversation(_params)
|
|
@conversation.open!
|
|
end
|
|
|
|
def pending_conversation(_params)
|
|
@conversation.pending!
|
|
end
|
|
|
|
def change_status(status)
|
|
@conversation.update!(status: status[0])
|
|
end
|
|
|
|
def change_priority(priority)
|
|
@conversation.update!(priority: (priority[0] == 'nil' ? nil : priority[0]))
|
|
end
|
|
|
|
def add_label(labels)
|
|
return if labels.empty?
|
|
|
|
@conversation.reload.add_labels(labels)
|
|
end
|
|
|
|
def assign_agent(agent_ids = [])
|
|
return @conversation.with_lock { @conversation.update!(assignee_id: nil) } if agent_ids[0] == 'nil'
|
|
|
|
agent_ids = [last_responding_agent_id] if agent_ids[0] == 'last_responding_agent'
|
|
return unless agent_belongs_to_inbox?(agent_ids)
|
|
|
|
@agent = @account.users.find_by(id: agent_ids)
|
|
return unless @agent.present? && @agent.confirmed?
|
|
|
|
# Locks the row so a concurrent writer (e.g. AutoAssignment::AssignmentService) can't
|
|
# interleave with a stale in-memory assignee_id and produce a spurious duplicate activity message.
|
|
@conversation.with_lock { @conversation.update!(assignee_id: @agent.id) }
|
|
end
|
|
|
|
def remove_label(labels)
|
|
return if labels.empty?
|
|
|
|
labels = @conversation.label_list - labels
|
|
@conversation.update(label_list: labels)
|
|
end
|
|
|
|
def assign_team(team_ids = [])
|
|
# 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.with_lock { @conversation.update!(team_id: nil) } if should_unassign
|
|
|
|
# check if team belongs to account only if team_id is present
|
|
# if team_id is nil, then it means that the team is being unassigned
|
|
return unless !team_ids[0].nil? && team_belongs_to_account?(team_ids)
|
|
|
|
@conversation.with_lock { @conversation.update!(team_id: team_ids[0]) }
|
|
end
|
|
|
|
def remove_assigned_agent(_params)
|
|
@conversation.with_lock { @conversation.update!(assignee_id: nil) }
|
|
end
|
|
|
|
def remove_assigned_team(_params)
|
|
@conversation.with_lock { @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
|
|
|
|
private
|
|
|
|
def last_responding_agent_id
|
|
@conversation.messages.outgoing.where(sender_type: 'User', private: false).last&.sender_id
|
|
end
|
|
|
|
def agent_belongs_to_inbox?(agent_ids)
|
|
member_ids = @conversation.inbox.members.pluck(:user_id)
|
|
assignable_agent_ids = member_ids + @account.administrators.ids
|
|
|
|
assignable_agent_ids.include?(agent_ids[0])
|
|
end
|
|
|
|
def team_belongs_to_account?(team_ids)
|
|
@account.team_ids.include?(team_ids[0])
|
|
end
|
|
|
|
def conversation_a_tweet?
|
|
return false if @conversation.additional_attributes.blank?
|
|
|
|
@conversation.additional_attributes['type'] == 'tweet'
|
|
end
|
|
end
|
|
|
|
ActionService.include_mod_with('ActionService')
|