From 6230f4d7965ce45c3eb7fd4e8532e0df3dcda27e Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Fri, 22 May 2020 18:14:18 +0530 Subject: [PATCH] Enhancement: Threaded email replies for a conversation (#885) (#886) * Added custom Message-ID and In-Reply-To headers for conversation reply emails * Added new global config for the default domain (This is used in the above headers) * Added migration to run the config loader to load the new global config value * The subject of the conversation reply mailer was made static (This is required for threaded emails) * Added required tests --- app/mailers/conversation_reply_mailer.rb | 38 +++++++++++++++---- config/installation_config.yml | 2 + config/locales/en.yml | 2 + db/migrate/20200522115645_reload_config.rb | 5 +++ db/schema.rb | 2 +- .../mailers/conversation_reply_mailer_spec.rb | 18 ++++++++- 6 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20200522115645_reload_config.rb diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index 0b0b75669..742e61c02 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -6,6 +6,7 @@ class ConversationReplyMailer < ApplicationMailer return unless smtp_config_set_or_development? @conversation = conversation + @account = @conversation.account @contact = @conversation.contact @agent = @conversation.assignee @@ -15,33 +16,56 @@ class ConversationReplyMailer < ApplicationMailer @messages = recap_messages + new_messages @messages = @messages.select(&:reportable?) - mail(to: @contact&.email, from: from_email, reply_to: reply_email, subject: mail_subject(@messages.last)) + mail({ + to: @contact&.email, + from: from_email, + reply_to: reply_email, + subject: mail_subject(@messages.last), + message_id: custom_message_id, + in_reply_to: in_reply_to_email + }) end private - def mail_subject(last_message, trim_length = 50) - subject_line = last_message&.content&.truncate(trim_length) || 'New messages on this conversation' + def mail_subject(_last_message, _trim_length = 50) + subject_line = I18n.t('conversations.reply.email_subject') "[##{@conversation.display_id}] #{subject_line}" end def reply_email if custom_domain_email_enabled? - "reply+to+#{@conversation.uuid}@#{@conversation.account.domain}" + "reply+to+#{@conversation.uuid}@#{@account.domain}" else @agent&.email end end def from_email - if custom_domain_email_enabled? && @conversation.account.support_email.present? - @conversation.account.support_email + if custom_domain_email_enabled? && @account.support_email.present? + @account.support_email else ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com') end end + def custom_message_id + "" + end + + def in_reply_to_email + "" + end + def custom_domain_email_enabled? - @custom_domain_email_enabled ||= @conversation.account.domain_emails_enabled? && @conversation.account.domain.present? + @custom_domain_email_enabled ||= @account.domain_emails_enabled? && @account.domain.present? + end + + def current_domain + if custom_domain_email_enabled? && @account.domain + @account.domain + else + GlobalConfig.get('FALLBACK_DOMAIN')['FALLBACK_DOMAIN'] + end end end diff --git a/config/installation_config.yml b/config/installation_config.yml index 13801a95b..8a49567e5 100644 --- a/config/installation_config.yml +++ b/config/installation_config.yml @@ -14,3 +14,5 @@ value: 'https://www.chatwoot.com/privacy-policy' - name: DISPLAY_MANIFEST value: true +- name: FALLBACK_DOMAIN + value: chatwoot.com diff --git a/config/locales/en.yml b/config/locales/en.yml index b11a27bda..3b9afe3e4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -54,3 +54,5 @@ en: typical_reply_message_body: "%{account_name} typically replies in a few hours." ways_to_reach_you_message_body: "Give the team a way to reach you." email_input_box_message_body: "Get notified by email" + reply: + email_subject: "New messages on this conversation" diff --git a/db/migrate/20200522115645_reload_config.rb b/db/migrate/20200522115645_reload_config.rb new file mode 100644 index 000000000..530ec9218 --- /dev/null +++ b/db/migrate/20200522115645_reload_config.rb @@ -0,0 +1,5 @@ +class ReloadConfig < ActiveRecord::Migration[6.0] + def change + ConfigLoader.new.process + end +end diff --git a/db/schema.rb b/db/schema.rb index 6965c2d03..cbc4c5d87 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.define(version: 2020_05_10_112339) do +ActiveRecord::Schema.define(version: 2020_05_22_115645) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index cb49dbe22..8868e59c3 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -19,7 +19,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now } it 'renders the subject' do - expect(mail.subject).to eq("[##{message.conversation.display_id}] #{message.content.truncate(30)}") + expect(mail.subject).to eq("[##{message.conversation.display_id}] New messages on this conversation") end it 'not have private notes' do @@ -44,6 +44,14 @@ RSpec.describe ConversationReplyMailer, type: :mailer do it 'renders the reply to email' do expect(mail.reply_to).to eq([message&.conversation&.assignee&.email]) end + + it 'sets the correct custom message id' do + expect(mail.message_id).to eq("") + end + + it 'sets the correct in reply to id' do + expect(mail.in_reply_to).to eq("") + end end context 'when the cutsom domain emails are enabled' do @@ -67,6 +75,14 @@ RSpec.describe ConversationReplyMailer, type: :mailer do it 'sets the from email to be the support email' do expect(mail.from).to eq([conversation.account.support_email]) end + + it 'sets the correct custom message id' do + expect(mail.message_id).to eq("conversation/#{conversation.uuid}/messages/#{message.id}@#{conversation.account.domain}") + end + + it 'sets the correct in reply to id' do + expect(mail.in_reply_to).to eq("account/#{conversation.account.id}/conversation/#{conversation.uuid}@#{conversation.account.domain}") + end end end end