From c16624dc5d9c1e1a29d1c0d75aebdf9eb64c6d27 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Tue, 10 Jan 2023 18:57:34 +0530 Subject: [PATCH] fix: Duplicate messages in Whatsapp Channel (#6220) fixes: #5433 --- .../whatsapp/incoming_message_base_service.rb | 46 +++++++++++-------- .../whatsapp/incoming_message_service_spec.rb | 27 ++++++----- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/app/services/whatsapp/incoming_message_base_service.rb b/app/services/whatsapp/incoming_message_base_service.rb index df9d433e5..83fceb452 100644 --- a/app/services/whatsapp/incoming_message_base_service.rb +++ b/app/services/whatsapp/incoming_message_base_service.rb @@ -7,26 +7,36 @@ class Whatsapp::IncomingMessageBaseService def perform processed_params - perform_statuses + if processed_params[:statuses].present? + process_statuses + elsif processed_params[:messages].present? + process_messages + end + end + + private + + def find_message_by_source_id(source_id) + return unless source_id + + @message = Message.find_by(source_id: source_id) + end + + def process_messages + # message allready exists so we don't need to process + return if find_message_by_source_id(@processed_params[:messages].first[:id]) set_contact return unless @contact set_conversation - - perform_messages + create_messages end - private + def process_statuses + return unless find_message_by_source_id(@processed_params[:statuses].first[:id]) - def perform_statuses - return if @processed_params[:statuses].blank? - - status = @processed_params[:statuses].first - @message = Message.find_by(source_id: status[:id]) - return unless @message - - update_message_with_status(@message, status) + update_message_with_status(@message, @processed_params[:statuses].first) end def update_message_with_status(message, status) @@ -38,8 +48,8 @@ class Whatsapp::IncomingMessageBaseService message.save! end - def perform_messages - return if @processed_params[:messages].blank? || unprocessable_message_type? + def create_messages + return if unprocessable_message_type? @message = @conversation.messages.build( content: message_content(@processed_params[:messages].first), @@ -50,7 +60,7 @@ class Whatsapp::IncomingMessageBaseService source_id: @processed_params[:messages].first[:id].to_s ) attach_files - attach_location + attach_location if message_type == 'location' @message.save! end @@ -60,9 +70,7 @@ class Whatsapp::IncomingMessageBaseService def message_content(message) # TODO: map interactive messages back to button messages in chatwoot - message.dig(:text, :body) || - message.dig(:button, :text) || - message.dig(:interactive, :button_reply, :title) || + message.dig(:text, :body) || message.dig(:button, :text) || message.dig(:interactive, :button_reply, :title) || message.dig(:interactive, :list_reply, :title) end @@ -142,8 +150,6 @@ class Whatsapp::IncomingMessageBaseService end def attach_location - return unless @processed_params[:messages].first[:type] == 'location' - location = @processed_params[:messages].first['location'] location_name = location['name'] ? "#{location['name']}, #{location['address']}" : '' @message.attachments.new( diff --git a/spec/services/whatsapp/incoming_message_service_spec.rb b/spec/services/whatsapp/incoming_message_service_spec.rb index 8978dbcad..3721d48cd 100644 --- a/spec/services/whatsapp/incoming_message_service_spec.rb +++ b/spec/services/whatsapp/incoming_message_service_spec.rb @@ -7,14 +7,16 @@ describe Whatsapp::IncomingMessageService do end let!(:whatsapp_channel) { create(:channel_whatsapp, sync_templates: false) } + let!(:params) do + { + 'contacts' => [{ 'profile' => { 'name' => 'Sojan Jose' }, 'wa_id' => '2423423243' }], + 'messages' => [{ 'from' => '2423423243', 'id' => 'SDFADSf23sfasdafasdfa', 'text' => { 'body' => 'Test' }, + 'timestamp' => '1633034394', 'type' => 'text' }] + }.with_indifferent_access + end context 'when valid text message params' do it 'creates appropriate conversations, message and contacts' do - params = { - 'contacts' => [{ 'profile' => { 'name' => 'Sojan Jose' }, 'wa_id' => '2423423243' }], - 'messages' => [{ 'from' => '2423423243', 'id' => 'SDFADSf23sfasdafasdfa', 'text' => { 'body' => 'Test' }, - 'timestamp' => '1633034394', 'type' => 'text' }] - }.with_indifferent_access described_class.new(inbox: whatsapp_channel.inbox, params: params).perform expect(whatsapp_channel.inbox.conversations.count).not_to eq(0) expect(Contact.all.first.name).to eq('Sojan Jose') @@ -22,12 +24,6 @@ describe Whatsapp::IncomingMessageService do end it 'appends to last conversation when if conversation already exisits' do - params = { - 'contacts' => [{ 'profile' => { 'name' => 'Sojan Jose' }, 'wa_id' => '2423423243' }], - 'messages' => [{ 'from' => '2423423243', 'id' => 'SDFADSf23sfasdafasdfa', 'text' => { 'body' => 'Test' }, - 'timestamp' => '1633034394', 'type' => 'text' }] - }.with_indifferent_access - contact_inbox = create(:contact_inbox, inbox: whatsapp_channel.inbox, source_id: params[:messages].first[:from]) 2.times.each { create(:conversation, inbox: whatsapp_channel.inbox, contact_inbox: contact_inbox) } last_conversation = create(:conversation, inbox: whatsapp_channel.inbox, contact_inbox: contact_inbox) @@ -37,6 +33,15 @@ describe Whatsapp::IncomingMessageService do # message appended to the last conversation expect(last_conversation.messages.last.content).to eq(params[:messages].first[:text][:body]) end + + it 'will not create duplicate messages when same message is received' do + described_class.new(inbox: whatsapp_channel.inbox, params: params).perform + expect(whatsapp_channel.inbox.messages.count).to eq(1) + + # this shouldn't create a duplicate message + described_class.new(inbox: whatsapp_channel.inbox, params: params).perform + expect(whatsapp_channel.inbox.messages.count).to eq(1) + end end context 'when unsupported message types' do