diff --git a/app/listeners/participation_listener.rb b/app/listeners/participation_listener.rb index d0a5f48c2..b9f94e252 100644 --- a/app/listeners/participation_listener.rb +++ b/app/listeners/participation_listener.rb @@ -3,6 +3,13 @@ class ParticipationListener < BaseListener def assignee_changed(event) conversation, _account = extract_conversation_and_account(event) - conversation.conversation_participants.find_or_create_by!(user_id: conversation.assignee_id) if conversation.assignee_id.present? + return if conversation.assignee_id.blank? + + conversation.conversation_participants.find_or_create_by!(user_id: conversation.assignee_id) + # We have observed race conditions triggering these errors + # example: Assignment happening via automation, while auto assignment is also configured. + rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid + Rails.logger.warn "Failed to create conversation participant for account #{conversation.account.id} " \ + ": user #{conversation.assignee_id} : conversation #{conversation.id}" end end diff --git a/app/models/concerns/assignment_handler.rb b/app/models/concerns/assignment_handler.rb index 0fab737ed..5dc89f779 100644 --- a/app/models/concerns/assignment_handler.rb +++ b/app/models/concerns/assignment_handler.rb @@ -32,7 +32,7 @@ module AssignmentHandler ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? }, TEAM_CHANGED => -> { saved_change_to_team_id? } }.each do |event, condition| - condition.call && dispatcher_dispatch(event) + condition.call && dispatcher_dispatch(event, previous_changes) end end diff --git a/spec/listeners/participation_listener_spec.rb b/spec/listeners/participation_listener_spec.rb index 69c030de6..9ed70fc33 100644 --- a/spec/listeners/participation_listener_spec.rb +++ b/spec/listeners/participation_listener_spec.rb @@ -9,8 +9,6 @@ describe ParticipationListener do before do create(:inbox_member, inbox: inbox, user: agent) - Current.user = nil - Current.account = nil end describe '#assignee_changed' do @@ -22,5 +20,19 @@ describe ParticipationListener do listener.assignee_changed(event) expect(conversation.conversation_participants.map(&:user_id)).to include(agent.id) end + + it 'does not fail if the conversation participant already exists' do + conversation.conversation_participants.create!(user: agent) + expect { listener.assignee_changed(event) }.not_to raise_error + end + + it 'logs a debug message if participant save fails due to a race condition' do + allow(Rails.logger).to receive(:warn) + allow(conversation).to receive(:conversation_participants).and_return(double) + allow(conversation.conversation_participants).to receive(:find_or_create_by!).and_raise(ActiveRecord::RecordNotUnique) + expect { listener.assignee_changed(event) }.not_to raise_error + expect(Rails.logger).to have_received(:warn).with('Failed to create conversation participant for account ' \ + "#{account.id} : user #{agent.id} : conversation #{conversation.id}") + end end end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 2f79b08f2..0ca7303ef 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -156,7 +156,7 @@ RSpec.describe Conversation do 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, performed_by: nil) + changed_attributes: changed_attributes, 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, performed_by: nil)