From 7e70f7a68ae5c6adf469297d1b2db4d23300bcc3 Mon Sep 17 00:00:00 2001 From: Pranav Date: Tue, 5 Aug 2025 00:47:06 -0700 Subject: [PATCH] fix: Disable automations on auto-reply emails (#12101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The term "sorcerer’s apprentice mode" is defined as a bug in a protocol where, under some circumstances, the receipt of a message causes multiple messages to be sent, each of which, when received, triggers the same bug. - RFC3834 Reference: https://github.com/chatwoot/chatwoot/pull/9606 This PR: - Adds an auto_reply attribute to message. - Adds an auto_reply attribute to conversation. - Disable conversation_created / conversation_opened event if auto_reply is set. - Disable message_created event if auto_reply is set. --------- Co-authored-by: Muhsin Keloth --- app/listeners/automation_rule_listener.rb | 11 +++- app/mailboxes/imap/imap_mailbox.rb | 1 + .../incoming_email_validity_helper.rb | 16 ----- app/mailboxes/support_mailbox.rb | 1 + app/models/message.rb | 6 ++ app/presenters/mail_presenter.rb | 3 +- .../hook_execution_service.rb | 1 + .../automation_rule_listener_spec.rb | 19 ++++++ spec/mailboxes/imap/imap_mailbox_spec.rb | 5 +- spec/mailboxes/reply_mailbox_spec.rb | 3 +- spec/mailboxes/support_mailbox_spec.rb | 2 +- spec/models/message_spec.rb | 65 +++++++++++++++++++ spec/presenters/mail_presenter_spec.rb | 9 ++- .../hook_execution_service_spec.rb | 29 +++++++++ 14 files changed, 147 insertions(+), 24 deletions(-) diff --git a/app/listeners/automation_rule_listener.rb b/app/listeners/automation_rule_listener.rb index 5dabda47e..6974e227a 100644 --- a/app/listeners/automation_rule_listener.rb +++ b/app/listeners/automation_rule_listener.rb @@ -17,7 +17,7 @@ class AutomationRuleListener < BaseListener end def conversation_created(event) - return if performed_by_automation?(event) + return if performed_by_automation?(event) || ignore_auto_reply_event?(event) conversation = event.data[:conversation] account = conversation.account @@ -34,7 +34,7 @@ class AutomationRuleListener < BaseListener end def conversation_opened(event) - return if performed_by_automation?(event) + return if performed_by_automation?(event) || ignore_auto_reply_event?(event) conversation = event.data[:conversation] account = conversation.account @@ -87,8 +87,13 @@ class AutomationRuleListener < BaseListener event.data[:performed_by].present? && event.data[:performed_by].instance_of?(AutomationRule) end + def ignore_auto_reply_event?(event) + conversation = event.data[:conversation] + conversation.additional_attributes['auto_reply'].present? + end + def ignore_message_created_event?(event) message = event.data[:message] - performed_by_automation?(event) || message.activity? + performed_by_automation?(event) || message.activity? || message.auto_reply_email? end end diff --git a/app/mailboxes/imap/imap_mailbox.rb b/app/mailboxes/imap/imap_mailbox.rb index 4e26756f6..bd591b05a 100644 --- a/app/mailboxes/imap/imap_mailbox.rb +++ b/app/mailboxes/imap/imap_mailbox.rb @@ -84,6 +84,7 @@ class Imap::ImapMailbox additional_attributes: { source: 'email', in_reply_to: in_reply_to, + auto_reply: @processed_mail.auto_reply?, mail_subject: @processed_mail.subject, initiated_at: { timestamp: Time.now.utc diff --git a/app/mailboxes/incoming_email_validity_helper.rb b/app/mailboxes/incoming_email_validity_helper.rb index 9483c9768..ce73a0b60 100644 --- a/app/mailboxes/incoming_email_validity_helper.rb +++ b/app/mailboxes/incoming_email_validity_helper.rb @@ -8,13 +8,6 @@ module IncomingEmailValidityHelper # This can happen in cases like bounce emails for invalid contact email address return false unless Devise.email_regexp.match?(@processed_mail.original_sender) - # Process bounced emails, as regular emails - return true if @processed_mail.bounced? - - # we skip processing auto reply emails like delivery status notifications - # out of office replies, etc. - return false if auto_reply_email? - true end @@ -24,13 +17,4 @@ module IncomingEmailValidityHelper true end - - def auto_reply_email? - if @processed_mail.auto_reply? - Rails.logger.info "is_auto_reply? : #{processed_mail.auto_reply?}" - true - else - false - end - end end diff --git a/app/mailboxes/support_mailbox.rb b/app/mailboxes/support_mailbox.rb index 5d80baef0..6279293b2 100644 --- a/app/mailboxes/support_mailbox.rb +++ b/app/mailboxes/support_mailbox.rb @@ -70,6 +70,7 @@ class SupportMailbox < ApplicationMailbox 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 diff --git a/app/models/message.rb b/app/models/message.rb index cc04a088e..bbb25df43 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -195,6 +195,12 @@ class Message < ApplicationRecord true end + def auto_reply_email? + return false unless incoming_email? || inbox.email? + + content_attributes.dig(:email, :auto_reply) == true + end + def valid_first_reply? return false unless human_response? && !private? return false if conversation.first_reply_created_at.present? diff --git a/app/presenters/mail_presenter.rb b/app/presenters/mail_presenter.rb index e57831c96..e98d2d085 100644 --- a/app/presenters/mail_presenter.rb +++ b/app/presenters/mail_presenter.rb @@ -103,7 +103,8 @@ class MailPresenter < SimpleDelegator references: references, subject: subject, text_content: text_content, - to: to + to: to, + auto_reply: auto_reply? } end diff --git a/app/services/message_templates/hook_execution_service.rb b/app/services/message_templates/hook_execution_service.rb index 7da475af3..6205c8f3c 100644 --- a/app/services/message_templates/hook_execution_service.rb +++ b/app/services/message_templates/hook_execution_service.rb @@ -4,6 +4,7 @@ class MessageTemplates::HookExecutionService def perform return if conversation.campaign.present? return if conversation.last_incoming_message.blank? + return if message.auto_reply_email? trigger_templates end diff --git a/spec/listeners/automation_rule_listener_spec.rb b/spec/listeners/automation_rule_listener_spec.rb index c08b6e402..e1c365f94 100644 --- a/spec/listeners/automation_rule_listener_spec.rb +++ b/spec/listeners/automation_rule_listener_spec.rb @@ -48,6 +48,13 @@ describe AutomationRuleListener do listener.conversation_created(event) expect(AutomationRules::ActionService).not_to have_received(:new).with(automation_rule, account, conversation) end + + it 'does not call AutomationRules::ActionService if conversation has auto_reply in additional_attributes' do + conversation.additional_attributes = { 'auto_reply' => true } + allow(condition_match).to receive(:present?).and_return(true) + listener.conversation_created(event) + expect(AutomationRules::ActionService).not_to have_received(:new).with(automation_rule, account, conversation) + end end end @@ -165,6 +172,18 @@ describe AutomationRuleListener do expect(AutomationRules::ActionService).not_to have_received(:new).with(automation_rule, account, conversation) end + it 'does not call AutomationRules::ActionService if message is auto reply email' do + email_channel = create(:channel_email, account: account) + email_inbox = create(:inbox, channel: email_channel, account: account) + email_conversation = create(:conversation, inbox: email_inbox, account: account) + email_message = create(:message, conversation: email_conversation, account: account, content_attributes: { email: { auto_reply: true } }) + email_event = Events::Base.new('message_created', Time.zone.now, { message: email_message }) + allow(condition_match).to receive(:present?).and_return(true) + + listener.message_created(email_event) + expect(AutomationRules::ActionService).not_to have_received(:new) + end + it 'does not call AutomationRules::ActionService if conditions do not match based on content' do message.update!(processed_message_content: 'hi', content: "hi\n\nhello") allow(condition_match).to receive(:present?).and_return(false) diff --git a/spec/mailboxes/imap/imap_mailbox_spec.rb b/spec/mailboxes/imap/imap_mailbox_spec.rb index fc94c98be..4535899cf 100644 --- a/spec/mailboxes/imap/imap_mailbox_spec.rb +++ b/spec/mailboxes/imap/imap_mailbox_spec.rb @@ -111,7 +111,8 @@ RSpec.describe Imap::ImapMailbox do let(:auto_reply_mail) { create_inbound_email_from_fixture('auto_reply.eml') } it 'does not create a new conversation' do - expect { class_instance.process(auto_reply_mail.mail, channel) }.not_to change(Conversation, :count) + expect { class_instance.process(auto_reply_mail.mail, channel) }.to change(Conversation, :count) + expect(Conversation.last.additional_attributes['auto_reply']).to be true end end @@ -120,6 +121,8 @@ RSpec.describe Imap::ImapMailbox do it 'processes the bounced email' do expect { class_instance.process(bounced_mail.mail, channel) }.to change(Message, :count) + expect(Message.last.content_attributes['email']['auto_reply']).to be true + expect(Conversation.last.additional_attributes['auto_reply']).to be true end end diff --git a/spec/mailboxes/reply_mailbox_spec.rb b/spec/mailboxes/reply_mailbox_spec.rb index 2320e3e07..167777218 100644 --- a/spec/mailboxes/reply_mailbox_spec.rb +++ b/spec/mailboxes/reply_mailbox_spec.rb @@ -12,7 +12,8 @@ RSpec.describe ReplyMailbox do let(:conversation) { create(:conversation, assignee: agent, inbox: create(:inbox, account: account, greeting_enabled: false), account: account) } let(:described_subject) { described_class.receive reply_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] + %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 context 'with reply uuid present' do diff --git a/spec/mailboxes/support_mailbox_spec.rb b/spec/mailboxes/support_mailbox_spec.rb index f75e3ef80..f6964285d 100644 --- a/spec/mailboxes/support_mailbox_spec.rb +++ b/spec/mailboxes/support_mailbox_spec.rb @@ -56,7 +56,7 @@ RSpec.describe SupportMailbox do 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] + text_content to auto_reply] end let(:conversation) { Conversation.where(inbox_id: channel_email.inbox).last } diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 6de36a55e..cd4bc6bc1 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -548,4 +548,69 @@ RSpec.describe Message do expect(presenter).to have_received(:outgoing_content) end end + + describe '#auto_reply_email?' do + context 'when message is not an incoming email and inbox is not email' do + let(:conversation) { create(:conversation) } + let(:message) { create(:message, conversation: conversation, message_type: :outgoing) } + + it 'returns false' do + expect(message.auto_reply_email?).to be false + end + end + + context 'when message is an incoming email' do + let(:email_channel) { create(:channel_email) } + let(:email_inbox) { create(:inbox, channel: email_channel) } + let(:conversation) { create(:conversation, inbox: email_inbox) } + + it 'returns false when auto_reply is not set to true' do + message = create( + :message, + conversation: conversation, + message_type: :incoming, + content_type: 'incoming_email', + content_attributes: {} + ) + expect(message.auto_reply_email?).to be false + end + + it 'returns true when auto_reply is set to true' do + message = create( + :message, + conversation: conversation, + message_type: :incoming, + content_type: 'incoming_email', + content_attributes: { email: { auto_reply: true } } + ) + expect(message.auto_reply_email?).to be true + end + end + + context 'when inbox is email' do + let(:email_channel) { create(:channel_email) } + let(:email_inbox) { create(:inbox, channel: email_channel) } + let(:conversation) { create(:conversation, inbox: email_inbox) } + + it 'returns false when auto_reply is not set to true' do + message = create( + :message, + conversation: conversation, + message_type: :outgoing, + content_attributes: {} + ) + expect(message.auto_reply_email?).to be false + end + + it 'returns true when auto_reply is set to true' do + message = create( + :message, + conversation: conversation, + message_type: :outgoing, + content_attributes: { email: { auto_reply: true } } + ) + expect(message.auto_reply_email?).to be true + end + end + end end diff --git a/spec/presenters/mail_presenter_spec.rb b/spec/presenters/mail_presenter_spec.rb index af6768e41..b9b23c8ce 100644 --- a/spec/presenters/mail_presenter_spec.rb +++ b/spec/presenters/mail_presenter_spec.rb @@ -49,13 +49,15 @@ RSpec.describe MailPresenter do :references, :subject, :text_content, - :to + :to, + :auto_reply ]) expect(data[:content_type]).to include('multipart/alternative') expect(data[:date].to_s).to eq('2020-04-20T04:20:20-04:00') expect(data[:message_id]).to eq(mail.message_id) expect(data[:multipart]).to be(true) expect(data[:subject]).to eq(decorated_mail.subject) + expect(data[:auto_reply]).to eq(decorated_mail.auto_reply?) end it 'give email from in downcased format' do @@ -136,6 +138,11 @@ RSpec.describe MailPresenter do expect(decorated_auto_reply_mail.auto_reply?).to be true expect(decorated_auto_reply_with_auto_submitted_mail.auto_reply?).to be true end + + it 'includes auto_reply status in serialized_data' do + expect(decorated_auto_reply_mail.serialized_data[:auto_reply]).to be true + expect(decorated_mail.serialized_data[:auto_reply]).to be_falsey + end end end end diff --git a/spec/services/message_templates/hook_execution_service_spec.rb b/spec/services/message_templates/hook_execution_service_spec.rb index b80c42fd8..13a820f17 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -111,6 +111,35 @@ describe MessageTemplates::HookExecutionService do end end + context 'when message is an auto reply email' do + it 'does not call any template hooks' do + contact = create(:contact) + conversation = create(:conversation, contact: contact) + conversation.inbox.update(greeting_enabled: true, enable_email_collect: true, greeting_message: 'Hi, this is a greeting message') + + message = create(:message, conversation: conversation, content_type: :incoming_email) + message.content_attributes = { email: { auto_reply: true } } + message.save! + + greeting_service = double + email_collect_service = double + out_of_office_service = double + + allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service) + allow(greeting_service).to receive(:perform).and_return(true) + allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(email_collect_service) + allow(email_collect_service).to receive(:perform).and_return(true) + allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) + allow(out_of_office_service).to receive(:perform).and_return(true) + + described_class.new(message: message).perform + + expect(MessageTemplates::Template::Greeting).not_to have_received(:new) + expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new) + expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) + end + end + context 'when it is after working hours' do it 'calls ::MessageTemplates::Template::OutOfOffice' do contact = create(:contact)