chore: Refactor MarkMessagesAsReadJob based on conversation id and time stamp (#8217)

- Mark all messages as read by providing the conversation ID and timestamp.
- For Instagram, ensure all previous messages that weren't marked as failed are now marked as read. This is because the read events are only triggered for the most recent message and not for any previous ones.

app/controllers/api/v1/widget/conversations_controller.rb

@@ -28,7 +28,7 @@ class Api::V1::Widget::ConversationsController < Api::V1::Widget::BaseController
 
     conversation.contact_last_seen_at = DateTime.now.utc
     conversation.save!
-    ::Conversations::MarkMessagesAsReadJob.perform_later(conversation)
+    ::Conversations::MarkMessagesAsReadJob.perform_later(conversation.id, conversation.contact_last_seen_at)
     head :ok
   end
 

app/controllers/public/api/v1/inboxes/conversations_controller.rb

@@ -23,7 +23,7 @@ class Public::Api::V1::Inboxes::ConversationsController < Public::Api::V1::Inbox
   def update_last_seen
     @conversation.contact_last_seen_at = DateTime.now.utc
     @conversation.save!
-    ::Conversations::MarkMessagesAsReadJob.perform_later(@conversation)
+    ::Conversations::MarkMessagesAsReadJob.perform_later(@conversation.id, @conversation.contact_last_seen_at)
     head :ok
   end
 

app/jobs/conversations/mark_messages_as_read_job.rb

@@ -1,12 +1,15 @@
 class Conversations::MarkMessagesAsReadJob < ApplicationJob
   queue_as :low
 
-  def perform(conversation)
+  def perform(conversation_id, timestamp)
+    conversation = Conversation.find_by(id: conversation_id)
+
+    return unless conversation
+
     # Mark every message created before the user's viewing time as read.
     conversation.messages.where(status: %w[sent delivered])
                 .where.not(message_type: 'incoming')
-                .where('created_at <= ?',
-                       conversation.contact_last_seen_at).find_each do |message|
+                .where('messages.created_at <= ?', timestamp).find_each do |message|
       message.update!(status: 'read')
     end
   end

app/services/instagram/read_status_service.rb

@@ -4,12 +4,7 @@ class Instagram::ReadStatusService
   def perform
     return if instagram_channel.blank?
 
-    process_status if message.present?
-  end
-
-  def process_status
-    @message.status = 'read'
-    @message.save!
+    ::Conversations::MarkMessagesAsReadJob.perform_later(message.conversation.id, message.created_at) if message.present?
   end
 
   def instagram_id

spec/controllers/api/v1/widget/conversations_controller_spec.rb

@@ -168,9 +168,12 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do
   describe 'POST /api/v1/widget/conversations/update_last_seen' do
     context 'with a conversation' do
       it 'returns the correct conversation params' do
+        current_time = DateTime.now.utc
+        allow(DateTime).to receive(:now).and_return(current_time)
+
         allow(Rails.configuration.dispatcher).to receive(:dispatch)
         expect(conversation.contact_last_seen_at).to be_nil
-        expect(Conversations::MarkMessagesAsReadJob).to receive(:perform_later).with(conversation)
+        expect(Conversations::MarkMessagesAsReadJob).to receive(:perform_later).with(conversation.id, current_time)
 
         post '/api/v1/widget/conversations/update_last_seen',
              headers: { 'X-Auth-Token' => token },

spec/controllers/public/api/v1/inbox/conversations_controller_spec.rb

@@ -80,8 +80,10 @@ RSpec.describe 'Public Inbox Contact Conversations API', type: :request do
     end
 
     it 'updates the last seen of the conversation contact' do
+      current_time = DateTime.now.utc
+      allow(DateTime).to receive(:now).and_return(current_time)
       contact_last_seen_at = conversation.contact_last_seen_at
-      expect(Conversations::MarkMessagesAsReadJob).to receive(:perform_later).with(conversation)
+      expect(Conversations::MarkMessagesAsReadJob).to receive(:perform_later).with(conversation.id, current_time)
       post update_last_seen_path
 
       expect(response).to have_http_status(:success)

spec/jobs/conversations/mark_messages_as_read_job_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe Conversations::MarkMessagesAsReadJob do
   context 'when called' do
     it 'marks all sent messages in a conversation as read' do
       expect do
-        described_class.perform_now(conversation)
+        described_class.perform_now(conversation.id, conversation.contact_last_seen_at)
         message.reload
       end.to change(message, :status).from('sent').to('read')
     end
@@ -24,7 +24,7 @@ RSpec.describe Conversations::MarkMessagesAsReadJob do
     it 'marks all delivered messages in a conversation as read' do
       message.update!(status: 'delivered')
       expect do
-        described_class.perform_now(conversation)
+        described_class.perform_now(conversation.id, conversation.contact_last_seen_at)
         message.reload
       end.to change(message, :status).from('delivered').to('read')
     end
@@ -32,7 +32,7 @@ RSpec.describe Conversations::MarkMessagesAsReadJob do
     it 'marks all templates messages in a conversation as read' do
       message.update!(status: 'delivered', message_type: 'template')
       expect do
-        described_class.perform_now(conversation)
+        described_class.perform_now(conversation.id, conversation.contact_last_seen_at)
         message.reload
       end.to change(message, :status).from('delivered').to('read')
     end
@@ -40,21 +40,27 @@ RSpec.describe Conversations::MarkMessagesAsReadJob do
     it 'does not mark failed messages as read' do
       message.update!(status: 'failed')
       expect do
-        described_class.perform_now(conversation)
+        described_class.perform_now(conversation.id, conversation.contact_last_seen_at)
       end.not_to change(message.reload, :status)
     end
 
     it 'does not mark incoming messages as read' do
       message.update!(message_type: 'incoming')
       expect do
-        described_class.perform_now(conversation)
+        described_class.perform_now(conversation.id, conversation.contact_last_seen_at)
       end.not_to change(message.reload, :status)
     end
 
     it 'does not mark messages created after the contact last seen time as read' do
       message.update!(created_at: DateTime.now.utc)
       expect do
-        described_class.perform_now(conversation)
+        described_class.perform_now(conversation.id, conversation.contact_last_seen_at)
+      end.not_to change(message.reload, :status)
+    end
+
+    it 'does not run the job if the conversation does not exist' do
+      expect do
+        described_class.perform_now(1212, conversation.contact_last_seen_at)
       end.not_to change(message.reload, :status)
     end
   end

spec/services/instagram/read_status_service_spec.rb

@@ -17,7 +17,11 @@ describe Instagram::ReadStatusService do
     context 'when messaging_seen callback is fired' do
       let(:message) { conversation.messages.last }
 
-      it 'updates the message status to read if the status is delivered' do
+      before do
+        allow(Conversations::MarkMessagesAsReadJob).to receive(:perform_later)
+      end
+
+      it 'enqueues the MarkMessagesAsReadJob with correct parameters if the message is found' do
         params = {
           recipient: {
             id: 'chatwoot-app-user-id-1'
@@ -27,10 +31,10 @@ describe Instagram::ReadStatusService do
           }
         }
         described_class.new(params: params).perform
-        expect(conversation.reload.messages.last.status).to eq('read')
+        expect(Conversations::MarkMessagesAsReadJob).to have_received(:perform_later).with(conversation.id, message.created_at)
       end
 
-      it 'does not update the status if message is not found' do
+      it 'does not enqueue the MarkMessagesAsReadJob if the message is not found' do
         params = {
           recipient: {
             id: 'chatwoot-app-user-id-1'
@@ -39,9 +43,8 @@ describe Instagram::ReadStatusService do
             mid: 'random-message-id'
           }
         }
-
         described_class.new(params: params).perform
-        expect(conversation.reload.messages.last.status).not_to eq('read')
+        expect(Conversations::MarkMessagesAsReadJob).not_to have_received(:perform_later)
       end
     end
   end