From 7ca192615d92c70ddf85423e143b003fae3df7bc Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Sun, 19 Jul 2020 15:35:55 +0530 Subject: [PATCH] Chore: Move some email configs to ENV variables (#1064) For the outgoing emails which has dependency on the incoming part as well like the conversation continuity part, some of the config variables used were entirely based on the account attributes. But this is not true in case of self hosted situations where you have multiple accounts and have a common config for incoming emails. So moved out some of the attributes entirely dependednt on the account to ENV with a fallback to the Global config. Also, with this changes the name of the agent will be shown in the email clinet with in the conversation rather than just the support email address. This has a huge UX impact on the cutomer. Modified all the necessary unit tests to reflect these changes. Updated the .env.example file for the new ENV variable. --- .env.example | 3 ++- app/mailers/conversation_reply_mailer.rb | 26 ++++++++++++------- config/installation_config.yml | 6 +++-- .../mailers/conversation_reply_mailer_spec.rb | 15 +++++++---- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/.env.example b/.env.example index 5adb4a848..110bd4292 100644 --- a/.env.example +++ b/.env.example @@ -40,7 +40,8 @@ SMTP_AUTHENTICATION= SMTP_ENABLE_STARTTLS_AUTO= # Mail Incoming - +# This is the domain set for the reply emails when conversation continuity is enabled +MAILER_INBOUND_EMAIL_DOMAIN= # Set this to appropriate ingress channel with regards to incoming emails # Possible values are : # :relay for Exim, Postfix, Qmail diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index a33c3ab72..2a8b2d29d 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -55,17 +55,17 @@ class ConversationReplyMailer < ApplicationMailer def reply_email if custom_domain_email_enabled? - "reply+#{@conversation.uuid}@#{@account.domain}" + "#{@agent.name} " else @agent&.email end end def from_email - if custom_domain_email_enabled? && @account.support_email.present? - @account.support_email + if custom_domain_email_enabled? + "#{@agent.name} <#{@account_support_email}>" else - ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com') + "#{@agent.name} <#{ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')}>" end end @@ -78,14 +78,22 @@ class ConversationReplyMailer < ApplicationMailer end def custom_domain_email_enabled? - @custom_domain_email_enabled ||= @account.domain_emails_enabled? && @account.domain.present? + @custom_domain_email_enabled ||= @account.domain_emails_enabled? && current_domain.present? && account_support_email.present? end def current_domain - if custom_domain_email_enabled? && @account.domain - @account.domain - else - GlobalConfig.get('FALLBACK_DOMAIN')['FALLBACK_DOMAIN'] + @current_domain ||= begin + @account.domain || + ENV.fetch('MAILER_INBOUND_EMAIL_DOMAIN', false) || + GlobalConfig.get('MAILER_INBOUND_EMAIL_DOMAIN')['MAILER_INBOUND_EMAIL_DOMAIN'] + end + end + + def account_support_email + @account_support_email ||= begin + @account.support_email || + GlobalConfig.get('MAILER_SUPPORT_EMAIL')['MAILER_SUPPORT_EMAIL'] || + ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com') end end end diff --git a/config/installation_config.yml b/config/installation_config.yml index 8a49567e5..4c21d6003 100644 --- a/config/installation_config.yml +++ b/config/installation_config.yml @@ -14,5 +14,7 @@ value: 'https://www.chatwoot.com/privacy-policy' - name: DISPLAY_MANIFEST value: true -- name: FALLBACK_DOMAIN - value: chatwoot.com +- name: MAILER_INBOUND_EMAIL_DOMAIN + value: +- name: MAILER_SUPPORT_EMAIL + value: diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index d82041eba..525839e0e 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -68,6 +68,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do let(:conversation) { create(:conversation, assignee: agent, inbox: inbox_member.inbox, account: account) } let!(:message) { create(:message, conversation: conversation, account: account) } let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now } + let(:domain) { account.domain || ENV.fetch('MAILER_INBOUND_EMAIL_DOMAIN', false) } it 'renders the receiver email' do expect(mail.to).to eq([message&.conversation&.contact&.email]) @@ -78,17 +79,18 @@ RSpec.describe ConversationReplyMailer, type: :mailer do end it 'sets the correct custom message id' do - expect(mail.message_id).to eq("") + expect(mail.message_id).to eq("conversation/#{conversation.uuid}/messages/#{message.id}@#{domain}") end it 'sets the correct in reply to id' do - expect(mail.in_reply_to).to eq("") + expect(mail.in_reply_to).to eq("account/#{conversation.account.id}/conversation/#{conversation.uuid}@#{domain}") end end context 'when the custom domain emails are enabled' do - let(:conversation) { create(:conversation, assignee: agent) } - let(:message) { create(:message, conversation: conversation) } + let(:account) { create(:account) } + let(:conversation) { create(:conversation, assignee: agent, account: account).reload } + let(:message) { create(:message, conversation: conversation, account: account, inbox: conversation.inbox) } let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now } before do @@ -96,15 +98,18 @@ RSpec.describe ConversationReplyMailer, type: :mailer do account.domain = 'example.com' account.support_email = 'support@example.com' account.domain_emails_enabled = true - account.save + account.save! end it 'sets reply to email to be based on the domain' do reply_to_email = "reply+#{message.conversation.uuid}@#{conversation.account.domain}" + reply_to = "#{agent.name} <#{reply_to_email}>" + expect(mail['REPLY-TO'].value).to eq(reply_to) expect(mail.reply_to).to eq([reply_to_email]) end it 'sets the from email to be the support email' do + expect(mail['FROM'].value).to eq("#{agent.name} <#{conversation.account.support_email}>") expect(mail.from).to eq([conversation.account.support_email]) end