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
This commit is contained in:
parent
ba9a0160ac
commit
a27737e91c
@ -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'
|
||||
|
||||
|
||||
@ -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
|
||||
@ -0,0 +1 @@
|
||||
json.merge! @attachment.push_event_data || {}
|
||||
@ -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]
|
||||
|
||||
@ -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({
|
||||
|
||||
@ -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
|
||||
Loading…
Reference in New Issue
Block a user