* [#139] Delayed emails for conversations * Added the setex and get methods to Redis wrapper * Set the priorities for the sidekiq queues * Was not able to use mailhog for testing email in local, switched back to letter opener and added comments on using the SMTP settings * Added after create hood in messages to queue the sending of mail after 2 minutes using sidekiq worker and also set the redis key for the conversation to avoid the email sending for every message * Added the sidekiq worker to send the email and delete the conversation redis key * Added the mailer and mail template * mailer sends the last 10 messages along with the new messages from the time it was queued * Send email only in development or if smtp config is set * Send email only in development or if smtp config is set * Set the SMTP_PORT in production variable * Adding redis to circle CI * Specs for the conversation email changes * Added specs for conversation email sidekiq worker * Added specs for conversation mailer * Added specs in message model for the after create hook for notify email * Send emails only when there is a reply from agent * set development to use mailhog * Adding comments for using letter opener
This commit is contained in:
parent
1d3ed016be
commit
d4b3ba4baa
@ -13,6 +13,7 @@ defaults: &defaults
|
|||||||
# CircleCI maintains a library of pre-built images
|
# CircleCI maintains a library of pre-built images
|
||||||
# documented at https://circleci.com/docs/2.0/circleci-images/
|
# documented at https://circleci.com/docs/2.0/circleci-images/
|
||||||
- image: circleci/postgres:9.4
|
- image: circleci/postgres:9.4
|
||||||
|
- image: circleci/redis:5.0.7-alpine
|
||||||
environment:
|
environment:
|
||||||
- CC_TEST_REPORTER_ID: b1b5c4447bf93f6f0b06a64756e35afd0810ea83649f03971cbf303b4449456f
|
- CC_TEST_REPORTER_ID: b1b5c4447bf93f6f0b06a64756e35afd0810ea83649f03971cbf303b4449456f
|
||||||
|
|
||||||
|
|||||||
@ -17,6 +17,8 @@ FB_APP_ID=
|
|||||||
MAILER_SENDER_EMAIL=accounts@chatwoot.com
|
MAILER_SENDER_EMAIL=accounts@chatwoot.com
|
||||||
SMTP_PORT=1025
|
SMTP_PORT=1025
|
||||||
SMTP_DOMAIN=chatwoot.com
|
SMTP_DOMAIN=chatwoot.com
|
||||||
|
# if you are running docker-compose, set SMTP_ADDRESS value as "mailhog",
|
||||||
|
# else set the value as "localhost"
|
||||||
SMTP_ADDRESS=mailhog
|
SMTP_ADDRESS=mailhog
|
||||||
SMTP_USERNAME=
|
SMTP_USERNAME=
|
||||||
SMTP_PASSWORD=
|
SMTP_PASSWORD=
|
||||||
|
|||||||
@ -16,6 +16,10 @@ Style/FrozenStringLiteralComment:
|
|||||||
Enabled: false
|
Enabled: false
|
||||||
Style/SymbolArray:
|
Style/SymbolArray:
|
||||||
Enabled: false
|
Enabled: false
|
||||||
|
Style/GlobalVars:
|
||||||
|
Exclude:
|
||||||
|
- 'config/initializers/redis.rb'
|
||||||
|
- 'lib/redis/alfred.rb'
|
||||||
Metrics/BlockLength:
|
Metrics/BlockLength:
|
||||||
Exclude:
|
Exclude:
|
||||||
- spec/**/*
|
- spec/**/*
|
||||||
|
|||||||
@ -4,4 +4,8 @@ class ApplicationMailer < ActionMailer::Base
|
|||||||
|
|
||||||
# helpers
|
# helpers
|
||||||
helper :frontend_urls
|
helper :frontend_urls
|
||||||
|
|
||||||
|
def smtp_config_set_or_development?
|
||||||
|
ENV.fetch('SMTP_ADDRESS', nil).present? || Rails.env.development?
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@ -3,7 +3,7 @@ class AssignmentMailer < ApplicationMailer
|
|||||||
layout 'mailer'
|
layout 'mailer'
|
||||||
|
|
||||||
def conversation_assigned(conversation, agent)
|
def conversation_assigned(conversation, agent)
|
||||||
return if ENV.fetch('SMTP_ADDRESS', nil).blank?
|
return unless smtp_config_set_or_development?
|
||||||
|
|
||||||
@agent = agent
|
@agent = agent
|
||||||
@conversation = conversation
|
@conversation = conversation
|
||||||
|
|||||||
26
app/mailers/conversation_mailer.rb
Normal file
26
app/mailers/conversation_mailer.rb
Normal file
@ -0,0 +1,26 @@
|
|||||||
|
class ConversationMailer < ApplicationMailer
|
||||||
|
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
|
||||||
|
layout 'mailer'
|
||||||
|
|
||||||
|
def new_message(conversation, message_queued_time)
|
||||||
|
return unless smtp_config_set_or_development?
|
||||||
|
|
||||||
|
@conversation = conversation
|
||||||
|
@contact = @conversation.contact
|
||||||
|
@agent = @conversation.assignee
|
||||||
|
|
||||||
|
recap_messages = @conversation.messages.where('created_at < ?', message_queued_time).order(created_at: :asc).last(10)
|
||||||
|
new_messages = @conversation.messages.where('created_at >= ?', message_queued_time)
|
||||||
|
|
||||||
|
@messages = recap_messages + new_messages
|
||||||
|
@messages = @messages.select(&:reportable?)
|
||||||
|
|
||||||
|
mail(to: @contact&.email, from: @agent&.email, subject: mail_subject(@messages.last))
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def mail_subject(last_message, trim_length = 30)
|
||||||
|
"[##{@conversation.display_id}] #{last_message.content.truncate(trim_length)}"
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -49,7 +49,8 @@ class Message < ApplicationRecord
|
|||||||
after_create :reopen_conversation,
|
after_create :reopen_conversation,
|
||||||
:dispatch_event,
|
:dispatch_event,
|
||||||
:send_reply,
|
:send_reply,
|
||||||
:execute_message_template_hooks
|
:execute_message_template_hooks,
|
||||||
|
:notify_via_mail
|
||||||
|
|
||||||
def channel_token
|
def channel_token
|
||||||
@token ||= inbox.channel.try(:page_access_token)
|
@token ||= inbox.channel.try(:page_access_token)
|
||||||
@ -94,4 +95,17 @@ class Message < ApplicationRecord
|
|||||||
def execute_message_template_hooks
|
def execute_message_template_hooks
|
||||||
::MessageTemplates::HookExecutionService.new(message: self).perform
|
::MessageTemplates::HookExecutionService.new(message: self).perform
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def notify_via_mail
|
||||||
|
conversation_mail_key = Redis::Alfred::CONVERSATION_MAILER_KEY % conversation.id
|
||||||
|
if Redis::Alfred.get(conversation_mail_key).nil? && conversation.contact.email? && outgoing?
|
||||||
|
# set a redis key for the conversation so that we don't need to send email for every
|
||||||
|
# new message that comes in and we dont enque the delayed sidekiq job for every message
|
||||||
|
Redis::Alfred.setex(conversation_mail_key, Time.zone.now)
|
||||||
|
|
||||||
|
# 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
|
||||||
|
ConversationEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
16
app/views/conversation_mailer/new_message.html.erb
Normal file
16
app/views/conversation_mailer/new_message.html.erb
Normal file
@ -0,0 +1,16 @@
|
|||||||
|
<p>Hi <%= @contact.name %>,</p>
|
||||||
|
|
||||||
|
<p>You have new messages on your conversation.</p>
|
||||||
|
|
||||||
|
<table>
|
||||||
|
<% @messages.each do |message| %>
|
||||||
|
<tr>
|
||||||
|
<td>
|
||||||
|
<b><%= message.incoming? ? 'You' : message.user.name %></b>
|
||||||
|
</td>
|
||||||
|
<td>: <%= message.content %></td>
|
||||||
|
</tr>
|
||||||
|
<% end %>
|
||||||
|
</table>
|
||||||
|
|
||||||
|
<p>Click <%= link_to 'here', app_conversation_url(id: @conversation.display_id) %> to get back to the conversation. </p>
|
||||||
15
app/workers/conversation_email_worker.rb
Normal file
15
app/workers/conversation_email_worker.rb
Normal file
@ -0,0 +1,15 @@
|
|||||||
|
class ConversationEmailWorker
|
||||||
|
include Sidekiq::Worker
|
||||||
|
sidekiq_options queue: :mailers
|
||||||
|
|
||||||
|
def perform(conversation_id, queued_time)
|
||||||
|
@conversation = Conversation.find(conversation_id)
|
||||||
|
|
||||||
|
# send the email
|
||||||
|
ConversationMailer.new_message(@conversation, queued_time).deliver_later
|
||||||
|
|
||||||
|
# delete the redis set from the first new message on the conversation
|
||||||
|
conversation_mail_key = Redis::Alfred::CONVERSATION_MAILER_KEY % @conversation.id
|
||||||
|
Redis::Alfred.delete(conversation_mail_key)
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -42,13 +42,17 @@ Rails.application.configure do
|
|||||||
|
|
||||||
config.action_mailer.perform_deliveries = true
|
config.action_mailer.perform_deliveries = true
|
||||||
config.action_mailer.raise_delivery_errors = true
|
config.action_mailer.raise_delivery_errors = true
|
||||||
config.action_mailer.delivery_method = :smtp
|
|
||||||
config.action_mailer.default_url_options = { host: 'localhost:3000' }
|
config.action_mailer.default_url_options = { host: 'localhost:3000' }
|
||||||
|
|
||||||
|
# If you want to use letter opener instead of mailhog for testing emails locally,
|
||||||
|
# uncomment the following line L49 and comment lines L51 through to L65
|
||||||
|
# config.action_mailer.delivery_method = :letter_opener
|
||||||
|
|
||||||
|
config.action_mailer.delivery_method = :smtp
|
||||||
smtp_settings = {
|
smtp_settings = {
|
||||||
port: ENV['SMTP_PORT'],
|
port: ENV['SMTP_PORT'] || 25,
|
||||||
domain: ENV['SMTP_DOMAIN'],
|
domain: ENV['SMTP_DOMAIN'] || 'localhost',
|
||||||
address: ENV['SMTP_ADDRESS']
|
address: ENV['SMTP_ADDRESS'] || 'chatwoot.com'
|
||||||
}
|
}
|
||||||
|
|
||||||
if ENV['SMTP_AUTHENTICATION'].present?
|
if ENV['SMTP_AUTHENTICATION'].present?
|
||||||
|
|||||||
@ -59,7 +59,7 @@ Rails.application.configure do
|
|||||||
config.action_mailer.default_url_options = { host: ENV['FRONTEND_URL'] }
|
config.action_mailer.default_url_options = { host: ENV['FRONTEND_URL'] }
|
||||||
config.action_mailer.smtp_settings = {
|
config.action_mailer.smtp_settings = {
|
||||||
address: ENV['SMTP_ADDRESS'],
|
address: ENV['SMTP_ADDRESS'],
|
||||||
port: 587,
|
port: ENV['SMTP_PORT'] || 587,
|
||||||
user_name: ENV['SMTP_USERNAME'],
|
user_name: ENV['SMTP_USERNAME'],
|
||||||
password: ENV['SMTP_PASSWORD'],
|
password: ENV['SMTP_PASSWORD'],
|
||||||
authentication: :login,
|
authentication: :login,
|
||||||
|
|||||||
@ -2,5 +2,6 @@ uri = URI.parse(ENV.fetch('REDIS_URL', 'redis://127.0.0.1:6379'))
|
|||||||
redis = Rails.env.test? ? MockRedis.new : Redis.new(url: uri)
|
redis = Rails.env.test? ? MockRedis.new : Redis.new(url: uri)
|
||||||
Nightfury.redis = Redis::Namespace.new('reports', redis: redis)
|
Nightfury.redis = Redis::Namespace.new('reports', redis: redis)
|
||||||
|
|
||||||
# Alfred - Used currently for Round Robin. Add here as you use it for more features
|
# Alfred - Used currently for round robin and conversation emails.
|
||||||
|
# Add here as you use it for more features
|
||||||
$alfred = Redis::Namespace.new('alfred', redis: redis, warning: true)
|
$alfred = Redis::Namespace.new('alfred', redis: redis, warning: true)
|
||||||
|
|||||||
@ -11,10 +11,10 @@
|
|||||||
# even put in dynamic logic, like a host-specific queue.
|
# even put in dynamic logic, like a host-specific queue.
|
||||||
# http://www.mikeperham.com/2013/11/13/advanced-sidekiq-host-specific-queues/
|
# http://www.mikeperham.com/2013/11/13/advanced-sidekiq-host-specific-queues/
|
||||||
:queues:
|
:queues:
|
||||||
- critical
|
- [critical, 5]
|
||||||
- default
|
- [default, 2]
|
||||||
- low
|
- [low, 1]
|
||||||
- mailers
|
- [mailers, 2]
|
||||||
|
|
||||||
# you can override concurrency based on environment
|
# you can override concurrency based on environment
|
||||||
production:
|
production:
|
||||||
|
|||||||
@ -1,4 +1,6 @@
|
|||||||
module Redis::Alfred
|
module Redis::Alfred
|
||||||
|
CONVERSATION_MAILER_KEY = 'CONVERSATION::%d'.freeze
|
||||||
|
|
||||||
class << self
|
class << self
|
||||||
def rpoplpush(source, destination)
|
def rpoplpush(source, destination)
|
||||||
$alfred.rpoplpush(source, destination)
|
$alfred.rpoplpush(source, destination)
|
||||||
@ -15,5 +17,13 @@ module Redis::Alfred
|
|||||||
def lrem(key, value, count = 0)
|
def lrem(key, value, count = 0)
|
||||||
$alfred.lrem(key, count, value)
|
$alfred.lrem(key, count, value)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def setex(key, value, expiry = 1.day)
|
||||||
|
$alfred.setex(key, expiry, value)
|
||||||
|
end
|
||||||
|
|
||||||
|
def get(key)
|
||||||
|
$alfred.get(key)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
30
spec/mailers/conversation_mailer_spec.rb
Normal file
30
spec/mailers/conversation_mailer_spec.rb
Normal file
@ -0,0 +1,30 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
RSpec.describe ConversationMailer, type: :mailer do
|
||||||
|
describe 'new_message' do
|
||||||
|
let(:agent) { create(:user, email: 'agent1@example.com') }
|
||||||
|
let(:conversation) { create(:conversation, assignee: agent) }
|
||||||
|
let(:message) { create(:message, conversation: conversation) }
|
||||||
|
let(:mail) { described_class.new_message(message.conversation, Time.zone.now).deliver_now }
|
||||||
|
let(:class_instance) { described_class.new }
|
||||||
|
|
||||||
|
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
|
||||||
|
|
||||||
|
it 'renders the subject' do
|
||||||
|
expect(mail.subject).to eq("[##{message.conversation.display_id}] #{message.content.truncate(30)}")
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders the receiver email' do
|
||||||
|
expect(mail.to).to eq([message&.conversation&.contact&.email])
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders the sender email' do
|
||||||
|
expect(mail.from).to eq([message&.conversation&.assignee&.email])
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -22,5 +22,11 @@ RSpec.describe Message, type: :model do
|
|||||||
expect(::MessageTemplates::HookExecutionService).to have_received(:new).with(message: message)
|
expect(::MessageTemplates::HookExecutionService).to have_received(:new).with(message: message)
|
||||||
expect(hook_execution_service).to have_received(:perform)
|
expect(hook_execution_service).to have_received(:perform)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'calls notify email method on after save' do
|
||||||
|
allow(message).to receive(:notify_via_mail).and_return(true)
|
||||||
|
message.save!
|
||||||
|
expect(message).to have_received(:notify_via_mail)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@ -5,6 +5,7 @@ require File.expand_path('../config/environment', __dir__)
|
|||||||
abort('The Rails environment is running in production mode!') if Rails.env.production?
|
abort('The Rails environment is running in production mode!') if Rails.env.production?
|
||||||
require 'rspec/rails'
|
require 'rspec/rails'
|
||||||
require 'pundit/rspec'
|
require 'pundit/rspec'
|
||||||
|
require 'sidekiq/testing'
|
||||||
|
|
||||||
# Add additional requires below this line. Rails is not loaded until this point!
|
# Add additional requires below this line. Rails is not loaded until this point!
|
||||||
|
|
||||||
|
|||||||
37
spec/workers/conversation_email_worker_spec.rb
Normal file
37
spec/workers/conversation_email_worker_spec.rb
Normal file
@ -0,0 +1,37 @@
|
|||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
Sidekiq::Testing.fake!
|
||||||
|
RSpec.describe ConversationEmailWorker, 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) }
|
||||||
|
|
||||||
|
describe 'testing ConversationEmailWorker' do
|
||||||
|
before do
|
||||||
|
conversation.save!
|
||||||
|
allow(Conversation).to receive(:find).and_return(conversation)
|
||||||
|
mailer = double
|
||||||
|
allow(ConversationMailer).to receive(:new_message).and_return(mailer)
|
||||||
|
allow(mailer).to receive(:deliver_later).and_return(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'worker jobs are enqueued in the mailers queue' do
|
||||||
|
described_class.perform_async
|
||||||
|
assert_equal :mailers, described_class.queue
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'goes into the jobs array for testing environment' do
|
||||||
|
expect do
|
||||||
|
described_class.perform_async
|
||||||
|
end.to change(described_class.jobs, :size).by(1)
|
||||||
|
described_class.new.perform(1, Time.zone.now)
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with actions performed by the worker' do
|
||||||
|
it 'calls ConversationMailer' do
|
||||||
|
described_class.new.perform(1, Time.zone.now)
|
||||||
|
expect(ConversationMailer).to have_received(:new_message)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
Reference in New Issue
Block a user