From ca4a766b82812ed3fcebde53fc0944b1c5e65c20 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Thu, 4 Mar 2021 13:59:59 +0530 Subject: [PATCH] chore: Enable email channel (#1851) --- .../dashboard/settings/inbox/FinishSetup.vue | 2 +- app/mailboxes/application_mailbox.rb | 5 +++- app/mailboxes/support_mailbox.rb | 2 +- app/mailers/conversation_reply_mailer.rb | 6 +---- app/models/account.rb | 8 ++++++ app/models/channel/email.rb | 25 +++++++++---------- app/presenters/mail_presenter.rb | 18 ++++--------- app/views/api/v1/models/_inbox.json.jbuilder | 2 +- config/features.yml | 2 +- ...43_rename_channel_email_forward_address.rb | 5 ++++ db/schema.rb | 6 ++--- db/seeds.rb | 4 +-- spec/factories/channel/channel_email.rb | 2 +- spec/mailboxes/application_mailbox_spec.rb | 13 +++++++--- .../mailers/conversation_reply_mailer_spec.rb | 2 +- 15 files changed, 54 insertions(+), 48 deletions(-) create mode 100644 db/migrate/20210303192243_rename_channel_email_forward_address.rb diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/FinishSetup.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/FinishSetup.vue index 0c19df46e..630bc0652 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/inbox/FinishSetup.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/FinishSetup.vue @@ -25,7 +25,7 @@ diff --git a/app/mailboxes/application_mailbox.rb b/app/mailboxes/application_mailbox.rb index 041e534db..a13110dfe 100644 --- a/app/mailboxes/application_mailbox.rb +++ b/app/mailboxes/application_mailbox.rb @@ -22,7 +22,7 @@ class ApplicationMailbox < ActionMailbox::Base proc do |inbound_mail_obj| is_a_support_email = false inbound_mail_obj.mail.to.each do |email| - channel = Channel::Email.find_by(email: email) + channel = Channel::Email.find_by('email = ? OR forward_to_email = ?', email, email) if channel.present? is_a_support_email = true break @@ -37,7 +37,10 @@ class ApplicationMailbox < ActionMailbox::Base end # routing should be defined below the referenced procs + + # routes as a reply to existing conversations routing(reply_mail? => :reply) + # routes as a new conversation in email channel routing(support_mail? => :support) routing(catch_all_mail? => :default) end diff --git a/app/mailboxes/support_mailbox.rb b/app/mailboxes/support_mailbox.rb index ac63d23ba..dad8b4bb1 100644 --- a/app/mailboxes/support_mailbox.rb +++ b/app/mailboxes/support_mailbox.rb @@ -19,7 +19,7 @@ class SupportMailbox < ApplicationMailbox def find_channel mail.to.each do |email| - @channel = Channel::Email.find_by(email: email) + @channel = Channel::Email.find_by('email = ? OR forward_to_email = ?', email, email) break if @channel.present? end raise 'Email channel/inbox not found' if @channel.nil? diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index 165e9cff7..93fb61668 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -142,11 +142,7 @@ class ConversationReplyMailer < ApplicationMailer end def current_domain - @current_domain ||= begin - @account.domain || - ENV.fetch('MAILER_INBOUND_EMAIL_DOMAIN', false) || - GlobalConfig.get('MAILER_INBOUND_EMAIL_DOMAIN')['MAILER_INBOUND_EMAIL_DOMAIN'] - end + @current_domain ||= @account.inbound_email_domain end def account_support_email diff --git a/app/models/account.rb b/app/models/account.rb index d6f004777..db163ca36 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -87,6 +87,14 @@ class Account < ApplicationRecord } end + def inbound_email_domain + domain || GlobalConfig.get('MAILER_INBOUND_EMAIL_DOMAIN')['MAILER_INBOUND_EMAIL_DOMAIN'] || ENV.fetch('MAILER_INBOUND_EMAIL_DOMAIN', false) + end + + def support_email + super || GlobalConfig.get('MAILER_SUPPORT_EMAIL')['MAILER_SUPPORT_EMAIL'] || ENV.fetch('MAILER_SENDER_EMAIL', nil) + end + private def notify_creation diff --git a/app/models/channel/email.rb b/app/models/channel/email.rb index 33e11e5e8..d68136b76 100644 --- a/app/models/channel/email.rb +++ b/app/models/channel/email.rb @@ -2,17 +2,17 @@ # # Table name: channel_email # -# id :bigint not null, primary key -# email :string not null -# forward_to_address :string not null -# created_at :datetime not null -# updated_at :datetime not null -# account_id :integer not null +# id :bigint not null, primary key +# email :string not null +# forward_to_email :string not null +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer not null # # Indexes # -# index_channel_email_on_email (email) UNIQUE -# index_channel_email_on_forward_to_address (forward_to_address) UNIQUE +# index_channel_email_on_email (email) UNIQUE +# index_channel_email_on_forward_to_email (forward_to_email) UNIQUE # class Channel::Email < ApplicationRecord @@ -21,10 +21,10 @@ class Channel::Email < ApplicationRecord validates :account_id, presence: true belongs_to :account validates :email, uniqueness: true - validates :forward_to_address, uniqueness: true + validates :forward_to_email, uniqueness: true has_one :inbox, as: :channel, dependent: :destroy - before_validation :ensure_forward_to_address, on: :create + before_validation :ensure_forward_to_email, on: :create def name 'Email' @@ -36,8 +36,7 @@ class Channel::Email < ApplicationRecord private - def ensure_forward_to_address - email_domain = InstallationConfig.find_by(name: 'MAILER_INBOUND_EMAIL_DOMAIN')&.value - self.forward_to_address ||= "#{SecureRandom.hex}@#{email_domain}" + def ensure_forward_to_email + self.forward_to_email ||= "#{SecureRandom.hex}@#{account.inbound_email_domain}" end end diff --git a/app/presenters/mail_presenter.rb b/app/presenters/mail_presenter.rb index 40070fd04..5e92ba528 100644 --- a/app/presenters/mail_presenter.rb +++ b/app/presenters/mail_presenter.rb @@ -91,13 +91,13 @@ class MailPresenter < SimpleDelegator end def quoted_text_regexes - return sender_agnostic_regexes if @account.nil? || account_support_email.blank? + return sender_agnostic_regexes if @account.nil? || @account.support_email.blank? [ - Regexp.new("From:\s* #{Regexp.escape(account_support_email)}", Regexp::IGNORECASE), - Regexp.new("<#{Regexp.escape(account_support_email)}>", Regexp::IGNORECASE), - Regexp.new("#{Regexp.escape(account_support_email)}\s+wrote:", Regexp::IGNORECASE), - Regexp.new("On(.*)#{Regexp.escape(account_support_email)}(.*)wrote:", Regexp::IGNORECASE) + Regexp.new("From:\s* #{Regexp.escape(@account.support_email)}", Regexp::IGNORECASE), + Regexp.new("<#{Regexp.escape(@account.support_email)}>", Regexp::IGNORECASE), + Regexp.new("#{Regexp.escape(@account.support_email)}\s+wrote:", Regexp::IGNORECASE), + Regexp.new("On(.*)#{Regexp.escape(@account.support_email)}(.*)wrote:", Regexp::IGNORECASE) ] + sender_agnostic_regexes end @@ -109,12 +109,4 @@ class MailPresenter < SimpleDelegator Regexp.new("from:\s*$", Regexp::IGNORECASE) ] end - - def account_support_email - @account_support_email ||= begin - @account.support_email || - GlobalConfig.get('MAILER_SUPPORT_EMAIL')['MAILER_SUPPORT_EMAIL'] || - ENV.fetch('MAILER_SENDER_EMAIL', nil) - end - end end diff --git a/app/views/api/v1/models/_inbox.json.jbuilder b/app/views/api/v1/models/_inbox.json.jbuilder index cee4e429d..db122de37 100644 --- a/app/views/api/v1/models/_inbox.json.jbuilder +++ b/app/views/api/v1/models/_inbox.json.jbuilder @@ -17,7 +17,7 @@ json.welcome_tagline resource.channel.try(:welcome_tagline) json.enable_auto_assignment resource.enable_auto_assignment json.web_widget_script resource.channel.try(:web_widget_script) json.website_token resource.channel.try(:website_token) -json.forward_to_address resource.channel.try(:forward_to_address) +json.forward_to_email resource.channel.try(:forward_to_email) json.phone_number resource.channel.try(:phone_number) json.selected_feature_flags resource.channel.try(:selected_feature_flags) json.reply_time resource.channel.try(:reply_time) diff --git a/config/features.yml b/config/features.yml index 54dd79f33..70064255a 100644 --- a/config/features.yml +++ b/config/features.yml @@ -2,7 +2,7 @@ - name: inbound_emails enabled: false - name: channel_email - enabled: false + enabled: true - name: channel_facebook enabled: true - name: channel_twitter diff --git a/db/migrate/20210303192243_rename_channel_email_forward_address.rb b/db/migrate/20210303192243_rename_channel_email_forward_address.rb new file mode 100644 index 000000000..1ccec24f5 --- /dev/null +++ b/db/migrate/20210303192243_rename_channel_email_forward_address.rb @@ -0,0 +1,5 @@ +class RenameChannelEmailForwardAddress < ActiveRecord::Migration[6.0] + def change + rename_column :channel_email, :forward_to_address, :forward_to_email + end +end diff --git a/db/schema.rb b/db/schema.rb index db05d6392..264ff1534 100644 --- a/db/schema.rb +++ b/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: 2021_02_22_131155) do +ActiveRecord::Schema.define(version: 2021_03_03_192243) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -131,11 +131,11 @@ ActiveRecord::Schema.define(version: 2021_02_22_131155) do create_table "channel_email", force: :cascade do |t| t.integer "account_id", null: false t.string "email", null: false - t.string "forward_to_address", null: false + t.string "forward_to_email", null: false t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.index ["email"], name: "index_channel_email_on_email", unique: true - t.index ["forward_to_address"], name: "index_channel_email_on_forward_to_address", unique: true + t.index ["forward_to_email"], name: "index_channel_email_on_forward_to_email", unique: true end create_table "channel_facebook_pages", id: :serial, force: :cascade do |t| diff --git a/db/seeds.rb b/db/seeds.rb index 60822d7f2..49227aef9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -13,9 +13,7 @@ unless Rails.env.production? SuperAdmin.create!(email: 'john@acme.inc', password: '123456') account = Account.create!( - name: 'Acme Inc', - domain: 'support.chatwoot.com', - support_email: ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com') + name: 'Acme Inc' ) user = User.new(name: 'John', email: 'john@acme.inc', password: '123456') diff --git a/spec/factories/channel/channel_email.rb b/spec/factories/channel/channel_email.rb index 4af1953fd..6e01dffdd 100644 --- a/spec/factories/channel/channel_email.rb +++ b/spec/factories/channel/channel_email.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory :channel_email, class: 'Channel::Email' do sequence(:email) { |n| "care-#{n}@example.com" } - sequence(:forward_to_address) { |n| "forward-#{n}@chatwoot.com" } + sequence(:forward_to_email) { |n| "forward-#{n}@chatwoot.com" } account after(:create) do |channel_email| create(:inbox, channel: channel_email, account: channel_email.account) diff --git a/spec/mailboxes/application_mailbox_spec.rb b/spec/mailboxes/application_mailbox_spec.rb index 54f0b8a44..be33732c4 100644 --- a/spec/mailboxes/application_mailbox_spec.rb +++ b/spec/mailboxes/application_mailbox_spec.rb @@ -29,13 +29,18 @@ RSpec.describe ApplicationMailbox, type: :mailbox do describe 'Support' do let!(:channel_email) { create(:channel_email) } - before do + it 'routes support emails to Support Mailbox when mail is to channel email' do # this email is hardcoded in the support.eml, that's why we are updating this - channel_email.email = 'care@example.com' - channel_email.save + channel_email.update(email: 'care@example.com') + dbl = double + expect(SupportMailbox).to receive(:new).and_return(dbl) + expect(dbl).to receive(:perform_processing).and_return(true) + described_class.route support_mail end - it 'routes support emails to Support Mailbox' do + it 'routes support emails to Support Mailbox when mail is to channel forward to email' do + # this email is hardcoded in the support.eml, that's why we are updating this + channel_email.update(forward_to_email: 'care@example.com') dbl = double expect(SupportMailbox).to receive(:new).and_return(dbl) expect(dbl).to receive(:perform_processing).and_return(true) diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index 4558c568f..8ddd11523 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -95,7 +95,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do let(:conversation) { create(:conversation, assignee: agent, inbox: inbox_member.inbox, account: account) } let!(:message) { create(:message, conversation: conversation, account: account) } let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now } - let(:domain) { account.domain || ENV.fetch('MAILER_INBOUND_EMAIL_DOMAIN', false) } + let(:domain) { account.inbound_email_domain } it 'renders the receiver email' do expect(mail.to).to eq([message&.conversation&.contact&.email])