chore: Migrate from the worker to jobs
This commit is contained in:
@@ -13,7 +13,10 @@ class SendReplyJob < ApplicationJob
|
||||
'Channel::Telegram' => ::Telegram::SendOnTelegramService,
|
||||
'Channel::Whatsapp' => ::Whatsapp::SendOnWhatsappService,
|
||||
'Channel::Sms' => ::Sms::SendOnSmsService,
|
||||
'Channel::Instagram' => ::Instagram::SendOnInstagramService
|
||||
'Channel::Instagram' => ::Instagram::SendOnInstagramService,
|
||||
'Channel::Email' => ::Email::SendOnEmailService,
|
||||
'Channel::WebWidget' => ::Messages::SendEmailNotificationService,
|
||||
'Channel::Api' => ::Messages::SendEmailNotificationService
|
||||
}
|
||||
|
||||
case channel_name
|
||||
|
||||
@@ -288,7 +288,6 @@ class Message < ApplicationRecord
|
||||
def execute_after_create_commit_callbacks
|
||||
# rails issue with order of active record callbacks being executed https://github.com/rails/rails/issues/20911
|
||||
reopen_conversation
|
||||
notify_via_mail
|
||||
set_conversation_activity
|
||||
dispatch_create_events
|
||||
send_reply
|
||||
@@ -374,48 +373,6 @@ class Message < ApplicationRecord
|
||||
::MessageTemplates::HookExecutionService.new(message: self).perform
|
||||
end
|
||||
|
||||
def email_notifiable_webwidget?
|
||||
inbox.web_widget? && inbox.channel.continuity_via_email
|
||||
end
|
||||
|
||||
def email_notifiable_api_channel?
|
||||
inbox.api? && inbox.account.feature_enabled?('email_continuity_on_api_channel')
|
||||
end
|
||||
|
||||
def email_notifiable_channel?
|
||||
email_notifiable_webwidget? || %w[Email].include?(inbox.inbox_type) || email_notifiable_api_channel?
|
||||
end
|
||||
|
||||
def can_notify_via_mail?
|
||||
return unless email_notifiable_message?
|
||||
return unless email_notifiable_channel?
|
||||
return if conversation.contact.email.blank?
|
||||
|
||||
true
|
||||
end
|
||||
|
||||
def notify_via_mail
|
||||
return unless can_notify_via_mail?
|
||||
|
||||
trigger_notify_via_mail
|
||||
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, id)
|
||||
ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, id)
|
||||
end
|
||||
|
||||
def conversation_mail_key
|
||||
format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: conversation.id)
|
||||
end
|
||||
|
||||
def validate_attachments_limit(_attachment)
|
||||
errors.add(:attachments, message: 'exceeded maximum allowed') if attachments.size >= NUMBER_OF_PERMITTED_ATTACHMENTS
|
||||
end
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
class EmailReplyWorker
|
||||
include Sidekiq::Worker
|
||||
sidekiq_options queue: :mailers, retry: 3
|
||||
class Email::SendOnEmailService < Base::SendOnChannelService
|
||||
private
|
||||
|
||||
def perform(message_id)
|
||||
message = Message.find(message_id)
|
||||
def channel_class
|
||||
Channel::Email
|
||||
end
|
||||
|
||||
def perform_reply
|
||||
return unless message.email_notifiable_message?
|
||||
|
||||
# send the email
|
||||
ConversationReplyMailer.with(account: message.account).email_reply(message).deliver_now
|
||||
rescue StandardError => e
|
||||
ChatwootExceptionTracker.new(e, account: message.account).capture_exception
|
||||
@@ -0,0 +1,39 @@
|
||||
class Messages::SendEmailNotificationService
|
||||
pattr_initialize [:message!]
|
||||
|
||||
def perform
|
||||
return unless should_send_email_notification?
|
||||
|
||||
conversation = message.conversation
|
||||
conversation_mail_key = format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: conversation.id)
|
||||
|
||||
# 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, message.id)
|
||||
ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, message.id)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def should_send_email_notification?
|
||||
return false unless message.email_notifiable_message?
|
||||
return false if message.conversation.contact.email.blank?
|
||||
|
||||
email_reply_enabled?
|
||||
end
|
||||
|
||||
def email_reply_enabled?
|
||||
inbox = message.inbox
|
||||
case inbox.channel.class.to_s
|
||||
when 'Channel::WebWidget'
|
||||
inbox.channel.continuity_via_email
|
||||
when 'Channel::Api'
|
||||
inbox.account.feature_enabled?('email_continuity_on_api_channel')
|
||||
else
|
||||
false
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -108,5 +108,32 @@ RSpec.describe SendReplyJob do
|
||||
expect(process_service).to receive(:perform)
|
||||
described_class.perform_now(message.id)
|
||||
end
|
||||
|
||||
it 'calls ::Email::SendOnEmailService when its email message' do
|
||||
email_channel = create(:channel_email)
|
||||
message = create(:message, conversation: create(:conversation, inbox: email_channel.inbox))
|
||||
allow(Email::SendOnEmailService).to receive(:new).with(message: message).and_return(process_service)
|
||||
expect(Email::SendOnEmailService).to receive(:new).with(message: message)
|
||||
expect(process_service).to receive(:perform)
|
||||
described_class.perform_now(message.id)
|
||||
end
|
||||
|
||||
it 'calls ::Messages::SendEmailNotificationService when its webwidget message' do
|
||||
webwidget_channel = create(:channel_widget)
|
||||
message = create(:message, conversation: create(:conversation, inbox: webwidget_channel.inbox))
|
||||
allow(Messages::SendEmailNotificationService).to receive(:new).with(message: message).and_return(process_service)
|
||||
expect(Messages::SendEmailNotificationService).to receive(:new).with(message: message)
|
||||
expect(process_service).to receive(:perform)
|
||||
described_class.perform_now(message.id)
|
||||
end
|
||||
|
||||
it 'calls ::Messages::SendEmailNotificationService when its api channel message' do
|
||||
api_channel = create(:channel_api)
|
||||
message = create(:message, conversation: create(:conversation, inbox: api_channel.inbox))
|
||||
allow(Messages::SendEmailNotificationService).to receive(:new).with(message: message).and_return(process_service)
|
||||
expect(Messages::SendEmailNotificationService).to receive(:new).with(message: message)
|
||||
expect(process_service).to receive(:perform)
|
||||
described_class.perform_now(message.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -333,20 +333,11 @@ RSpec.describe Message 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)
|
||||
it 'calls SendReplyJob for all channels' do
|
||||
allow(SendReplyJob).to receive(:perform_later).and_return(true)
|
||||
message.message_type = 'outgoing'
|
||||
message.content_attributes = { email: { text_content: { quoted: 'quoted text' } } }
|
||||
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)
|
||||
message.save!
|
||||
expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
|
||||
expect(SendReplyJob).to have_received(:perform_later).with(message.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
require 'rails_helper'
|
||||
|
||||
describe Email::SendOnEmailService do
|
||||
let(:account) { create(:account) }
|
||||
let(:email_channel) { create(:channel_email, account: account) }
|
||||
let(:inbox) { create(:inbox, account: account, channel: email_channel) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
let(:message) { create(:message, conversation: conversation, message_type: 'outgoing') }
|
||||
let(:service) { described_class.new(message: message) }
|
||||
|
||||
describe '#perform' do
|
||||
context 'when message is email notifiable' do
|
||||
before do
|
||||
allow(ConversationReplyMailer).to receive_message_chain(:with, :email_reply, :deliver_now)
|
||||
end
|
||||
|
||||
it 'sends email via ConversationReplyMailer' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyMailer).to have_received(:with).with(account: message.account)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when message is not email notifiable' do
|
||||
let(:message) { create(:message, conversation: conversation, message_type: 'incoming') }
|
||||
|
||||
before do
|
||||
allow(ConversationReplyMailer).to receive_message_chain(:with, :email_reply, :deliver_now)
|
||||
end
|
||||
|
||||
it 'does not send email' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyMailer).not_to have_received(:with)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when an error occurs' do
|
||||
let(:error_message) { 'SMTP connection failed' }
|
||||
let(:error) { StandardError.new(error_message) }
|
||||
|
||||
before do
|
||||
allow(ConversationReplyMailer).to receive_message_chain(:with, :email_reply, :deliver_now).and_raise(error)
|
||||
allow(ChatwootExceptionTracker).to receive(:new).and_return(double(capture_exception: true))
|
||||
allow(Messages::StatusUpdateService).to receive(:new).and_return(double(perform: true))
|
||||
end
|
||||
|
||||
it 'captures the exception' do
|
||||
expect(ChatwootExceptionTracker).to receive(:new).with(error, account: message.account)
|
||||
|
||||
service.perform
|
||||
end
|
||||
|
||||
it 'updates message status to failed' do
|
||||
expect(Messages::StatusUpdateService).to receive(:new).with(message, 'failed', error_message)
|
||||
|
||||
service.perform
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#channel_class' do
|
||||
it 'returns Channel::Email' do
|
||||
expect(service.send(:channel_class)).to eq(Channel::Email)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'inheritance' do
|
||||
it 'inherits from Base::SendOnChannelService' do
|
||||
expect(described_class.superclass).to eq(Base::SendOnChannelService)
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,166 @@
|
||||
require 'rails_helper'
|
||||
|
||||
describe Messages::SendEmailNotificationService do
|
||||
let(:account) { create(:account) }
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
let(:message) { create(:message, conversation: conversation, message_type: 'outgoing') }
|
||||
let(:service) { described_class.new(message: message) }
|
||||
|
||||
describe '#perform' do
|
||||
context 'when email notification should be sent' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_widget, account: account, continuity_via_email: true)) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before do
|
||||
conversation.contact.update!(email: 'test@example.com')
|
||||
allow(Redis::Alfred).to receive(:get).and_return(nil)
|
||||
allow(Redis::Alfred).to receive(:setex)
|
||||
allow(ConversationReplyEmailWorker).to receive(:perform_in)
|
||||
end
|
||||
|
||||
it 'schedules ConversationReplyEmailWorker' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyEmailWorker).to have_received(:perform_in).with(
|
||||
2.minutes,
|
||||
conversation.id,
|
||||
message.id
|
||||
)
|
||||
end
|
||||
|
||||
it 'sets redis key to prevent duplicate emails' do
|
||||
expected_key = format(Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: conversation.id)
|
||||
|
||||
service.perform
|
||||
|
||||
expect(Redis::Alfred).to have_received(:setex).with(expected_key, message.id)
|
||||
end
|
||||
|
||||
context 'when redis key already exists' do
|
||||
before do
|
||||
allow(Redis::Alfred).to receive(:get).and_return('existing_key')
|
||||
end
|
||||
|
||||
it 'does not schedule worker' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
|
||||
end
|
||||
|
||||
it 'does not set redis key' do
|
||||
service.perform
|
||||
|
||||
expect(Redis::Alfred).not_to have_received(:setex)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when email notification should not be sent' do
|
||||
before do
|
||||
allow(ConversationReplyEmailWorker).to receive(:perform_in)
|
||||
end
|
||||
|
||||
context 'when message is not email notifiable' do
|
||||
let(:message) { create(:message, conversation: conversation, message_type: 'incoming') }
|
||||
|
||||
it 'does not schedule worker' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when contact has no email' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_widget, account: account, continuity_via_email: true)) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before do
|
||||
conversation.contact.update!(email: nil)
|
||||
end
|
||||
|
||||
it 'does not schedule worker' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when channel does not support email notifications' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_sms, account: account)) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before do
|
||||
conversation.contact.update!(email: 'test@example.com')
|
||||
end
|
||||
|
||||
it 'does not schedule worker' do
|
||||
service.perform
|
||||
|
||||
expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#should_send_email_notification?' do
|
||||
context 'with WebWidget channel' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_widget, account: account, continuity_via_email: true)) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before do
|
||||
conversation.contact.update!(email: 'test@example.com')
|
||||
end
|
||||
|
||||
it 'returns true when continuity_via_email is enabled' do
|
||||
expect(service.send(:should_send_email_notification?)).to be true
|
||||
end
|
||||
|
||||
context 'when continuity_via_email is disabled' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_widget, account: account, continuity_via_email: false)) }
|
||||
|
||||
it 'returns false' do
|
||||
expect(service.send(:should_send_email_notification?)).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with API channel' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_api, account: account)) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before do
|
||||
conversation.contact.update!(email: 'test@example.com')
|
||||
allow(account).to receive(:feature_enabled?).and_return(false)
|
||||
allow(account).to receive(:feature_enabled?).with('email_continuity_on_api_channel').and_return(true)
|
||||
end
|
||||
|
||||
it 'returns true when email_continuity_on_api_channel feature is enabled' do
|
||||
expect(service.send(:should_send_email_notification?)).to be true
|
||||
end
|
||||
|
||||
context 'when email_continuity_on_api_channel feature is disabled' do
|
||||
before do
|
||||
allow(account).to receive(:feature_enabled?).and_return(false)
|
||||
allow(account).to receive(:feature_enabled?).with('email_continuity_on_api_channel').and_return(false)
|
||||
end
|
||||
|
||||
it 'returns false' do
|
||||
expect(service.send(:should_send_email_notification?)).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with other channels' do
|
||||
let(:inbox) { create(:inbox, account: account, channel: create(:channel_email, account: account)) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
|
||||
before do
|
||||
conversation.contact.update!(email: 'test@example.com')
|
||||
end
|
||||
|
||||
it 'returns false' do
|
||||
expect(service.send(:should_send_email_notification?)).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -1,58 +0,0 @@
|
||||
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
|
||||
context 'when emails are successfully sent' 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_now).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_now)
|
||||
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_now)
|
||||
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_now)
|
||||
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_now)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when emails are not sent' 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_now).and_raise(ArgumentError)
|
||||
end
|
||||
|
||||
it 'mark message as failed' do
|
||||
expect { described_class.new.perform(message.id) }.to change { message.reload.status }.from('sent').to('failed')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user