From 6b3f1114fdb117c2953c01eb824194b504c8996d Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Thu, 26 Feb 2026 14:45:15 +0400 Subject: [PATCH] fix(slack): Show correct sender name and avatar for Slack replies (#13624) --- .../components-next/message/Message.vue | 17 ++- .../widget/components/AgentMessage.vue | 14 ++- .../slack/slack_message_helper.rb | 26 +++- .../slack/incoming_message_builder_spec.rb | 114 ++++++++++++++++-- 4 files changed, 151 insertions(+), 20 deletions(-) diff --git a/app/javascript/dashboard/components-next/message/Message.vue b/app/javascript/dashboard/components-next/message/Message.vue index 66234984c..3de9d2d04 100644 --- a/app/javascript/dashboard/components-next/message/Message.vue +++ b/app/javascript/dashboard/components-next/message/Message.vue @@ -129,6 +129,7 @@ const props = defineProps({ inReplyTo: { type: Object, default: null }, // eslint-disable-line vue/no-unused-properties isEmailInbox: { type: Boolean, default: false }, private: { type: Boolean, default: false }, + additionalAttributes: { type: Object, default: () => ({}) }, // eslint-disable-line vue/no-unused-properties sender: { type: Object, default: null }, senderId: { type: Number, default: null }, senderType: { type: String, default: null }, @@ -172,7 +173,10 @@ const variant = computed(() => { return MESSAGE_VARIANTS.AGENT; } - const isBot = !props.sender || props.sender.type === SENDER_TYPES.AGENT_BOT; + const isBot = + props.sender?.type === SENDER_TYPES.AGENT_BOT || + props.senderType === SENDER_TYPES.AGENT_BOT || + (!props.sender && !props.additionalAttributes?.senderName); if (isBot && props.messageType === MESSAGE_TYPES.OUTGOING) { return MESSAGE_VARIANTS.BOT; } @@ -450,12 +454,13 @@ const avatarInfo = computed(() => { }; } - // If no sender, return bot info + // If no sender, check for Slack (or other integration) sender info if (!props.sender) { - return { - name: t('CONVERSATION.BOT'), - src: '', - }; + const { senderName, senderAvatarUrl } = props.additionalAttributes || {}; + if (senderName) { + return { name: senderName, src: senderAvatarUrl ?? '' }; + } + return { name: t('CONVERSATION.BOT'), src: '' }; } const { sender } = props; diff --git a/app/javascript/widget/components/AgentMessage.vue b/app/javascript/widget/components/AgentMessage.vue index e8d245ddb..a13f7bf65 100755 --- a/app/javascript/widget/components/AgentMessage.vue +++ b/app/javascript/widget/components/AgentMessage.vue @@ -72,6 +72,10 @@ export default { return this.message.sender.available_name || this.message.sender.name; } + if (this.message.additional_attributes?.sender_name) { + return this.message.additional_attributes.sender_name; + } + if (this.useInboxAvatarForBot) { return this.channelConfig.websiteName; } @@ -87,9 +91,13 @@ export default { return displayImage; } - return this.message.sender - ? this.message.sender.avatar_url - : displayImage; + if (this.message.sender) { + return this.message.sender.avatar_url; + } + + return ( + this.message.additional_attributes?.sender_avatar_url || displayImage + ); }, hasRecordedResponse() { return ( diff --git a/lib/integrations/slack/slack_message_helper.rb b/lib/integrations/slack/slack_message_helper.rb index 0ee328fb3..3af57a4c3 100644 --- a/lib/integrations/slack/slack_message_helper.rb +++ b/lib/integrations/slack/slack_message_helper.rb @@ -27,6 +27,10 @@ module Integrations::Slack::SlackMessageHelper end def create_message + resolved_sender, sender_name, sender_avatar_url = resolve_slack_sender + slack_sender_attrs = {} + slack_sender_attrs[:sender_name] = sender_name if sender_name + slack_sender_attrs[:sender_avatar_url] = sender_avatar_url if sender_avatar_url @message = conversation.messages.build( message_type: :outgoing, account_id: conversation.account_id, @@ -34,7 +38,8 @@ module Integrations::Slack::SlackMessageHelper content: Slack::Messages::Formatting.unescape(params[:event][:text] || ''), external_source_id_slack: params[:event][:ts], private: private_note?, - sender: sender + sender: resolved_sender, + additional_attributes: slack_sender_attrs ) process_attachments(params[:event][:files]) if attachments_present? @message.save! @@ -81,9 +86,22 @@ module Integrations::Slack::SlackMessageHelper @conversation ||= Conversation.where(identifier: params[:event][:thread_ts]).first end - def sender - user_email = slack_client.users_info(user: params[:event][:user])[:user][:profile][:email] - conversation.account.users.from_email(user_email) + def resolve_slack_sender + return [nil, nil, nil] unless params[:event][:user] + + slack_user = slack_client.users_info(user: params[:event][:user])[:user] + chatwoot_user = conversation.account.users.from_email(slack_user[:profile][:email]) + return [chatwoot_user, nil, nil] if chatwoot_user + + sender_name = slack_user.dig(:profile, :display_name).presence || + slack_user[:real_name].presence || + slack_user[:name] + sender_avatar_url = slack_user.dig(:profile, :image_192).presence + [nil, sender_name, sender_avatar_url] + rescue Slack::Web::Api::Errors::MissingScope + raise + rescue StandardError + [nil, nil, nil] end def private_note? diff --git a/spec/lib/integrations/slack/incoming_message_builder_spec.rb b/spec/lib/integrations/slack/incoming_message_builder_spec.rb index 2ce206489..65234767e 100644 --- a/spec/lib/integrations/slack/incoming_message_builder_spec.rb +++ b/spec/lib/integrations/slack/incoming_message_builder_spec.rb @@ -69,7 +69,7 @@ describe Integrations::Slack::IncomingMessageBuilder do expect(hook).not_to be_nil messages_count = conversation.messages.count builder = described_class.new(message_params) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) 2.times.each { builder.perform } expect(conversation.messages.count).to eql(messages_count + 1) expect(conversation.messages.last.content).to eql('this is test https://chatwoot.com Hey @Sojan Test again') @@ -79,7 +79,7 @@ describe Integrations::Slack::IncomingMessageBuilder do expect(hook).not_to be_nil messages_count = conversation.messages.count builder = described_class.new(message_params) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) builder.perform expect(conversation.messages.count).to eql(messages_count + 1) expect(conversation.messages.last.content).to eql('this is test https://chatwoot.com Hey @Sojan Test again') @@ -89,7 +89,7 @@ describe Integrations::Slack::IncomingMessageBuilder do expect(hook).not_to be_nil messages_count = conversation.messages.count builder = described_class.new(private_message_params) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) builder.perform expect(conversation.messages.count).to eql(messages_count + 1) expect(conversation.messages.last.content).to eql('pRivate: A private note message') @@ -130,7 +130,7 @@ describe Integrations::Slack::IncomingMessageBuilder do messages_count = conversation.messages.count message_with_attachments[:event][:files] = nil builder = described_class.new(message_with_attachments) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) builder.perform expect(conversation.messages.count).to eql(messages_count) end @@ -139,7 +139,7 @@ describe Integrations::Slack::IncomingMessageBuilder do expect(hook).not_to be_nil messages_count = conversation.messages.count builder = described_class.new(message_with_attachments) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) builder.perform expect(conversation.messages.count).to eql(messages_count + 1) expect(conversation.messages.last.content).to eql('this is test https://chatwoot.com Hey @Sojan Test again') @@ -152,7 +152,7 @@ describe Integrations::Slack::IncomingMessageBuilder do message_with_attachments[:event][:text] = 'Attached File!' builder = described_class.new(message_with_attachments) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) builder.perform expect(conversation.messages.count).to eql(messages_count) @@ -165,13 +165,113 @@ describe Integrations::Slack::IncomingMessageBuilder do video_attachment_params[:event][:files][0][:mimetype] = 'video/mp4' builder = described_class.new(video_attachment_params) - allow(builder).to receive(:sender).and_return(nil) + allow(builder).to receive(:resolve_slack_sender).and_return([nil, nil, nil]) expect { builder.perform }.not_to raise_error expect(conversation.messages.last.attachments).to be_any end end + context 'when resolving slack sender' do + let(:builder) { described_class.new(message_params) } + + before do + allow(builder).to receive(:slack_client).and_return(slack_client) + end + + context 'when slack user email matches a chatwoot agent' do + before do + create(:user, account: conversation.account, email: 'agent@example.com') + slack_response = { + user: { + profile: { email: 'agent@example.com', display_name: 'Muhsin K', image_192: 'https://avatars.slack-edge.com/avatar.png' }, + real_name: 'Muhsin K', + name: 'muhsink' + } + } + allow(slack_client).to receive(:users_info) + .with(user: message_params[:event][:user]) + .and_return(slack_response) + end + + it 'sets the matched agent as message sender' do + builder.perform + expect(conversation.messages.last.sender).to eq(conversation.account.users.from_email('agent@example.com')) + end + + it 'does not store sender_name in additional_attributes' do + builder.perform + expect(conversation.messages.last.additional_attributes).not_to have_key('sender_name') + end + end + + context 'when slack user email does not match any chatwoot agent' do + before do + slack_response = { + user: { + profile: { email: 'unknown@example.com', display_name: 'Muhsin K', image_192: 'https://avatars.slack-edge.com/avatar.png' }, + real_name: 'Muhsin K', + name: 'muhsink' + } + } + allow(slack_client).to receive(:users_info) + .with(user: message_params[:event][:user]) + .and_return(slack_response) + end + + it 'saves sender_name from slack display_name in additional_attributes' do + builder.perform + expect(conversation.messages.last.sender).to be_nil + expect(conversation.messages.last.additional_attributes['sender_name']).to eq('Muhsin K') + end + + it 'saves sender_avatar_url from slack profile image in additional_attributes' do + builder.perform + expect(conversation.messages.last.additional_attributes['sender_avatar_url']) + .to eq('https://avatars.slack-edge.com/avatar.png') + end + + it 'falls back to real_name when display_name is blank' do + allow(slack_client).to receive(:users_info).and_return({ + user: { + profile: { email: 'unknown@example.com', display_name: '', + image_192: nil }, real_name: 'Muhsin K', name: 'muhsink' + } + }) + builder.perform + expect(conversation.messages.last.additional_attributes['sender_name']).to eq('Muhsin K') + end + + it 'falls back to slack username when display_name and real_name are both blank' do + allow(slack_client).to receive(:users_info).and_return({ + user: { + profile: { email: 'unknown@example.com', display_name: '', + image_192: nil }, real_name: '', name: 'muhsink' + } + }) + builder.perform + expect(conversation.messages.last.additional_attributes['sender_name']).to eq('muhsink') + end + end + + context 'when the slack API call raises an error' do + before do + allow(slack_client).to receive(:users_info).and_raise(StandardError, 'API error') + end + + it 'creates the message with nil sender' do + expect { builder.perform }.not_to raise_error + expect(conversation.messages.last.sender).to be_nil + end + + it 'does not store sender info in additional_attributes' do + builder.perform + expect(conversation.messages.last.additional_attributes).not_to have_key('sender_name') + expect(conversation.messages.last.additional_attributes).not_to have_key('sender_avatar_url') + end + end + end + context 'when link shared' do let(:link_shared) do {