diff --git a/.codeclimate.yml b/.codeclimate.yml
index ac38c27bb..50d5360fd 100644
--- a/.codeclimate.yml
+++ b/.codeclimate.yml
@@ -14,6 +14,10 @@ plugins:
checks:
similar-code:
enabled: false
+ method-count:
+ enabled: true
+ config:
+ threshold: 25
exclude_patterns:
- "spec/"
- "**/specs/"
diff --git a/app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue b/app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue
index 2de33e6af..661515adf 100644
--- a/app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue
+++ b/app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue
@@ -22,8 +22,8 @@
/>
diff --git a/app/javascript/dashboard/components/widgets/conversation/bubble/Actions.vue b/app/javascript/dashboard/components/widgets/conversation/bubble/Actions.vue
index 59a0a9896..597a5504d 100644
--- a/app/javascript/dashboard/components/widgets/conversation/bubble/Actions.vue
+++ b/app/javascript/dashboard/components/widgets/conversation/bubble/Actions.vue
@@ -1,6 +1,12 @@
{{ readableTime }}
+
+
+
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,
- .ion-android-open {
+ .icon {
color: var(--white);
}
}
@@ -201,4 +211,8 @@ export default {
}
}
}
+
+.delivered-icon {
+ margin-left: -var(--space-normal);
+}
diff --git a/app/javascript/dashboard/i18n/locale/en/chatlist.json b/app/javascript/dashboard/i18n/locale/en/chatlist.json
index 7262c4f9a..cda9f28c9 100644
--- a/app/javascript/dashboard/i18n/locale/en/chatlist.json
+++ b/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",
diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue
index b33ce4ed3..9d63e5781 100644
--- a/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue
+++ b/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() {
diff --git a/app/javascript/shared/mixins/inboxMixin.js b/app/javascript/shared/mixins/inboxMixin.js
index a2112ca80..db28c849a 100644
--- a/app/javascript/shared/mixins/inboxMixin.js
+++ b/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 this.facebookBadge;
- }
- if (this.isATwilioChannel) {
- return this.twilioBadge;
- }
- return this.channelType;
+ return badgeKey || this.channelType;
},
isAWhatsappChannel() {
return (
diff --git a/app/mailboxes/application_mailbox.rb b/app/mailboxes/application_mailbox.rb
index 7e4324460..29a408635 100644
--- a/app/mailboxes/application_mailbox.rb
+++ b/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?
diff --git a/app/mailboxes/reply_mailbox.rb b/app/mailboxes/reply_mailbox.rb
index 4ac0b093f..000ad5138 100644
--- a/app/mailboxes/reply_mailbox.rb
+++ b/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
#
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
diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb
index fd658a4b0..a36450ec7 100644
--- a/app/mailers/conversation_reply_mailer.rb
+++ b/app/mailers/conversation_reply_mailer.rb
@@ -2,14 +2,14 @@ class ConversationReplyMailer < ApplicationMailer
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot ')
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)
- new_messages = @conversation.messages.chat.where('created_at >= ?', message_queued_time)
+ recap_messages = @conversation.messages.chat.where('id < ?', last_queued_id).last(10)
+ 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,
- cc: cc_bcc_emails[0],
- bcc: cc_bcc_emails[1]
+ in_reply_to: in_reply_to_email
})
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} "
+ "#{assignee_name} from #{@inbox.name} "
else
@inbox.email_address || @agent&.email
end
@@ -129,7 +147,9 @@ class ConversationReplyMailer < ApplicationMailer
end
def custom_message_id
- ""
+ last_message = @message || @messages&.last
+
+ ""
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
diff --git a/app/models/contact_inbox.rb b/app/models/contact_inbox.rb
index fe268b605..6f36b8735 100644
--- a/app/models/contact_inbox.rb
+++ b/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?
diff --git a/app/models/message.rb b/app/models/message.rb
index 83705f4ea..7a6fe0bc0 100644
--- a/app/models/message.rb
+++ b/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)
- 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
+ Redis::Alfred.setex(conversation_mail_key, id)
+ ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, id)
end
def conversation_mail_key
diff --git a/app/views/mailers/conversation_reply_mailer/email_reply.html.erb b/app/views/mailers/conversation_reply_mailer/email_reply.html.erb
new file mode 100644
index 000000000..197c6ce60
--- /dev/null
+++ b/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 [click here to view]
+ <% end %>
+<% end %>
diff --git a/app/workers/conversation_reply_email_worker.rb b/app/workers/conversation_reply_email_worker.rb
index 1f97b43ee..0eddecaf7 100644
--- a/app/workers/conversation_reply_email_worker.rb
+++ b/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?
- ConversationReplyMailer.with(account: @conversation.account).reply_without_summary(@conversation, queued_time).deliver_later
+ if @conversation.messages.incoming&.last&.content_type == 'incoming_email'
+ 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
diff --git a/app/workers/email_reply_worker.rb b/app/workers/email_reply_worker.rb
new file mode 100644
index 000000000..1dc3c704b
--- /dev/null
+++ b/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
diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb
index 9fe50b837..c89509785 100644
--- a/spec/mailers/conversation_reply_mailer_spec.rb
+++ b/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(:cc_mail) { described_class.reply_with_summary(cc_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, 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
diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb
index 4e0933ec2..bc8d79ac5 100644
--- a/spec/models/message_spec.rb
+++ b/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)
diff --git a/spec/workers/conversation_reply_email_worker_spec.rb b/spec/workers/conversation_reply_email_worker_spec.rb
index 469431c61..936242a06 100644
--- a/spec/workers/conversation_reply_email_worker_spec.rb
+++ b/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
diff --git a/spec/workers/email_reply_worker_spec.rb b/spec/workers/email_reply_worker_spec.rb
new file mode 100644
index 000000000..dc2d83e2b
--- /dev/null
+++ b/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