Merge branch 'develop' into feat/conversation-finder-strategies

This commit is contained in:
Shivam Mishra
2025-11-04 20:07:24 +05:30
committed by GitHub
19 changed files with 744 additions and 13 deletions
+1
View File
@@ -21,6 +21,7 @@ gem 'telephone_number'
gem 'time_diff'
gem 'tzinfo-data'
gem 'valid_email2'
gem 'email-provider-info'
# compress javascript config.assets.js_compressor
gem 'uglifier'
##-- used for single column multiple binary flags in notification settings/feature flagging --##
+2
View File
@@ -270,6 +270,7 @@ GEM
concurrent-ruby (~> 1.0)
http (>= 3.0)
ruby2_keywords
email-provider-info (0.0.1)
email_reply_trimmer (0.1.13)
erubi (1.13.0)
et-orbi (1.2.11)
@@ -1016,6 +1017,7 @@ DEPENDENCIES
dotenv-rails (>= 3.0.0)
down
elastic-apm
email-provider-info
email_reply_trimmer
facebook-messenger
factory_bot_rails (>= 6.4.3)
+4 -9
View File
@@ -153,15 +153,6 @@ class Message < ApplicationRecord
merge_sender_attributes(data)
end
def search_data
data = attributes.symbolize_keys
data[:conversation] = conversation.present? ? conversation_push_event_data : nil
data[:attachments] = attachments.map(&:push_event_data) if attachments.present?
data[:sender] = sender.push_event_data if sender
data[:inbox] = inbox
data
end
def conversation_push_event_data
{
assignee_id: conversation.assignee_id,
@@ -259,6 +250,10 @@ class Message < ApplicationRecord
true
end
def search_data
Messages::SearchDataPresenter.new(self).search_data
end
private
def prevent_message_flooding
@@ -0,0 +1,58 @@
class Messages::SearchDataPresenter < SimpleDelegator
def search_data
{
**searchable_content,
**message_attributes,
**search_additional_data,
conversation: conversation_data
}
end
private
def searchable_content
{
content: content,
attachments: attachment_data,
content_attributes: content_attributes_data
}
end
def message_attributes
{
account_id: account_id,
inbox_id: inbox_id,
conversation_id: conversation_id,
message_type: message_type,
private: private,
created_at: created_at,
source_id: source_id,
sender_id: sender_id,
sender_type: sender_type
}
end
def attachment_data
attachments.filter_map do |a|
{ transcribed_text: a.meta&.dig('transcribed_text') }
end.presence
end
def content_attributes_data
email_subject = content_attributes.dig(:email, :subject)
return {} if email_subject.blank?
{ email: { subject: email_subject } }
end
def conversation_data
{ id: conversation.display_id }
end
def search_additional_data
{
campaign_id: additional_attributes&.dig('campaign_id'),
automation_rule_id: content_attributes&.dig('automation_rule_id')
}
end
end
@@ -0,0 +1,16 @@
class AddUniqueIndexToCompaniesDomain < ActiveRecord::Migration[7.1]
def up
remove_index :companies, name: 'index_companies_on_domain_and_account_id', if_exists: true
add_index :companies, [:account_id, :domain],
unique: true,
name: 'index_companies_on_account_and_domain',
where: 'domain IS NOT NULL'
end
def down
remove_index :companies, name: 'index_companies_on_account_and_domain', if_exists: true
add_index :companies, [:domain, :account_id],
name: 'index_companies_on_domain_and_account_id'
end
end
+1 -1
View File
@@ -577,8 +577,8 @@ ActiveRecord::Schema[7.1].define(version: 2025_10_22_152158) do
t.bigint "account_id", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["account_id", "domain"], name: "index_companies_on_account_and_domain", unique: true, where: "(domain IS NOT NULL)"
t.index ["account_id"], name: "index_companies_on_account_id"
t.index ["domain", "account_id"], name: "index_companies_on_domain_and_account_id"
t.index ["name", "account_id"], name: "index_companies_on_name_and_account_id"
end
@@ -0,0 +1,54 @@
class Migration::CompanyAccountBatchJob < ApplicationJob
queue_as :low
def perform(account)
account.contacts
.where.not(email: nil)
.find_in_batches(batch_size: 1000) do |contact_batch|
process_contact_batch(contact_batch, account)
end
end
private
def process_contact_batch(contacts, account)
contacts.each do |contact|
next unless should_process?(contact)
company = find_or_create_company(contact, account)
# rubocop:disable Rails/SkipsModelValidations
contact.update_column(:company_id, company.id) if company
# rubocop:enable Rails/SkipsModelValidations
end
end
def should_process?(contact)
return false if contact.company_id.present?
return false if contact.email.blank?
Companies::BusinessEmailDetectorService.new(contact.email).perform
end
def find_or_create_company(contact, account)
domain = extract_domain(contact.email)
company_name = derive_company_name(contact, domain)
Company.find_or_create_by!(account: account, domain: domain) do |company|
company.name = company_name
end
rescue ActiveRecord::RecordNotUnique
# Race condition: Another job created it between our check and create
# just find the one that was created
Company.find_by(account: account, domain: domain)
end
def extract_domain(email)
email.split('@').last&.downcase
end
def derive_company_name(contact, domain)
contact.additional_attributes&.dig('company_name').presence ||
domain.split('.').first.tr('-_', ' ').titleize
end
end
@@ -0,0 +1,17 @@
class Migration::CompanyBackfillJob < ApplicationJob
queue_as :low
def perform
Rails.logger.info 'Starting company backfill migration...'
account_count = 0
Account.find_in_batches(batch_size: 100) do |accounts|
accounts.each do |account|
Rails.logger.info "Enqueuing company backfill for account #{account.id}"
Migration::CompanyAccountBatchJob.perform_later(account)
account_count += 1
end
end
Rails.logger.info "Company backfill migration complete. Enqueued jobs for #{account_count} accounts."
end
end
+4 -3
View File
@@ -12,9 +12,9 @@
#
# Indexes
#
# index_companies_on_account_id (account_id)
# index_companies_on_domain_and_account_id (domain,account_id)
# index_companies_on_name_and_account_id (name,account_id)
# index_companies_on_account_and_domain (account_id,domain) UNIQUE WHERE (domain IS NOT NULL)
# index_companies_on_account_id (account_id)
# index_companies_on_name_and_account_id (name,account_id)
#
class Company < ApplicationRecord
include Avatarable
@@ -24,6 +24,7 @@ class Company < ApplicationRecord
with: /\A[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)+\z/,
message: I18n.t('errors.companies.domain.invalid')
}
validates :domain, uniqueness: { scope: :account_id }, if: -> { domain.present? }
validates :description, length: { maximum: Limits::COMPANY_DESCRIPTION_LENGTH_LIMIT }
belongs_to :account
@@ -2,5 +2,30 @@ module Enterprise::Concerns::Contact
extend ActiveSupport::Concern
included do
belongs_to :company, optional: true
after_commit :associate_company_from_email,
on: [:create, :update],
if: :should_associate_company?
end
private
def should_associate_company?
# Only trigger if:
# 1. Contact has an email
# 2. Contact doesn't have a compan yet
# 3. Email was just set/changed
# 4. Email was previously nil (first time getting email)
email.present? &&
company_id.nil? &&
saved_change_to_email? &&
saved_change_to_email.first.nil?
end
def associate_company_from_email
Contacts::CompanyAssociationService.new.associate_company_from_email(self)
rescue StandardError => e
Rails.logger.error("Failed to associate company for contact #{id}: #{e.message}")
# Don't fail the contact save if the company association fails
end
end
@@ -0,0 +1,19 @@
class Companies::BusinessEmailDetectorService
attr_reader :email
def initialize(email)
@email = email
end
def perform
return false if email.blank?
address = ValidEmail2::Address.new(email)
return false unless address.valid?
return false if address.disposable_domain?
provider = EmailProviderInfo.call(email)
provider.nil?
end
end
@@ -0,0 +1,46 @@
class Contacts::CompanyAssociationService
def associate_company_from_email(contact)
return nil if skip_association?(contact)
company = find_or_create_company(contact)
# rubocop:disable Rails/SkipsModelValidations
# Intentionally using update_column here to:
# 1. Avoid triggering callbacks
# 2. Improve performance (We're only setting company_id, no need for validation)
contact.update_column(:company_id, company.id) if company
# rubocop:enable Rails/SkipsModelValidations
company
end
private
def skip_association?(contact)
return true if contact.company_id.present?
return true if contact.email.blank?
detector = Companies::BusinessEmailDetectorService.new(contact.email)
return true unless detector.perform
false
end
def find_or_create_company(contact)
domain = extract_domain(contact.email)
company_name = derive_company_name(contact, domain)
Company.find_or_create_by!(account: contact.account, domain: domain) do |company|
company.name = company_name
end
rescue ActiveRecord::RecordNotUnique
# If another process created it first, just find that
Company.find_by(account: contact.account, domain: domain)
end
def extract_domain(email)
email.split('@').last&.downcase
end
def derive_company_name(contact, domain)
contact.additional_attributes&.dig('company_name') || domain.split('.').first.tr('-_', ' ').titleize
end
end
+12
View File
@@ -0,0 +1,12 @@
namespace :companies do
desc 'Backfill companies from existing contact email domains'
task backfill: :environment do
puts 'Starting company backfill migration...'
puts 'This will process all accounts and create companies from contact email domains.'
puts 'The job will run in the background via Sidekiq'
puts ''
Migration::CompanyBackfillJob.perform_later
puts 'Company backfill job has been enqueued.'
puts 'Monitor progress in logs or Sidekiq dashboard.'
end
end
@@ -0,0 +1,133 @@
require 'rails_helper'
RSpec.describe Migration::CompanyAccountBatchJob, type: :job do
let(:account) { create(:account) }
describe '#perform' do
before do
# Stub EmailProvideInfo to control behavior in tests
allow(EmailProviderInfo).to receive(:call) do |email|
domain = email.split('@').last&.downcase
case domain
when 'gmail.com', 'yahoo.com', 'hotmail.com', 'uol.com.br'
'free_provider' # generic free provider name
end
end
end
context 'when contact has business email' do
let!(:contact) { create(:contact, account: account, email: 'user@acme.com') }
it 'creates a company and associates the contact' do
# Clean up companies created by Part 2's callback
Company.delete_all
# rubocop:disable Rails/SkipsModelValidations
contact.update_column(:company_id, nil)
# rubocop:enable Rails/SkipsModelValidations
expect do
described_class.perform_now(account)
end.to change(Company, :count).by(1)
contact.reload
expect(contact.company).to be_present
expect(contact.company.domain).to eq('acme.com')
expect(contact.company.name).to eq('Acme')
end
end
context 'when contact has free email' do
let!(:contact) { create(:contact, account: account, email: 'user@gmail.com') }
it 'does not create a company' do
expect do
described_class.perform_now(account)
end.not_to change(Company, :count)
contact.reload
expect(contact.company_id).to be_nil
end
end
context 'when contact has company_name in additional_attributes' do
let!(:contact) do
create(:contact, account: account, email: 'user@acme.com', additional_attributes: { 'company_name' => 'Acme Corporation' })
end
it 'uses the saved company name' do
described_class.perform_now(account)
contact.reload
expect(contact.company.name).to eq('Acme Corporation')
end
end
context 'when contact already has a company' do
let!(:existing_company) { create(:company, account: account, domain: 'existing.com') }
let!(:contact) do
create(:contact, account: account, email: 'user@acme.com', company: existing_company)
end
it 'does not change the existing company' do
described_class.perform_now(account)
contact.reload
expect(contact.company_id).to eq(existing_company.id)
end
end
context 'when multiple contacts have the same domain' do
let!(:contact1) { create(:contact, account: account, email: 'user1@acme.com') }
let!(:contact2) { create(:contact, account: account, email: 'user2@acme.com') }
it 'creates only one company for the domain' do
# Clean up companies created by Part 2's callback
Company.delete_all
# rubocop:disable Rails/SkipsModelValidations
contact1.update_column(:company_id, nil)
contact2.update_column(:company_id, nil)
# rubocop:enable Rails/SkipsModelValidations
expect do
described_class.perform_now(account)
end.to change(Company, :count).by(1)
contact1.reload
contact2.reload
expect(contact1.company_id).to eq(contact2.company_id)
expect(contact1.company.domain).to eq('acme.com')
end
end
context 'when contact has no email' do
let!(:contact) { create(:contact, account: account, email: nil) }
it 'skips the contact' do
expect do
described_class.perform_now(account)
end.not_to change(Company, :count)
contact.reload
expect(contact.company_id).to be_nil
end
end
context 'when processing large batch' do
before do
contacts_data = Array.new(2000) do |i|
{
account_id: account.id,
email: "user#{i}@company#{i % 100}.com",
name: "User #{i}",
created_at: Time.current,
updated_at: Time.current
}
end
# rubocop:disable Rails/SkipsModelValidations
Contact.insert_all(contacts_data)
# rubocop:enable Rails/SkipsModelValidations
end
it 'processes all contacts in batches' do
expect do
described_class.perform_now(account)
end.to change(Company, :count).by(100)
expect(account.contacts.where.not(company_id: nil).count).to eq(2000)
end
end
end
end
@@ -0,0 +1,31 @@
require 'rails_helper'
RSpec.describe Migration::CompanyBackfillJob, type: :job do
describe '#perform' do
it 'enqueues the job' do
expect { described_class.perform_later }
.to have_enqueued_job(described_class)
.on_queue('low')
end
context 'when accounts exist' do
let!(:account1) { create(:account) }
let!(:account2) { create(:account) }
it 'enqueues CompanyAccountBatchJob for each account' do
expect do
described_class.perform_now
end.to have_enqueued_job(Migration::CompanyAccountBatchJob)
.with(account1)
.and have_enqueued_job(Migration::CompanyAccountBatchJob)
.with(account2)
end
end
context 'when no accounts exist' do
it 'completes without error' do
expect { described_class.perform_now }.not_to raise_error
end
end
end
end
@@ -0,0 +1,61 @@
require 'rails_helper'
RSpec.describe Contact, type: :model do
describe 'company auto-association' do
let(:account) { create(:account) }
context 'when creating a new contact with business email' do
it 'automatically creates and associates a company' do
expect do
create(:contact, email: 'john@acme.com', account: account)
end.to change(Company, :count).by(1)
contact = described_class.last
expect(contact.company).to be_present
expect(contact.company.domain).to eq('acme.com')
end
it 'does not create company for free email providers' do
expect do
create(:contact, email: 'john@gmail.com', account: account)
end.not_to change(Company, :count)
end
end
context 'when updating a contact to add email for first time' do
it 'creates and associates company' do
contact = create(:contact, email: nil, account: account)
expect do
contact.update(email: 'john@acme.com')
end.to change(Company, :count).by(1)
contact.reload
expect(contact.company.domain).to eq('acme.com')
end
end
context 'when updating a contact that already has a company' do
it 'does not change company when email changes' do
existing_company = create(:company, domain: 'oldcompany.com', account: account)
contact = create(:contact, email: 'john@oldcompany.com', company: existing_company, account: account)
expect do
contact.update(email: 'john@new_company.com')
end.not_to change(Company, :count)
contact.reload
expect(contact.company).to eq(existing_company)
end
end
context 'when multiple contacts share the same domain' do
it 'associates all contacts with the same company' do
contacts = ['john@acme.com', 'jane@acme.com', 'bob@acme.com']
contacts.each do |contact|
create(:contact, email: contact, account: account)
end
expect(Company.where(domain: 'acme.com', account: account).count).to eq(1)
company = Company.find_by(domain: 'acme.com', account: account)
expect(company.contacts.count).to eq(contacts.length)
end
end
end
end
@@ -0,0 +1,99 @@
require 'rails_helper'
RSpec.describe Companies::BusinessEmailDetectorService, type: :service do
let(:service) { described_class.new(email) }
describe '#perform' do
context 'when email is from a business domain' do
let(:email) { 'user@acme.com' }
let(:valid_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: false) }
before do
allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
allow(EmailProviderInfo).to receive(:call).with(email).and_return(nil)
end
it 'returns true' do
expect(service.perform).to be(true)
end
end
context 'when email is from gmail' do
let(:email) { 'user@gmail.com' }
let(:valid_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: false) }
before do
allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
allow(EmailProviderInfo).to receive(:call).with(email).and_return('gmail')
end
it 'returns false' do
expect(service.perform).to be(false)
end
end
context 'when email is from Brazilian free provider' do
let(:email) { 'user@uol.com.br' }
let(:valid_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: false) }
before do
allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
allow(EmailProviderInfo).to receive(:call).with(email).and_return('uol')
end
it 'returns false' do
expect(service.perform).to be(false)
end
end
context 'when email is disposable' do
let(:email) { 'user@mailinator.com' }
let(:disposable_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: true) }
it 'returns false' do
allow(ValidEmail2::Address).to receive(:new).with(email).and_return(disposable_email_address)
expect(service.perform).to be(false)
end
end
context 'when email is invalid format' do
let(:email) { 'invalid-email' }
let(:invalid_email_address) { instance_double(ValidEmail2::Address, valid?: false) }
it 'returns false' do
allow(ValidEmail2::Address).to receive(:new).with(email).and_return(invalid_email_address)
expect(service.perform).to be(false)
end
end
context 'when email is nil' do
let(:email) { nil }
it 'remains false' do
expect(service.perform).to be(false)
end
end
context 'when email is empty string' do
let(:email) { '' }
it 'returns false' do
expect(service.perform).to be(false)
end
end
context 'when email domain is uppercase' do
let(:email) { 'user@GMAIL.COM' }
let(:valid_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: false) }
before do
allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
allow(EmailProviderInfo).to receive(:call).with(email).and_return('gmail')
end
it 'returns false (case insensitive)' do
expect(service.perform).to be(false)
end
end
end
end
@@ -0,0 +1,83 @@
require 'rails_helper'
RSpec.describe Contacts::CompanyAssociationService, type: :service do
let(:account) { create(:account) }
let(:service) { described_class.new }
describe '#associate_company_from_email' do
context 'when contact has business email and no company' do
it 'creates a new company and associates it' do
contact = create(:contact, email: 'john@acme.com', account: account, company_id: nil)
Company.delete_all # Delete any companies created by the callback
# rubocop:disable Rails/SkipsModelValidations
contact.update_column(:company_id, nil) # Delete the company association created by the callback
# rubocop:enable Rails/SkipsModelValidations
valid_email_address = instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: false)
allow(ValidEmail2::Address).to receive(:new).with('john@acme.com').and_return(valid_email_address)
allow(EmailProviderInfo).to receive(:call).with('john@acme.com').and_return(nil)
expect do
service.associate_company_from_email(contact)
end.to change(Company, :count).by(1)
contact.reload
expect(contact.company).to be_present
expect(contact.company.domain).to eq('acme.com')
expect(contact.company.name).to eq('Acme')
end
it 'reuses existing company with same domain' do
existing_company = create(:company, domain: 'acme.com', account: account)
contact = create(:contact, email: 'john@acme.com', account: account, company_id: nil)
# rubocop:disable Rails/SkipsModelValidations
contact.update_column(:company_id, nil) # Delete the company association created by the callback
# rubocop:enable Rails/SkipsModelValidations
valid_email_address = instance_double(ValidEmail2::Address, valid?: true, disposable_domain?: false)
allow(ValidEmail2::Address).to receive(:new).with('john@acme.com').and_return(valid_email_address)
allow(EmailProviderInfo).to receive(:call).with('john@acme.com').and_return(nil)
expect do
service.associate_company_from_email(contact)
end.not_to change(Company, :count)
contact.reload
expect(contact.company).to eq(existing_company)
end
end
context 'when contact already has a company' do
it 'skips association and returns nil' do
existing_company = create(:company, account: account)
contact = create(:contact, email: 'john@acme.com', account: account, company_id: existing_company.id)
result = service.associate_company_from_email(contact)
expect(result).to be_nil
contact.reload
expect(contact.company).to eq(existing_company)
end
end
context 'when contact has free email provider' do
it 'skips association for email' do
contact = create(:contact, email: 'john@gmail.com', account: account, company_id: nil)
expect do
service.associate_company_from_email(contact)
end.not_to change(Company, :count)
contact.reload
expect(contact.company).to be_nil
end
end
context 'when contact has no email' do
it 'skips association' do
contact = create(:contact, email: nil, account: account, company_id: nil)
result = service.associate_company_from_email(contact)
expect(result).to be_nil
expect(contact.reload.company).to be_nil
end
end
end
end
@@ -0,0 +1,78 @@
require 'rails_helper'
RSpec.describe Messages::SearchDataPresenter do
let(:presenter) { described_class.new(message) }
let(:account) { create(:account) }
let(:inbox) { create(:inbox, account: account) }
let(:contact) { create(:contact, account: account) }
let(:conversation) { create(:conversation, account: account, inbox: inbox, contact: contact) }
let(:message) { create(:message, account: account, inbox: inbox, conversation: conversation, sender: contact) }
describe '#search_data' do
let(:expected_data) do
{
content: message.content,
account_id: message.account_id,
inbox_id: message.inbox_id,
conversation_id: message.conversation_id,
message_type: message.message_type,
private: message.private,
created_at: message.created_at,
source_id: message.source_id,
sender_id: message.sender_id,
sender_type: message.sender_type,
conversation: {
id: conversation.display_id
}
}
end
it 'returns search index payload with core fields' do
expect(presenter.search_data).to include(expected_data)
end
context 'with attachments' do
before do
attachment = message.attachments.new(account_id: message.account_id, file_type: :image)
attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
attachment.meta = { 'transcribed_text' => 'Hello world' }
end
it 'includes attachment transcriptions' do
attachments_data = presenter.search_data[:attachments]
expect(attachments_data).to be_an(Array)
expect(attachments_data.first).to include(transcribed_text: 'Hello world')
end
end
context 'with email content attributes' do
before do
message.update(
content_attributes: { email: { subject: 'Test Subject' } }
)
end
it 'includes email subject' do
content_attrs = presenter.search_data[:content_attributes]
expect(content_attrs[:email][:subject]).to eq('Test Subject')
end
end
context 'with campaign and automation data' do
before do
message.update(
additional_attributes: { 'campaign_id' => '123' },
content_attributes: { 'automation_rule_id' => '456' }
)
end
it 'includes campaign_id' do
expect(presenter.search_data[:campaign_id]).to eq('123')
end
it 'includes automation_rule_id' do
expect(presenter.search_data[:automation_rule_id]).to eq('456')
end
end
end
end