fix: Remove IMAP and SMTP email validation (#4435)

* Remove IMAP and SMTP email validation
* Rename imap_email & smtp_email columns to imap_login & smtp_login respectively.
* Use channel email domain if inbound email domain not present

app/helpers/api/v1/inboxes_helper.rb

@@ -14,7 +14,7 @@ module Api::V1::InboxesHelper
     Mail.defaults do
       retriever_method :imap, { address: channel_data[:imap_address],
                                 port: channel_data[:imap_port],
-                                user_name: channel_data[:imap_email],
+                                user_name: channel_data[:imap_login],
                                 password: channel_data[:imap_password],
                                 enable_ssl: channel_data[:imap_enable_ssl] }
     end
@@ -33,7 +33,7 @@ module Api::V1::InboxesHelper
   end
 
   def check_smtp_connection(channel_data, smtp)
-    smtp.start(channel_data[:smtp_domain], channel_data[:smtp_email], channel_data[:smtp_password],
+    smtp.start(channel_data[:smtp_domain], channel_data[:smtp_login], channel_data[:smtp_password],
                channel_data[:smtp_authentication]&.to_sym || :login)
     smtp.finish unless smtp&.nil?
   end

app/javascript/dashboard/i18n/locale/en/inboxMgmt.json

@@ -486,9 +486,9 @@
         "LABEL": "Port",
         "PLACE_HOLDER": "Port"
       },
-      "EMAIL": {
-        "LABEL": "Email",
-        "PLACE_HOLDER": "Email"
+      "LOGIN": {
+        "LABEL": "Login",
+        "PLACE_HOLDER": "Login"
       },
       "PASSWORD": {
         "LABEL": "Password",
@@ -514,9 +514,9 @@
         "LABEL": "Port",
         "PLACE_HOLDER": "Port"
       },
-      "EMAIL": {
-        "LABEL": "Email",
-        "PLACE_HOLDER": "Email"
+      "LOGIN": {
+        "LABEL": "Login",
+        "PLACE_HOLDER": "Login"
       },
       "PASSWORD": {
         "LABEL": "Password",

app/javascript/dashboard/routes/dashboard/settings/inbox/ImapSettings.vue

@@ -34,12 +34,12 @@
             @blur="$v.port.$touch"
           />
           <woot-input
-            v-model="email"
-            :class="{ error: $v.email.$error }"
+            v-model="login"
+            :class="{ error: $v.login.$error }"
             class="medium-9 columns"
-            :label="$t('INBOX_MGMT.IMAP.EMAIL.LABEL')"
-            :placeholder="$t('INBOX_MGMT.IMAP.EMAIL.PLACE_HOLDER')"
-            @blur="$v.email.$touch"
+            :label="$t('INBOX_MGMT.IMAP.LOGIN.LABEL')"
+            :placeholder="$t('INBOX_MGMT.IMAP.LOGIN.PLACE_HOLDER')"
+            @blur="$v.login.$touch"
           />
           <woot-input
             v-model="password"
@@ -73,7 +73,7 @@
 import { mapGetters } from 'vuex';
 import alertMixin from 'shared/mixins/alertMixin';
 import SettingsSection from 'dashboard/components/SettingsSection';
-import { required, minLength, email } from 'vuelidate/lib/validators';
+import { required, minLength } from 'vuelidate/lib/validators';
 
 export default {
   components: {
@@ -91,7 +91,7 @@ export default {
       isIMAPEnabled: false,
       address: '',
       port: '',
-      email: '',
+      login: '',
       password: '',
       isSSLEnabled: true,
     };
@@ -99,7 +99,7 @@ export default {
   validations: {
     address: { required },
     port: { required, minLength: minLength(2) },
-    email: { required, email },
+    login: { required },
     password: { required },
   },
   computed: {
@@ -119,14 +119,14 @@ export default {
         imap_enabled,
         imap_address,
         imap_port,
-        imap_email,
+        imap_login,
         imap_password,
         imap_enable_ssl,
       } = this.inbox;
       this.isIMAPEnabled = imap_enabled;
       this.address = imap_address;
       this.port = imap_port;
-      this.email = imap_email;
+      this.login = imap_login;
       this.password = imap_password;
       this.isSSLEnabled = imap_enable_ssl;
     },
@@ -140,7 +140,7 @@ export default {
             imap_enabled: this.isIMAPEnabled,
             imap_address: this.address,
             imap_port: this.port,
-            imap_email: this.email,
+            imap_login: this.login,
             imap_password: this.password,
             imap_enable_ssl: this.isSSLEnabled,
             imap_inbox_synced_at: this.isIMAPEnabled

app/javascript/dashboard/routes/dashboard/settings/inbox/SmtpSettings.vue

@@ -33,12 +33,12 @@
             @blur="$v.port.$touch"
           />
           <woot-input
-            v-model="email"
-            :class="{ error: $v.email.$error }"
+            v-model="login"
+            :class="{ error: $v.login.$error }"
             class="medium-9 columns"
-            :label="$t('INBOX_MGMT.SMTP.EMAIL.LABEL')"
-            :placeholder="$t('INBOX_MGMT.SMTP.EMAIL.PLACE_HOLDER')"
-            @blur="$v.email.$touch"
+            :label="$t('INBOX_MGMT.SMTP.LOGIN.LABEL')"
+            :placeholder="$t('INBOX_MGMT.SMTP.LOGIN.PLACE_HOLDER')"
+            @blur="$v.login.$touch"
           />
           <woot-input
             v-model="password"
@@ -91,7 +91,7 @@
 import { mapGetters } from 'vuex';
 import alertMixin from 'shared/mixins/alertMixin';
 import SettingsSection from 'dashboard/components/SettingsSection';
-import { required, minLength, email } from 'vuelidate/lib/validators';
+import { required, minLength } from 'vuelidate/lib/validators';
 import InputRadioGroup from './components/InputRadioGroup';
 import SingleSelectDropdown from './components/SingleSelectDropdown';
 
@@ -113,7 +113,7 @@ export default {
       isSMTPEnabled: false,
       address: '',
       port: '',
-      email: '',
+      login: '',
       password: '',
       domain: '',
       ssl: false,
@@ -147,7 +147,7 @@ export default {
       required,
       minLength: minLength(2),
     },
-    email: { required, email },
+    login: { required },
     password: { required },
     domain: { required },
   },
@@ -168,7 +168,7 @@ export default {
         smtp_enabled,
         smtp_address,
         smtp_port,
-        smtp_email,
+        smtp_login,
         smtp_password,
         smtp_domain,
         smtp_enable_starttls_auto,
@@ -179,7 +179,7 @@ export default {
       this.isSMTPEnabled = smtp_enabled;
       this.address = smtp_address;
       this.port = smtp_port;
-      this.email = smtp_email;
+      this.login = smtp_login;
       this.password = smtp_password;
       this.domain = smtp_domain;
       this.starttls = smtp_enable_starttls_auto;
@@ -220,7 +220,7 @@ export default {
             smtp_enabled: this.isSMTPEnabled,
             smtp_address: this.address,
             smtp_port: this.port,
-            smtp_email: this.email,
+            smtp_login: this.login,
             smtp_password: this.password,
             smtp_domain: this.domain,
             smtp_enable_ssl_tls: this.ssl,

app/jobs/inboxes/fetch_imap_emails_job.rb

@@ -26,7 +26,7 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob
     Mail.defaults do
       retriever_method :imap, address: channel.imap_address,
                               port: channel.imap_port,
-                              user_name: channel.imap_email,
+                              user_name: channel.imap_login,
                               password: channel.imap_password,
                               enable_ssl: channel.imap_enable_ssl
     end

app/mailers/conversation_reply_mailer.rb

@@ -128,11 +128,11 @@ class ConversationReplyMailer < ApplicationMailer
   def custom_message_id
     last_message = @message || @messages&.last
 
-    "<conversation/#{@conversation.uuid}/messages/#{last_message&.id}@#{@account.inbound_email_domain}>"
+    "<conversation/#{@conversation.uuid}/messages/#{last_message&.id}@#{channel_email_domain}>"
   end
 
   def in_reply_to_email
-    conversation_reply_email_id || "<account/#{@account.id}/conversation/#{@conversation.uuid}@#{@account.inbound_email_domain}>"
+    conversation_reply_email_id || "<account/#{@account.id}/conversation/#{@conversation.uuid}@#{channel_email_domain}>"
   end
 
   def conversation_reply_email_id

app/mailers/conversation_reply_mailer_helper.rb

@@ -26,7 +26,7 @@ module ConversationReplyMailerHelper
     smtp_settings = {
       address: @channel.smtp_address,
       port: @channel.smtp_port,
-      user_name: @channel.smtp_email,
+      user_name: @channel.smtp_login,
       password: @channel.smtp_password,
       domain: @channel.smtp_domain,
       tls: @channel.smtp_enable_ssl_tls,
@@ -48,10 +48,18 @@ module ConversationReplyMailerHelper
   end
 
   def email_from
-    email_smtp_enabled ? @channel.smtp_email : from_email_with_name
+    email_smtp_enabled ? @channel.email : from_email_with_name
   end
 
   def email_reply_to
-    email_imap_enabled ? @channel.imap_email : reply_email
+    email_imap_enabled ? @channel.email : reply_email
+  end
+
+  # Use channel email domain in case of account email domain is not set for custom message_id and in_reply_to
+  def channel_email_domain
+    return @account.inbound_email_domain if @account.inbound_email_domain.present?
+
+    email = @inbox.channel.try(:email)
+    email.present? ? email.split('@').last : raise(StandardError, 'Channel email domain not present.')
   end
 end

app/models/channel/email.rb

@@ -6,19 +6,19 @@
 #  email                     :string           not null
 #  forward_to_email          :string           not null
 #  imap_address              :string           default("")
-#  imap_email                :string           default("")
 #  imap_enable_ssl           :boolean          default(TRUE)
 #  imap_enabled              :boolean          default(FALSE)
 #  imap_inbox_synced_at      :datetime
+#  imap_login                :string           default("")
 #  imap_password             :string           default("")
 #  imap_port                 :integer          default(0)
 #  smtp_address              :string           default("")
 #  smtp_authentication       :string           default("login")
 #  smtp_domain               :string           default("")
-#  smtp_email                :string           default("")
 #  smtp_enable_ssl_tls       :boolean          default(FALSE)
 #  smtp_enable_starttls_auto :boolean          default(TRUE)
 #  smtp_enabled              :boolean          default(FALSE)
+#  smtp_login                :string           default("")
 #  smtp_openssl_verify_mode  :string           default("none")
 #  smtp_password             :string           default("")
 #  smtp_port                 :integer          default(0)
@@ -37,8 +37,8 @@ class Channel::Email < ApplicationRecord
   include Reauthorizable
 
   self.table_name = 'channel_email'
-  EDITABLE_ATTRS = [:email, :imap_enabled, :imap_email, :imap_password, :imap_address, :imap_port, :imap_enable_ssl, :imap_inbox_synced_at,
-                    :smtp_enabled, :smtp_email, :smtp_password, :smtp_address, :smtp_port, :smtp_domain, :smtp_enable_starttls_auto,
+  EDITABLE_ATTRS = [:email, :imap_enabled, :imap_login, :imap_password, :imap_address, :imap_port, :imap_enable_ssl, :imap_inbox_synced_at,
+                    :smtp_enabled, :smtp_login, :smtp_password, :smtp_address, :smtp_port, :smtp_domain, :smtp_enable_starttls_auto,
                     :smtp_enable_ssl_tls, :smtp_openssl_verify_mode, :smtp_authentication].freeze
 
   validates :email, uniqueness: true

app/views/api/v1/models/_inbox.json.jbuilder

@@ -53,7 +53,7 @@ if resource.email?
   json.email resource.channel.try(:email)
 
   ## IMAP
-  json.imap_email resource.channel.try(:imap_email)
+  json.imap_login resource.channel.try(:imap_login)
   json.imap_password resource.channel.try(:imap_password)
   json.imap_address resource.channel.try(:imap_address)
   json.imap_port resource.channel.try(:imap_port)
@@ -61,7 +61,7 @@ if resource.email?
   json.imap_enable_ssl resource.channel.try(:imap_enable_ssl)
 
   ## SMTP
-  json.smtp_email resource.channel.try(:smtp_email)
+  json.smtp_login resource.channel.try(:smtp_login)
   json.smtp_password resource.channel.try(:smtp_password)
   json.smtp_address resource.channel.try(:smtp_address)
   json.smtp_port resource.channel.try(:smtp_port)

config/schedule.yml

@@ -16,6 +16,6 @@ trigger_scheduled_items_job:
 
 # executed At every 5th minute..
 trigger_imap_email_inboxes_job:
-  cron: '*/5 * * * *'
+  cron: '*/1 * * * *'
   class: 'Inboxes::FetchImapEmailInboxesJob'
   queue: scheduled_jobs

db/migrate/20220409044943_rename_imap_email_and_smtp_email_columns.rb

@@ -0,0 +1,6 @@
+class RenameImapEmailAndSmtpEmailColumns < ActiveRecord::Migration[6.1]
+  def change
+    rename_column :channel_email, :imap_email, :imap_login
+    rename_column :channel_email, :smtp_email, :smtp_login
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2022_04_05_092033) do
+ActiveRecord::Schema.define(version: 2022_04_09_044943) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
@@ -190,14 +190,14 @@ ActiveRecord::Schema.define(version: 2022_04_05_092033) do
     t.boolean "imap_enabled", default: false
     t.string "imap_address", default: ""
     t.integer "imap_port", default: 0
-    t.string "imap_email", default: ""
+    t.string "imap_login", default: ""
     t.string "imap_password", default: ""
     t.boolean "imap_enable_ssl", default: true
     t.datetime "imap_inbox_synced_at"
     t.boolean "smtp_enabled", default: false
     t.string "smtp_address", default: ""
     t.integer "smtp_port", default: 0
-    t.string "smtp_email", default: ""
+    t.string "smtp_login", default: ""
     t.string "smtp_password", default: ""
     t.string "smtp_domain", default: ""
     t.boolean "smtp_enable_starttls_auto", default: true

spec/controllers/api/v1/accounts/inboxes_controller_spec.rb

@@ -426,7 +426,7 @@ RSpec.describe 'Inboxes API', type: :request do
                   imap_enabled: true,
                   imap_address: 'imap.gmail.com',
                   imap_port: 993,
-                  imap_email: 'imaptest@gmail.com'
+                  imap_login: 'imaptest@gmail.com'
                 }
               },
               as: :json
@@ -496,7 +496,7 @@ RSpec.describe 'Inboxes API', type: :request do
                   smtp_enabled: true,
                   smtp_address: 'smtp.gmail.com',
                   smtp_port: 587,
-                  smtp_email: 'smtptest@gmail.com',
+                  smtp_login: 'smtptest@gmail.com',
                   smtp_enable_starttls_auto: true,
                   smtp_openssl_verify_mode: 'peer'
                 }
@@ -525,7 +525,7 @@ RSpec.describe 'Inboxes API', type: :request do
                 channel: {
                   smtp_enabled: true,
                   smtp_address: 'smtp.gmail.com',
-                  smtp_email: 'smtptest@gmail.com',
+                  smtp_login: 'smtptest@gmail.com',
                   smtp_port: 587,
                   smtp_enable_ssl_tls: true,
                   smtp_openssl_verify_mode: 'none'

spec/jobs/inboxes/fetch_imap_email_inboxes_job_spec.rb

@@ -3,7 +3,7 @@ require 'rails_helper'
 RSpec.describe Inboxes::FetchImapEmailInboxesJob, type: :job do
   let(:account) { create(:account) }
   let(:imap_email_channel) do
-    create(:channel_email, imap_enabled: true, imap_address: 'imap.gmail.com', imap_port: 993, imap_email: 'imap@gmail.com',
+    create(:channel_email, imap_enabled: true, imap_address: 'imap.gmail.com', imap_port: 993, imap_login: 'imap@gmail.com',
                            imap_password: 'password', account: account)
   end
   let(:email_inbox) { create(:inbox, channel: imap_email_channel, account: account) }

spec/jobs/inboxes/fetch_imap_emails_job_spec.rb

@@ -3,7 +3,7 @@ require 'rails_helper'
 RSpec.describe Inboxes::FetchImapEmailsJob, type: :job do
   let(:account) { create(:account) }
   let(:imap_email_channel) do
-    create(:channel_email, imap_enabled: true, imap_address: 'imap.gmail.com', imap_port: 993, imap_email: 'imap@gmail.com',
+    create(:channel_email, imap_enabled: true, imap_address: 'imap.gmail.com', imap_port: 993, imap_login: 'imap@gmail.com',
                            imap_password: 'password', imap_inbox_synced_at: Time.now.utc - 10, account: account)
   end
   let(:email_inbox) { create(:inbox, channel: imap_email_channel, account: account) }

spec/mailboxes/imap/imap_mailbox_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Imap::ImapMailbox, type: :mailbox do
     let(:agent) { create(:user, email: 'agent@example.com', account: account) }
     let(:channel) do
       create(:channel_email, imap_enabled: true, imap_address: 'imap.gmail.com',
-                             imap_port: 993, imap_email: 'imap@gmail.com', imap_password: 'password',
+                             imap_port: 993, imap_login: 'imap@gmail.com', imap_password: 'password',
                              account: account)
     end
     let(:inbox) { create(:inbox, channel: channel, account: account) }

spec/mailers/conversation_reply_mailer_spec.rb

@@ -156,7 +156,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
 
     context 'when smtp enabled for email channel' do
       let(:smtp_email_channel) do
-        create(:channel_email, smtp_enabled: true, smtp_address: 'smtp.gmail.com', smtp_port: 587, smtp_email: 'smtp@gmail.com',
+        create(:channel_email, smtp_enabled: true, smtp_address: 'smtp.gmail.com', smtp_port: 587, smtp_login: 'smtp@gmail.com',
                                smtp_password: 'password', smtp_domain: 'smtp.gmail.com', account: account)
       end
       let(:conversation) { create(:conversation, assignee: agent, inbox: smtp_email_channel.inbox, account: account).reload }
@@ -251,5 +251,24 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
         expect(mail.in_reply_to).to eq("account/#{conversation.account.id}/conversation/#{conversation.uuid}@#{conversation.account.domain}")
       end
     end
+
+    context 'when inbound email domain is not enabled' do
+      let(:new_account) { create(:account, domain: nil) }
+      let!(:email_channel) { create(:channel_email, account: new_account) }
+      let!(:inbox) { create(:inbox, channel: email_channel, account: new_account) }
+      let(:inbox_member) { create(:inbox_member, user: agent, inbox: inbox) }
+      let(:conversation) { create(:conversation, assignee: agent, inbox: inbox_member.inbox, account: new_account) }
+      let!(:message) { create(:message, conversation: conversation, account: new_account) }
+      let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
+      let(:domain) { inbox.channel.email.split('@').last }
+
+      it 'sets the correct custom message id' do
+        expect(mail.message_id).to eq("conversation/#{conversation.uuid}/messages/#{message.id}@#{domain}")
+      end
+
+      it 'sets the correct in reply to id' do
+        expect(mail.in_reply_to).to eq("account/#{conversation.account.id}/conversation/#{conversation.uuid}@#{domain}")
+      end
+    end
   end
 end