diff --git a/app/mailboxes/application_mailbox.rb b/app/mailboxes/application_mailbox.rb index 88e0954a0..a77f5ea43 100644 --- a/app/mailboxes/application_mailbox.rb +++ b/app/mailboxes/application_mailbox.rb @@ -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 - # - 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+@ + # checks if follows this pattern: reply+@ def reply_uuid_mail?(inbound_mail) inbound_mail.mail.to&.any? do |email| conversation_uuid = email.split('@')[0] diff --git a/app/mailboxes/reply_mailbox.rb b/app/mailboxes/reply_mailbox.rb index c7068c951..8147c58db 100644 --- a/app/mailboxes/reply_mailbox.rb +++ b/app/mailboxes/reply_mailbox.rb @@ -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 diff --git a/app/mailboxes/support_mailbox.rb b/app/mailboxes/support_mailbox.rb deleted file mode 100644 index 6279293b2..000000000 --- a/app/mailboxes/support_mailbox.rb +++ /dev/null @@ -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 diff --git a/app/services/mailbox/conversation_finder.rb b/app/services/mailbox/conversation_finder.rb index da8173b2f..2f8128ecd 100644 --- a/app/services/mailbox/conversation_finder.rb +++ b/app/services/mailbox/conversation_finder.rb @@ -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 diff --git a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb new file mode 100644 index 000000000..5997f1682 --- /dev/null +++ b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb @@ -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 diff --git a/spec/mailboxes/application_mailbox_spec.rb b/spec/mailboxes/application_mailbox_spec.rb index 33bbf9de8..1bc4d0fd1 100644 --- a/spec/mailboxes/application_mailbox_spec.rb +++ b/spec/mailboxes/application_mailbox_spec.rb @@ -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 diff --git a/spec/mailboxes/support_mailbox_spec.rb b/spec/mailboxes/support_mailbox_spec.rb deleted file mode 100644 index 0dbfbbe3b..000000000 --- a/spec/mailboxes/support_mailbox_spec.rb +++ /dev/null @@ -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 ') - 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 diff --git a/spec/services/mailbox/conversation_finder_spec.rb b/spec/services/mailbox/conversation_finder_spec.rb index 1544ac5c4..313ca409a 100644 --- a/spec/services/mailbox/conversation_finder_spec.rb +++ b/spec/services/mailbox/conversation_finder_spec.rb @@ -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 diff --git a/spec/services/mailbox/conversation_finder_strategies/new_conversation_strategy_spec.rb b/spec/services/mailbox/conversation_finder_strategies/new_conversation_strategy_spec.rb new file mode 100644 index 000000000..0984fac3e --- /dev/null +++ b/spec/services/mailbox/conversation_finder_strategies/new_conversation_strategy_spec.rb @@ -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 = '' + 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'] = '' + 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('') + 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 ' + 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