From d758df8807925718dfd3a4a4dfc3d21d89e31baa Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Sun, 17 Jan 2021 22:43:32 +0530 Subject: [PATCH] fix: Add correct thread message_id to the email message (#1659) Co-authored-by: Sojan --- .rubocop.yml | 2 ++ app/mailboxes/support_mailbox.rb | 1 + app/mailers/conversation_reply_mailer.rb | 26 ++++++++++++++++--- app/models/channel/email.rb | 4 +-- app/models/message.rb | 21 ++++++++++----- .../conversation_reply_email_worker.rb | 6 ++++- 6 files changed, 47 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 65a5a1308..01951faea 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -11,6 +11,8 @@ Metrics/ClassLength: Max: 125 Exclude: - 'app/models/conversation.rb' + - 'app/mailers/conversation_reply_mailer.rb' + - 'app/models/message.rb' RSpec/ExampleLength: Max: 25 Style/Documentation: diff --git a/app/mailboxes/support_mailbox.rb b/app/mailboxes/support_mailbox.rb index ea9e16f1b..ac63d23ba 100644 --- a/app/mailboxes/support_mailbox.rb +++ b/app/mailboxes/support_mailbox.rb @@ -47,6 +47,7 @@ class SupportMailbox < ApplicationMailbox contact_inbox_id: @contact_inbox.id, additional_attributes: { source: 'email', + mail_subject: @processed_mail.subject, initiated_at: { timestamp: Time.now.utc } diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index 24ccfb7b6..ee3ef8239 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -66,6 +66,10 @@ class ConversationReplyMailer < ApplicationMailer @inbox = @conversation.inbox end + def should_use_conversation_email_address? + @inbox.inbox_type == 'Email' || inbound_email_enabled? + end + def conversation_already_viewed? # whether contact already saw the message on widget return unless @conversation.contact_last_seen_at @@ -83,12 +87,18 @@ class ConversationReplyMailer < ApplicationMailer end def mail_subject + return "Re: #{incoming_mail_subject}" if incoming_mail_subject + subject_line = I18n.t('conversations.reply.email_subject') "[##{@conversation.display_id}] #{subject_line}" end + def incoming_mail_subject + @incoming_mail_subject ||= @conversation.additional_attributes['mail_subject'] + end + def reply_email - if inbound_email_enabled? + if should_use_conversation_email_address? "#{assignee_name} " else @inbox.email_address || @agent&.email @@ -96,7 +106,7 @@ class ConversationReplyMailer < ApplicationMailer end def from_email_with_name - if inbound_email_enabled? + if should_use_conversation_email_address? "#{assignee_name} <#{account_support_email}>" else "#{assignee_name} <#{from_email_address}>" @@ -114,7 +124,17 @@ class ConversationReplyMailer < ApplicationMailer end def in_reply_to_email - "" + conversation_reply_email_id || "" + end + + def conversation_reply_email_id + content_attributes = @conversation.messages.incoming.last&.content_attributes + + if content_attributes && content_attributes['email'] && content_attributes['message_id'] + "<#{@conversation.messages.incoming.last.content_attributes['email']['message_id']}>" + end + + nil end def inbound_email_enabled? diff --git a/app/models/channel/email.rb b/app/models/channel/email.rb index 582a1f26e..33e11e5e8 100644 --- a/app/models/channel/email.rb +++ b/app/models/channel/email.rb @@ -37,7 +37,7 @@ class Channel::Email < ApplicationRecord private def ensure_forward_to_address - # TODO : implement better logic here - self.forward_to_address ||= "#{SecureRandom.hex}@xyc.com" + email_domain = InstallationConfig.find_by(name: 'MAILER_INBOUND_EMAIL_DOMAIN')&.value + self.forward_to_address ||= "#{SecureRandom.hex}@#{email_domain}" end end diff --git a/app/models/message.rb b/app/models/message.rb index af0ca42d8..064441e3b 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -128,8 +128,7 @@ class Message < ApplicationRecord private def execute_after_create_commit_callbacks - # rails issue with order of active record callbacks being executed - # https://github.com/rails/rails/issues/20911 + # rails issue with order of active record callbacks being executed https://github.com/rails/rails/issues/20911 set_conversation_activity dispatch_create_events send_reply @@ -169,8 +168,7 @@ class Message < ApplicationRecord def can_notify_via_mail? return unless email_notifiable_message? - return false if conversation.contact.email.blank? - return false unless %w[Website Email].include? inbox.inbox_type + return false if conversation.contact.email.blank? || !(%w[Website Email].include? inbox.inbox_type) true end @@ -178,10 +176,19 @@ class Message < ApplicationRecord def notify_via_mail return unless can_notify_via_mail? - # set a redis key for the conversation so that we don't need to send email for every new message + trigger_notify_via_mail + end + + def trigger_notify_via_mail + # will set a redis key for the conversation so that we don't need to send email for every new message # last few messages coupled together is sent every 2 minutes rather than one email for each message - if Redis::Alfred.get(conversation_mail_key).nil? - Redis::Alfred.setex(conversation_mail_key, Time.zone.now) + # if redis key exists there is an unprocessed job that will take care of delivering the email + return if Redis::Alfred.get(conversation_mail_key).present? + + Redis::Alfred.setex(conversation_mail_key, Time.zone.now) + if inbox.inbox_type == 'Email' + ConversationReplyEmailWorker.perform_in(2.seconds, conversation.id, Time.zone.now) + else ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now) end end diff --git a/app/workers/conversation_reply_email_worker.rb b/app/workers/conversation_reply_email_worker.rb index d0c1311fd..c352711b0 100644 --- a/app/workers/conversation_reply_email_worker.rb +++ b/app/workers/conversation_reply_email_worker.rb @@ -7,7 +7,7 @@ class ConversationReplyEmailWorker @conversation = Conversation.find(conversation_id) # send the email - if @conversation.messages.incoming&.last&.content_type == 'incoming_email' + if @conversation.messages.incoming&.last&.content_type == 'incoming_email' || email_inbox? ConversationReplyMailer.reply_without_summary(@conversation, queued_time).deliver_later else ConversationReplyMailer.reply_with_summary(@conversation, queued_time).deliver_later @@ -19,6 +19,10 @@ class ConversationReplyEmailWorker private + def email_inbox? + @conversation.inbox&.inbox_type == 'Email' + end + def conversation_mail_key format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: @conversation.id) end