From 05b8486538b1f6500ae58f53607a99681b8ebfa9 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Mon, 16 Sep 2024 16:14:35 +0530 Subject: [PATCH] fix: `message_type` inconsistency across message end points (#10108) The `before_type_cast` method sometimes returns a string for `message_type`, creating inconsistencies in different payloads. This pull request will remove all `before_type_cast` usage and replace it with `to_i` methods. --- app/models/message.rb | 2 +- .../v1/accounts/search/_message.json.jbuilder | 2 +- .../api/v1/models/_conversation.json.jbuilder | 2 +- .../api/v1/models/_message.json.jbuilder | 2 +- .../v1/models/_widget_message.json.jbuilder | 2 +- .../v1/widget/messages/create.json.jbuilder | 2 +- .../v1/widget/messages/index.json.jbuilder | 2 +- .../api/v1/models/_message.json.jbuilder | 2 +- .../api/v1/accounts/search_controller_spec.rb | 1 + .../widget/conversations_controller_spec.rb | 41 +++++++++++++------ .../api/v1/widget/messages_controller_spec.rb | 1 + .../api/v1/inbox/messages_controller_spec.rb | 1 + spec/models/message_spec.rb | 2 +- 13 files changed, 40 insertions(+), 22 deletions(-) diff --git a/app/models/message.rb b/app/models/message.rb index 6244d050c..836fc7de5 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -137,7 +137,7 @@ class Message < ApplicationRecord def push_event_data data = attributes.symbolize_keys.merge( created_at: created_at.to_i, - message_type: message_type_before_type_cast, + message_type: message_type.to_i, conversation_id: conversation.display_id, conversation: conversation_push_event_data ) diff --git a/app/views/api/v1/accounts/search/_message.json.jbuilder b/app/views/api/v1/accounts/search/_message.json.jbuilder index 98ac728d6..d9416cce0 100644 --- a/app/views/api/v1/accounts/search/_message.json.jbuilder +++ b/app/views/api/v1/accounts/search/_message.json.jbuilder @@ -1,6 +1,6 @@ json.id message.id json.content message.content -json.message_type message.message_type_before_type_cast +json.message_type message.message_type.to_i json.content_type message.content_type json.source_id message.source_id json.inbox_id message.inbox_id diff --git a/app/views/api/v1/models/_conversation.json.jbuilder b/app/views/api/v1/models/_conversation.json.jbuilder index 68dfd2c40..dc8343228 100644 --- a/app/views/api/v1/models/_conversation.json.jbuilder +++ b/app/views/api/v1/models/_conversation.json.jbuilder @@ -17,7 +17,7 @@ json.messages do json.content message.content json.id message.id json.sender_name message.sender.name if message.sender - json.message_type message.message_type_before_type_cast + json.message_type message.message_type.to_i json.created_at message.created_at.to_i end end diff --git a/app/views/api/v1/models/_message.json.jbuilder b/app/views/api/v1/models/_message.json.jbuilder index 583bc6fa6..87dc9297d 100644 --- a/app/views/api/v1/models/_message.json.jbuilder +++ b/app/views/api/v1/models/_message.json.jbuilder @@ -3,7 +3,7 @@ json.content message.content json.inbox_id message.inbox_id json.echo_id message.echo_id if message.echo_id json.conversation_id message.conversation.display_id -json.message_type message.message_type_before_type_cast +json.message_type message.message_type.to_i json.content_type message.content_type json.status message.status json.content_attributes message.content_attributes diff --git a/app/views/api/v1/models/_widget_message.json.jbuilder b/app/views/api/v1/models/_widget_message.json.jbuilder index 8aa42aba3..36ce34dec 100644 --- a/app/views/api/v1/models/_widget_message.json.jbuilder +++ b/app/views/api/v1/models/_widget_message.json.jbuilder @@ -1,6 +1,6 @@ json.id resource.id json.content resource.content -json.message_type resource.message_type_before_type_cast +json.message_type resource.message_type.to_i json.content_type resource.content_type json.content_attributes resource.content_attributes json.created_at resource.created_at.to_i diff --git a/app/views/api/v1/widget/messages/create.json.jbuilder b/app/views/api/v1/widget/messages/create.json.jbuilder index fbd6ab5e5..81d4cd771 100644 --- a/app/views/api/v1/widget/messages/create.json.jbuilder +++ b/app/views/api/v1/widget/messages/create.json.jbuilder @@ -2,7 +2,7 @@ json.id @message.id json.content @message.content json.inbox_id @message.inbox_id json.conversation_id @message.conversation.display_id -json.message_type @message.message_type_before_type_cast +json.message_type @message.message_type.to_i json.created_at @message.created_at.to_i json.private @message.private json.source_id @message.source_id diff --git a/app/views/api/v1/widget/messages/index.json.jbuilder b/app/views/api/v1/widget/messages/index.json.jbuilder index 29e80815d..f178a0990 100644 --- a/app/views/api/v1/widget/messages/index.json.jbuilder +++ b/app/views/api/v1/widget/messages/index.json.jbuilder @@ -2,7 +2,7 @@ json.payload do json.array! @messages do |message| json.id message.id json.content message.content - json.message_type message.message_type_before_type_cast + json.message_type message.message_type.to_i json.content_type message.content_type json.content_attributes message.content_attributes json.created_at message.created_at.to_i diff --git a/app/views/public/api/v1/models/_message.json.jbuilder b/app/views/public/api/v1/models/_message.json.jbuilder index 8aa42aba3..36ce34dec 100644 --- a/app/views/public/api/v1/models/_message.json.jbuilder +++ b/app/views/public/api/v1/models/_message.json.jbuilder @@ -1,6 +1,6 @@ json.id resource.id json.content resource.content -json.message_type resource.message_type_before_type_cast +json.message_type resource.message_type.to_i json.content_type resource.content_type json.content_attributes resource.content_attributes json.created_at resource.created_at.to_i diff --git a/spec/controllers/api/v1/accounts/search_controller_spec.rb b/spec/controllers/api/v1/accounts/search_controller_spec.rb index b5644cebf..894c4e788 100644 --- a/spec/controllers/api/v1/accounts/search_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/search_controller_spec.rb @@ -112,6 +112,7 @@ RSpec.describe 'Search', type: :request do expect(response_data[:payload].keys).to contain_exactly(:messages) expect(response_data[:payload][:messages].length).to eq 2 + expect(response_data[:payload][:messages].first[:message_type]).to eq 0 end end end diff --git a/spec/controllers/api/v1/widget/conversations_controller_spec.rb b/spec/controllers/api/v1/widget/conversations_controller_spec.rb index e4f5a4bde..6966c87ea 100644 --- a/spec/controllers/api/v1/widget/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/widget/conversations_controller_spec.rb @@ -13,6 +13,21 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do Widget::TokenService.new(payload: { source_id: second_session.source_id, inbox_id: web_widget.inbox.id }).generate_token end + def conversation_params + { + website_token: web_widget.website_token, + contact: { + name: 'contact-name', + email: 'contact-email@chatwoot.com', + phone_number: '+919745313456' + }, + message: { + content: 'This is a test message' + }, + custom_attributes: { order_id: '12345' } + } + end + describe 'GET /api/v1/widget/conversations' do context 'with a conversation' do it 'returns the correct conversation params' do @@ -47,21 +62,10 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do end describe 'POST /api/v1/widget/conversations' do - it 'creates a conversation' do + it 'creates a conversation with correct details' do post '/api/v1/widget/conversations', headers: { 'X-Auth-Token' => token }, - params: { - website_token: web_widget.website_token, - contact: { - name: 'contact-name', - email: 'contact-email@chatwoot.com', - phone_number: '+919745313456' - }, - message: { - content: 'This is a test message' - }, - custom_attributes: { order_id: '12345' } - }, + params: conversation_params, as: :json expect(response).to have_http_status(:success) @@ -70,8 +74,19 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do expect(json_response['contact']['email']).to eq 'contact-email@chatwoot.com' expect(json_response['contact']['phone_number']).to eq '+919745313456' expect(json_response['contact']['name']).to eq 'contact-name' + end + + it 'creates a conversation with correct message and custom attributes' do + post '/api/v1/widget/conversations', + headers: { 'X-Auth-Token' => token }, + params: conversation_params, + as: :json + + expect(response).to have_http_status(:success) + json_response = response.parsed_body expect(json_response['custom_attributes']['order_id']).to eq '12345' expect(json_response['messages'][0]['content']).to eq 'This is a test message' + expect(json_response['messages'][0]['message_type']).to eq 0 end it 'create a conversation with a name and without an email' do diff --git a/spec/controllers/api/v1/widget/messages_controller_spec.rb b/spec/controllers/api/v1/widget/messages_controller_spec.rb index 11c916514..2174a1a53 100644 --- a/spec/controllers/api/v1/widget/messages_controller_spec.rb +++ b/spec/controllers/api/v1/widget/messages_controller_spec.rb @@ -54,6 +54,7 @@ RSpec.describe '/api/v1/widget/messages', type: :request do expect(response).to have_http_status(:success) json_response = response.parsed_body expect(json_response['content']).to eq(message_params[:content]) + expect(json_response['message_type']).to eq 0 end it 'does not create the message' do diff --git a/spec/controllers/public/api/v1/inbox/messages_controller_spec.rb b/spec/controllers/public/api/v1/inbox/messages_controller_spec.rb index a7e055b19..3dd43e3bd 100644 --- a/spec/controllers/public/api/v1/inbox/messages_controller_spec.rb +++ b/spec/controllers/public/api/v1/inbox/messages_controller_spec.rb @@ -26,6 +26,7 @@ RSpec.describe 'Public Inbox Contact Conversation Messages API', type: :request expect(response).to have_http_status(:success) data = response.parsed_body expect(data['content']).to eq('hello') + expect(data['message_type']).to eq(0) end it 'does not create the message' do diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 4c66e37ee..3300254d5 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -79,7 +79,7 @@ RSpec.describe Message do external_source_ids: message.external_source_ids, id: message.id, inbox_id: message.inbox_id, - message_type: message.message_type_before_type_cast, + message_type: message.message_type.to_i, private: message.private, processed_message_content: message.processed_message_content, sender_id: message.sender_id,