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

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)

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