From cda65ea3397c84b29b9e7ec1a85927be11964be3 Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Sun, 1 Mar 2020 19:06:13 +0530
Subject: [PATCH] Feature: Conversation creation email notifications (#576)

* Clean up the mailers

* Disable assignment mailer if setting is turned off

* Email notifications on conversation create

* Specs
---
 app/dispatchers/async_dispatcher.rb           |  2 +-
 app/listeners/email_notification_listener.rb  | 10 ++++
 .../conversation_notifications_mailer.rb      | 21 ++++++++
 app/mailers/application_mailer.rb             |  1 +
 app/mailers/assignment_mailer.rb              | 12 -----
 ...mailer.rb => conversation_reply_mailer.rb} |  4 +-
 app/models/conversation.rb                    |  7 +--
 app/models/message.rb                         |  2 +-
 .../conversation_assigned.html.erb            |  0
 .../conversation_created.html.erb             |  5 ++
 .../reply_with_summary.html.erb}              |  0
 ....rb => conversation_reply_email_worker.rb} |  4 +-
 .../email_notification_listener_spec.rb       | 52 +++++++++++++++++++
 .../conversation_notifications_mailer_spec.rb | 39 ++++++++++++++
 ...c.rb => conversation_reply_mailer_spec.rb} |  6 +--
 spec/models/conversation_spec.rb              | 16 +++++-
 ...> conversation_reply_email_worker_spec.rb} | 10 ++--
 17 files changed, 160 insertions(+), 31 deletions(-)
 create mode 100644 app/listeners/email_notification_listener.rb
 create mode 100644 app/mailers/agent_notifications/conversation_notifications_mailer.rb
 delete mode 100644 app/mailers/assignment_mailer.rb
 rename app/mailers/{conversation_mailer.rb => conversation_reply_mailer.rb} (88%)
 rename app/views/{assignment_mailer => mailers/agent_notifications/conversation_notifications_mailer}/conversation_assigned.html.erb (100%)
 create mode 100644 app/views/mailers/agent_notifications/conversation_notifications_mailer/conversation_created.html.erb
 rename app/views/{conversation_mailer/new_message.html.erb => mailers/conversation_reply_mailer/reply_with_summary.html.erb} (100%)
 rename app/workers/{conversation_email_worker.rb => conversation_reply_email_worker.rb} (76%)
 create mode 100644 spec/listeners/email_notification_listener_spec.rb
 create mode 100644 spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb
 rename spec/mailers/{conversation_mailer_spec.rb => conversation_reply_mailer_spec.rb} (81%)
 rename spec/workers/{conversation_email_worker_spec.rb => conversation_reply_email_worker_spec.rb} (74%)

diff --git a/app/dispatchers/async_dispatcher.rb b/app/dispatchers/async_dispatcher.rb
index 49c63654d..dff0c9d92 100644
--- a/app/dispatchers/async_dispatcher.rb
+++ b/app/dispatchers/async_dispatcher.rb
@@ -5,7 +5,7 @@ class AsyncDispatcher < BaseDispatcher
   end
 
   def listeners
-    listeners = [ReportingListener.instance, WebhookListener.instance]
+    listeners = [EmailNotificationListener.instance, ReportingListener.instance, WebhookListener.instance]
     listeners << SubscriptionListener.instance if ENV['BILLING_ENABLED']
     listeners
   end
diff --git a/app/listeners/email_notification_listener.rb b/app/listeners/email_notification_listener.rb
new file mode 100644
index 000000000..c9dfcfb09
--- /dev/null
+++ b/app/listeners/email_notification_listener.rb
@@ -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
diff --git a/app/mailers/agent_notifications/conversation_notifications_mailer.rb b/app/mailers/agent_notifications/conversation_notifications_mailer.rb
new file mode 100644
index 000000000..98650013c
--- /dev/null
+++ b/app/mailers/agent_notifications/conversation_notifications_mailer.rb
@@ -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
diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb
index 24f927f14..e83bbf437 100644
--- a/app/mailers/application_mailer.rb
+++ b/app/mailers/application_mailer.rb
@@ -1,6 +1,7 @@
 class ApplicationMailer < ActionMailer::Base
   default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
   layout 'mailer'
+  append_view_path Rails.root.join('app/views/mailers')
 
   # helpers
   helper :frontend_urls
diff --git a/app/mailers/assignment_mailer.rb b/app/mailers/assignment_mailer.rb
deleted file mode 100644
index 54920f2cf..000000000
--- a/app/mailers/assignment_mailer.rb
+++ /dev/null
@@ -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
diff --git a/app/mailers/conversation_mailer.rb b/app/mailers/conversation_reply_mailer.rb
similarity index 88%
rename from app/mailers/conversation_mailer.rb
rename to app/mailers/conversation_reply_mailer.rb
index 44f15f8b1..9fe65d231 100644
--- a/app/mailers/conversation_mailer.rb
+++ b/app/mailers/conversation_reply_mailer.rb
@@ -1,8 +1,8 @@
-class ConversationMailer < ApplicationMailer
+class ConversationReplyMailer < ApplicationMailer
   default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
   layout 'mailer'
 
-  def new_message(conversation, message_queued_time)
+  def reply_with_summary(conversation, message_queued_time)
     return unless smtp_config_set_or_development?
 
     @conversation = conversation
diff --git a/app/models/conversation.rb b/app/models/conversation.rb
index 6facd6ad6..51f53039a 100644
--- a/app/models/conversation.rb
+++ b/app/models/conversation.rb
@@ -112,8 +112,11 @@ class Conversation < ApplicationRecord
 
   def send_email_notification_to_assignee
     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
 
   def self_assign?(assignee_id)
@@ -156,8 +159,6 @@ class Conversation < ApplicationRecord
   end
 
   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 if assignee
 
diff --git a/app/models/message.rb b/app/models/message.rb
index 633e44d34..7d7b8acbc 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -133,7 +133,7 @@ class Message < ApplicationRecord
 
       # 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)
+      ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now)
     end
   end
 end
diff --git a/app/views/assignment_mailer/conversation_assigned.html.erb b/app/views/mailers/agent_notifications/conversation_notifications_mailer/conversation_assigned.html.erb
similarity index 100%
rename from app/views/assignment_mailer/conversation_assigned.html.erb
rename to app/views/mailers/agent_notifications/conversation_notifications_mailer/conversation_assigned.html.erb
diff --git a/app/views/mailers/agent_notifications/conversation_notifications_mailer/conversation_created.html.erb b/app/views/mailers/agent_notifications/conversation_notifications_mailer/conversation_created.html.erb
new file mode 100644
index 000000000..091cce51f
--- /dev/null
+++ b/app/views/mailers/agent_notifications/conversation_notifications_mailer/conversation_created.html.erb
@@ -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>
diff --git a/app/views/conversation_mailer/new_message.html.erb b/app/views/mailers/conversation_reply_mailer/reply_with_summary.html.erb
similarity index 100%
rename from app/views/conversation_mailer/new_message.html.erb
rename to app/views/mailers/conversation_reply_mailer/reply_with_summary.html.erb
diff --git a/app/workers/conversation_email_worker.rb b/app/workers/conversation_reply_email_worker.rb
similarity index 76%
rename from app/workers/conversation_email_worker.rb
rename to app/workers/conversation_reply_email_worker.rb
index 161c32521..8fed7cc12 100644
--- a/app/workers/conversation_email_worker.rb
+++ b/app/workers/conversation_reply_email_worker.rb
@@ -1,4 +1,4 @@
-class ConversationEmailWorker
+class ConversationReplyEmailWorker
   include Sidekiq::Worker
   sidekiq_options queue: :mailers
 
@@ -6,7 +6,7 @@ class ConversationEmailWorker
     @conversation = Conversation.find(conversation_id)
 
     # 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
     conversation_mail_key = Redis::Alfred::CONVERSATION_MAILER_KEY % @conversation.id
diff --git a/spec/listeners/email_notification_listener_spec.rb b/spec/listeners/email_notification_listener_spec.rb
new file mode 100644
index 000000000..22b5be07c
--- /dev/null
+++ b/spec/listeners/email_notification_listener_spec.rb
@@ -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
diff --git a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb
new file mode 100644
index 000000000..40883b1d6
--- /dev/null
+++ b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb
@@ -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
diff --git a/spec/mailers/conversation_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb
similarity index 81%
rename from spec/mailers/conversation_mailer_spec.rb
rename to spec/mailers/conversation_reply_mailer_spec.rb
index d0dd16b95..fb6935642 100644
--- a/spec/mailers/conversation_mailer_spec.rb
+++ b/spec/mailers/conversation_reply_mailer_spec.rb
@@ -2,12 +2,12 @@
 
 require 'rails_helper'
 
-RSpec.describe ConversationMailer, type: :mailer do
-  describe 'new_message' do
+RSpec.describe ConversationReplyMailer, type: :mailer do
+  describe 'reply_with_summary' 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(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now }
     let(:class_instance) { described_class.new }
 
     before do
diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb
index ae1d5fad7..baeb04748 100644
--- a/spec/models/conversation_spec.rb
+++ b/spec/models/conversation_spec.rb
@@ -34,7 +34,7 @@ RSpec.describe Conversation, type: :model do
       new_assignee
 
       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)
       Current.user = old_assignee
 
@@ -58,7 +58,7 @@ RSpec.describe Conversation, type: :model do
         .with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation)
 
       # 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?
     end
@@ -117,11 +117,23 @@ RSpec.describe Conversation, type: :model do
     let(:agent) do
       create(:user, email: 'agent@example.com', account: conversation.account, role: :agent)
     end
+    let(:assignment_mailer) { double(deliver: true) }
 
     it 'assigns the agent to conversation' do
       expect(update_assignee).to eq(true)
       expect(conversation.reload.assignee).to eq(agent)
     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
 
   describe '#toggle_status' do
diff --git a/spec/workers/conversation_email_worker_spec.rb b/spec/workers/conversation_reply_email_worker_spec.rb
similarity index 74%
rename from spec/workers/conversation_email_worker_spec.rb
rename to spec/workers/conversation_reply_email_worker_spec.rb
index aafd41ee8..0ebcb58d3 100644
--- a/spec/workers/conversation_email_worker_spec.rb
+++ b/spec/workers/conversation_reply_email_worker_spec.rb
@@ -1,17 +1,17 @@
 require 'rails_helper'
 
 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(:scheduled_job) { described_class.perform_at(perform_at, 1, Time.zone.now) }
   let(:conversation) { build(:conversation, display_id: nil) }
 
-  describe 'testing ConversationEmailWorker' do
+  describe 'testing ConversationSummaryEmailWorker' 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(ConversationReplyMailer).to receive(:reply_with_summary).and_return(mailer)
       allow(mailer).to receive(:deliver_later).and_return(true)
     end
 
@@ -28,9 +28,9 @@ RSpec.describe ConversationEmailWorker, type: :worker do
     end
 
     context 'with actions performed by the worker' do
-      it 'calls ConversationMailer' do
+      it 'calls ConversationSummaryMailer' do
         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