fix: avoid Slack file upload API for fallback messages (#10461)

Skip calling the Slack file upload API for message types such as
fallback (e.g., Facebook and location messages) that lack actual file
data in attachments. This prevents unnecessary API calls and resolves a
Sentry error currently occurring in production.

fixes: https://github.com/chatwoot/chatwoot/issues/10460
This commit is contained in:
Sojan Jose
2024-11-26 12:56:40 +08:00
committed by GitHub
parent b9d888d8ab
commit 12a82b6459
4 changed files with 43 additions and 2 deletions

View File

@@ -69,6 +69,10 @@ class Attachment < ApplicationRecord
end
end
def with_attached_file?
[:image, :audio, :video, :file].include?(file_type.to_sym)
end
private
def file_metadata

View File

@@ -117,6 +117,8 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService
end
def upload_file
return unless message.attachments.first.with_attached_file?
result = slack_client.files_upload({
channels: hook.reference_id,
initial_comment: 'Attached File!',

View File

@@ -180,6 +180,28 @@ describe Integrations::Slack::SendOnSlackService do
expect(message.attachments).to be_any
end
it 'will not call file_upload if attachment does not have a file (e.g facebook - fallback type)' do
expect(slack_client).to receive(:chat_postMessage).with(
channel: hook.reference_id,
text: message.content,
username: "#{message.sender.name} (Contact)",
thread_ts: conversation.identifier,
icon_url: anything,
unfurl_links: true
).and_return(slack_message)
message.attachments.new(account_id: message.account_id, file_type: :fallback)
expect(slack_client).not_to receive(:files_upload)
message.save!
builder.perform
expect(message.external_source_id_slack).to eq 'cw-origin-6789.12345'
expect(message.attachments).to be_any
end
it 'sent a template message on slack' do
builder = described_class.new(message: template_message, hook: hook)
allow(builder).to receive(:slack_client).and_return(slack_client)

View File

@@ -1,8 +1,9 @@
require 'rails_helper'
RSpec.describe Attachment do
let!(:message) { create(:message) }
describe 'external url validations' do
let(:message) { create(:message) }
let(:attachment) { message.attachments.new(account_id: message.account_id, file_type: :image) }
before do
@@ -25,13 +26,25 @@ RSpec.describe Attachment do
describe 'download_url' do
it 'returns valid download url' do
message = create(:message)
attachment = message.attachments.new(account_id: message.account_id, file_type: :image)
attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
expect(attachment.download_url).not_to be_nil
end
end
describe 'with_attached_file?' do
it 'returns true if its an attachment with file' do
attachment = message.attachments.new(account_id: message.account_id, file_type: :image)
attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
expect(attachment.with_attached_file?).to be true
end
it 'returns false if its an attachment with out a file' do
attachment = message.attachments.new(account_id: message.account_id, file_type: :fallback)
expect(attachment.with_attached_file?).to be false
end
end
describe 'push_event_data for instagram story mentions' do
let(:instagram_message) { create(:message, :instagram_story_mention) }