refactor: strategy pattern for mailbox conversation finding (#12766)

Co-authored-by: Pranav <pranav@chatwoot.com>
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
Shivam Mishra
2025-11-10 20:47:18 +05:30
committed by GitHub
parent fb1aa085cf
commit 615e81731c
19 changed files with 1527 additions and 553 deletions

View File

@@ -4,38 +4,21 @@ class ApplicationMailbox < ActionMailbox::Base
# Last part is the regex for the UUID
# Eg: email should be something like : reply+6bdc3f4d-0bec-4515-a284-5d916fdde489@domain.com
REPLY_EMAIL_UUID_PATTERN = /^reply\+([0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12})$/i
CONVERSATION_MESSAGE_ID_PATTERN = %r{conversation/([a-zA-Z0-9-]*?)/messages/(\d+?)@(\w+\.\w+)}
# routes as a reply to existing conversations
# Route all emails to verified channels to the unified reply mailbox
# The ConversationFinder will determine if it's a reply or new conversation
routing(
->(inbound_mail) { valid_to_address?(inbound_mail) && (reply_uuid_mail?(inbound_mail) || in_reply_to_mail?(inbound_mail)) } => :reply
)
# routes as a new conversation in email channel
routing(
->(inbound_mail) { valid_to_address?(inbound_mail) && EmailChannelFinder.new(inbound_mail.mail).perform.present? } => :support
lambda { |inbound_mail|
valid_to_address?(inbound_mail) &&
(reply_uuid_mail?(inbound_mail) || EmailChannelFinder.new(inbound_mail.mail).perform.present?)
} => :reply
)
# catchall
routing(all: :default)
class << self
# checks if follow this pattern then send it to reply_mailbox
# <account/#{@account.id}/conversation/#{@conversation.uuid}@#{@account.inbound_email_domain}>
def in_reply_to_mail?(inbound_mail)
in_reply_to = inbound_mail.mail.in_reply_to
in_reply_to.present? && (
in_reply_to_matches?(in_reply_to) || Message.exists?(source_id: in_reply_to)
)
end
def in_reply_to_matches?(in_reply_to)
Array.wrap(in_reply_to).any? { it.match?(CONVERSATION_MESSAGE_ID_PATTERN) }
end
# checks if follow this pattern send it to reply_mailbox
# reply+<conversation-uuid>@<mailer-domain.com>
# checks if follows this pattern: reply+<conversation-uuid>@<mailer-domain.com>
def reply_uuid_mail?(inbound_mail)
inbound_mail.mail.to&.any? do |email|
conversation_uuid = email.split('@')[0]

View File

@@ -3,6 +3,8 @@ class Imap::ImapMailbox
include IncomingEmailValidityHelper
attr_accessor :channel, :account, :inbox, :conversation, :processed_mail
FALLBACK_CONVERSATION_PATTERN = %r{account/(\d+)/conversation/([a-zA-Z0-9-]+)@}
def process(mail, channel)
@inbound_mail = mail
@channel = channel
@@ -49,19 +51,32 @@ class Imap::ImapMailbox
end
def find_conversation_by_reference_ids
return if @inbound_mail.references.blank? && in_reply_to.present?
return if @inbound_mail.references.blank?
message = find_message_by_references
if message.present?
conversation = @inbox.conversations.find_by(id: message.conversation_id)
return conversation if conversation.present?
end
return if message.nil?
@inbox.conversations.find(message.conversation_id)
# FALLBACK_PATTERN use to find a conversation that is started by an agent (no incoming message yet)
conversation_id = find_conversation_by_references
@inbox.conversations.find_by(uuid: conversation_id) if conversation_id.present?
end
def in_reply_to
@processed_mail.in_reply_to
end
def find_conversation_by_references
references = Array.wrap(@inbound_mail.references)
references.each do |message_id|
match = FALLBACK_CONVERSATION_PATTERN.match(message_id)
return match[2] if match.present?
end
end
def find_message_by_references
message_to_return = nil

View File

@@ -1,88 +1,38 @@
class ReplyMailbox < ApplicationMailbox
attr_accessor :conversation_uuid, :processed_mail
attr_accessor :conversation, :processed_mail
# Last part is the regex for the UUID
# Eg: email should be something like : reply+6bdc3f4d-0bec-4515-a284-5d916fdde489@domain.com
EMAIL_PART_PATTERN = /^reply\+([0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12})$/i
before_processing :conversation_uuid_from_to_address,
:find_relative_conversation
before_processing :find_conversation
def process
return if @conversation.blank?
# Return early if no conversation was found (e.g., notification emails, suspended accounts)
return unless @conversation
decorate_mail
create_message
add_attachments_to_message
# Wrap everything in a transaction to ensure atomicity
# This prevents orphan conversations if message/attachment creation fails
# and ensures idempotency on job retry (conversation won't be duplicated)
ActiveRecord::Base.transaction do
persist_conversation_if_needed
decorate_mail
create_message
add_attachments_to_message
end
end
private
def find_relative_conversation
if @conversation_uuid
find_conversation_with_uuid
elsif mail.in_reply_to.present?
find_conversation_with_in_reply_to
end
def find_conversation
@conversation = Mailbox::ConversationFinder.new(mail).find
# Log when email is rejected
Rails.logger.info "Email #{mail.message_id} rejected - no conversation found" unless @conversation
end
def conversation_uuid_from_to_address
@mail = MailPresenter.new(mail)
def persist_conversation_if_needed
# Save the conversation if it's a new record (from NewConversationStrategy)
# We persist here instead of in the strategy to maintain transaction integrity
return unless @conversation.new_record?
return if @mail.mail_receiver.blank?
@mail.mail_receiver.each do |email|
username = email.split('@')[0]
match_result = username.match(ApplicationMailbox::REPLY_EMAIL_UUID_PATTERN)
if match_result
@conversation_uuid = match_result.captures
break
end
end
@conversation_uuid
end
# find conversation uuid from below pattern
# reply+<conversation-uuid>@<mailer-domain.com>
def find_conversation_with_uuid
@conversation = Conversation.find_by(uuid: conversation_uuid)
validate_resource @conversation
end
def find_conversation_by_uuid(match_result)
@conversation_uuid = match_result.captures[0]
find_conversation_with_uuid
end
def find_conversation_by_message_id(in_reply_to)
@message = Message.find_by(source_id: in_reply_to)
@conversation = @message.conversation if @message.present?
@conversation_uuid = @conversation.uuid if @conversation.present?
end
# find conversation uuid from below pattern
# <conversation/#{@conversation.uuid}/messages/#{@messages&.last&.id}@#{@account.inbound_email_domain}>
def find_conversation_with_in_reply_to
match_result = nil
in_reply_to_addresses = mail.in_reply_to
in_reply_to_addresses = [in_reply_to_addresses] if in_reply_to_addresses.is_a?(String)
in_reply_to_addresses.each do |in_reply_to|
match_result = in_reply_to.match(::ApplicationMailbox::CONVERSATION_MESSAGE_ID_PATTERN)
break if match_result
end
find_by_in_reply_to_addresses(match_result, in_reply_to_addresses)
end
def find_by_in_reply_to_addresses(match_result, in_reply_to_addresses)
find_conversation_by_uuid(match_result) if match_result
find_conversation_by_message_id(in_reply_to_addresses) if @conversation.blank?
end
def validate_resource(resource)
Rails.logger.error "[App::Mailboxes::ReplyMailbox] Email conversation with uuid: #{conversation_uuid} not found" if resource.nil?
resource
@conversation.save!
Rails.logger.info "Created new conversation #{@conversation.id} for email #{mail.message_id}"
end
def decorate_mail

View File

@@ -1,94 +0,0 @@
class SupportMailbox < ApplicationMailbox
include IncomingEmailValidityHelper
attr_accessor :channel, :account, :inbox, :conversation, :processed_mail
before_processing :find_channel,
:load_account,
:load_inbox,
:decorate_mail
def process
Rails.logger.info "Processing email #{mail.message_id} from #{original_sender_email} to #{mail.to} with subject #{mail.subject}"
# Skip processing email if it belongs to any of the edge cases
return unless incoming_email_from_valid_email?
ActiveRecord::Base.transaction do
find_or_create_contact
find_or_create_conversation
create_message
add_attachments_to_message
end
end
private
def find_channel
find_channel_with_to_mail if @channel.blank?
raise 'Email channel/inbox not found' if @channel.nil?
@channel
end
def find_channel_with_to_mail
@channel = EmailChannelFinder.new(mail).perform
end
def load_account
@account = @channel.account
end
def load_inbox
@inbox = @channel.inbox
end
def decorate_mail
@processed_mail = MailPresenter.new(mail, @account)
end
def find_conversation_by_in_reply_to
return if in_reply_to.blank?
@account.conversations.where("additional_attributes->>'in_reply_to' = ?", in_reply_to).first
end
def in_reply_to
mail['In-Reply-To'].try(:value)
end
def original_sender_email
@processed_mail.original_sender&.downcase
end
def find_or_create_conversation
@conversation = find_conversation_by_in_reply_to || ::Conversation.create!({
account_id: @account.id,
inbox_id: @inbox.id,
contact_id: @contact.id,
contact_inbox_id: @contact_inbox.id,
additional_attributes: {
in_reply_to: in_reply_to,
source: 'email',
auto_reply: @processed_mail.auto_reply?,
mail_subject: @processed_mail.subject,
initiated_at: {
timestamp: Time.now.utc
}
}
})
end
def find_or_create_contact
@contact = @inbox.contacts.from_email(original_sender_email)
if @contact.present?
@contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact)
else
create_contact
end
end
def identify_contact_name
processed_mail.sender_name || processed_mail.from.first.split('@').first
end
end