Skip to content

Commit

Permalink
Chore: Provide fixed attachment URLs for Channels (#4507)
Browse files Browse the repository at this point in the history
Prior to this change, The attachment URL sent from Chatwoot to 3rd party integrations like Whatsapp and Facebook
involved a 301 redirect before the original content is served. This causes intermittent breakages for the sent attachments.

fixes: #3632
ref: https://blog.saeloun.com/2021/09/14/rails-7-adds-expiring-urls-to-active-storage.html
  • Loading branch information
sojan-official committed Apr 20, 2022
1 parent 2b2252b commit 2c73df4
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 11 deletions.
5 changes: 4 additions & 1 deletion app/models/attachment.rb
Expand Up @@ -44,12 +44,15 @@ def push_event_data
base_data.merge(file_metadata)
end

# NOTE: the URl returned does a 301 redirect to the actual file
def file_url
file.attached? ? url_for(file) : ''
end

# NOTE: for External services use this methods since redirect doesn't work effectively in a lot of cases
def download_url
file.attached? ? rails_storage_proxy_url(file) : ''
ActiveStorage::Current.host = Rails.application.routes.default_url_options[:host] if ActiveStorage::Current.host.blank?
file.attached? ? file.blob.url : ''
end

def thumb_url
Expand Down
2 changes: 1 addition & 1 deletion app/models/channel/telegram.rb
Expand Up @@ -95,7 +95,7 @@ def send_attachments(message)
when 'file'
telegram_attachment[:type] = 'document'
end
telegram_attachment[:media] = attachment.file_url
telegram_attachment[:media] = attachment.download_url
telegram_attachments << telegram_attachment
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/channel/whatsapp.rb
Expand Up @@ -85,7 +85,7 @@ def send_text_message(phone_number, message)
def send_attachment_message(phone_number, message)
attachment = message.attachments.first
type = %w[image audio video].include?(attachment.file_type) ? attachment.file_type : 'document'
attachment_url = attachment.file_url
attachment_url = attachment.download_url
response = HTTParty.post(
"#{api_base_path}/messages",
headers: api_headers,
Expand Down
2 changes: 1 addition & 1 deletion app/services/facebook/send_on_facebook_service.rb
Expand Up @@ -36,7 +36,7 @@ def fb_attachment_message_params
attachment: {
type: attachment_type(attachment),
payload: {
url: attachment.file_url
url: attachment.download_url
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion app/services/instagram/send_on_instagram_service.rb
Expand Up @@ -39,7 +39,7 @@ def attachament_message_params
attachment: {
type: attachment_type(attachment),
payload: {
url: attachment.file_url
url: attachment.download_url
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/services/twilio/send_on_twilio_service.rb
Expand Up @@ -25,7 +25,7 @@ def message_params
end

def attachments
message.attachments.map(&:file_url)
message.attachments.map(&:download_url)
end

def inbox
Expand Down
4 changes: 2 additions & 2 deletions config/environments/test.rb
Expand Up @@ -20,8 +20,8 @@
config.public_file_server.headers = {
'Cache-Control' => "public, max-age=#{1.hour.to_i}"
}
config.action_mailer.default_url_options = { host: 'localhost', port: 3000 }
Rails.application.routes.default_url_options = { host: 'localhost', port: 3000 }
config.action_mailer.default_url_options = { host: 'http://localhost:3000' }
Rails.application.routes.default_url_options = { host: 'http://localhost:3000' }

# Show full error reports and disable caching.
config.consider_all_requests_local = true
Expand Down
2 changes: 1 addition & 1 deletion lib/integrations/slack/send_on_slack_service.rb
Expand Up @@ -73,7 +73,7 @@ def upload_file
end

def file_type
File.extname(message.attachments.first.file_url).strip.downcase[1..]
File.extname(message.attachments.first.download_url).strip.downcase[1..]
end

def file_information
Expand Down
12 changes: 12 additions & 0 deletions spec/models/attachment_spec.rb
@@ -0,0 +1,12 @@
require 'rails_helper'

RSpec.describe Attachment, type: :model 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: File.open(Rails.root.join('spec/assets/avatar.png')), filename: 'avatar.png', content_type: 'image/png')
expect(attachment.download_url).not_to eq nil
end
end
end
3 changes: 2 additions & 1 deletion spec/services/facebook/send_on_facebook_service_spec.rb
Expand Up @@ -57,6 +57,7 @@
attachment = message.attachments.new(account_id: message.account_id, file_type: :image)
attachment.file.attach(io: File.open(Rails.root.join('spec/assets/avatar.png')), filename: 'avatar.png', content_type: 'image/png')
message.save!
allow(attachment).to receive(:download_url).and_return('url1')
::Facebook::SendOnFacebookService.new(message: message).perform
expect(bot).to have_received(:deliver).with({
recipient: { id: contact_inbox.source_id },
Expand All @@ -70,7 +71,7 @@
attachment: {
type: 'image',
payload: {
url: attachment.file_url
url: 'url1'
}
}
},
Expand Down
4 changes: 3 additions & 1 deletion spec/services/sms/send_on_sms_service_spec.rb
Expand Up @@ -36,12 +36,14 @@
allow(HTTParty).to receive(:post).and_return(sms_request)
allow(sms_request).to receive(:success?).and_return(true)
allow(sms_request).to receive(:parsed_response).and_return({ 'id' => '123456789' })
allow(attachment).to receive(:download_url).and_return('url1')
allow(attachment2).to receive(:download_url).and_return('url2')
expect(HTTParty).to receive(:post).with(
'https://messaging.bandwidth.com/api/v2/users/1/messages',
basic_auth: { username: '1', password: '1' },
headers: { 'Content-Type' => 'application/json' },
body: { 'to' => '+123456789', 'from' => sms_channel.phone_number, 'text' => 'test', 'applicationId' => '1',
'media' => [attachment.download_url, attachment2.download_url] }.to_json
'media' => %w[url1 url2] }.to_json
)
described_class.new(message: message).perform
expect(message.reload.source_id).to eq('123456789')
Expand Down

0 comments on commit 2c73df4

Please sign in to comment.