feat: Display sent status of emails in email channel (#3125)

.codeclimate.yml

@@ -14,6 +14,10 @@ plugins:
 checks:
   similar-code:
     enabled: false
+  method-count:
+    enabled: true
+    config:
+      threshold: 25
 exclude_patterns:
   - "spec/"
   - "**/specs/"

app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue

@@ -22,8 +22,8 @@
       />
       <reply-email-head
         v-if="showReplyHead"
-        @set-emails="setCcEmails"
         :clear-mails="clearMails"
+        @set-emails="setCcEmails"
       />
       <resizable-text-area
         v-if="!showRichContentEditor"
@@ -278,8 +278,8 @@ export default {
       }
       return !this.message.trim().replace(/\n/g, '').length;
     },
-    showReplyHead(){
+    showReplyHead() {
-      return this.isAnEmailChannel;
+      return !this.isOnPrivateNote && this.isAnEmailChannel;
     },
   },
   watch: {
@@ -465,11 +465,11 @@ export default {
         messagePayload.file = attachment.resource.file;
       }
 
-      if(this.ccEmails) {
+      if (this.ccEmails) {
         messagePayload.ccEmails = this.ccEmails;
       }
 
-      if(this.bccEmails) {
+      if (this.bccEmails) {
         messagePayload.bccEmails = this.bccEmails;
       }
 
@@ -480,8 +480,8 @@ export default {
     },
     setCcEmails(value) {
       this.bccEmails = value.bccEmails;
-      this.ccEmails = value.ccEmails
+      this.ccEmails = value.ccEmails;
-    }
+    },
   },
 };
 </script>

app/javascript/dashboard/components/widgets/conversation/bubble/Actions.vue

@@ -1,6 +1,12 @@
 <template>
   <div class="message-text--metadata">
     <span class="time">{{ readableTime }}</span>
+    <span v-if="showSentIndicator" class="time">
+      <i
+        v-tooltip.top-start="$t('CHAT_LIST.SENT')"
+        class="icon ion-checkmark"
+      />
+    </span>
     <i
       v-if="isEmail"
       v-tooltip.top-start="$t('CHAT_LIST.RECEIVED_VIA_EMAIL')"
@@ -36,8 +42,10 @@
 <script>
 import { MESSAGE_TYPE } from 'shared/constants/messages';
 import { BUS_EVENTS } from 'shared/constants/busEvents';
+import inboxMixin from 'shared/mixins/inboxMixin';
 
 export default {
+  mixins: [inboxMixin],
   props: {
     sender: {
       type: Object,
@@ -99,6 +107,9 @@ export default {
       return `https://twitter.com/${screenName ||
         this.inbox.name}/status/${sourceId}`;
     },
+    showSentIndicator() {
+      return this.isOutgoing && this.sourceId && this.isAnEmailChannel;
+    },
   },
   methods: {
     onTweetReply() {
@@ -128,8 +139,7 @@ export default {
 }
 
 .right {
-  .ion-reply,
+  .icon {
-  .ion-android-open {
     color: var(--white);
   }
 }
@@ -201,4 +211,8 @@ export default {
     }
   }
 }
+
+.delivered-icon {
+  margin-left: -var(--space-normal);
+}
 </style>

app/javascript/dashboard/i18n/locale/en/chatlist.json

@@ -85,6 +85,7 @@
     "RECEIVED_VIA_EMAIL": "Received via email",
     "VIEW_TWEET_IN_TWITTER": "View tweet in Twitter",
     "REPLY_TO_TWEET": "Reply to this tweet",
+    "SENT": "Sent successfully",
     "NO_MESSAGES": "No Messages",
     "NO_CONTENT": "No content available",
     "HIDE_QUOTED_TEXT": "Hide Quoted Text",

app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue

@@ -486,6 +486,9 @@ export default {
       if (this.isATwilioSMSChannel || this.isATwilioWhatsappChannel) {
         return `${this.inbox.name} (${this.inbox.phone_number})`;
       }
+      if (this.isAnEmailChannel) {
+        return `${this.inbox.name} (${this.inbox.email})`;
+      }
       return this.inbox.name;
     },
     messengerScript() {

app/javascript/shared/mixins/inboxMixin.js

@@ -64,16 +64,17 @@ export default {
       return this.chatAdditionalAttributes.type || 'facebook';
     },
     inboxBadge() {
+      const badgeKey = '';
       if (this.isATwitterInbox) {
-        return this.twitterBadge;
+        badgeKey = this.twitterBadge;
+      } else if (this.isAFacebookInbox) {
+        badgeKey = this.facebookBadge;
+      } else if (this.isATwilioChannel) {
+        badgeKey = this.twilioBadge;
+      } else if (this.isAWhatsappChannel) {
+        badgeKey = 'whatsapp';
       }
-      if (this.isAFacebookInbox) {
+      return badgeKey || this.channelType;
-        return this.facebookBadge;
-      }
-      if (this.isATwilioChannel) {
-        return this.twilioBadge;
-      }
-      return this.channelType;
     },
     isAWhatsappChannel() {
       return (

app/mailboxes/application_mailbox.rb

@@ -40,7 +40,7 @@ class ApplicationMailbox < ActionMailbox::Base
   def self.in_reply_to_mail?(inbound_mail_obj, is_a_reply_email)
     return if is_a_reply_email
 
-    in_reply_to = inbound_mail_obj.mail['In-Reply-To'].value
+    in_reply_to = inbound_mail_obj.mail.in_reply_to
 
     return false if in_reply_to.blank?
 

app/mailboxes/reply_mailbox.rb

@@ -20,7 +20,7 @@ class ReplyMailbox < ApplicationMailbox
   def find_relative_conversation
     if @conversation_uuid
       find_conversation_with_uuid
-    elsif mail['In-Reply-To'].try(:value).present?
+    elsif mail.in_reply_to.present?
       find_conversation_with_in_reply_to
     end
   end
@@ -63,7 +63,7 @@ class ReplyMailbox < ApplicationMailbox
   # find conversation uuid from below pattern
   # <conversation/#{@conversation.uuid}/messages/#{@messages&.last&.id}@#{@account.inbound_email_domain}>
   def find_conversation_with_in_reply_to
-    in_reply_to = mail['In-Reply-To'].try(:value)
+    in_reply_to = mail.in_reply_to
     match_result = in_reply_to.match(ApplicationMailbox::CONVERSATION_MESSAGE_ID_PATTERN) if in_reply_to.present?
 
     if match_result

app/mailers/conversation_reply_mailer.rb

@@ -2,14 +2,14 @@ class ConversationReplyMailer < ApplicationMailer
   default from: ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')
   layout :choose_layout
 
-  def reply_with_summary(conversation, message_queued_time)
+  def reply_with_summary(conversation, last_queued_id)
     return unless smtp_config_set_or_development?
 
     init_conversation_attributes(conversation)
     return if conversation_already_viewed?
 
-    recap_messages = @conversation.messages.chat.where('created_at < ?', message_queued_time).last(10)
+    recap_messages = @conversation.messages.chat.where('id < ?', last_queued_id).last(10)
-    new_messages = @conversation.messages.chat.where('created_at >= ?', message_queued_time)
+    new_messages = @conversation.messages.chat.where('id >= ?', last_queued_id)
     @messages = recap_messages + new_messages
     @messages = @messages.select(&:email_reply_summarizable?)
 
@@ -25,13 +25,13 @@ class ConversationReplyMailer < ApplicationMailer
          })
   end
 
-  def reply_without_summary(conversation, message_queued_time)
+  def reply_without_summary(conversation, last_queued_id)
     return unless smtp_config_set_or_development?
 
     init_conversation_attributes(conversation)
     return if conversation_already_viewed?
 
-    @messages = @conversation.messages.chat.where(message_type: [:outgoing, :template]).where('created_at >= ?', message_queued_time)
+    @messages = @conversation.messages.chat.where(message_type: [:outgoing, :template]).where('id >= ?', last_queued_id)
     @messages = @messages.reject { |m| m.template? && !m.input_csat? }
     return false if @messages.count.zero?
 
@@ -41,12 +41,30 @@ class ConversationReplyMailer < ApplicationMailer
            reply_to: reply_email,
            subject: mail_subject,
            message_id: custom_message_id,
-           in_reply_to: in_reply_to_email,
+           in_reply_to: in_reply_to_email
-           cc: cc_bcc_emails[0],
-           bcc: cc_bcc_emails[1]
          })
   end
 
+  def email_reply(message)
+    return unless smtp_config_set_or_development?
+
+    init_conversation_attributes(message.conversation)
+    @message = message
+
+    reply_mail_object = mail({
+                               to: @contact&.email,
+                               from: from_email_with_name,
+                               reply_to: reply_email,
+                               subject: mail_subject,
+                               message_id: custom_message_id,
+                               in_reply_to: in_reply_to_email,
+                               cc: cc_bcc_emails[0],
+                               bcc: cc_bcc_emails[1]
+                             })
+
+    message.update(source_id: reply_mail_object.message_id)
+  end
+
   def conversation_transcript(conversation, to_email)
     return unless smtp_config_set_or_development?
 
@@ -104,7 +122,7 @@ class ConversationReplyMailer < ApplicationMailer
 
   def reply_email
     if should_use_conversation_email_address?
-      "#{assignee_name} <reply+#{@conversation.uuid}@#{@account.inbound_email_domain}>"
+      "#{assignee_name} from #{@inbox.name} <reply+#{@conversation.uuid}@#{@account.inbound_email_domain}>"
     else
       @inbox.email_address || @agent&.email
     end
@@ -129,7 +147,9 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def custom_message_id
-    "<conversation/#{@conversation.uuid}/messages/#{@messages&.last&.id}@#{@account.inbound_email_domain}>"
+    last_message = @message || @messages&.last
+
+    "<conversation/#{@conversation.uuid}/messages/#{last_message&.id}@#{@account.inbound_email_domain}>"
   end
 
   def in_reply_to_email
@@ -161,7 +181,7 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def choose_layout
-    return false if action_name == 'reply_without_summary'
+    return false if action_name == 'reply_without_summary' || action_name == 'email_reply'
 
     'mailer/base'
   end

app/models/contact_inbox.rb

@@ -61,7 +61,7 @@ class ContactInbox < ApplicationRecord
   end
 
   def validate_email_source_id
-    errors.add(:source_id, "invalid source id for Email inbox. valid Regex #{Device.email_regexp}") unless Devise.email_regexp.match?(source_id)
+    errors.add(:source_id, "invalid source id for Email inbox. valid Regex #{Devise.email_regexp}") unless Devise.email_regexp.match?(source_id)
   end
 
   def valid_source_id_format?

app/models/message.rb

@@ -205,17 +205,15 @@ class Message < ApplicationRecord
   end
 
   def trigger_notify_via_mail
+    return EmailReplyWorker.perform_in(1.second, id) if inbox.inbox_type == 'Email'
+
     # 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 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)
+    Redis::Alfred.setex(conversation_mail_key, id)
-    if inbox.inbox_type == 'Email'
+    ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, id)
-      ConversationReplyEmailWorker.perform_in(2.seconds, conversation.id, Time.zone.now)
-    else
-      ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now)
-    end
   end
 
   def conversation_mail_key

app/views/mailers/conversation_reply_mailer/email_reply.html.erb

@@ -0,0 +1,8 @@
+<% if @message.content %>
+  <%= CommonMarker.render_html(@message.content).html_safe %>
+<% end %>
+<% if @message.attachments %>
+  <% @message.attachments.each do |attachment| %>
+    attachment [<a href="<%= attachment.file_url %>" _target="blank">click here to view</a>]
+  <% end %>
+<% end %>

app/workers/conversation_reply_email_worker.rb

@@ -3,14 +3,14 @@ class ConversationReplyEmailWorker
   include Sidekiq::Worker
   sidekiq_options queue: :mailers
 
-  def perform(conversation_id, queued_time)
+  def perform(conversation_id, last_queued_id)
     @conversation = Conversation.find(conversation_id)
 
     # send the email
-    if @conversation.messages.incoming&.last&.content_type == 'incoming_email' || email_inbox?
+    if @conversation.messages.incoming&.last&.content_type == 'incoming_email'
-      ConversationReplyMailer.with(account: @conversation.account).reply_without_summary(@conversation, queued_time).deliver_later
+      ConversationReplyMailer.with(account: @conversation.account).reply_without_summary(@conversation, last_queued_id).deliver_later
     else
-      ConversationReplyMailer.with(account: @conversation.account).reply_with_summary(@conversation, queued_time).deliver_later
+      ConversationReplyMailer.with(account: @conversation.account).reply_with_summary(@conversation, last_queued_id).deliver_later
     end
 
     # delete the redis set from the first new message on the conversation

app/workers/email_reply_worker.rb

@@ -0,0 +1,14 @@
+class EmailReplyWorker
+  include Sidekiq::Worker
+  sidekiq_options queue: :mailers
+
+  def perform(message_id)
+    message = Message.find(message_id)
+
+    return unless message.outgoing? || message.input_csat?
+    return if message.private?
+
+    # send the email
+    ConversationReplyMailer.with(account: message.account).email_reply(message).deliver_later
+  end
+end

spec/mailers/conversation_reply_mailer_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
     let!(:account) { create(:account) }
     let!(:agent) { create(:user, email: 'agent1@example.com', account: account) }
     let(:class_instance) { described_class.new }
+    let(:email_channel) { create(:channel_email, account: account) }
 
     before do
       allow(described_class).to receive(:new).and_return(class_instance)
@@ -35,8 +36,8 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
                })
       end
       let(:private_message) { create(:message, account: account, content: 'This is a private message', conversation: conversation) }
-      let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now }
+      let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
-      let(:cc_mail) { described_class.reply_with_summary(cc_message.conversation, Time.zone.now).deliver_now }
+      let(:cc_mail) { described_class.reply_with_summary(cc_message.conversation, message.id).deliver_now }
 
       it 'renders the subject' do
         expect(mail.subject).to eq("[##{message.conversation.display_id}] New messages on this conversation")
@@ -66,7 +67,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
     context 'without assignee' do
       let(:conversation) { create(:conversation, assignee: nil) }
       let(:message) { create(:message, conversation: conversation) }
-      let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now }
+      let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
 
       it 'has correct name' do
         expect(mail[:from].display_names).to eq(['Notifications from Inbox'])
@@ -84,7 +85,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
                account: account,
                message_type: 'outgoing').reload
       end
-      let(:mail) { described_class.reply_without_summary(message_1.conversation, Time.zone.now - 1.minute).deliver_now }
+      let(:mail) { described_class.reply_without_summary(message_2.conversation, message_2.id).deliver_now }
 
       before do
         message_2.save
@@ -113,12 +114,30 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
       end
     end
 
+    context 'with email reply' do
+      let(:conversation) { create(:conversation, assignee: agent, inbox: email_channel.inbox, account: account).reload }
+      let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') }
+      let(:mail) { described_class.email_reply(message).deliver_now }
+
+      it 'renders the subject' do
+        expect(mail.subject).to eq("[##{message.conversation.display_id}] New messages on this conversation")
+      end
+
+      it 'renders the body' do
+        expect(mail.decoded).to include message.content
+      end
+
+      it 'updates the source_id' do
+        expect(mail.message_id).to eq message.source_id
+      end
+    end
+
     context 'when custom domain and email is not enabled' do
       let(:inbox) { create(:inbox, account: account) }
       let(:inbox_member) { create(:inbox_member, user: agent, inbox: inbox) }
       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(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
       let(:domain) { account.inbound_email_domain }
 
       it 'renders the receiver email' do
@@ -142,7 +161,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
       let(:inbox) { create(:inbox, account: account, email_address: 'noreply@chatwoot.com') }
       let(:conversation) { create(:conversation, assignee: agent, inbox: 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(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
 
       it 'set reply to email address as inbox email address' do
         expect(mail.from).to eq([inbox.email_address])
@@ -154,7 +173,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
       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 }
+      let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
 
       before do
         account = conversation.account
@@ -166,7 +185,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
 
       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.available_name} <#{reply_to_email}>"
+        reply_to = "#{agent.available_name} from #{conversation.inbox.name} <#{reply_to_email}>"
         expect(mail['REPLY-TO'].value).to eq(reply_to)
         expect(mail.reply_to).to eq([reply_to_email])
       end

spec/models/message_spec.rb

@@ -70,6 +70,14 @@ RSpec.describe Message, type: :model do
       expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
     end
 
+    it 'calls EmailReply worker if the channel is email' do
+      message.inbox = create(:inbox, account: message.account, channel: build(:channel_email, account: message.account))
+      allow(EmailReplyWorker).to receive(:perform_in).and_return(true)
+      message.message_type = 'outgoing'
+      message.save!
+      expect(EmailReplyWorker).to have_received(:perform_in).with(1.second, message.id)
+    end
+
     it 'wont call notify email method unless its website or email channel' do
       message.inbox = create(:inbox, account: message.account, channel: build(:channel_api, account: message.account))
       allow(ConversationReplyEmailWorker).to receive(:perform_in).and_return(true)

spec/workers/conversation_reply_email_worker_spec.rb

@@ -2,8 +2,6 @@ require 'rails_helper'
 
 Sidekiq::Testing.fake!
 RSpec.describe ConversationReplyEmailWorker, type: :worker do
-  let(:perform_at) { (Time.zone.today + 6.hours).to_datetime }
-  let(:scheduled_job) { described_class.perform_at(perform_at, 1, Time.zone.now) }
   let(:conversation) { build(:conversation, display_id: nil) }
   let(:message) { build(:message, conversation: conversation, content_type: 'incoming_email', inbox: conversation.inbox) }
   let(:mailer) { double }
@@ -29,18 +27,18 @@ RSpec.describe ConversationReplyEmailWorker, type: :worker do
       expect do
         described_class.perform_async
       end.to change(described_class.jobs, :size).by(1)
-      described_class.new.perform(1, Time.zone.now)
+      described_class.new.perform(1, message.id)
     end
 
     context 'with actions performed by the worker' do
       it 'calls ConversationSummaryMailer#reply_with_summary when last incoming message was not email' do
-        described_class.new.perform(1, Time.zone.now)
+        described_class.new.perform(1, message.id)
         expect(mailer).to have_received(:reply_with_summary)
       end
 
       it 'calls ConversationSummaryMailer#reply_without_summary when last incoming message was from email' do
         message.save
-        described_class.new.perform(1, Time.zone.now)
+        described_class.new.perform(1, message.id)
         expect(mailer).to have_received(:reply_without_summary)
       end
     end

spec/workers/email_reply_worker_spec.rb

@@ -0,0 +1,44 @@
+require 'rails_helper'
+
+RSpec.describe EmailReplyWorker, type: :worker do
+  let(:account) { create(:account) }
+  let(:channel) { create(:channel_email, account: account) }
+  let(:message) { create(:message, message_type: :outgoing, inbox: channel.inbox, account: account) }
+  let(:private_message) { create(:message, private: true, message_type: :outgoing, inbox: channel.inbox, account: account) }
+  let(:incoming_message) { create(:message, message_type: :incoming, inbox: channel.inbox, account: account) }
+  let(:template_message) { create(:message, message_type: :template, content_type: :input_csat, inbox: channel.inbox, account: account) }
+  let(:mailer) { double }
+  let(:mailer_action) { double }
+
+  describe '#perform' do
+    before do
+      allow(ConversationReplyMailer).to receive(:with).and_return(mailer)
+      allow(mailer).to receive(:email_reply).and_return(mailer_action)
+      allow(mailer_action).to receive(:deliver_later).and_return(true)
+    end
+
+    it 'calls mailer action with message' do
+      described_class.new.perform(message.id)
+      expect(mailer).to have_received(:email_reply).with(message)
+      expect(mailer_action).to have_received(:deliver_later)
+    end
+
+    it 'does not call mailer action with a private message' do
+      described_class.new.perform(private_message.id)
+      expect(mailer).not_to have_received(:email_reply)
+      expect(mailer_action).not_to have_received(:deliver_later)
+    end
+
+    it 'calls mailer action with a CSAT message' do
+      described_class.new.perform(template_message.id)
+      expect(mailer).to have_received(:email_reply).with(template_message)
+      expect(mailer_action).to have_received(:deliver_later)
+    end
+
+    it 'does not call mailer action with an incoming message' do
+      described_class.new.perform(incoming_message.id)
+      expect(mailer).not_to have_received(:email_reply)
+      expect(mailer_action).not_to have_received(:deliver_later)
+    end
+  end
+end