From bce4e9b3a70268caac685070845fda7708432f0b Mon Sep 17 00:00:00 2001 From: Gabriel Jablonski Date: Tue, 24 Feb 2026 14:45:07 -0300 Subject: [PATCH] fix: clear source_id when retrying message to prevent skipping (#222) * fix: clear source_id when retrying message to prevent skipping * fix: validate message status and type before retrying to ensure proper handling --- .../conversations/messages_controller.rb | 3 +- .../conversations/messages_controller_spec.rb | 40 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/accounts/conversations/messages_controller.rb b/app/controllers/api/v1/accounts/conversations/messages_controller.rb index e9157b876..1675e4b43 100644 --- a/app/controllers/api/v1/accounts/conversations/messages_controller.rb +++ b/app/controllers/api/v1/accounts/conversations/messages_controller.rb @@ -32,10 +32,11 @@ class Api::V1::Accounts::Conversations::MessagesController < Api::V1::Accounts:: def retry return if message.blank? + return head :unprocessable_entity unless message.failed? && (message.outgoing? || message.template?) service = Messages::StatusUpdateService.new(message, 'sent') service.perform - message.update!(content_attributes: {}) + message.update!(content_attributes: {}, source_id: nil) ::SendReplyJob.perform_later(message.id) rescue StandardError => e render_could_not_create_error(e.message) diff --git a/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb index 9a29ece34..2f1d06bd1 100644 --- a/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations/messages_controller_spec.rb @@ -360,7 +360,7 @@ RSpec.describe 'Conversation Messages API', type: :request do end describe 'POST /api/v1/accounts/{account.id}/conversations/:conversation_id/messages/:id/retry' do - let(:message) { create(:message, account: account, status: :failed, content_attributes: { external_error: 'error' }) } + let(:message) { create(:message, account: account, message_type: :outgoing, status: :failed, content_attributes: { external_error: 'error' }) } context 'when it is an unauthenticated user' do it 'returns unauthorized' do @@ -385,6 +385,44 @@ RSpec.describe 'Conversation Messages API', type: :request do expect(message.reload.status).to eq('sent') expect(message.reload.content_attributes['external_error']).to be_nil end + + it 'clears source_id so the send job does not skip the message' do + message.update!(source_id: 'wamid.old_message_id') + + post "/api/v1/accounts/#{account.id}/conversations/#{message.conversation.display_id}/messages/#{message.id}/retry", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(message.reload.source_id).to be_nil + end + end + + context 'when the message is not failed or not outgoing' do + let(:agent) { create(:user, account: account, role: :agent) } + let(:sent_message) { create(:message, account: account, message_type: :outgoing, status: :sent) } + let(:incoming_failed) { create(:message, account: account, message_type: :incoming, status: :failed) } + + before do + create(:inbox_member, inbox: sent_message.conversation.inbox, user: agent) + create(:inbox_member, inbox: incoming_failed.conversation.inbox, user: agent) + end + + it 'returns unprocessable_entity for non-failed messages' do + post "/api/v1/accounts/#{account.id}/conversations/#{sent_message.conversation.display_id}/messages/#{sent_message.id}/retry", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'returns unprocessable_entity for incoming messages' do + post "/api/v1/accounts/#{account.id}/conversations/#{incoming_failed.conversation.display_id}/messages/#{incoming_failed.id}/retry", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end end context 'when the message id is invalid' do