refactor: unify email routing with NewConversationStrategy fallback
Replace separate SupportMailbox with NewConversationStrategy as the final fallback in the conversation finder chain. This prevents emails from being silently dropped when routing rules match but no existing conversation is found. Previously, emails could route to ReplyMailbox (based on headers or channel matching) but get dropped if ReferencesStrategy failed to find a conversation. This happened when two Chatwoot accounts emailed each other - References header contained conversation IDs from both accounts, routing matched globally, but scoped queries found nothing.
This commit is contained in:
Shivam Mishra
@@ -4,52 +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+)}
|
||||
CONVERSATION_FALLBACK_ID_PATTERN = %r{account/(\d+)/conversation/([a-zA-Z0-9-]+)@}
|
||||
|
||||
# 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
|
||||
references = inbound_mail.mail.references
|
||||
|
||||
# Check in_reply_to first
|
||||
return true if in_reply_to.present? && (
|
||||
in_reply_to_matches?(in_reply_to) || Message.exists?(source_id: in_reply_to)
|
||||
)
|
||||
|
||||
# Fallback to checking references header
|
||||
references.present? && references_match?(references)
|
||||
end
|
||||
|
||||
def in_reply_to_matches?(in_reply_to)
|
||||
Array.wrap(in_reply_to).any? { |id| id.match?(CONVERSATION_MESSAGE_ID_PATTERN) }
|
||||
end
|
||||
|
||||
def references_match?(references)
|
||||
Array.wrap(references).any? do |reference|
|
||||
reference.match?(CONVERSATION_MESSAGE_ID_PATTERN) ||
|
||||
reference.match?(CONVERSATION_FALLBACK_ID_PATTERN) ||
|
||||
Message.exists?(source_id: reference)
|
||||
end
|
||||
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]
|
||||
|
||||
@@ -4,8 +4,7 @@ class ReplyMailbox < ApplicationMailbox
|
||||
before_processing :find_conversation
|
||||
|
||||
def process
|
||||
return if @conversation.blank?
|
||||
|
||||
# ConversationFinder with NewConversationStrategy ensures @conversation is always present
|
||||
decorate_mail
|
||||
create_message
|
||||
add_attachments_to_message
|
||||
@@ -15,10 +14,6 @@ class ReplyMailbox < ApplicationMailbox
|
||||
|
||||
def find_conversation
|
||||
@conversation = Mailbox::ConversationFinder.new(mail).find
|
||||
|
||||
return unless @conversation.nil?
|
||||
|
||||
Rails.logger.error "[ReplyMailbox] No conversation found for email #{mail.message_id}"
|
||||
end
|
||||
|
||||
def decorate_mail
|
||||
|
||||
@@ -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
|
||||
@@ -2,7 +2,8 @@ class Mailbox::ConversationFinder
|
||||
DEFAULT_STRATEGIES = [
|
||||
Mailbox::ConversationFinderStrategies::ReceiverUuidStrategy,
|
||||
Mailbox::ConversationFinderStrategies::InReplyToStrategy,
|
||||
Mailbox::ConversationFinderStrategies::ReferencesStrategy
|
||||
Mailbox::ConversationFinderStrategies::ReferencesStrategy,
|
||||
Mailbox::ConversationFinderStrategies::NewConversationStrategy
|
||||
].freeze
|
||||
|
||||
def initialize(mail, strategies: DEFAULT_STRATEGIES)
|
||||
@@ -21,8 +22,8 @@ class Mailbox::ConversationFinder
|
||||
return conversation
|
||||
end
|
||||
|
||||
# No strategy matched
|
||||
Rails.logger.info 'No conversation found via any strategy'
|
||||
# Should not reach here if NewConversationStrategy is in the chain
|
||||
Rails.logger.error 'No conversation found via any strategy (NewConversationStrategy missing?)'
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox::ConversationFinderStrategies::BaseStrategy
|
||||
include MailboxHelper
|
||||
|
||||
attr_accessor :processed_mail, :account, :inbox, :contact, :contact_inbox, :conversation
|
||||
|
||||
# This strategy always succeeds by creating a new conversation if none was found.
|
||||
# It should be used as the last strategy in the chain to ensure emails are never dropped.
|
||||
def find
|
||||
channel = EmailChannelFinder.new(mail).perform
|
||||
return nil unless channel # No valid channel found
|
||||
|
||||
@account = channel.account
|
||||
@inbox = channel.inbox
|
||||
@processed_mail = MailPresenter.new(mail, @account)
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
find_or_create_contact
|
||||
create_conversation
|
||||
end
|
||||
|
||||
@conversation
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
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 original_sender_email
|
||||
@processed_mail.original_sender&.downcase
|
||||
end
|
||||
|
||||
def identify_contact_name
|
||||
@processed_mail.sender_name || @processed_mail.from.first.split('@').first
|
||||
end
|
||||
|
||||
def create_conversation
|
||||
@conversation = ::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 in_reply_to
|
||||
mail['In-Reply-To'].try(:value)
|
||||
end
|
||||
end
|
||||
@@ -41,28 +41,31 @@ RSpec.describe ApplicationMailbox do
|
||||
describe 'Support' do
|
||||
let!(:channel_email) { create(:channel_email) }
|
||||
|
||||
it 'routes support emails to Support Mailbox when mail is to channel email' do
|
||||
it 'routes support emails to Reply Mailbox when mail is to channel email' do
|
||||
# this email is hardcoded in the support.eml, that's why we are updating this
|
||||
# With NewConversationStrategy, all channel emails route to ReplyMailbox
|
||||
channel_email.update(email: 'care@example.com')
|
||||
dbl = double
|
||||
expect(SupportMailbox).to receive(:new).and_return(dbl)
|
||||
expect(ReplyMailbox).to receive(:new).and_return(dbl)
|
||||
expect(dbl).to receive(:perform_processing).and_return(true)
|
||||
described_class.route support_mail
|
||||
end
|
||||
|
||||
it 'routes support emails to Support Mailbox when mail is to channel forward to email' do
|
||||
it 'routes support emails to Reply Mailbox when mail is to channel forward to email' do
|
||||
# this email is hardcoded in the support.eml, that's why we are updating this
|
||||
# With NewConversationStrategy, all channel emails route to ReplyMailbox
|
||||
channel_email.update(forward_to_email: 'care@example.com')
|
||||
dbl = double
|
||||
expect(SupportMailbox).to receive(:new).and_return(dbl)
|
||||
expect(ReplyMailbox).to receive(:new).and_return(dbl)
|
||||
expect(dbl).to receive(:perform_processing).and_return(true)
|
||||
described_class.route support_mail
|
||||
end
|
||||
|
||||
it 'routes support emails to Support Mailbox with cc email' do
|
||||
it 'routes support emails to Reply Mailbox with cc email' do
|
||||
# With NewConversationStrategy, all channel emails route to ReplyMailbox
|
||||
channel_email.update(email: 'test@example.com')
|
||||
dbl = double
|
||||
expect(SupportMailbox).to receive(:new).and_return(dbl)
|
||||
expect(ReplyMailbox).to receive(:new).and_return(dbl)
|
||||
expect(dbl).to receive(:perform_processing).and_return(true)
|
||||
described_class.route reply_cc_mail
|
||||
end
|
||||
|
||||
@@ -1,352 +0,0 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe SupportMailbox do
|
||||
include ActionMailbox::TestHelper
|
||||
|
||||
describe 'when a chatwoot notification email is received' do
|
||||
let(:account) { create(:account) }
|
||||
let!(:channel_email) { create(:channel_email, email: 'sojan@chatwoot.com', account: account) }
|
||||
let(:notification_mail) { create_inbound_email_from_fixture('notification.eml') }
|
||||
let(:described_subject) { described_class.receive notification_mail }
|
||||
let(:conversation) { Conversation.where(inbox_id: channel_email.inbox).last }
|
||||
|
||||
it 'shouldnt create a conversation in the channel' do
|
||||
described_subject
|
||||
expect(conversation.present?).to be(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when bounced email with out a sender is recieved' do
|
||||
let(:account) { create(:account) }
|
||||
let(:bounced_email) { create_inbound_email_from_fixture('bounced_with_no_from.eml') }
|
||||
let(:described_subject) { described_class.receive bounced_email }
|
||||
|
||||
it 'shouldnt throw an error' do
|
||||
create(:channel_email, email: 'support@example.com', account: account)
|
||||
expect { described_subject }.not_to raise_error
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when an account is suspended' do
|
||||
let(:account) { create(:account, status: :suspended) }
|
||||
let(:agent) { create(:user, email: 'agent1@example.com', account: account) }
|
||||
let!(:channel_email) { create(:channel_email, account: account) }
|
||||
let(:support_mail) { create_inbound_email_from_fixture('support.eml') }
|
||||
let(:described_subject) { described_class.receive support_mail }
|
||||
let(:conversation) { Conversation.where(inbox_id: channel_email.inbox).last }
|
||||
|
||||
before do
|
||||
# this email is hardcoded in the support.eml, that's why we are updating this
|
||||
channel_email.email = 'care@example.com'
|
||||
channel_email.save!
|
||||
end
|
||||
|
||||
it 'shouldnt create a conversation in the channel' do
|
||||
described_subject
|
||||
expect(conversation.present?).to be(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'add mail as a new ticket in the email inbox' do
|
||||
let(:account) { create(:account) }
|
||||
let(:agent) { create(:user, email: 'agent1@example.com', account: account) }
|
||||
let!(:channel_email) { create(:channel_email, account: account) }
|
||||
let(:support_mail) { create_inbound_email_from_fixture('support.eml') }
|
||||
let(:support_in_reply_to_mail) { create_inbound_email_from_fixture('support_in_reply_to.eml') }
|
||||
let(:described_subject) { described_class.receive support_mail }
|
||||
let(:serialized_attributes) do
|
||||
%w[bcc cc content_type date from html_content in_reply_to message_id multipart number_of_attachments references subject
|
||||
text_content to auto_reply]
|
||||
end
|
||||
let(:conversation) { Conversation.where(inbox_id: channel_email.inbox).last }
|
||||
|
||||
before do
|
||||
# this email is hardcoded in the support.eml, that's why we are updating this
|
||||
channel_email.email = 'care@example.com'
|
||||
channel_email.save!
|
||||
end
|
||||
|
||||
describe 'covers email address format' do
|
||||
before do
|
||||
described_class.receive support_in_reply_to_mail
|
||||
end
|
||||
|
||||
it 'creates contact with proper email address' do
|
||||
expect(support_in_reply_to_mail.mail['reply_to'].try(:value)).to eq('Sony Mathew <sony@chatwoot.com>')
|
||||
expect(conversation.contact.email).to eq('sony@chatwoot.com')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'covers basic ticket creation' do
|
||||
before do
|
||||
described_subject
|
||||
end
|
||||
|
||||
it 'create the conversation in the inbox of the email channel' do
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
expect(conversation.additional_attributes['source']).to eq('email')
|
||||
expect(conversation.contact.email).to eq(support_mail.mail.from.first)
|
||||
end
|
||||
|
||||
it 'create a new contact as the sender of the email' do
|
||||
email_sender = Mail::Address.new(support_mail.mail[:from].value).name
|
||||
expect(conversation.messages.last.sender.email).to eq(support_mail.mail.from.first)
|
||||
expect(conversation.contact.name).to eq(email_sender)
|
||||
end
|
||||
|
||||
it 'add the mail content as new message on the conversation' do
|
||||
expect(conversation.messages.last.content).to eq("Let's talk about these images:")
|
||||
end
|
||||
|
||||
it 'add the attachments' do
|
||||
expect(conversation.messages.last.attachments.count).to eq(2)
|
||||
end
|
||||
|
||||
it 'have proper content_attributes with details of email' do
|
||||
expect(conversation.messages.last.content_attributes[:email].keys).to eq(serialized_attributes)
|
||||
end
|
||||
|
||||
it 'set proper content_type' do
|
||||
expect(conversation.messages.last.content_type).to eq('incoming_email')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'email with references header' do
|
||||
let(:mail_with_references) { create_inbound_email_from_fixture('mail_with_references.eml') }
|
||||
let(:described_subject) { described_class.receive mail_with_references }
|
||||
|
||||
before do
|
||||
# reuse the existing channel_email that's already set to 'care@example.com'
|
||||
described_subject
|
||||
end
|
||||
|
||||
it 'includes references in the message content_attributes' do
|
||||
message = conversation.messages.last
|
||||
email_attributes = message.content_attributes['email']
|
||||
|
||||
expect(email_attributes['references']).to be_present
|
||||
expect(email_attributes['references']).to eq(['4e6e35f5a38b4_479f13bb90078178@small-app-01.mail', 'test-reference-id'])
|
||||
end
|
||||
|
||||
it 'includes references in serialized email attributes' do
|
||||
message = conversation.messages.last
|
||||
expect(message.content_attributes['email'].keys).to include('references')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'Sender without name' do
|
||||
let(:support_mail_without_sender_name) { create_inbound_email_from_fixture('support_without_sender_name.eml') }
|
||||
let(:described_subject) { described_class.receive support_mail_without_sender_name }
|
||||
|
||||
it 'create a new contact with the email' do
|
||||
described_subject
|
||||
email_sender = support_mail_without_sender_name.mail.from.first.split('@').first
|
||||
expect(conversation.messages.last.sender.email).to eq(support_mail.mail.from.first)
|
||||
expect(conversation.contact.name).to eq(email_sender)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'Sender with upcase mail address' do
|
||||
let(:support_mail_without_sender_name) { create_inbound_email_from_fixture('support_without_sender_name.eml') }
|
||||
let(:described_subject) { described_class.receive support_mail_without_sender_name }
|
||||
|
||||
it 'create a new inbox with the email case insensitive' do
|
||||
described_subject
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'handle inbox contacts' do
|
||||
let!(:contact) { create(:contact, account: account, email: support_mail.mail.from.first) }
|
||||
let!(:contact_inbox) { create(:contact_inbox, inbox: channel_email.inbox, contact: contact) }
|
||||
|
||||
it 'does not create new contact if that contact exists in the inbox' do
|
||||
expect do
|
||||
described_subject
|
||||
end
|
||||
.to(not_change { Contact.count }
|
||||
.and(not_change { ContactInbox.count }))
|
||||
|
||||
expect(conversation.messages.last.sender.id).to eq(contact.id)
|
||||
expect(conversation.contact_inbox).to eq(contact_inbox)
|
||||
end
|
||||
|
||||
context 'with uppercase reply-to' do
|
||||
let(:support_mail) { create_inbound_email_from_fixture('support_uppercase.eml') }
|
||||
let!(:contact) { create(:contact, account: account, email: support_mail.mail.from.first) }
|
||||
let!(:contact_inbox) { create(:contact_inbox, inbox: channel_email.inbox, contact: contact) }
|
||||
|
||||
it 'does not create new contact if that contact exists in the inbox' do
|
||||
expect do
|
||||
described_subject
|
||||
end
|
||||
.to(not_change { Contact.count }
|
||||
.and(not_change { ContactInbox.count }))
|
||||
|
||||
expect(conversation.messages.last.sender.id).to eq(contact.id)
|
||||
expect(conversation.contact_inbox).to eq(contact_inbox)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'group email sender' do
|
||||
let(:group_sender_support_mail) { create_inbound_email_from_fixture('group_sender_support.eml') }
|
||||
let(:described_subject) { described_class.receive group_sender_support_mail }
|
||||
|
||||
before do
|
||||
# this email is hardcoded eml fixture file that's why we are updating this
|
||||
channel_email.email = 'support@chatwoot.com'
|
||||
channel_email.save!
|
||||
end
|
||||
|
||||
it 'create new contact with original sender' do
|
||||
described_subject
|
||||
email_sender = Mail::Address.new(group_sender_support_mail.mail[:from].value).name
|
||||
|
||||
expect(conversation.contact.email).to eq(group_sender_support_mail.mail['X-Original-Sender'].value)
|
||||
expect(conversation.contact.name).to eq(email_sender)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when mail has in reply to email' do
|
||||
let(:reply_mail_without_uuid) { create_inbound_email_from_fixture('reply_mail_without_uuid.eml') }
|
||||
let(:described_subject) { described_class.receive reply_mail_without_uuid }
|
||||
let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) }
|
||||
|
||||
before do
|
||||
email_channel
|
||||
reply_mail_without_uuid.mail['In-Reply-To'] = 'conversation/6bdc3f4d-0bec-4515-a284-5d916fdde489/messages/123'
|
||||
end
|
||||
|
||||
it 'create channel with reply to mail' do
|
||||
described_subject
|
||||
conversation_1 = Conversation.last
|
||||
|
||||
expect(conversation_1.messages.last.content).to eq("Let's talk about these images:")
|
||||
expect(conversation_1.additional_attributes['in_reply_to']).to eq('conversation/6bdc3f4d-0bec-4515-a284-5d916fdde489/messages/123')
|
||||
end
|
||||
|
||||
it 'append message to email conversation with same in reply to' do
|
||||
described_subject
|
||||
conversation_1 = Conversation.last
|
||||
|
||||
expect(conversation_1.messages.last.content).to eq("Let's talk about these images:")
|
||||
expect(conversation_1.additional_attributes['in_reply_to']).to eq('conversation/6bdc3f4d-0bec-4515-a284-5d916fdde489/messages/123')
|
||||
expect(conversation_1.messages.count).to eq(1)
|
||||
|
||||
reply_mail_without_uuid.mail['In-Reply-To'] = 'conversation/6bdc3f4d-0bec-4515-a284-5d916fdde489/messages/123'
|
||||
reply_mail_without_uuid.mail['Message-Id'] = '0CB459E0-0336-41DA-BC88-E6E28C697SFC@chatwoot.com'
|
||||
|
||||
described_class.receive reply_mail_without_uuid
|
||||
|
||||
expect(conversation_1.messages.last.content).to eq("Let's talk about these images:")
|
||||
expect(conversation_1.additional_attributes['in_reply_to']).to eq('conversation/6bdc3f4d-0bec-4515-a284-5d916fdde489/messages/123')
|
||||
expect(conversation_1.messages.count).to eq(2)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'Sender with reply_to email address' do
|
||||
let(:reply_to_mail) { create_inbound_email_from_fixture('reply_to.eml') }
|
||||
let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) }
|
||||
|
||||
it 'prefer reply-to over from address' do
|
||||
email_channel
|
||||
described_class.receive reply_to_mail
|
||||
|
||||
conversation_1 = Conversation.last
|
||||
email = conversation_1.messages.last.content_attributes['email']
|
||||
|
||||
expect(reply_to_mail.mail['From'].value).to be_present
|
||||
expect(conversation_1.messages.last.content).to eq("Let's talk about these images:")
|
||||
expect(reply_to_mail.mail['Reply-To'].value).to include(email['from'][0])
|
||||
expect(reply_to_mail.mail['Reply-To'].value).to include(conversation_1.contact.email)
|
||||
expect(reply_to_mail.mail['From'].value).not_to include(conversation_1.contact.email)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when mail part is not present' do
|
||||
let(:support_mail) { create_inbound_email_from_fixture('support_1.eml') }
|
||||
let(:only_text) { create_inbound_email_from_fixture('only_text.eml') }
|
||||
let(:only_html) { create_inbound_email_from_fixture('only_html.eml') }
|
||||
let(:only_attachments) { create_inbound_email_from_fixture('only_attachments.eml') }
|
||||
let(:html_and_attachments) { create_inbound_email_from_fixture('html_and_attachments.eml') }
|
||||
let(:described_subject) { described_class.receive support_mail }
|
||||
|
||||
it 'Considers raw html mail body' do
|
||||
described_subject
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
|
||||
expect(conversation.messages.last.content_attributes['email']['html_content']['reply']).to include(
|
||||
<<~BODY.chomp
|
||||
Hi,
|
||||
|
||||
We are providing you platform from here you can sell paid posts on your website.
|
||||
|
||||
Chatwoot | CS team | [C](https://d33wubrfki0l68.cloudfront.net/973467c532160fd8b940300a43fa85fa2d060307/dc9a0/static/brand-73f58cdefae282ae74cebfa74c1d7003.svg)
|
||||
|
||||
Skype: live:.cid.something
|
||||
|
||||
[]
|
||||
BODY
|
||||
)
|
||||
expect(conversation.messages.last.content_attributes['email']['subject']).to eq('Get Paid to post an article')
|
||||
end
|
||||
|
||||
it 'Considers only text body' do
|
||||
described_class.receive only_text
|
||||
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
|
||||
expect(conversation.messages.last.content).to eq('text only mail')
|
||||
expect(conversation.messages.last.content_attributes['email']['subject']).to eq('test text only mail')
|
||||
end
|
||||
|
||||
it 'Considers only html body' do
|
||||
described_class.receive only_html
|
||||
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
|
||||
expect(conversation.messages.last.content).to eq(
|
||||
<<~BODY.chomp
|
||||
This is html only mail
|
||||
BODY
|
||||
)
|
||||
expect(conversation.messages.last.content_attributes['email']['subject']).to eq('test html only mail')
|
||||
end
|
||||
|
||||
it 'Considers only attachments' do
|
||||
described_class.receive only_attachments
|
||||
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
|
||||
expect(conversation.messages.last.content).to be_nil
|
||||
expect(conversation.messages.last.attachments.count).to eq(1)
|
||||
expect(conversation.messages.last.content_attributes['email']['subject']).to eq('only attachments')
|
||||
end
|
||||
|
||||
it 'Considers html and attachments' do
|
||||
described_class.receive html_and_attachments
|
||||
|
||||
expect(conversation.inbox.id).to eq(channel_email.inbox.id)
|
||||
|
||||
expect(conversation.messages.last.content).to eq('This is html and attachments only mail')
|
||||
expect(conversation.messages.last.attachments.count).to eq(1)
|
||||
expect(conversation.messages.last.content_attributes['email']['subject']).to eq('attachment with html')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'when BCC processing is disabled for account' do
|
||||
before do
|
||||
allow(GlobalConfigService).to receive(:load).with('SKIP_INCOMING_BCC_PROCESSING', '').and_return(account.id.to_s)
|
||||
end
|
||||
|
||||
it 'does not process BCC-only emails' do
|
||||
bcc_mail = create_inbound_email_from_fixture('support.eml')
|
||||
bcc_mail.mail['to'] = nil
|
||||
bcc_mail.mail['bcc'] = 'care@example.com'
|
||||
|
||||
expect { described_class.receive bcc_mail }.to raise_error('Email channel/inbox not found')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -69,17 +69,27 @@ RSpec.describe Mailbox::ConversationFinder do
|
||||
end
|
||||
|
||||
context 'when no strategy finds conversation' do
|
||||
# With NewConversationStrategy in default strategies, this scenario only happens
|
||||
# when using custom strategies that exclude NewConversationStrategy
|
||||
let(:finding_strategies) do
|
||||
[
|
||||
Mailbox::ConversationFinderStrategies::ReceiverUuidStrategy,
|
||||
Mailbox::ConversationFinderStrategies::InReplyToStrategy,
|
||||
Mailbox::ConversationFinderStrategies::ReferencesStrategy
|
||||
]
|
||||
end
|
||||
|
||||
it 'returns nil' do
|
||||
finder = described_class.new(mail)
|
||||
finder = described_class.new(mail, strategies: finding_strategies)
|
||||
expect(finder.find).to be_nil
|
||||
end
|
||||
|
||||
it 'logs that no conversation was found' do
|
||||
allow(Rails.logger).to receive(:info)
|
||||
finder = described_class.new(mail)
|
||||
allow(Rails.logger).to receive(:error)
|
||||
finder = described_class.new(mail, strategies: finding_strategies)
|
||||
finder.find
|
||||
|
||||
expect(Rails.logger).to have_received(:info).with('No conversation found via any strategy')
|
||||
expect(Rails.logger).to have_received(:error).with('No conversation found via any strategy (NewConversationStrategy missing?)')
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
+156
@@ -0,0 +1,156 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Mailbox::ConversationFinderStrategies::NewConversationStrategy do
|
||||
let(:account) { create(:account) }
|
||||
let(:email_channel) { create(:channel_email, account: account) }
|
||||
let(:mail) { Mail.new }
|
||||
|
||||
before do
|
||||
mail.to = [email_channel.email]
|
||||
mail.from = 'sender@example.com'
|
||||
mail.subject = 'Test Subject'
|
||||
mail.message_id = '<test@example.com>'
|
||||
end
|
||||
|
||||
describe '#find' do
|
||||
context 'when channel is found' do
|
||||
context 'with new contact' do
|
||||
it 'creates a new conversation with new contact' do
|
||||
strategy = described_class.new(mail)
|
||||
|
||||
expect do
|
||||
conversation = strategy.find
|
||||
expect(conversation).to be_a(Conversation)
|
||||
expect(conversation.inbox).to eq(email_channel.inbox)
|
||||
expect(conversation.account).to eq(account)
|
||||
end.to change(Conversation, :count).by(1)
|
||||
.and change(Contact, :count).by(1)
|
||||
.and change(ContactInbox, :count).by(1)
|
||||
end
|
||||
|
||||
it 'sets conversation attributes correctly' do
|
||||
strategy = described_class.new(mail)
|
||||
conversation = strategy.find
|
||||
|
||||
expect(conversation.additional_attributes['source']).to eq('email')
|
||||
expect(conversation.additional_attributes['mail_subject']).to eq('Test Subject')
|
||||
expect(conversation.additional_attributes['initiated_at']).to have_key('timestamp')
|
||||
end
|
||||
|
||||
it 'sets contact attributes correctly' do
|
||||
strategy = described_class.new(mail)
|
||||
conversation = strategy.find
|
||||
|
||||
expect(conversation.contact.email).to eq('sender@example.com')
|
||||
expect(conversation.contact.name).to eq('sender')
|
||||
end
|
||||
end
|
||||
|
||||
context 'with existing contact' do
|
||||
let!(:existing_contact) { create(:contact, email: 'sender@example.com', account: account) }
|
||||
let!(:contact_inbox) { create(:contact_inbox, contact: existing_contact, inbox: email_channel.inbox) }
|
||||
|
||||
it 'creates conversation with existing contact' do
|
||||
strategy = described_class.new(mail)
|
||||
|
||||
expect do
|
||||
conversation = strategy.find
|
||||
expect(conversation).to be_a(Conversation)
|
||||
expect(conversation.contact).to eq(existing_contact)
|
||||
end.to change(Conversation, :count).by(1)
|
||||
.and not_change(Contact, :count)
|
||||
.and not_change(ContactInbox, :count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when mail has In-Reply-To header' do
|
||||
before do
|
||||
mail['In-Reply-To'] = '<previous-message@example.com>'
|
||||
end
|
||||
|
||||
it 'stores in_reply_to in additional_attributes' do
|
||||
strategy = described_class.new(mail)
|
||||
conversation = strategy.find
|
||||
|
||||
expect(conversation.additional_attributes['in_reply_to']).to eq('<previous-message@example.com>')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when mail is auto reply' do
|
||||
before do
|
||||
mail['X-Autoreply'] = 'yes'
|
||||
end
|
||||
|
||||
it 'marks conversation as auto_reply' do
|
||||
strategy = described_class.new(mail)
|
||||
conversation = strategy.find
|
||||
|
||||
expect(conversation.additional_attributes['auto_reply']).to be true
|
||||
end
|
||||
end
|
||||
|
||||
context 'when sender has name in From header' do
|
||||
before do
|
||||
mail.from = 'John Doe <john@example.com>'
|
||||
end
|
||||
|
||||
it 'uses sender name from mail' do
|
||||
strategy = described_class.new(mail)
|
||||
conversation = strategy.find
|
||||
|
||||
expect(conversation.contact.name).to eq('John Doe')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when channel is not found' do
|
||||
before do
|
||||
mail.to = ['nonexistent@example.com']
|
||||
end
|
||||
|
||||
it 'returns nil' do
|
||||
strategy = described_class.new(mail)
|
||||
|
||||
expect do
|
||||
result = strategy.find
|
||||
expect(result).to be_nil
|
||||
end.not_to change(Conversation, :count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when contact creation fails' do
|
||||
before do
|
||||
allow_any_instance_of(ContactInboxWithContactBuilder).to receive(:perform).and_raise(ActiveRecord::RecordInvalid)
|
||||
end
|
||||
|
||||
it 'rolls back the transaction' do
|
||||
strategy = described_class.new(mail)
|
||||
|
||||
expect do
|
||||
strategy.find
|
||||
end.to raise_error(ActiveRecord::RecordInvalid)
|
||||
.and not_change(Conversation, :count)
|
||||
.and not_change(Contact, :count)
|
||||
.and not_change(ContactInbox, :count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when conversation creation fails' do
|
||||
before do
|
||||
# Make conversation creation fail
|
||||
allow(Conversation).to receive(:create!).and_raise(ActiveRecord::RecordInvalid)
|
||||
end
|
||||
|
||||
it 'rolls back the transaction' do
|
||||
strategy = described_class.new(mail)
|
||||
|
||||
expect do
|
||||
strategy.find
|
||||
end.to raise_error(ActiveRecord::RecordInvalid)
|
||||
.and not_change(Conversation, :count)
|
||||
.and not_change(Contact, :count)
|
||||
.and not_change(ContactInbox, :count)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user