fix(slack): Show correct sender name and avatar for Slack replies (#13624)
This commit is contained in:
@@ -129,6 +129,7 @@ const props = defineProps({
|
|||||||
inReplyTo: { type: Object, default: null }, // eslint-disable-line vue/no-unused-properties
|
inReplyTo: { type: Object, default: null }, // eslint-disable-line vue/no-unused-properties
|
||||||
isEmailInbox: { type: Boolean, default: false },
|
isEmailInbox: { type: Boolean, default: false },
|
||||||
private: { 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 },
|
sender: { type: Object, default: null },
|
||||||
senderId: { type: Number, default: null },
|
senderId: { type: Number, default: null },
|
||||||
senderType: { type: String, default: null },
|
senderType: { type: String, default: null },
|
||||||
@@ -172,7 +173,10 @@ const variant = computed(() => {
|
|||||||
return MESSAGE_VARIANTS.AGENT;
|
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) {
|
if (isBot && props.messageType === MESSAGE_TYPES.OUTGOING) {
|
||||||
return MESSAGE_VARIANTS.BOT;
|
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) {
|
if (!props.sender) {
|
||||||
return {
|
const { senderName, senderAvatarUrl } = props.additionalAttributes || {};
|
||||||
name: t('CONVERSATION.BOT'),
|
if (senderName) {
|
||||||
src: '',
|
return { name: senderName, src: senderAvatarUrl ?? '' };
|
||||||
};
|
}
|
||||||
|
return { name: t('CONVERSATION.BOT'), src: '' };
|
||||||
}
|
}
|
||||||
|
|
||||||
const { sender } = props;
|
const { sender } = props;
|
||||||
|
|||||||
@@ -72,6 +72,10 @@ export default {
|
|||||||
return this.message.sender.available_name || this.message.sender.name;
|
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) {
|
if (this.useInboxAvatarForBot) {
|
||||||
return this.channelConfig.websiteName;
|
return this.channelConfig.websiteName;
|
||||||
}
|
}
|
||||||
@@ -87,9 +91,13 @@ export default {
|
|||||||
return displayImage;
|
return displayImage;
|
||||||
}
|
}
|
||||||
|
|
||||||
return this.message.sender
|
if (this.message.sender) {
|
||||||
? this.message.sender.avatar_url
|
return this.message.sender.avatar_url;
|
||||||
: displayImage;
|
}
|
||||||
|
|
||||||
|
return (
|
||||||
|
this.message.additional_attributes?.sender_avatar_url || displayImage
|
||||||
|
);
|
||||||
},
|
},
|
||||||
hasRecordedResponse() {
|
hasRecordedResponse() {
|
||||||
return (
|
return (
|
||||||
|
|||||||
@@ -27,6 +27,10 @@ module Integrations::Slack::SlackMessageHelper
|
|||||||
end
|
end
|
||||||
|
|
||||||
def create_message
|
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 = conversation.messages.build(
|
||||||
message_type: :outgoing,
|
message_type: :outgoing,
|
||||||
account_id: conversation.account_id,
|
account_id: conversation.account_id,
|
||||||
@@ -34,7 +38,8 @@ module Integrations::Slack::SlackMessageHelper
|
|||||||
content: Slack::Messages::Formatting.unescape(params[:event][:text] || ''),
|
content: Slack::Messages::Formatting.unescape(params[:event][:text] || ''),
|
||||||
external_source_id_slack: params[:event][:ts],
|
external_source_id_slack: params[:event][:ts],
|
||||||
private: private_note?,
|
private: private_note?,
|
||||||
sender: sender
|
sender: resolved_sender,
|
||||||
|
additional_attributes: slack_sender_attrs
|
||||||
)
|
)
|
||||||
process_attachments(params[:event][:files]) if attachments_present?
|
process_attachments(params[:event][:files]) if attachments_present?
|
||||||
@message.save!
|
@message.save!
|
||||||
@@ -81,9 +86,22 @@ module Integrations::Slack::SlackMessageHelper
|
|||||||
@conversation ||= Conversation.where(identifier: params[:event][:thread_ts]).first
|
@conversation ||= Conversation.where(identifier: params[:event][:thread_ts]).first
|
||||||
end
|
end
|
||||||
|
|
||||||
def sender
|
def resolve_slack_sender
|
||||||
user_email = slack_client.users_info(user: params[:event][:user])[:user][:profile][:email]
|
return [nil, nil, nil] unless params[:event][:user]
|
||||||
conversation.account.users.from_email(user_email)
|
|
||||||
|
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
|
end
|
||||||
|
|
||||||
def private_note?
|
def private_note?
|
||||||
|
|||||||
@@ -69,7 +69,7 @@ describe Integrations::Slack::IncomingMessageBuilder do
|
|||||||
expect(hook).not_to be_nil
|
expect(hook).not_to be_nil
|
||||||
messages_count = conversation.messages.count
|
messages_count = conversation.messages.count
|
||||||
builder = described_class.new(message_params)
|
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 }
|
2.times.each { builder.perform }
|
||||||
expect(conversation.messages.count).to eql(messages_count + 1)
|
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')
|
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
|
expect(hook).not_to be_nil
|
||||||
messages_count = conversation.messages.count
|
messages_count = conversation.messages.count
|
||||||
builder = described_class.new(message_params)
|
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
|
builder.perform
|
||||||
expect(conversation.messages.count).to eql(messages_count + 1)
|
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')
|
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
|
expect(hook).not_to be_nil
|
||||||
messages_count = conversation.messages.count
|
messages_count = conversation.messages.count
|
||||||
builder = described_class.new(private_message_params)
|
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
|
builder.perform
|
||||||
expect(conversation.messages.count).to eql(messages_count + 1)
|
expect(conversation.messages.count).to eql(messages_count + 1)
|
||||||
expect(conversation.messages.last.content).to eql('pRivate: A private note message')
|
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
|
messages_count = conversation.messages.count
|
||||||
message_with_attachments[:event][:files] = nil
|
message_with_attachments[:event][:files] = nil
|
||||||
builder = described_class.new(message_with_attachments)
|
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
|
builder.perform
|
||||||
expect(conversation.messages.count).to eql(messages_count)
|
expect(conversation.messages.count).to eql(messages_count)
|
||||||
end
|
end
|
||||||
@@ -139,7 +139,7 @@ describe Integrations::Slack::IncomingMessageBuilder do
|
|||||||
expect(hook).not_to be_nil
|
expect(hook).not_to be_nil
|
||||||
messages_count = conversation.messages.count
|
messages_count = conversation.messages.count
|
||||||
builder = described_class.new(message_with_attachments)
|
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
|
builder.perform
|
||||||
expect(conversation.messages.count).to eql(messages_count + 1)
|
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')
|
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!'
|
message_with_attachments[:event][:text] = 'Attached File!'
|
||||||
builder = described_class.new(message_with_attachments)
|
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
|
builder.perform
|
||||||
|
|
||||||
expect(conversation.messages.count).to eql(messages_count)
|
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'
|
video_attachment_params[:event][:files][0][:mimetype] = 'video/mp4'
|
||||||
|
|
||||||
builder = described_class.new(video_attachment_params)
|
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 { builder.perform }.not_to raise_error
|
||||||
expect(conversation.messages.last.attachments).to be_any
|
expect(conversation.messages.last.attachments).to be_any
|
||||||
end
|
end
|
||||||
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
|
context 'when link shared' do
|
||||||
let(:link_shared) do
|
let(:link_shared) do
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user