Merge branch 'develop' into feat/reporting-events-rollup
This commit is contained in:
@@ -130,11 +130,15 @@ class MailPresenter < SimpleDelegator
|
||||
end
|
||||
|
||||
def sender_name
|
||||
Mail::Address.new((@mail[:reply_to] || @mail[:from]).value).name
|
||||
parse_mail_address((@mail[:reply_to] || @mail[:from]).value)&.name
|
||||
end
|
||||
|
||||
def original_sender
|
||||
from_email_address(@mail[:reply_to].try(:value)) || @mail['X-Original-Sender'].try(:value) || from_email_address(from.first)
|
||||
[
|
||||
@mail[:reply_to]&.value,
|
||||
@mail['X-Original-Sender']&.value,
|
||||
@mail[:from]&.value
|
||||
].filter_map { |email| parse_mail_address(email)&.address }.first
|
||||
end
|
||||
|
||||
def headers_data
|
||||
@@ -147,10 +151,6 @@ class MailPresenter < SimpleDelegator
|
||||
headers.presence
|
||||
end
|
||||
|
||||
def from_email_address(email)
|
||||
Mail::Address.new(email).address
|
||||
end
|
||||
|
||||
def email_forwarded_for
|
||||
@mail['X-Forwarded-For'].try(:value)
|
||||
end
|
||||
@@ -175,11 +175,20 @@ class MailPresenter < SimpleDelegator
|
||||
|
||||
def notification_email_from_chatwoot?
|
||||
# notification emails are send via mailer sender email address. so it should match
|
||||
original_sender == Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')).address
|
||||
configured_sender = Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')).address
|
||||
original_sender.to_s.casecmp?(configured_sender)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def parse_mail_address(email)
|
||||
return if email.blank?
|
||||
|
||||
Mail::Address.new(email)
|
||||
rescue Mail::Field::ParseError, Mail::Field::IncompleteParseError
|
||||
nil
|
||||
end
|
||||
|
||||
def auto_submitted?
|
||||
@mail['Auto-Submitted'].present? && @mail['Auto-Submitted'].value != 'no'
|
||||
end
|
||||
|
||||
@@ -2,7 +2,6 @@ class MessageTemplates::HookExecutionService
|
||||
pattr_initialize [:message!]
|
||||
|
||||
def perform
|
||||
return if conversation.campaign.present?
|
||||
return if conversation.last_incoming_message.blank?
|
||||
return if message.auto_reply_email?
|
||||
|
||||
@@ -21,6 +20,7 @@ class MessageTemplates::HookExecutionService
|
||||
end
|
||||
|
||||
def should_send_out_of_office_message?
|
||||
return false if conversation.campaign.present?
|
||||
# should not send if its a tweet message
|
||||
return false if conversation.tweet?
|
||||
# should not send for outbound messages
|
||||
@@ -37,6 +37,7 @@ class MessageTemplates::HookExecutionService
|
||||
end
|
||||
|
||||
def should_send_greeting?
|
||||
return false if conversation.campaign.present?
|
||||
# should not send if its a tweet message
|
||||
return false if conversation.tweet?
|
||||
|
||||
@@ -49,6 +50,8 @@ class MessageTemplates::HookExecutionService
|
||||
|
||||
# TODO: we should be able to reduce this logic once we have a toggle for email collect messages
|
||||
def should_send_email_collect?
|
||||
return false if conversation.campaign.present?
|
||||
|
||||
!contact_has_email? && inbox.web_widget? && !email_collect_was_sent?
|
||||
end
|
||||
|
||||
|
||||
@@ -93,6 +93,10 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob
|
||||
end
|
||||
|
||||
def send_out_of_office_message_if_applicable
|
||||
# Campaign conversations should never receive OOO templates — the campaign itself
|
||||
# serves as the initial outreach, and OOO would be confusing in that context.
|
||||
return if @conversation.campaign.present?
|
||||
|
||||
::MessageTemplates::Template::OutOfOffice.perform_if_applicable(@conversation)
|
||||
end
|
||||
|
||||
|
||||
@@ -68,6 +68,10 @@ module Enterprise::MessageTemplates::HookExecutionService
|
||||
end
|
||||
|
||||
def send_out_of_office_message_after_handoff
|
||||
# Campaign conversations should never receive OOO templates — the campaign itself
|
||||
# serves as the initial outreach, and OOO would be confusing in that context.
|
||||
return if conversation.campaign.present?
|
||||
|
||||
::MessageTemplates::Template::OutOfOffice.perform_if_applicable(conversation)
|
||||
end
|
||||
|
||||
|
||||
@@ -42,6 +42,10 @@ class Captain::Tools::HandoffTool < Captain::Tools::BasePublicTool
|
||||
end
|
||||
|
||||
def send_out_of_office_message_if_applicable(conversation)
|
||||
# Campaign conversations should never receive OOO templates — the campaign itself
|
||||
# serves as the initial outreach, and OOO would be confusing in that context.
|
||||
return if conversation.campaign.present?
|
||||
|
||||
::MessageTemplates::Template::OutOfOffice.perform_if_applicable(conversation)
|
||||
end
|
||||
|
||||
|
||||
@@ -102,7 +102,8 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService
|
||||
def send_message
|
||||
post_message if message_content.present?
|
||||
upload_files if message.attachments.any?
|
||||
rescue Slack::Web::Api::Errors::AccountInactive, Slack::Web::Api::Errors::MissingScope, Slack::Web::Api::Errors::InvalidAuth,
|
||||
rescue Slack::Web::Api::Errors::IsArchived, Slack::Web::Api::Errors::AccountInactive, Slack::Web::Api::Errors::MissingScope,
|
||||
Slack::Web::Api::Errors::InvalidAuth,
|
||||
Slack::Web::Api::Errors::ChannelNotFound, Slack::Web::Api::Errors::NotInChannel => e
|
||||
Rails.logger.error e
|
||||
hook.prompt_reauthorization!
|
||||
|
||||
@@ -238,6 +238,77 @@ RSpec.describe MessageTemplates::HookExecutionService do
|
||||
end
|
||||
end
|
||||
|
||||
context 'when conversation has a campaign' do
|
||||
let(:campaign) { create(:campaign, account: account) }
|
||||
let(:campaign_conversation) { create(:conversation, inbox: inbox, account: account, contact: contact, status: :pending, campaign: campaign) }
|
||||
|
||||
it 'schedules captain response job for incoming messages on pending campaign conversations' do
|
||||
expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(campaign_conversation, assistant)
|
||||
|
||||
create(:message, conversation: campaign_conversation, message_type: :incoming)
|
||||
end
|
||||
|
||||
it 'does not send greeting template on campaign conversations' do
|
||||
inbox.update!(greeting_enabled: true, greeting_message: 'Hello! How can we help you?', enable_email_collect: false)
|
||||
|
||||
greeting_service = instance_double(MessageTemplates::Template::Greeting)
|
||||
allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service)
|
||||
allow(greeting_service).to receive(:perform).and_return(true)
|
||||
|
||||
create(:message, conversation: campaign_conversation, message_type: :incoming)
|
||||
|
||||
expect(MessageTemplates::Template::Greeting).not_to have_received(:new)
|
||||
end
|
||||
|
||||
it 'does not send out of office template on campaign conversations' do
|
||||
inbox.update!(working_hours_enabled: true, out_of_office_message: 'We are currently closed')
|
||||
inbox.working_hours.find_by(day_of_week: Time.current.in_time_zone(inbox.timezone).wday).update!(
|
||||
closed_all_day: true,
|
||||
open_all_day: false
|
||||
)
|
||||
|
||||
out_of_office_service = instance_double(MessageTemplates::Template::OutOfOffice)
|
||||
allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service)
|
||||
allow(out_of_office_service).to receive(:perform).and_return(true)
|
||||
|
||||
create(:message, conversation: campaign_conversation, message_type: :incoming)
|
||||
|
||||
expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new)
|
||||
end
|
||||
|
||||
it 'does not send email collect template on campaign conversations' do
|
||||
contact.update!(email: nil)
|
||||
inbox.update!(enable_email_collect: true)
|
||||
|
||||
email_collect_service = instance_double(MessageTemplates::Template::EmailCollect)
|
||||
allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(email_collect_service)
|
||||
allow(email_collect_service).to receive(:perform).and_return(true)
|
||||
|
||||
create(:message, conversation: campaign_conversation, message_type: :incoming)
|
||||
|
||||
expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new)
|
||||
end
|
||||
|
||||
it 'does not send out of office template after handoff on campaign conversations when quota is exceeded' do
|
||||
account.update!(
|
||||
limits: { 'captain_responses' => 100 },
|
||||
custom_attributes: account.custom_attributes.merge('captain_responses_usage' => 100)
|
||||
)
|
||||
inbox.update!(
|
||||
working_hours_enabled: true,
|
||||
out_of_office_message: 'We are currently closed'
|
||||
)
|
||||
inbox.working_hours.find_by(day_of_week: Time.current.in_time_zone(inbox.timezone).wday).update!(
|
||||
closed_all_day: true,
|
||||
open_all_day: false
|
||||
)
|
||||
|
||||
expect do
|
||||
create(:message, conversation: campaign_conversation, message_type: :incoming)
|
||||
end.not_to(change { campaign_conversation.messages.template.count })
|
||||
end
|
||||
end
|
||||
|
||||
context 'when Captain quota is exceeded and handoff happens' do
|
||||
before do
|
||||
account.update!(
|
||||
|
||||
@@ -178,5 +178,85 @@ RSpec.describe MailPresenter do
|
||||
expect(decorated_mail.serialized_data[:auto_reply]).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
describe 'malformed sender headers' do
|
||||
let(:mail_with_malformed_from) do
|
||||
Mail.new do
|
||||
header['From'] = 'Kevin McDonald <info@example.com'
|
||||
to 'Inbox <inbox@example.com>'
|
||||
subject :header
|
||||
body 'Hi'
|
||||
end
|
||||
end
|
||||
|
||||
let(:mail_with_malformed_reply_to) do
|
||||
Mail.new do
|
||||
from 'Sender <sender@example.com>'
|
||||
to 'Inbox <inbox@example.com>'
|
||||
subject :header
|
||||
body 'Hi'
|
||||
header['Reply-To'] = 'Reply User <reply@example.com'
|
||||
end
|
||||
end
|
||||
|
||||
let(:mail_with_original_sender_header) do
|
||||
Mail.new do
|
||||
from 'Sender <sender@example.com>'
|
||||
to 'Inbox <inbox@example.com>'
|
||||
subject :header
|
||||
body 'Hi'
|
||||
header['Reply-To'] = 'Reply User <reply@example.com'
|
||||
header['X-Original-Sender'] = 'Forwarded Sender <forwarded.sender@example.com>'
|
||||
end
|
||||
end
|
||||
|
||||
let(:mail_with_invalid_original_sender_header) do
|
||||
Mail.new do
|
||||
from 'Sender <sender@example.com>'
|
||||
to 'Inbox <inbox@example.com>'
|
||||
subject :header
|
||||
body 'Hi'
|
||||
header['Reply-To'] = 'Reply User <reply@example.com'
|
||||
header['X-Original-Sender'] = 'not an email address'
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns nil sender values when from header is malformed' do
|
||||
presenter = described_class.new(mail_with_malformed_from)
|
||||
|
||||
expect(presenter.original_sender).to be_nil
|
||||
expect(presenter.sender_name).to be_nil
|
||||
expect(presenter.notification_email_from_chatwoot?).to be(false)
|
||||
end
|
||||
|
||||
it 'falls back to from header when reply_to is malformed' do
|
||||
presenter = described_class.new(mail_with_malformed_reply_to)
|
||||
expect(presenter.original_sender).to eq('sender@example.com')
|
||||
end
|
||||
|
||||
it 'uses parsed X-Original-Sender value when available' do
|
||||
presenter = described_class.new(mail_with_original_sender_header)
|
||||
expect(presenter.original_sender).to eq('forwarded.sender@example.com')
|
||||
end
|
||||
|
||||
it 'falls back to from when X-Original-Sender is invalid' do
|
||||
presenter = described_class.new(mail_with_invalid_original_sender_header)
|
||||
expect(presenter.original_sender).to eq('sender@example.com')
|
||||
end
|
||||
|
||||
it 'matches notification sender emails case-insensitively' do
|
||||
mail_with_uppercase_sender = Mail.new do
|
||||
from 'Chatwoot <ACCOUNTS@CHATWOOT.COM>'
|
||||
to 'Inbox <inbox@example.com>'
|
||||
subject :header
|
||||
body 'Hi'
|
||||
end
|
||||
|
||||
with_modified_env MAILER_SENDER_EMAIL: 'Chatwoot <accounts@chatwoot.com>' do
|
||||
presenter = described_class.new(mail_with_uppercase_sender)
|
||||
expect(presenter.notification_email_from_chatwoot?).to be(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -111,6 +111,40 @@ describe MessageTemplates::HookExecutionService do
|
||||
end
|
||||
end
|
||||
|
||||
context 'when conversation has a campaign' do
|
||||
let(:campaign) { create(:campaign) }
|
||||
|
||||
it 'does not call ::MessageTemplates::Template::Greeting on campaign conversations' do
|
||||
contact = create(:contact, email: nil)
|
||||
conversation = create(:conversation, contact: contact, campaign: campaign)
|
||||
conversation.inbox.update(greeting_enabled: true, greeting_message: 'Hi, this is a greeting message', enable_email_collect: false)
|
||||
|
||||
greeting_service = double
|
||||
allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service)
|
||||
allow(greeting_service).to receive(:perform).and_return(true)
|
||||
|
||||
create(:message, conversation: conversation)
|
||||
|
||||
expect(MessageTemplates::Template::Greeting).not_to have_received(:new)
|
||||
end
|
||||
|
||||
it 'does not call ::MessageTemplates::Template::OutOfOffice on campaign conversations' do
|
||||
contact = create(:contact)
|
||||
conversation = create(:conversation, contact: contact, campaign: campaign)
|
||||
|
||||
conversation.inbox.update(working_hours_enabled: true, out_of_office_message: 'We are out of office')
|
||||
conversation.inbox.working_hours.today.update!(closed_all_day: true)
|
||||
|
||||
out_of_office_service = double
|
||||
allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service)
|
||||
allow(out_of_office_service).to receive(:perform).and_return(true)
|
||||
|
||||
create(:message, conversation: conversation)
|
||||
|
||||
expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when message is an auto reply email' do
|
||||
it 'does not call any template hooks' do
|
||||
contact = create(:contact)
|
||||
|
||||
Reference in New Issue
Block a user