diff --git a/app/jobs/avatar/avatar_from_favicon_job.rb b/app/jobs/avatar/avatar_from_favicon_job.rb new file mode 100644 index 000000000..6459cf77b --- /dev/null +++ b/app/jobs/avatar/avatar_from_favicon_job.rb @@ -0,0 +1,11 @@ +class Avatar::AvatarFromFaviconJob < ApplicationJob + queue_as :purgable + + def perform(company) + return if company.domain.blank? + return if company.avatar.attached? + + favicon_url = "https://www.google.com/s2/favicons?domain=#{company.domain}&sz=256" + Avatar::AvatarFromUrlJob.perform_now(company, favicon_url) + end +end diff --git a/app/jobs/companies/fetch_avatars_job.rb b/app/jobs/companies/fetch_avatars_job.rb new file mode 100644 index 000000000..d29e91e37 --- /dev/null +++ b/app/jobs/companies/fetch_avatars_job.rb @@ -0,0 +1,17 @@ +class Companies::FetchAvatarsJob < ApplicationJob + queue_as :low + + def perform(account_id) + account = Account.find(account_id) + companies = account.companies.where.not(domain: [nil, '']) + .left_joins(:avatar_attachment) + .where(active_storage_attachments: { id: nil }) + + total_companies = companies.count + companies.find_each do |company| + Avatar::AvatarFromFaviconJob.perform_later(company) + end + + Rails.logger.info "Queued #{total_companies} companies from account #{account_id} for favicon fetch" + end +end diff --git a/enterprise/app/models/company.rb b/enterprise/app/models/company.rb index d96344f9b..8581675bd 100644 --- a/enterprise/app/models/company.rb +++ b/enterprise/app/models/company.rb @@ -30,11 +30,13 @@ class Company < ApplicationRecord belongs_to :account has_many :contacts, dependent: :nullify + after_create_commit :fetch_favicon, if: -> { domain.present? } scope :ordered_by_name, -> { order(:name) } scope :search_by_name_or_domain, lambda { |query| where('name ILIKE :search OR domain ILIKE :search', search: "%#{query.strip}%") } + scope :order_on_contacts_count, lambda { |direction| order( Arel::Nodes::SqlLiteral.new( @@ -42,4 +44,10 @@ class Company < ApplicationRecord ) ) } + + private + + def fetch_favicon + Avatar::AvatarFromFaviconJob.set(wait: 5.seconds).perform_later(self) + end end diff --git a/enterprise/lib/tasks/companies.rake b/enterprise/lib/tasks/companies.rake new file mode 100644 index 000000000..729591d38 --- /dev/null +++ b/enterprise/lib/tasks/companies.rake @@ -0,0 +1,31 @@ +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 + + desc 'Fetch favicons for companies without avatars' + task fetch_missing_avatars: :environment do + account_ids = companies_without_avatars + + account_ids.each do |account_id| + Companies::FetchAvatarsJob.perform_later(account_id) + end + + puts "Queued #{account_ids.count} accounts for favicon fetch" + end +end + +def companies_without_avatars + Company.left_joins(:avatar_attachment) + .where(active_storage_attachments: { id: nil }) + .where.not(domain: [nil, '']) + .distinct + .pluck(:account_id) +end diff --git a/lib/tasks/companies.rake b/lib/tasks/companies.rake deleted file mode 100644 index 11fb5dc10..000000000 --- a/lib/tasks/companies.rake +++ /dev/null @@ -1,12 +0,0 @@ -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 diff --git a/spec/builders/v2/report_builder_spec.rb b/spec/builders/v2/report_builder_spec.rb index 3f86b0348..b6bf83f0a 100644 --- a/spec/builders/v2/report_builder_spec.rb +++ b/spec/builders/v2/report_builder_spec.rb @@ -16,7 +16,9 @@ describe V2::ReportBuilder do create(:inbox_member, user: user, inbox: inbox) gravatar_url = 'https://www.gravatar.com' + favicon_url = 'https://www.google.com/s2/favicons' stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404) + stub_request(:get, /#{Regexp.escape(favicon_url)}.*/).to_return(status: 404) perform_enqueued_jobs do 10.times do diff --git a/spec/builders/v2/reports/label_summary_builder_spec.rb b/spec/builders/v2/reports/label_summary_builder_spec.rb index f0eb6cefd..750f82241 100644 --- a/spec/builders/v2/reports/label_summary_builder_spec.rb +++ b/spec/builders/v2/reports/label_summary_builder_spec.rb @@ -18,6 +18,11 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do end let(:builder) { described_class.new(account: account, params: params) } + def stub_avatar_requests + stub_request(:get, %r{\Ahttps://www\.gravatar\.com.*}).to_return(status: 404) + stub_request(:get, %r{\Ahttps://www\.google\.com/s2/favicons.*}).to_return(status: 404) + end + describe '#initialize' do let(:business_hours) { false } @@ -85,8 +90,7 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do inbox = create(:inbox, account: account) create(:inbox_member, user: user, inbox: inbox) - gravatar_url = 'https://www.gravatar.com' - stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404) + stub_avatar_requests perform_enqueued_jobs do # Create conversations with label_1 @@ -223,8 +227,7 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do inbox = create(:inbox, account: account) create(:inbox_member, user: user, inbox: inbox) - gravatar_url = 'https://www.gravatar.com' - stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404) + stub_avatar_requests perform_enqueued_jobs do # Conversation within range @@ -281,8 +284,7 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do inbox = create(:inbox, account: account) create(:inbox_member, user: user, inbox: inbox) - gravatar_url = 'https://www.gravatar.com' - stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404) + stub_avatar_requests perform_enqueued_jobs do conversation = create(:conversation, account: account, @@ -338,8 +340,7 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do inbox = create(:inbox, account: account2) create(:inbox_member, user: user, inbox: inbox) - gravatar_url = 'https://www.gravatar.com' - stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404) + stub_avatar_requests perform_enqueued_jobs do conversation = create(:conversation, account: account2, @@ -349,13 +350,8 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do conversation.label_list conversation.save! - # First resolution conversation.resolved! - - # Reopen conversation conversation.open! - - # Second resolution conversation.resolved! end end diff --git a/spec/enterprise/jobs/avatar/avatar_from_favicon_job_spec.rb b/spec/enterprise/jobs/avatar/avatar_from_favicon_job_spec.rb new file mode 100644 index 000000000..5cd767de4 --- /dev/null +++ b/spec/enterprise/jobs/avatar/avatar_from_favicon_job_spec.rb @@ -0,0 +1,27 @@ +require 'rails_helper' + +RSpec.describe Avatar::AvatarFromFaviconJob do + let(:company) { create(:company, domain: 'wikipedia.org') } + let(:favicon_url) { 'https://www.google.com/s2/favicons?domain=wikipedia.org&sz=256' } + + it 'calls AvatarFromUrlJob with Google Favicon URL' do + expect(Avatar::AvatarFromUrlJob).to receive(:perform_now).with(company, favicon_url) + described_class.perform_now(company) + end + + it 'does not call AvatarFromUrlJob when domain is blank' do + company.update(domain: '') + expect(Avatar::AvatarFromUrlJob).not_to receive(:perform_now) + described_class.perform_now(company) + end + + it 'does not call AvatarFromUrlJob when avatar is already attached' do + company.avatar.attach( + io: Rails.root.join('spec/assets/avatar.png').open, + filename: 'avatar.png', + content_type: 'image/png' + ) + expect(Avatar::AvatarFromUrlJob).not_to receive(:perform_now) + described_class.perform_now(company) + end +end diff --git a/spec/enterprise/jobs/companies/fetch_avatars_job_spec.rb b/spec/enterprise/jobs/companies/fetch_avatars_job_spec.rb new file mode 100644 index 000000000..902cec6fd --- /dev/null +++ b/spec/enterprise/jobs/companies/fetch_avatars_job_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe Companies::FetchAvatarsJob do + let(:account) { create(:account) } + let!(:company_with_avatar) { create(:company, account: account, domain: 'example.com') } + let!(:company_without_avatar) { create(:company, account: account, domain: 'wikipedia.org') } + let!(:company_no_domain) { create(:company, account: account, domain: nil) } + + before do + # Attach avatar to first company + company_with_avatar.avatar.attach( + io: Rails.root.join('spec/assets/avatar.png').open, + filename: 'avatar.png', + content_type: 'image/png' + ) + end + + it 'queues Avatar::AvatarFromFaviconJob only for companies without avatars' do + expect(Avatar::AvatarFromFaviconJob).to receive(:perform_later).with(company_without_avatar).once + expect(Avatar::AvatarFromFaviconJob).not_to receive(:perform_later).with(company_with_avatar) + expect(Avatar::AvatarFromFaviconJob).not_to receive(:perform_later).with(company_no_domain) + + described_class.perform_now(account.id) + end +end diff --git a/spec/enterprise/models/conversation_spec.rb b/spec/enterprise/models/conversation_spec.rb index 50d9ee993..f7116eb54 100644 --- a/spec/enterprise/models/conversation_spec.rb +++ b/spec/enterprise/models/conversation_spec.rb @@ -6,9 +6,14 @@ RSpec.describe Conversation, type: :model do end describe 'SLA policy updates' do - let!(:conversation) { create(:conversation) } + let(:conversation) { create(:conversation) } let!(:sla_policy) { create(:sla_policy, account: conversation.account) } + before do + stub_request(:get, %r{\Ahttps://www\.gravatar\.com.*}).to_return(status: 404) + stub_request(:get, %r{\Ahttps://www\.google\.com/s2/favicons.*}).to_return(status: 404) + end + it 'generates an activity message when the SLA policy is updated' do conversation.update!(sla_policy_id: sla_policy.id) diff --git a/spec/services/account/sign_up_email_validation_service_spec.rb b/spec/services/account/sign_up_email_validation_service_spec.rb index 3f907f02c..a4b1231be 100644 --- a/spec/services/account/sign_up_email_validation_service_spec.rb +++ b/spec/services/account/sign_up_email_validation_service_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Account::SignUpEmailValidationService, type: :service do it 'raises InvalidEmail with invalid message' do allow(ValidEmail2::Address).to receive(:new).with(email).and_return(invalid_email_address) expect { service.perform }.to raise_error do |error| - expect(error).to be_a(CustomExceptions::Account::InvalidEmail) + expect(error.class.name).to eq('CustomExceptions::Account::InvalidEmail') expect(error.message).to eq(I18n.t('errors.signup.invalid_email')) end end @@ -32,7 +32,7 @@ RSpec.describe Account::SignUpEmailValidationService, type: :service do it 'raises InvalidEmail with blocked domain message' do allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address) expect { service.perform }.to raise_error do |error| - expect(error).to be_a(CustomExceptions::Account::InvalidEmail) + expect(error.class.name).to eq('CustomExceptions::Account::InvalidEmail') expect(error.message).to eq(I18n.t('errors.signup.blocked_domain')) end end @@ -44,7 +44,7 @@ RSpec.describe Account::SignUpEmailValidationService, type: :service do it 'raises InvalidEmail with blocked domain message' do allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address) expect { service.perform }.to raise_error do |error| - expect(error).to be_a(CustomExceptions::Account::InvalidEmail) + expect(error.class.name).to eq('CustomExceptions::Account::InvalidEmail') expect(error.message).to eq(I18n.t('errors.signup.blocked_domain')) end end @@ -56,7 +56,7 @@ RSpec.describe Account::SignUpEmailValidationService, type: :service do it 'raises InvalidEmail with disposable message' do allow(ValidEmail2::Address).to receive(:new).with(email).and_return(disposable_email_address) expect { service.perform }.to raise_error do |error| - expect(error).to be_a(CustomExceptions::Account::InvalidEmail) + expect(error.class.name).to eq('CustomExceptions::Account::InvalidEmail') expect(error.message).to eq(I18n.t('errors.signup.disposable_email')) end end