From 12a82b6459f7ff229bc0c0428616bad7a00582ab Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Tue, 26 Nov 2024 12:56:40 +0800
Subject: [PATCH] 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
---
 app/models/attachment.rb                      |  4 ++++
 .../slack/send_on_slack_service.rb            |  2 ++
 .../slack/send_on_slack_service_spec.rb       | 22 +++++++++++++++++++
 spec/models/attachment_spec.rb                | 17 ++++++++++++--
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index b9ed804d3..65e56a21f 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -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
diff --git a/lib/integrations/slack/send_on_slack_service.rb b/lib/integrations/slack/send_on_slack_service.rb
index aa9d3ad5b..20a65905c 100644
--- a/lib/integrations/slack/send_on_slack_service.rb
+++ b/lib/integrations/slack/send_on_slack_service.rb
@@ -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!',
diff --git a/spec/lib/integrations/slack/send_on_slack_service_spec.rb b/spec/lib/integrations/slack/send_on_slack_service_spec.rb
index 3474cbfcd..6524154b4 100644
--- a/spec/lib/integrations/slack/send_on_slack_service_spec.rb
+++ b/spec/lib/integrations/slack/send_on_slack_service_spec.rb
@@ -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)
diff --git a/spec/models/attachment_spec.rb b/spec/models/attachment_spec.rb
index 6b05d26fb..b1d19bd0e 100644
--- a/spec/models/attachment_spec.rb
+++ b/spec/models/attachment_spec.rb
@@ -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) }