From 47bdb6d2bb44e61a3595724ba2efe20962df6815 Mon Sep 17 00:00:00 2001 From: Pranav Date: Wed, 24 Sep 2025 11:36:53 -0700 Subject: [PATCH] feat: Clean up email configuration for from and reply to emails (#12453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We first added conversation continuity for the live chat widget, and then carried the same logic over to email channels. The problem was that this added a reply+conversationUUID@domain.com as the reply-to for emails, which was unnecessary. For email channels, the reply-to can just be the channel’s own email address. That extra layer made things more complex than it needed to be. In this PR, I’ve cleaned up the config so it’s simpler. The table below shows how it’ll work going forward. --- | Type | From Email | Reply To Email | | -- | -- | -- | | Standard IMAP, SMTP email channel | channel.email | channel.email | | Google OAuth Email channel | channel.email | channel.email | | Microsoft OAuth Email channel | channel.email | channel.email | | Email forwarded to Chatwoot, brought their own SMTP | channel.email | channel.email | | Imap to fetch email, Use Chatwoot's SMTP | channel.email if verified with Chatwoot's SMTP provider. Otherwise account support email | channel.email | | Email forwarded to Chatwoot, Use Chatwoot's SMTP | channel.email if verified with Chatwoot's SMTP provider. Otherwise account support email | channel.email | | -- | -- | -- | | Website Live Chat - Conversation Continuity Inbound Emails enabled| Account Support Email | reply+{conversation-uuid}@{account_domain} | | Website Live Chat - Conversation Continuity Inbound Emails disabled| Account Support Email | Account Support Email | Fixes https://github.com/chatwoot/chatwoot/issues/10614 Fixes https://github.com/chatwoot/chatwoot/issues/10521 Fixes https://github.com/chatwoot/chatwoot/issues/10300 Fixes https://github.com/chatwoot/chatwoot/issues/10091 Fixes https://github.com/chatwoot/chatwoot/issues/4890 Fixes https://github.com/chatwoot/chatwoot/issues/10676 Fixes https://github.com/chatwoot/chatwoot/issues/10756 Fixes https://github.com/chatwoot/chatwoot/issues/11515 Fixes https://github.com/chatwoot/chatwoot/issues/9471 --------- Co-authored-by: Sojan Jose Co-authored-by: Muhsin Keloth --- app/builders/email/base_builder.rb | 54 ++++++ app/builders/email/from_builder.rb | 51 ++++++ app/builders/email/reply_to_builder.rb | 21 +++ .../conversation_reply_mailer_helper.rb | 4 + app/models/channel/email.rb | 3 +- config/features.yml | 6 + ...917012759_add_verified_to_channel_email.rb | 5 + db/schema.rb | 3 +- spec/builders/email/from_builder_spec.rb | 169 ++++++++++++++++++ spec/builders/email/reply_to_builder_spec.rb | 108 +++++++++++ 10 files changed, 422 insertions(+), 2 deletions(-) create mode 100644 app/builders/email/base_builder.rb create mode 100644 app/builders/email/from_builder.rb create mode 100644 app/builders/email/reply_to_builder.rb create mode 100644 db/migrate/20250917012759_add_verified_to_channel_email.rb create mode 100644 spec/builders/email/from_builder_spec.rb create mode 100644 spec/builders/email/reply_to_builder_spec.rb diff --git a/app/builders/email/base_builder.rb b/app/builders/email/base_builder.rb new file mode 100644 index 000000000..731b1b0f5 --- /dev/null +++ b/app/builders/email/base_builder.rb @@ -0,0 +1,54 @@ +class Email::BaseBuilder + pattr_initialize [:inbox!] + + private + + def channel + @channel ||= inbox.channel + end + + def account + @account ||= inbox.account + end + + def conversation + @conversation ||= message.conversation + end + + def custom_sender_name + message&.sender&.available_name || I18n.t('conversations.reply.email.header.notifications') + end + + def sender_name(sender_email) + # Friendly: from + # Professional: + if inbox.friendly? + I18n.t( + 'conversations.reply.email.header.friendly_name', + sender_name: custom_sender_name, + business_name: business_name, + from_email: sender_email + ) + else + I18n.t( + 'conversations.reply.email.header.professional_name', + business_name: business_name, + from_email: sender_email + ) + end + end + + def business_name + inbox.business_name || inbox.sanitized_name + end + + def account_support_email + # Parse the email to ensure it's in the correct format, the user + # can save it in the format "Name " + parse_email(account.support_email) + end + + def parse_email(email_string) + Mail::Address.new(email_string).address + end +end diff --git a/app/builders/email/from_builder.rb b/app/builders/email/from_builder.rb new file mode 100644 index 000000000..fff33dc0a --- /dev/null +++ b/app/builders/email/from_builder.rb @@ -0,0 +1,51 @@ +class Email::FromBuilder < Email::BaseBuilder + pattr_initialize [:inbox!, :message!] + + def build + return sender_name(account_support_email) unless inbox.email? + + from_email = case email_channel_type + when :standard_imap_smtp, + :google_oauth, + :microsoft_oauth, + :forwarding_own_smtp + channel.email + when :imap_chatwoot_smtp, + :forwarding_chatwoot_smtp + channel.verified_for_sending ? channel.email : account_support_email + else + account_support_email + end + + sender_name(from_email) + end + + private + + def email_channel_type + return :google_oauth if channel.google? + return :microsoft_oauth if channel.microsoft? + return :standard_imap_smtp if imap_and_smtp_enabled? + return :imap_chatwoot_smtp if imap_enabled_without_smtp? + return :forwarding_own_smtp if forwarding_with_own_smtp? + return :forwarding_chatwoot_smtp if forwarding_without_smtp? + + :unknown + end + + def imap_and_smtp_enabled? + channel.imap_enabled && channel.smtp_enabled + end + + def imap_enabled_without_smtp? + channel.imap_enabled && !channel.smtp_enabled + end + + def forwarding_with_own_smtp? + !channel.imap_enabled && channel.smtp_enabled + end + + def forwarding_without_smtp? + !channel.imap_enabled && !channel.smtp_enabled + end +end diff --git a/app/builders/email/reply_to_builder.rb b/app/builders/email/reply_to_builder.rb new file mode 100644 index 000000000..d330c922a --- /dev/null +++ b/app/builders/email/reply_to_builder.rb @@ -0,0 +1,21 @@ +class Email::ReplyToBuilder < Email::BaseBuilder + pattr_initialize [:inbox!, :message!] + + def build + reply_to = if inbox.email? + channel.email + elsif inbound_email_enabled? + "reply+#{conversation.uuid}@#{account.inbound_email_domain}" + else + account_support_email + end + + sender_name(reply_to) + end + + private + + def inbound_email_enabled? + account.feature_enabled?('inbound_emails') && account.inbound_email_domain.present? + end +end diff --git a/app/mailers/conversation_reply_mailer_helper.rb b/app/mailers/conversation_reply_mailer_helper.rb index 55f7fe12b..2fe3695f9 100644 --- a/app/mailers/conversation_reply_mailer_helper.rb +++ b/app/mailers/conversation_reply_mailer_helper.rb @@ -118,10 +118,14 @@ module ConversationReplyMailerHelper end def email_from + return Email::FromBuilder.new(inbox: @inbox, message: current_message).build if @account.feature_enabled?(:reply_mailer_migration) + email_oauth_enabled || email_smtp_enabled ? channel_email_with_name : from_email_with_name end def email_reply_to + return Email::ReplyToBuilder.new(inbox: @inbox, message: current_message).build if @account.feature_enabled?(:reply_mailer_migration) + email_imap_enabled ? @channel.email : reply_email end diff --git a/app/models/channel/email.rb b/app/models/channel/email.rb index 126596204..a8fadb61e 100644 --- a/app/models/channel/email.rb +++ b/app/models/channel/email.rb @@ -23,6 +23,7 @@ # smtp_openssl_verify_mode :string default("none") # smtp_password :string default("") # smtp_port :integer default(0) +# verified_for_sending :boolean default(FALSE), not null # created_at :datetime not null # updated_at :datetime not null # account_id :integer not null @@ -42,7 +43,7 @@ class Channel::Email < ApplicationRecord self.table_name = 'channel_email' EDITABLE_ATTRS = [:email, :imap_enabled, :imap_login, :imap_password, :imap_address, :imap_port, :imap_enable_ssl, :smtp_enabled, :smtp_login, :smtp_password, :smtp_address, :smtp_port, :smtp_domain, :smtp_enable_starttls_auto, - :smtp_enable_ssl_tls, :smtp_openssl_verify_mode, :smtp_authentication, :provider].freeze + :smtp_enable_ssl_tls, :smtp_openssl_verify_mode, :smtp_authentication, :provider, :verified_for_sending].freeze validates :email, uniqueness: true validates :forward_to_email, uniqueness: true diff --git a/config/features.yml b/config/features.yml index 40083812a..3d896b483 100644 --- a/config/features.yml +++ b/config/features.yml @@ -214,3 +214,9 @@ enabled: false premium: true chatwoot_internal: true +- name: reply_mailer_migration + # This feature is temporary only to migrate reply mailer to new email builder + # Once the migration is done, this feature can be removed + display_name: Reply Mailer Migration + enabled: false + chatwoot_internal: true diff --git a/db/migrate/20250917012759_add_verified_to_channel_email.rb b/db/migrate/20250917012759_add_verified_to_channel_email.rb new file mode 100644 index 000000000..4c88f3432 --- /dev/null +++ b/db/migrate/20250917012759_add_verified_to_channel_email.rb @@ -0,0 +1,5 @@ +class AddVerifiedToChannelEmail < ActiveRecord::Migration[7.1] + def change + add_column :channel_email, :verified_for_sending, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 00b6f9109..d5f0c244c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2025_09_16_024703) do +ActiveRecord::Schema[7.1].define(version: 2025_09_17_012759) do # These extensions should be enabled to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" @@ -422,6 +422,7 @@ ActiveRecord::Schema[7.1].define(version: 2025_09_16_024703) do t.boolean "smtp_enable_ssl_tls", default: false t.jsonb "provider_config", default: {} t.string "provider" + t.boolean "verified_for_sending", default: false, null: false t.index ["email"], name: "index_channel_email_on_email", unique: true t.index ["forward_to_email"], name: "index_channel_email_on_forward_to_email", unique: true end diff --git a/spec/builders/email/from_builder_spec.rb b/spec/builders/email/from_builder_spec.rb new file mode 100644 index 000000000..fe8ad65cb --- /dev/null +++ b/spec/builders/email/from_builder_spec.rb @@ -0,0 +1,169 @@ +require 'rails_helper' + +RSpec.describe Email::FromBuilder do + let(:account) { create(:account, support_email: 'support@example.com') } + let(:agent) { create(:user, account: account) } + let(:conversation) { create(:conversation, account: account) } + let(:current_message) { create(:message, conversation: conversation, sender: agent, message_type: :outgoing) } + + describe '#build' do + context 'when inbox is not an email channel' do + let(:channel) { create(:channel_api, account: account) } + let(:inbox) { create(:inbox, channel: channel, account: account) } + + it 'returns account support email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('support@example.com') + end + + context 'with friendly inbox' do + let(:inbox) { create(:inbox, channel: channel, account: account, sender_name_type: :friendly) } + + it 'returns friendly formatted sender name with support email' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include(agent.available_name) + expect(result).to include('support@example.com') + end + end + + context 'with professional inbox' do + let(:inbox) { create(:inbox, channel: channel, account: account, sender_name_type: :professional) } + + it 'returns professional formatted sender name with support email' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('support@example.com') + end + end + end + + context 'when inbox is an email channel' do + let(:channel) { create(:channel_email, email: 'care@example.com', account: account) } + let(:inbox) { create(:inbox, channel: channel, account: account) } + + context 'with standard IMAP/SMTP configuration' do + before do + channel.update!( + imap_enabled: true, + smtp_enabled: true, + imap_address: 'imap.example.com', + smtp_address: 'smtp.example.com' + ) + end + + it 'returns channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + + context 'with Google OAuth configuration' do + before do + channel.update!( + provider: 'google', + imap_enabled: true, + provider_config: { access_token: 'token', refresh_token: 'refresh' } + ) + end + + it 'returns channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + + context 'with Microsoft OAuth configuration' do + before do + channel.update!( + provider: 'microsoft', + imap_enabled: true, + provider_config: { access_token: 'token', refresh_token: 'refresh' } + ) + end + + it 'returns channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + + context 'with forwarding and own SMTP configuration' do + before do + channel.update!( + imap_enabled: false, + smtp_enabled: true, + smtp_address: 'smtp.example.com' + ) + end + + it 'returns channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + + context 'with IMAP enabled and Chatwoot SMTP and channel is verified_for_sending' do + before do + channel.update!(verified_for_sending: true, imap_enabled: true, smtp_enabled: false) + end + + it 'returns channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + + context 'with IMAP enabled and Chatwoot SMTP and channel is not verified_for_sending' do + before do + channel.update!(verified_for_sending: false, imap_enabled: true, smtp_enabled: false) + end + + it 'returns account support email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('support@example.com') + end + end + + context 'with forwarding and Chatwoot SMTP and channel is verified_for_sending' do + before do + channel.update!(verified_for_sending: true, imap_enabled: false, smtp_enabled: false) + end + + it 'returns channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + + context 'with forwarding and Chatwoot SMTP and channel is not verified_for_sending' do + before { channel.update!(verified_for_sending: false, imap_enabled: false, smtp_enabled: false) } + + it 'returns account support email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('support@example.com') + end + end + end + end +end diff --git a/spec/builders/email/reply_to_builder_spec.rb b/spec/builders/email/reply_to_builder_spec.rb new file mode 100644 index 000000000..8c54a5c48 --- /dev/null +++ b/spec/builders/email/reply_to_builder_spec.rb @@ -0,0 +1,108 @@ +require 'rails_helper' + +RSpec.describe Email::ReplyToBuilder do + let(:account) { create(:account, domain: 'mail.example.com', support_email: 'support@example.com') } + let(:agent) { create(:user, account: account) } + let(:conversation) { create(:conversation, account: account) } + let(:current_message) { create(:message, conversation: conversation, sender: agent, message_type: :outgoing) } + let(:inbox) { create(:inbox, account: account) } + + describe '#build' do + context 'when inbox is an email channel' do + let(:channel) { create(:channel_email, email: 'care@example.com', account: account) } + let(:inbox) { create(:inbox, channel: channel, account: account) } + + it 'returns the channel email with sender name formatting' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + + context 'with friendly inbox' do + let(:inbox) do + create(:inbox, channel: channel, account: account, greeting_enabled: true, greeting_message: 'Hello', sender_name_type: :friendly) + end + + it 'returns friendly formatted sender name' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include(agent.available_name) + expect(result).to include('care@example.com') + end + end + + context 'with professional inbox' do + let(:inbox) { create(:inbox, channel: channel, account: account, sender_name_type: :professional) } + + it 'returns professional formatted sender name' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('care@example.com') + end + end + end + + context 'when inbox is not an email channel' do + let(:channel) { create(:channel_api, account: account) } + let(:inbox) { create(:inbox, channel: channel, account: account) } + + context 'with inbound email enabled' do + before do + account.enable_features('inbound_emails') + account.update!(domain: 'mail.example.com', support_email: 'support@example.com') + end + + it 'returns reply email with conversation uuid' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include("reply+#{conversation.uuid}@mail.example.com") + end + end + + context 'when support_email has display name format and inbound emails are disabled' do + before do + account.disable_features('inbound_emails') + account.update!(support_email: 'Support ') + end + + it 'returns account support email with display name' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include("#{inbox.name} ") + end + end + + context 'when feature is disabled' do + before do + account.disable_features('inbound_emails') + end + + it 'returns account support email' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('support@example.com') + end + end + + context 'when inbound email domain is missing' do + before do + account.enable_features('inbound_emails') + account.update!(domain: nil) + end + + it 'returns account support email' do + builder = described_class.new(inbox: inbox, message: current_message) + result = builder.build + + expect(result).to include('support@example.com') + end + end + end + end +end