From 96efc44b8271313bcdf80f928e2f4dffb017f1ec Mon Sep 17 00:00:00 2001
From: Sony Mathew <ynos1234@gmail.com>
Date: Sun, 19 Jul 2020 23:08:07 +0530
Subject: [PATCH] Chore: Feature lock email settings in UI (#1065)

* Chore: Feature lock email settings in UI

The email settings under account settings needed to be
feature locked in a way different from teh current way for it
to be enabled for accounts in a self hosted scenario.

Some refactorings were also done along with this change.

1. There was a feature flag defined in code in account model called
domain_emails_enabled was used to check if the inbound emails was
enabled for the account. But there was already a feature flag called
"inbound_emails" defined in features.yml. So changed to use this to
check if inbound emails are enabled for an account.
2. Renamed and re-purposed existing `domain_emails_enabled` to
`custom_email_domain_enabled` to use for feature toggling the UI
for email settings.
3. To enable & disable multiple features using the featurable concern
we were passing an array of values. Changed this to accept a comma
separated set of values.

* Chore: Feature lock email settings in UI

Fixed the specs for accounts controller & removed
unneccessary code from Account seetings component in UI

* Chore: Convert newlines to <br>s

Removed the layout used while sending replies in
conversation continuity.

Converted the newlines in the messages to <br/> tags
for the correct HTML rendering.

* Chore: Bug fix in reply email domain

Renamed the function custom_email_domain_enabled  to
inbound_email_enabled.

Fixed bug on setting reply emails's domain.
---
 app/controllers/api/v1/accounts_controller.rb |  4 +-
 .../i18n/locale/en/generalSettings.json       | 15 +++----
 .../dashboard/settings/account/Index.vue      | 44 +++++++------------
 app/mailers/conversation_reply_mailer.rb      | 20 ++++++---
 app/models/account.rb                         |  2 +-
 app/models/concerns/featurable.rb             |  4 +-
 app/views/api/v1/accounts/show.json.jbuilder  |  2 +-
 .../api/v1/accounts/update.json.jbuilder      |  2 +-
 .../reply_without_summary.html.erb            |  2 +-
 ...709145000_remove_multiple_feature_flags.rb |  4 +-
 .../api/v1/accounts_controller_spec.rb        |  4 +-
 spec/factories/accounts.rb                    |  2 +-
 .../mailers/conversation_reply_mailer_spec.rb |  2 +-
 13 files changed, 47 insertions(+), 60 deletions(-)

diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index 3fb0509e3..cf60de614 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -32,7 +32,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def update
-    @account.update!(account_params.slice(:name, :locale, :domain, :support_email, :domain_emails_enabled))
+    @account.update!(account_params.slice(:name, :locale, :domain, :support_email))
   end
 
   def update_active_at
@@ -57,7 +57,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def account_params
-    params.permit(:account_name, :email, :name, :locale, :domain, :support_email, :domain_emails_enabled)
+    params.permit(:account_name, :email, :name, :locale, :domain, :support_email)
   end
 
   def check_signup_enabled
diff --git a/app/javascript/dashboard/i18n/locale/en/generalSettings.json b/app/javascript/dashboard/i18n/locale/en/generalSettings.json
index 31b5f9fe2..0a3fc0b8e 100644
--- a/app/javascript/dashboard/i18n/locale/en/generalSettings.json
+++ b/app/javascript/dashboard/i18n/locale/en/generalSettings.json
@@ -24,8 +24,8 @@
         "ERROR": ""
       },
       "DOMAIN": {
-        "LABEL": "Domain",
-        "PLACEHOLDER": "Your website domain",
+        "LABEL": "Incoming Email Domain",
+        "PLACEHOLDER": "The domain where you will receive the emails",
         "ERROR": ""
       },
       "SUPPORT_EMAIL": {
@@ -33,14 +33,9 @@
         "PLACEHOLDER": "Your company's support email",
         "ERROR": ""
       },
-      "ENABLE_DOMAIN_EMAIL": {
-        "LABEL": "Enable domain email",
-        "PLACEHOLDER": "Enable the custom domain email",
-        "ERROR": "",
-        "OPTIONS": {
-          "ENABLED": "Enabled",
-          "DISABLED": "Disabled"
-        }
+      "FEATURES": {
+        "INBOUND_EMAIL_ENABLED": "Conversation continuity with emails is enabled for your account.",
+        "CUSTOM_EMAIL_DOMAIN_ENABLED": "You can receive emails in your custom domain now."
       }
     }
   }
diff --git a/app/javascript/dashboard/routes/dashboard/settings/account/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/account/Index.vue
index d8d86a052..8ade5fd8e 100644
--- a/app/javascript/dashboard/routes/dashboard/settings/account/Index.vue
+++ b/app/javascript/dashboard/routes/dashboard/settings/account/Index.vue
@@ -36,7 +36,15 @@
               {{ $t('GENERAL_SETTINGS.FORM.LANGUAGE.ERROR') }}
             </span>
           </label>
-          <label>
+          <label v-if="featureInboundEmailEnabled">
+            {{ $t('GENERAL_SETTINGS.FORM.FEATURES.INBOUND_EMAIL_ENABLED') }}
+          </label>
+          <label v-if="featureCustomDomainEmailEnabled">
+            {{
+              $t('GENERAL_SETTINGS.FORM.FEATURES.CUSTOM_EMAIL_DOMAIN_ENABLED')
+            }}
+          </label>
+          <label v-if="featureCustomDomainEmailEnabled">
             {{ $t('GENERAL_SETTINGS.FORM.DOMAIN.LABEL') }}
             <input
               v-model="domain"
@@ -44,29 +52,7 @@
               :placeholder="$t('GENERAL_SETTINGS.FORM.DOMAIN.PLACEHOLDER')"
             />
           </label>
-          <label v-if="featureInboundEmailEnabled">
-            {{ $t('GENERAL_SETTINGS.FORM.ENABLE_DOMAIN_EMAIL.LABEL') }}
-            <select v-model="domainEmailsEnabled">
-              <option value="true">
-                {{
-                  $t(
-                    'GENERAL_SETTINGS.FORM.ENABLE_DOMAIN_EMAIL.OPTIONS.ENABLED'
-                  )
-                }}
-              </option>
-              <option value="false">
-                {{
-                  $t(
-                    'GENERAL_SETTINGS.FORM.ENABLE_DOMAIN_EMAIL.OPTIONS.DISABLED'
-                  )
-                }}
-              </option>
-            </select>
-            <p class="help-text">
-              {{ $t('GENERAL_SETTINGS.FORM.ENABLE_DOMAIN_EMAIL.PLACEHOLDER') }}
-            </p>
-          </label>
-          <label v-if="featureInboundEmailEnabled">
+          <label v-if="featureCustomDomainEmailEnabled">
             {{ $t('GENERAL_SETTINGS.FORM.SUPPORT_EMAIL.LABEL') }}
             <input
               v-model="supportEmail"
@@ -106,7 +92,6 @@ export default {
       name: '',
       locale: 'en',
       domain: '',
-      domainEmailsEnabled: false,
       supportEmail: '',
       features: {},
     };
@@ -132,6 +117,10 @@ export default {
     featureInboundEmailEnabled() {
       return !!this.features.inbound_emails;
     },
+
+    featureCustomDomainEmailEnabled() {
+      return this.featureInboundEmailEnabled && !!this.customEmailDomainEnabled;
+    },
   },
   mounted() {
     if (!this.id) {
@@ -148,7 +137,7 @@ export default {
           id,
           domain,
           support_email,
-          domain_emails_enabled,
+          custom_email_domain_enabled,
           features,
         } = this.getAccount(this.accountId);
 
@@ -158,7 +147,7 @@ export default {
         this.id = id;
         this.domain = domain;
         this.supportEmail = support_email;
-        this.domainEmailsEnabled = domain_emails_enabled;
+        this.customEmailDomainEnabled = custom_email_domain_enabled;
         this.features = features;
       } catch (error) {
         // Ignore error
@@ -177,7 +166,6 @@ export default {
           name: this.name,
           domain: this.domain,
           support_email: this.supportEmail,
-          domain_emails_enabled: this.domainEmailsEnabled,
         });
         Vue.config.lang = this.locale;
         this.showAlert(this.$t('GENERAL_SETTINGS.UPDATE.SUCCESS'));
diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb
index 2a8b2d29d..ccea7184a 100644
--- a/app/mailers/conversation_reply_mailer.rb
+++ b/app/mailers/conversation_reply_mailer.rb
@@ -1,6 +1,6 @@
 class ConversationReplyMailer < ApplicationMailer
   default from: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
-  layout 'mailer'
+  layout :choose_layout
 
   def reply_with_summary(conversation, message_queued_time)
     return unless smtp_config_set_or_development?
@@ -54,16 +54,16 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def reply_email
-    if custom_domain_email_enabled?
-      "#{@agent.name} <reply+#{@conversation.uuid}@#{@account.domain}>"
+    if inbound_email_enabled?
+      "#{@agent.name} <reply+#{@conversation.uuid}@#{current_domain}>"
     else
       @agent&.email
     end
   end
 
   def from_email
-    if custom_domain_email_enabled?
-      "#{@agent.name} <#{@account_support_email}>"
+    if inbound_email_enabled?
+      "#{@agent.name} <#{account_support_email}>"
     else
       "#{@agent.name} <#{ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')}>"
     end
@@ -77,8 +77,8 @@ class ConversationReplyMailer < ApplicationMailer
     "<account/#{@account.id}/conversation/#{@conversation.uuid}@#{current_domain}>"
   end
 
-  def custom_domain_email_enabled?
-    @custom_domain_email_enabled ||= @account.domain_emails_enabled? && current_domain.present? && account_support_email.present?
+  def inbound_email_enabled?
+    @inbound_email_enabled ||= @account.feature_enabled?('inbound_emails') && current_domain.present? && account_support_email.present?
   end
 
   def current_domain
@@ -96,4 +96,10 @@ class ConversationReplyMailer < ApplicationMailer
         ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')
     end
   end
+
+  def choose_layout
+    return false if action_name == 'reply_without_summary'
+
+    'mailer'
+  end
 end
diff --git a/app/models/account.rb b/app/models/account.rb
index 7e97365e5..12fb04921 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -26,7 +26,7 @@ class Account < ApplicationRecord
   }.freeze
 
   ACCOUNT_SETTINGS_FLAGS = {
-    1 => :domain_emails_enabled
+    1 => :custom_email_domain_enabled
   }.freeze
 
   validates :name, presence: true
diff --git a/app/models/concerns/featurable.rb b/app/models/concerns/featurable.rb
index 720f44cfa..7c8a44264 100644
--- a/app/models/concerns/featurable.rb
+++ b/app/models/concerns/featurable.rb
@@ -18,13 +18,13 @@ module Featurable
     before_create :enable_default_features
   end
 
-  def enable_features(names)
+  def enable_features(*names)
     names.each do |name|
       send("feature_#{name}=", true)
     end
   end
 
-  def disable_features(names)
+  def disable_features(*names)
     names.each do |name|
       send("feature_#{name}=", false)
     end
diff --git a/app/views/api/v1/accounts/show.json.jbuilder b/app/views/api/v1/accounts/show.json.jbuilder
index 446d1accb..b9d8ba444 100644
--- a/app/views/api/v1/accounts/show.json.jbuilder
+++ b/app/views/api/v1/accounts/show.json.jbuilder
@@ -2,6 +2,6 @@ json.id @account.id
 json.name @account.name
 json.locale @account.locale
 json.domain @account.domain
-json.domain_emails_enabled @account.domain_emails_enabled
+json.custom_email_domain_enabled @account.custom_email_domain_enabled
 json.support_email @account.support_email
 json.features @account.all_features
diff --git a/app/views/api/v1/accounts/update.json.jbuilder b/app/views/api/v1/accounts/update.json.jbuilder
index 26d1c640e..68acc570a 100644
--- a/app/views/api/v1/accounts/update.json.jbuilder
+++ b/app/views/api/v1/accounts/update.json.jbuilder
@@ -2,6 +2,6 @@ json.id @account.id
 json.name @account.name
 json.locale @account.locale
 json.domain @account.domain
-json.domain_emails_enabled @account.domain_emails_enabled
+json.custom_email_domain_enabled @account.custom_email_domain_enabled
 json.support_email @account.support_email
 json.features @account.enabled_features
diff --git a/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb b/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb
index 8d7d73978..950db5867 100644
--- a/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb
+++ b/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb
@@ -1,7 +1,7 @@
 <% @messages.each do |message| %>
   <p>
     <% if message.content %>
-      <%= message.content %>
+      <%= message.content.gsub("\n", "<br/>").html_safe %>
     <% end %>
     <% if message.attachments %>
       <% message.attachments.each do |attachment| %>
diff --git a/db/migrate/20200709145000_remove_multiple_feature_flags.rb b/db/migrate/20200709145000_remove_multiple_feature_flags.rb
index 82a9ede27..a7384a529 100644
--- a/db/migrate/20200709145000_remove_multiple_feature_flags.rb
+++ b/db/migrate/20200709145000_remove_multiple_feature_flags.rb
@@ -12,8 +12,8 @@ class RemoveMultipleFeatureFlags < ActiveRecord::Migration[6.0]
     twitter_config = feature_config.value.find { |value| value['name'] == 'channel_twitter' }
     Account.find_in_batches do |account_batch|
       account_batch.each do |account|
-        account.enable_features(['channel_facebook']) if facebook_config['enabled']
-        account.enable_features(['channel_twitter']) if twitter_config['enabled']
+        account.enable_features('channel_facebook') if facebook_config['enabled']
+        account.enable_features('channel_twitter') if twitter_config['enabled']
         account.save!
       end
     end
diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb
index 17eea3043..7c40447b1 100644
--- a/spec/controllers/api/v1/accounts_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts_controller_spec.rb
@@ -169,8 +169,7 @@ RSpec.describe 'Accounts API', type: :request do
         name: 'New Name',
         locale: 'en',
         domain: 'example.com',
-        support_email: 'care@example.com',
-        domain_emails_enabled: true
+        support_email: 'care@example.com'
       }
 
       it 'modifies an account' do
@@ -183,7 +182,6 @@ RSpec.describe 'Accounts API', type: :request do
         expect(account.reload.name).to eq(params[:name])
         expect(account.reload.locale).to eq(params[:locale])
         expect(account.reload.domain).to eq(params[:domain])
-        expect(account.reload.domain_emails_enabled).to eq(params[:domain_emails_enabled])
         expect(account.reload.support_email).to eq(params[:support_email])
       end
     end
diff --git a/spec/factories/accounts.rb b/spec/factories/accounts.rb
index 25b6c2f4b..aa8a9b7e6 100644
--- a/spec/factories/accounts.rb
+++ b/spec/factories/accounts.rb
@@ -3,7 +3,7 @@
 FactoryBot.define do
   factory :account do
     sequence(:name) { |n| "Account #{n}" }
-    domain_emails_enabled { false }
+    custom_email_domain_enabled { false }
     domain { 'test.com' }
     support_email { 'support@test.com' }
   end
diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb
index 525839e0e..dbe254258 100644
--- a/spec/mailers/conversation_reply_mailer_spec.rb
+++ b/spec/mailers/conversation_reply_mailer_spec.rb
@@ -97,7 +97,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
         account = conversation.account
         account.domain = 'example.com'
         account.support_email = 'support@example.com'
-        account.domain_emails_enabled = true
+        account.enable_features('inbound_emails')
         account.save!
       end