fix: Add correct thread message_id to the email message (#1659)

Co-authored-by: Sojan <sojan@pepalo.com>

.rubocop.yml

@@ -11,6 +11,8 @@ Metrics/ClassLength:
   Max: 125
   Exclude:
     - 'app/models/conversation.rb'
+    - 'app/mailers/conversation_reply_mailer.rb'
+    - 'app/models/message.rb'
 RSpec/ExampleLength:
   Max: 25
 Style/Documentation:

app/mailboxes/support_mailbox.rb

@@ -47,6 +47,7 @@ class SupportMailbox < ApplicationMailbox
                                              contact_inbox_id: @contact_inbox.id,
                                              additional_attributes: {
                                                source: 'email',
+                                               mail_subject: @processed_mail.subject,
                                                initiated_at: {
                                                  timestamp: Time.now.utc
                                                }

app/mailers/conversation_reply_mailer.rb

@@ -66,6 +66,10 @@ class ConversationReplyMailer < ApplicationMailer
     @inbox = @conversation.inbox
   end
 
+  def should_use_conversation_email_address?
+    @inbox.inbox_type == 'Email' || inbound_email_enabled?
+  end
+
   def conversation_already_viewed?
     # whether contact already saw the message on widget
     return unless @conversation.contact_last_seen_at
@@ -83,12 +87,18 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def mail_subject
+    return "Re: #{incoming_mail_subject}" if incoming_mail_subject
+
     subject_line = I18n.t('conversations.reply.email_subject')
     "[##{@conversation.display_id}] #{subject_line}"
   end
 
+  def incoming_mail_subject
+    @incoming_mail_subject ||= @conversation.additional_attributes['mail_subject']
+  end
+
   def reply_email
-    if inbound_email_enabled?
+    if should_use_conversation_email_address?
       "#{assignee_name} <reply+#{@conversation.uuid}@#{current_domain}>"
     else
       @inbox.email_address || @agent&.email
@@ -96,7 +106,7 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def from_email_with_name
-    if inbound_email_enabled?
+    if should_use_conversation_email_address?
       "#{assignee_name} <#{account_support_email}>"
     else
       "#{assignee_name} <#{from_email_address}>"
@@ -114,7 +124,17 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def in_reply_to_email
-    "<account/#{@account.id}/conversation/#{@conversation.uuid}@#{current_domain}>"
+    conversation_reply_email_id || "<account/#{@account.id}/conversation/#{@conversation.uuid}@#{current_domain}>"
+  end
+
+  def conversation_reply_email_id
+    content_attributes = @conversation.messages.incoming.last&.content_attributes
+
+    if content_attributes && content_attributes['email'] && content_attributes['message_id']
+      "<#{@conversation.messages.incoming.last.content_attributes['email']['message_id']}>"
+    end
+
+    nil
   end
 
   def inbound_email_enabled?

app/models/channel/email.rb

@@ -37,7 +37,7 @@ class Channel::Email < ApplicationRecord
   private
 
   def ensure_forward_to_address
-    # TODO : implement better logic here
+    email_domain = InstallationConfig.find_by(name: 'MAILER_INBOUND_EMAIL_DOMAIN')&.value
-    self.forward_to_address ||= "#{SecureRandom.hex}@xyc.com"
+    self.forward_to_address ||= "#{SecureRandom.hex}@#{email_domain}"
   end
 end

app/models/message.rb

@@ -128,8 +128,7 @@ class Message < ApplicationRecord
   private
 
   def execute_after_create_commit_callbacks
-    # rails issue with order of active record callbacks being executed
+    # rails issue with order of active record callbacks being executed https://github.com/rails/rails/issues/20911
-    # https://github.com/rails/rails/issues/20911
     set_conversation_activity
     dispatch_create_events
     send_reply
@@ -169,8 +168,7 @@ class Message < ApplicationRecord
 
   def can_notify_via_mail?
     return unless email_notifiable_message?
-    return false if conversation.contact.email.blank?
+    return false if conversation.contact.email.blank? || !(%w[Website Email].include? inbox.inbox_type)
-    return false unless %w[Website Email].include? inbox.inbox_type
 
     true
   end
@@ -178,10 +176,19 @@ class Message < ApplicationRecord
   def notify_via_mail
     return unless can_notify_via_mail?
 
-    # set a redis key for the conversation so that we don't need to send email for every new message
+    trigger_notify_via_mail
+  end
+
+  def trigger_notify_via_mail
+    # will set a redis key for the conversation so that we don't need to send email for every new message
     # last few messages coupled together is sent every 2 minutes rather than one email for each message
-    if Redis::Alfred.get(conversation_mail_key).nil?
+    # if redis key exists there is an unprocessed job that will take care of delivering the email
+    return if Redis::Alfred.get(conversation_mail_key).present?
+
     Redis::Alfred.setex(conversation_mail_key, Time.zone.now)
+    if inbox.inbox_type == 'Email'
+      ConversationReplyEmailWorker.perform_in(2.seconds, conversation.id, Time.zone.now)
+    else
       ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now)
     end
   end

app/workers/conversation_reply_email_worker.rb

@@ -7,7 +7,7 @@ class ConversationReplyEmailWorker
     @conversation = Conversation.find(conversation_id)
 
     # send the email
-    if @conversation.messages.incoming&.last&.content_type == 'incoming_email'
+    if @conversation.messages.incoming&.last&.content_type == 'incoming_email' || email_inbox?
       ConversationReplyMailer.reply_without_summary(@conversation, queued_time).deliver_later
     else
       ConversationReplyMailer.reply_with_summary(@conversation, queued_time).deliver_later
@@ -19,6 +19,10 @@ class ConversationReplyEmailWorker
 
   private
 
+  def email_inbox?
+    @conversation.inbox&.inbox_type == 'Email'
+  end
+
   def conversation_mail_key
     format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: @conversation.id)
   end