Feature: Conversation creation email notifications (#576)

* Clean up the mailers

* Disable assignment mailer if setting is turned off

* Email notifications on conversation create

* Specs
This commit is contained in:
Sojan Jose 2020-03-01 19:06:13 +05:30 committed by GitHub
parent d6237dfc59
commit cda65ea339
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 160 additions and 31 deletions

View file

@ -5,7 +5,7 @@ class AsyncDispatcher < BaseDispatcher
end end
def listeners def listeners
listeners = [ReportingListener.instance, WebhookListener.instance] listeners = [EmailNotificationListener.instance, ReportingListener.instance, WebhookListener.instance]
listeners << SubscriptionListener.instance if ENV['BILLING_ENABLED'] listeners << SubscriptionListener.instance if ENV['BILLING_ENABLED']
listeners listeners
end end

View file

@ -0,0 +1,10 @@
class EmailNotificationListener < BaseListener
def conversation_created(event)
conversation, _account, _timestamp = extract_conversation_and_account(event)
conversation.inbox.members.each do |agent|
next unless agent.notification_settings.find_by(account_id: conversation.account_id).conversation_creation?
AgentNotifications::ConversationNotificationsMailer.conversation_created(conversation, agent).deliver_later
end
end
end

View file

@ -0,0 +1,21 @@
class AgentNotifications::ConversationNotificationsMailer < ApplicationMailer
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
layout 'mailer'
def conversation_created(conversation, agent)
return unless smtp_config_set_or_development?
@agent = agent
@conversation = conversation
subject = "#{@agent.name}, A new conversation [ID - #{@conversation.display_id}] has been created in #{@conversation.inbox&.name}."
mail(to: @agent.email, subject: subject)
end
def conversation_assigned(conversation, agent)
return unless smtp_config_set_or_development?
@agent = agent
@conversation = conversation
mail(to: @agent.email, subject: "#{@agent.name}, A new conversation [ID - #{@conversation.display_id}] has been assigned to you.")
end
end

View file

@ -1,6 +1,7 @@
class ApplicationMailer < ActionMailer::Base class ApplicationMailer < ActionMailer::Base
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com') default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
layout 'mailer' layout 'mailer'
append_view_path Rails.root.join('app/views/mailers')
# helpers # helpers
helper :frontend_urls helper :frontend_urls

View file

@ -1,12 +0,0 @@
class AssignmentMailer < ApplicationMailer
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
layout 'mailer'
def conversation_assigned(conversation, agent)
return unless smtp_config_set_or_development?
@agent = agent
@conversation = conversation
mail(to: @agent.email, subject: "#{@agent.name}, A new conversation [ID - #{@conversation.display_id}] has been assigned to you.")
end
end

View file

@ -1,8 +1,8 @@
class ConversationMailer < ApplicationMailer class ConversationReplyMailer < ApplicationMailer
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com') default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
layout 'mailer' layout 'mailer'
def new_message(conversation, message_queued_time) def reply_with_summary(conversation, message_queued_time)
return unless smtp_config_set_or_development? return unless smtp_config_set_or_development?
@conversation = conversation @conversation = conversation

View file

@ -112,8 +112,11 @@ class Conversation < ApplicationRecord
def send_email_notification_to_assignee def send_email_notification_to_assignee
return if self_assign?(assignee_id) return if self_assign?(assignee_id)
return unless saved_change_to_assignee_id?
return if assignee_id.blank?
return if assignee.notification_settings.find_by(account_id: account_id).not_conversation_assignment?
AssignmentMailer.conversation_assigned(self, assignee).deliver_later if saved_change_to_assignee_id? && assignee_id.present? AgentNotifications::ConversationNotificationsMailer.conversation_assigned(self, assignee).deliver_later
end end
def self_assign?(assignee_id) def self_assign?(assignee_id)
@ -156,8 +159,6 @@ class Conversation < ApplicationRecord
end end
def run_round_robin def run_round_robin
# return unless conversation.account.has_feature?(round_robin)
# return unless conversation.account.round_robin_enabled?
return unless inbox.enable_auto_assignment return unless inbox.enable_auto_assignment
return if assignee return if assignee

View file

@ -133,7 +133,7 @@ class Message < ApplicationRecord
# Since this is live chat, send the email after few minutes so the only one email with # Since this is live chat, send the email after few minutes so the only one email with
# last few messages coupled together is sent rather than email for each message # last few messages coupled together is sent rather than email for each message
ConversationEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now) ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now)
end end
end end
end end

View file

@ -0,0 +1,5 @@
<p>Hi <%= @agent.name %>,</p>
<p>Time to save the world. A new conversation has been created in <%= @conversation.inbox.name %></p>
<p>Click <%= link_to 'here', app_conversation_url(id: @conversation.display_id) %> to get cracking. </p>

View file

@ -1,4 +1,4 @@
class ConversationEmailWorker class ConversationReplyEmailWorker
include Sidekiq::Worker include Sidekiq::Worker
sidekiq_options queue: :mailers sidekiq_options queue: :mailers
@ -6,7 +6,7 @@ class ConversationEmailWorker
@conversation = Conversation.find(conversation_id) @conversation = Conversation.find(conversation_id)
# send the email # send the email
ConversationMailer.new_message(@conversation, queued_time).deliver_later ConversationReplyMailer.reply_with_summary(@conversation, queued_time).deliver_later
# delete the redis set from the first new message on the conversation # delete the redis set from the first new message on the conversation
conversation_mail_key = Redis::Alfred::CONVERSATION_MAILER_KEY % @conversation.id conversation_mail_key = Redis::Alfred::CONVERSATION_MAILER_KEY % @conversation.id

View file

@ -0,0 +1,52 @@
require 'rails_helper'
describe EmailNotificationListener do
let(:listener) { described_class.instance }
let!(:account) { create(:account) }
let!(:user) { create(:user, account: account) }
let!(:agent_with_notification) { create(:user, account: account) }
let!(:agent_with_out_notification) { create(:user, account: account) }
let!(:inbox) { create(:inbox, account: account) }
let!(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) }
describe 'conversation_created' do
let(:event_name) { :'conversation.created' }
before do
creation_mailer = double
allow(AgentNotifications::ConversationNotificationsMailer).to receive(:conversation_created).and_return(creation_mailer)
allow(creation_mailer).to receive(:deliver_later).and_return(true)
end
context 'when conversation is created' do
it 'sends email to inbox members who have notifications turned on' do
notification_setting = agent_with_notification.notification_settings.first
notification_setting.selected_email_flags = [:conversation_creation]
notification_setting.save!
create(:inbox_member, user: agent_with_notification, inbox: inbox)
conversation.reload
event = Events::Base.new(event_name, Time.zone.now, conversation: conversation)
listener.conversation_created(event)
expect(AgentNotifications::ConversationNotificationsMailer).to have_received(:conversation_created)
.with(conversation, agent_with_notification)
end
it 'does not send and email to inbox members who have notifications turned off' do
notification_setting = agent_with_notification.notification_settings.first
notification_setting.unselect_all_email_flags
notification_setting.save!
create(:inbox_member, user: agent_with_out_notification, inbox: inbox)
conversation.reload
event = Events::Base.new(event_name, Time.zone.now, conversation: conversation)
listener.conversation_created(event)
expect(AgentNotifications::ConversationNotificationsMailer).not_to have_received(:conversation_created)
.with(conversation, agent_with_out_notification)
end
end
end
end

View file

@ -0,0 +1,39 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe AgentNotifications::ConversationNotificationsMailer, type: :mailer do
let(:class_instance) { described_class.new }
let(:agent) { create(:user, email: 'agent1@example.com') }
let(:conversation) { create(:conversation, assignee: agent) }
before do
allow(described_class).to receive(:new).and_return(class_instance)
allow(class_instance).to receive(:smtp_config_set_or_development?).and_return(true)
end
describe 'conversation_created' do
let(:mail) { described_class.conversation_created(conversation, agent).deliver_now }
it 'renders the subject' do
expect(mail.subject).to eq("#{agent.name}, A new conversation [ID - #{conversation
.display_id}] has been created in #{conversation.inbox&.name}.")
end
it 'renders the receiver email' do
expect(mail.to).to eq([agent.email])
end
end
describe 'conversation_assigned' do
let(:mail) { described_class.conversation_assigned(conversation, agent).deliver_now }
it 'renders the subject' do
expect(mail.subject).to eq("#{agent.name}, A new conversation [ID - #{conversation.display_id}] has been assigned to you.")
end
it 'renders the receiver email' do
expect(mail.to).to eq([agent.email])
end
end
end

View file

@ -2,12 +2,12 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe ConversationMailer, type: :mailer do RSpec.describe ConversationReplyMailer, type: :mailer do
describe 'new_message' do describe 'reply_with_summary' do
let(:agent) { create(:user, email: 'agent1@example.com') } let(:agent) { create(:user, email: 'agent1@example.com') }
let(:conversation) { create(:conversation, assignee: agent) } let(:conversation) { create(:conversation, assignee: agent) }
let(:message) { create(:message, conversation: conversation) } let(:message) { create(:message, conversation: conversation) }
let(:mail) { described_class.new_message(message.conversation, Time.zone.now).deliver_now } let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now }
let(:class_instance) { described_class.new } let(:class_instance) { described_class.new }
before do before do

View file

@ -34,7 +34,7 @@ RSpec.describe Conversation, type: :model do
new_assignee new_assignee
allow(Rails.configuration.dispatcher).to receive(:dispatch) allow(Rails.configuration.dispatcher).to receive(:dispatch)
allow(AssignmentMailer).to receive(:conversation_assigned).and_return(assignment_mailer) allow(AgentNotifications::ConversationNotificationsMailer).to receive(:conversation_assigned).and_return(assignment_mailer)
allow(assignment_mailer).to receive(:deliver_later) allow(assignment_mailer).to receive(:deliver_later)
Current.user = old_assignee Current.user = old_assignee
@ -58,7 +58,7 @@ RSpec.describe Conversation, type: :model do
.with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation) .with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation)
# send_email_notification_to_assignee # send_email_notification_to_assignee
expect(AssignmentMailer).to have_received(:conversation_assigned).with(conversation, new_assignee) expect(AgentNotifications::ConversationNotificationsMailer).to have_received(:conversation_assigned).with(conversation, new_assignee)
expect(assignment_mailer).to have_received(:deliver_later) if ENV.fetch('SMTP_ADDRESS', nil).present? expect(assignment_mailer).to have_received(:deliver_later) if ENV.fetch('SMTP_ADDRESS', nil).present?
end end
@ -117,11 +117,23 @@ RSpec.describe Conversation, type: :model do
let(:agent) do let(:agent) do
create(:user, email: 'agent@example.com', account: conversation.account, role: :agent) create(:user, email: 'agent@example.com', account: conversation.account, role: :agent)
end end
let(:assignment_mailer) { double(deliver: true) }
it 'assigns the agent to conversation' do it 'assigns the agent to conversation' do
expect(update_assignee).to eq(true) expect(update_assignee).to eq(true)
expect(conversation.reload.assignee).to eq(agent) expect(conversation.reload.assignee).to eq(agent)
end end
it 'does not send assignment mailer if notification setting is turned off' do
allow(AgentNotifications::ConversationNotificationsMailer).to receive(:conversation_assigned).and_return(assignment_mailer)
notification_setting = agent.notification_settings.first
notification_setting.unselect_all_email_flags
notification_setting.save!
expect(update_assignee).to eq(true)
expect(AgentNotifications::ConversationNotificationsMailer).not_to have_received(:conversation_assigned).with(conversation, agent)
end
end end
describe '#toggle_status' do describe '#toggle_status' do

View file

@ -1,17 +1,17 @@
require 'rails_helper' require 'rails_helper'
Sidekiq::Testing.fake! Sidekiq::Testing.fake!
RSpec.describe ConversationEmailWorker, type: :worker do RSpec.describe ConversationReplyEmailWorker, type: :worker do
let(:perform_at) { (Time.zone.today + 6.hours).to_datetime } let(:perform_at) { (Time.zone.today + 6.hours).to_datetime }
let(:scheduled_job) { described_class.perform_at(perform_at, 1, Time.zone.now) } let(:scheduled_job) { described_class.perform_at(perform_at, 1, Time.zone.now) }
let(:conversation) { build(:conversation, display_id: nil) } let(:conversation) { build(:conversation, display_id: nil) }
describe 'testing ConversationEmailWorker' do describe 'testing ConversationSummaryEmailWorker' do
before do before do
conversation.save! conversation.save!
allow(Conversation).to receive(:find).and_return(conversation) allow(Conversation).to receive(:find).and_return(conversation)
mailer = double mailer = double
allow(ConversationMailer).to receive(:new_message).and_return(mailer) allow(ConversationReplyMailer).to receive(:reply_with_summary).and_return(mailer)
allow(mailer).to receive(:deliver_later).and_return(true) allow(mailer).to receive(:deliver_later).and_return(true)
end end
@ -28,9 +28,9 @@ RSpec.describe ConversationEmailWorker, type: :worker do
end end
context 'with actions performed by the worker' do context 'with actions performed by the worker' do
it 'calls ConversationMailer' do it 'calls ConversationSummaryMailer' do
described_class.new.perform(1, Time.zone.now) described_class.new.perform(1, Time.zone.now)
expect(ConversationMailer).to have_received(:new_message) expect(ConversationReplyMailer).to have_received(:reply_with_summary)
end end
end end
end end