From a27737e91c16101eeb74d5c86d1f2781d1132015 Mon Sep 17 00:00:00 2001 From: Gabriel Jablonski Date: Thu, 25 Dec 2025 19:27:47 -0300 Subject: [PATCH] feat: allow updating attachment metadata (#172) * feat: allow updating attachment metadata * feat: allow updating attachment metadata * feat: add tests for handling requests without meta parameter and empty meta parameter --- app/builders/messages/message_builder.rb | 27 ++- .../conversations/attachments_controller.rb | 34 ++++ .../attachments/update.json.jbuilder | 1 + config/routes.rb | 1 + .../builders/messages/message_builder_spec.rb | 28 +++ .../attachments_controller_spec.rb | 187 ++++++++++++++++++ 6 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v1/accounts/conversations/attachments_controller.rb create mode 100644 app/views/api/v1/accounts/conversations/attachments/update.json.jbuilder create mode 100644 spec/controllers/api/v1/accounts/conversations/attachments_controller_spec.rb diff --git a/app/builders/messages/message_builder.rb b/app/builders/messages/message_builder.rb index af73315f8..b5f516946 100644 --- a/app/builders/messages/message_builder.rb +++ b/app/builders/messages/message_builder.rb @@ -14,6 +14,7 @@ class Messages::MessageBuilder # rubocop:disable Metrics/ClassLength @message_type = params[:message_type] || 'outgoing' @attachments = params[:attachments] @is_recorded_audio = params[:is_recorded_audio] + @attachments_metadata = normalize_attachments_metadata(params[:attachments_metadata]) @automation_rule = content_attributes&.dig(:automation_rule_id) return unless params.instance_of?(ActionController::Parameters) @@ -69,7 +70,14 @@ class Messages::MessageBuilder # rubocop:disable Metrics/ClassLength end end - def process_metadata(attachment) # rubocop:disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity + def process_metadata(attachment) + meta = {} + meta.merge!(recorded_audio_metadata(attachment) || {}) + meta.merge!(custom_attachment_metadata(attachment) || {}) + meta.presence + end + + def recorded_audio_metadata(attachment) # rubocop:disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity # NOTE: `is_recorded_audio` can be either a boolean or an array of file names. return unless @is_recorded_audio return { is_recorded_audio: true } if @is_recorded_audio == true @@ -85,6 +93,23 @@ class Messages::MessageBuilder # rubocop:disable Metrics/ClassLength nil end + def custom_attachment_metadata(attachment) + return unless @attachments_metadata.is_a?(Hash) + + filename = attachment.respond_to?(:original_filename) ? attachment.original_filename : nil + return unless filename + + metadata = @attachments_metadata[filename] + metadata.to_h if metadata.present? + end + + def normalize_attachments_metadata(metadata) + return if metadata.blank? + + metadata = metadata.to_unsafe_h if metadata.respond_to?(:to_unsafe_h) + metadata.deep_stringify_keys + end + def process_emails return unless @conversation.inbox&.inbox_type == 'Email' diff --git a/app/controllers/api/v1/accounts/conversations/attachments_controller.rb b/app/controllers/api/v1/accounts/conversations/attachments_controller.rb new file mode 100644 index 000000000..0c2dc9977 --- /dev/null +++ b/app/controllers/api/v1/accounts/conversations/attachments_controller.rb @@ -0,0 +1,34 @@ +class Api::V1::Accounts::Conversations::AttachmentsController < Api::V1::Accounts::Conversations::BaseController + before_action :set_message + before_action :set_attachment + before_action :validate_meta_size, only: [:update] + + MAX_META_SIZE = 16.kilobytes + + def update + @attachment.update!(permitted_params) + @attachment.message.send_update_event + end + + private + + def set_message + @message = @conversation.messages.find(params[:message_id]) + end + + def set_attachment + @attachment = @message.attachments.find(params[:id]) + end + + def permitted_params + params.permit(meta: {}) + end + + def validate_meta_size + return if params[:meta].blank? + + return unless params[:meta].to_json.bytesize > MAX_META_SIZE + + render json: { error: "Metadata size exceeds maximum allowed (#{MAX_META_SIZE / 1024}KB)" }, status: :unprocessable_entity + end +end diff --git a/app/views/api/v1/accounts/conversations/attachments/update.json.jbuilder b/app/views/api/v1/accounts/conversations/attachments/update.json.jbuilder new file mode 100644 index 000000000..681a469e4 --- /dev/null +++ b/app/views/api/v1/accounts/conversations/attachments/update.json.jbuilder @@ -0,0 +1 @@ +json.merge! @attachment.push_event_data || {} diff --git a/config/routes.rb b/config/routes.rb index 84ce07ffe..20e47f1d2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -125,6 +125,7 @@ Rails.application.routes.draw do post :translate post :retry end + resources :attachments, only: [:update] end resources :assignments, only: [:create] resources :labels, only: [:create, :index] diff --git a/spec/builders/messages/message_builder_spec.rb b/spec/builders/messages/message_builder_spec.rb index 5778d4e1a..1cb1b07da 100644 --- a/spec/builders/messages/message_builder_spec.rb +++ b/spec/builders/messages/message_builder_spec.rb @@ -158,6 +158,34 @@ describe Messages::MessageBuilder do expect(message.attachments.first.meta).to eq({ 'is_recorded_audio' => true }) end + it 'creates attachment with custom metadata from attachments_metadata param' do + params[:attachments_metadata] = { 'avatar.png' => { description: 'Profile picture', source: 'upload' } } + + message = message_builder + + expect(message.attachments.first.meta).to include('description' => 'Profile picture', 'source' => 'upload') + end + + it 'does not apply metadata when filename key does not match' do + params[:attachments_metadata] = { 'other_file.png' => { description: 'Wrong file' } } + + message = message_builder + + expect(message.attachments.first.meta).to be_nil + end + + it 'merges is_recorded_audio with attachments_metadata' do + params[:is_recorded_audio] = true + params[:attachments_metadata] = { 'avatar.png' => { description: 'Audio note' } } + + message = message_builder + + expect(message.attachments.first.meta).to eq({ + 'is_recorded_audio' => true, + 'description' => 'Audio note' + }) + end + context 'when DIRECT_UPLOAD_ENABLED' do let(:params) do ActionController::Parameters.new({ diff --git a/spec/controllers/api/v1/accounts/conversations/attachments_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/attachments_controller_spec.rb new file mode 100644 index 000000000..61e5ffc3c --- /dev/null +++ b/spec/controllers/api/v1/accounts/conversations/attachments_controller_spec.rb @@ -0,0 +1,187 @@ +require 'rails_helper' + +RSpec.describe 'Conversation Message Attachments API', type: :request do + let!(:account) { create(:account) } + let!(:inbox) { create(:inbox, account: account) } + let!(:conversation) { create(:conversation, inbox: inbox, account: account) } + let!(:message) { create(:message, conversation: conversation, account: account) } + let!(:attachment) { message.attachments.create!(account: account, file_type: :fallback, fallback_title: 'Test attachment') } + + describe 'PATCH /api/v1/accounts/{account.id}/conversations/{conversation_id}/messages/{message_id}/attachments/{id}' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ) + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user with access to conversation' do + let(:agent) { create(:user, account: account, role: :agent) } + + before do + create(:inbox_member, inbox: conversation.inbox, user: agent) + end + + it 'updates attachment meta' do + params = { meta: { description: 'Audio recording from meeting', source: 'microphone' } } + + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(attachment.reload.meta).to eq({ 'description' => 'Audio recording from meeting', 'source' => 'microphone' }) + end + + it 'returns the updated attachment data' do + params = { meta: { description: 'Test attachment' } } + + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + response_data = response.parsed_body + expect(response_data['id']).to eq(attachment.id) + expect(response_data['meta']).to eq({ 'description' => 'Test attachment' }) + end + + it 'triggers message update event' do + params = { meta: { description: 'Updated description' } } + + allow(Rails.configuration.dispatcher).to receive(:dispatch) + + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'message.updated', anything, hash_including(message: an_instance_of(Message)) + ) + end + + it 'handles request without meta parameter' do + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: {}, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(attachment.reload.meta).to eq({}) + end + + it 'handles empty meta parameter' do + attachment.update!(meta: { existing: 'data' }) + + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: { meta: {} }, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(attachment.reload.meta).to eq({}) + end + + it 'rejects metadata that exceeds size limit' do + large_value = 'x' * 17_000 + params = { meta: { large_field: large_value } } + + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['error']).to include('Metadata size exceeds maximum') + end + + it 'returns not found for non-existent attachment' do + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: 0 + ), + params: { meta: { key: 'value' } }, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:not_found) + end + + it 'returns not found for non-existent message' do + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: 0, + id: attachment.id + ), + params: { meta: { key: 'value' } }, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:not_found) + end + end + + context 'when it is an authenticated user without access to conversation' do + let(:agent) { create(:user, account: account, role: :agent) } + + it 'returns unauthorized' do + params = { meta: { description: 'Test' } } + + patch api_v1_account_conversation_message_attachment_url( + account_id: account.id, + conversation_id: conversation.display_id, + message_id: message.id, + id: attachment.id + ), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + end +end