fix: assignee_changed callback not getting triggered during conversation creation (#9334)

The reload method in our callback was refreshing the object and hence the saved_change_to_assignee_id? Method wasn't working in the following callbacks.

This impacted the listeners subscribing to the event `ASSIGNEE_CHANGE`, `TEAM_CHANGE` etc
This commit is contained in:
Sojan Jose
2024-05-06 11:48:17 -07:00
committed by GitHub
parent 2af0d58deb
commit f6d7f3b665
9 changed files with 59 additions and 27 deletions

View File

@@ -10,14 +10,15 @@ class AsyncDispatcher < BaseDispatcher
def listeners def listeners
[ [
AutomationRuleListener.instance,
CampaignListener.instance, CampaignListener.instance,
CsatSurveyListener.instance, CsatSurveyListener.instance,
HookListener.instance, HookListener.instance,
InstallationWebhookListener.instance, InstallationWebhookListener.instance,
NotificationListener.instance, NotificationListener.instance,
ParticipationListener.instance,
ReportingEventListener.instance, ReportingEventListener.instance,
WebhookListener.instance, WebhookListener.instance
AutomationRuleListener.instance
] ]
end end
end end

View File

@@ -0,0 +1,8 @@
class ParticipationListener < BaseListener
include Events::Types
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?
end
end

View File

@@ -38,7 +38,6 @@ module AssignmentHandler
def process_assignment_changes def process_assignment_changes
process_assignment_activities process_assignment_activities
process_participant_assignment
end end
def process_assignment_activities def process_assignment_activities
@@ -49,10 +48,4 @@ module AssignmentHandler
create_assignee_change_activity(user_name) create_assignee_change_activity(user_name)
end end
end end
def process_participant_assignment
return unless saved_change_to_assignee_id? && assignee_id.present?
conversation_participants.find_or_create_by!(user_id: assignee_id)
end
end end

View File

@@ -15,7 +15,6 @@ module AutoAssignmentHandler
return unless should_run_auto_assignment? return unless should_run_auto_assignment?
::AutoAssignment::AgentAssignmentService.new(conversation: self, allowed_agent_ids: inbox.member_ids_with_assignment_capacity).perform ::AutoAssignment::AgentAssignmentService.new(conversation: self, allowed_agent_ids: inbox.member_ids_with_assignment_capacity).perform
conversation_participants.find_or_create_by(user_id: assignee_id) if assignee_id.present?
end end
def should_run_auto_assignment? def should_run_auto_assignment?

View File

@@ -110,7 +110,7 @@ class Conversation < ApplicationRecord
after_update_commit :execute_after_update_commit_callbacks after_update_commit :execute_after_update_commit_callbacks
after_create_commit :notify_conversation_creation after_create_commit :notify_conversation_creation
after_commit :set_display_id, unless: :display_id? after_create_commit :load_attributes_created_by_db_triggers
delegate :auto_resolve_duration, to: :account delegate :auto_resolve_duration, to: :account
@@ -257,8 +257,13 @@ class Conversation < ApplicationRecord
assignee_id.present? && Current.user&.id == assignee_id assignee_id.present? && Current.user&.id == assignee_id
end end
def set_display_id def load_attributes_created_by_db_triggers
reload # Display id is set via a trigger in the database
# So we need to specifically fetch it after the record is created
# We can't use reload because it will clear the previous changes, which we need for the dispatcher
obj_from_db = self.class.find(id)
self[:display_id] = obj_from_db[:display_id]
self[:uuid] = obj_from_db[:uuid]
end end
def notify_status_change def notify_status_change

View File

@@ -21,7 +21,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
describe '#perform - SLA misses' do describe '#perform - SLA misses' do
context 'when first response SLA is missed' do context 'when first response SLA is missed' do
before { sla_policy.update(first_response_time_threshold: 1.hour) } before { applied_sla.sla_policy.update(first_response_time_threshold: 1.hour) }
it 'updates the SLA status to missed and logs a warning' do it 'updates the SLA status to missed and logs a warning' do
allow(Rails.logger).to receive(:warn) allow(Rails.logger).to receive(:warn)
@@ -42,7 +42,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
context 'when next response SLA is missed' do context 'when next response SLA is missed' do
before do before do
sla_policy.update(next_response_time_threshold: 1.hour) applied_sla.sla_policy.update(next_response_time_threshold: 1.hour)
conversation.update(first_reply_created_at: 5.hours.ago, waiting_since: 5.hours.ago) conversation.update(first_reply_created_at: 5.hours.ago, waiting_since: 5.hours.ago)
end end
@@ -64,7 +64,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
end end
context 'when resolution time SLA is missed' do context 'when resolution time SLA is missed' do
before { sla_policy.update(resolution_time_threshold: 1.hour) } before { applied_sla.sla_policy.update(resolution_time_threshold: 1.hour) }
it 'updates the SLA status to missed and logs a warning' do it 'updates the SLA status to missed and logs a warning' do
allow(Rails.logger).to receive(:warn) allow(Rails.logger).to receive(:warn)
@@ -89,7 +89,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
context 'when resolved conversation with resolution time SLA is missed' do context 'when resolved conversation with resolution time SLA is missed' do
before do before do
conversation.resolved! conversation.resolved!
sla_policy.update(resolution_time_threshold: 1.hour) applied_sla.sla_policy.update(resolution_time_threshold: 1.hour)
end end
it 'does not update the SLA status to missed' do it 'does not update the SLA status to missed' do
@@ -100,7 +100,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
context 'when multiple SLAs are missed' do context 'when multiple SLAs are missed' do
before do before do
sla_policy.update(first_response_time_threshold: 1.hour, next_response_time_threshold: 1.hour, resolution_time_threshold: 1.hour) applied_sla.sla_policy.update(first_response_time_threshold: 1.hour, next_response_time_threshold: 1.hour, resolution_time_threshold: 1.hour)
conversation.update(first_reply_created_at: 5.hours.ago, waiting_since: 5.hours.ago) conversation.update(first_reply_created_at: 5.hours.ago, waiting_since: 5.hours.ago)
end end
@@ -119,7 +119,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
describe '#perform - SLA hits' do describe '#perform - SLA hits' do
context 'when first response SLA is hit' do context 'when first response SLA is hit' do
before do before do
sla_policy.update(first_response_time_threshold: 6.hours) applied_sla.sla_policy.update(first_response_time_threshold: 6.hours)
conversation.update(first_reply_created_at: 30.minutes.ago) conversation.update(first_reply_created_at: 30.minutes.ago)
end end
@@ -142,7 +142,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
context 'when next response SLA is hit' do context 'when next response SLA is hit' do
before do before do
sla_policy.update(next_response_time_threshold: 6.hours) applied_sla.sla_policy.update(next_response_time_threshold: 6.hours)
conversation.update(first_reply_created_at: 30.minutes.ago, waiting_since: nil) conversation.update(first_reply_created_at: 30.minutes.ago, waiting_since: nil)
end end
@@ -164,7 +164,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
context 'when resolution time SLA is hit' do context 'when resolution time SLA is hit' do
before do before do
sla_policy.update(resolution_time_threshold: 8.hours) applied_sla.sla_policy.update(resolution_time_threshold: 8.hours)
conversation.resolved! conversation.resolved!
end end
@@ -182,7 +182,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do
describe 'SLA evaluation with frt hit, multiple nrt misses and rt miss' do describe 'SLA evaluation with frt hit, multiple nrt misses and rt miss' do
before do before do
# Setup SLA Policy thresholds # Setup SLA Policy thresholds
sla_policy.update( applied_sla.sla_policy.update(
first_response_time_threshold: 2.hours, # Hit frt first_response_time_threshold: 2.hours, # Hit frt
next_response_time_threshold: 1.hour, # Miss nrt multiple times next_response_time_threshold: 1.hour, # Miss nrt multiple times
resolution_time_threshold: 4.hours # Miss rt resolution_time_threshold: 4.hours # Miss rt

View File

@@ -0,0 +1,26 @@
require 'rails_helper'
describe ParticipationListener do
let(:listener) { described_class.instance }
let!(:account) { create(:account) }
let!(:admin) { create(:user, account: account, role: :administrator) }
let!(:inbox) { create(:inbox, account: account) }
let!(:agent) { create(:user, account: account, role: :agent) }
let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: agent) }
before do
create(:inbox_member, inbox: inbox, user: agent)
Current.user = nil
Current.account = nil
end
describe '#assignee_changed' do
let(:event_name) { :assignee_changed }
let!(:event) { Events::Base.new(event_name, Time.zone.now, conversation: conversation) }
it 'adds the assignee as a participant to the conversation' do
expect(conversation.conversation_participants.map(&:user_id)).not_to include(admin.id)
listener.assignee_changed(event)
expect(conversation.conversation_participants.map(&:user_id)).to include(agent.id)
end
end
end

View File

@@ -26,10 +26,6 @@ shared_examples_for 'auto_assignment_handler' do
expect(conversation.reload.assignee).to eq(agent) expect(conversation.reload.assignee).to eq(agent)
end end
it 'adds assignee to conversation participants' do
expect(conversation.conversation_participants.map(&:user)).to include(agent)
end
it 'will not auto assign agent if enable_auto_assignment is false' do it 'will not auto assign agent if enable_auto_assignment is false' do
inbox.update(enable_auto_assignment: false) inbox.update(enable_auto_assignment: false)

View File

@@ -107,7 +107,11 @@ RSpec.describe Message do
end end
describe 'message create event' do describe 'message create event' do
let(:conversation) { create(:conversation) } let!(:conversation) { create(:conversation) }
before do
conversation.reload
end
it 'updates the conversation first reply created at if it is the first outgoing message' do it 'updates the conversation first reply created at if it is the first outgoing message' do
expect(conversation.first_reply_created_at).to be_nil expect(conversation.first_reply_created_at).to be_nil