chore: Ability to Disable Gravatars (#5027)

fixes: #3853

- Introduced DISABLE_GRAVATAR Global Config, which will stop chatwoot from making API requests to gravatar
- Cleaned up avatar-related logic and centralized it into the avatarable concern
- Added specs for the missing cases
- Added migration for existing installations to move the avatar to attachment, rather than making the API that results in 404.

app/actions/contact_identify_action.rb

@@ -104,7 +104,7 @@ class ContactIdentifyAction
     # TODO: replace reject { |_k, v| v.blank? } with compact_blank when rails is upgraded
     @contact.discard_invalid_attrs if discard_invalid_attrs
     @contact.save!
-    ContactAvatarJob.perform_later(@contact, params[:avatar_url]) if params[:avatar_url].present?
+    Avatar::AvatarFromUrlJob.perform_later(@contact, params[:avatar_url]) if params[:avatar_url].present?
   end
 
   def merge_contact(base_contact, merge_contact)

app/builders/contact_builder.rb

@@ -23,7 +23,7 @@ class ContactBuilder
   end
 
   def update_contact_avatar(contact)
-    ::ContactAvatarJob.perform_later(contact, contact_attributes[:avatar_url]) if contact_attributes[:avatar_url]
+    ::Avatar::AvatarFromUrlJob.perform_later(contact, contact_attributes[:avatar_url]) if contact_attributes[:avatar_url]
   end
 
   def create_contact

app/builders/messages/facebook/message_builder.rb

@@ -58,7 +58,7 @@ class Messages::Facebook::MessageBuilder < Messages::Messenger::MessageBuilder
     return if contact_params[:remote_avatar_url].blank?
     return if @contact.avatar.attached?
 
-    ContactAvatarJob.perform_later(@contact, contact_params[:remote_avatar_url])
+    Avatar::AvatarFromUrlJob.perform_later(@contact, contact_params[:remote_avatar_url])
   end
 
   def conversation

app/controllers/api/v1/accounts/callbacks_controller.rb

@@ -90,9 +90,7 @@ class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController
   end
 
   def set_avatar(facebook_inbox, page_id)
-    avatar_file = Down.download(
+    avatar_url = "https://graph.facebook.com/#{page_id}/picture?type=large"
-      "http://graph.facebook.com/#{page_id}/picture?type=large"
+    Avatar::AvatarFromUrlJob.perform_later(facebook_inbox, avatar_url)
-    )
-    facebook_inbox.avatar.attach(io: avatar_file, filename: avatar_file.original_filename, content_type: avatar_file.content_type)
   end
 end

app/controllers/api/v1/accounts/contacts_controller.rb

@@ -166,13 +166,7 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
   end
 
   def process_avatar
-    if permitted_params[:avatar].blank? && permitted_params[:avatar_url].present?
+    ::Avatar::AvatarFromUrlJob.perform_later(@contact, params[:avatar_url]) if params[:avatar_url].present?
-      ::ContactAvatarJob.perform_later(@contact, params[:avatar_url])
-    elsif permitted_params[:avatar].blank? && permitted_params[:email].present?
-      hash = Digest::MD5.hexdigest(params[:email])
-      gravatar_url = "https://www.gravatar.com/avatar/#{hash}?d=404"
-      ::ContactAvatarJob.perform_later(@contact, gravatar_url)
-    end
   end
 
   def render_error(error, error_status)

app/jobs/avatar/avatar_from_gravatar_job.rb

@@ -0,0 +1,13 @@
+class Avatar::AvatarFromGravatarJob < ApplicationJob
+  queue_as :low
+
+  def perform(avatarable, email)
+    return if GlobalConfigService.load('DISABLE_GRAVATAR', '').present?
+    return if email.blank?
+    return if avatarable.avatar_url.present?
+
+    hash = Digest::MD5.hexdigest(email)
+    gravatar_url = "https://www.gravatar.com/avatar/#{hash}?d=404"
+    Avatar::AvatarFromUrlJob.perform_later(avatarable, gravatar_url)
+  end
+end

app/jobs/avatar/avatar_from_url_job.rb

@@ -0,0 +1,15 @@
+class Avatar::AvatarFromUrlJob < ApplicationJob
+  queue_as :default
+
+  def perform(avatarable, avatar_url)
+    return unless avatarable.respond_to?(:avatar)
+
+    avatar_file = Down.download(
+      avatar_url,
+      max_size: 15 * 1024 * 1024
+    )
+    avatarable.avatar.attach(io: avatar_file, filename: avatar_file.original_filename, content_type: avatar_file.content_type)
+  rescue Down::NotFound, Down::Error => e
+    Rails.logger.error "Exception: invalid avatar url #{avatar_url} : #{e.message}"
+  end
+end

app/jobs/contact_avatar_job.rb

@@ -1,15 +0,0 @@
-class ContactAvatarJob < ApplicationJob
-  queue_as :default
-
-  def perform(contact, avatar_url)
-    avatar_file = Down.download(
-      avatar_url,
-      max_size: 15 * 1024 * 1024
-    )
-    contact.avatar.attach(io: avatar_file, filename: avatar_file.original_filename, content_type: avatar_file.content_type)
-  rescue Down::NotFound
-    contact.avatar.attachment.destroy! if contact.avatar.attached?
-  rescue Down::Error => e
-    Rails.logger.error "Exception: invalid avatar url #{avatar_url} : #{e.message}"
-  end
-end

app/models/concerns/avatarable.rb

@@ -7,19 +7,24 @@ module Avatarable
   included do
     has_one_attached :avatar
     validate :acceptable_avatar, if: -> { avatar.changed? }
+    after_save :fetch_avatar_from_gravatar
   end
 
   def avatar_url
     return url_for(avatar.representation(resize: '250x250')) if avatar.attached? && avatar.representable?
 
-    if [SuperAdmin, User, Contact].include?(self.class) && email.present?
-      hash = Digest::MD5.hexdigest(email)
-      return "https://www.gravatar.com/avatar/#{hash}?d=404"
-    end
-
     ''
   end
 
+  def fetch_avatar_from_gravatar
+    return unless saved_changes.key?(:email)
+    return if email.blank?
+
+    # Incase avatar_url is supplied, we don't want to fetch avatar from gravatar
+    # So we will wait for it to be processed
+    Avatar::AvatarFromGravatarJob.set(wait: 30.seconds).perform_later(self, email)
+  end
+
   def acceptable_avatar
     return unless avatar.attached?
 

app/services/instagram/webhooks_base_service.rb

@@ -16,6 +16,6 @@ class Instagram::WebhooksBaseService
     )
 
     @contact = @contact_inbox.contact
-    ContactAvatarJob.perform_later(@contact, user['profile_pic']) if user['profile_pic']
+    Avatar::AvatarFromUrlJob.perform_later(@contact, user['profile_pic']) if user['profile_pic']
   end
 end

app/services/telegram/incoming_message_service.rb

@@ -45,7 +45,7 @@ class Telegram::IncomingMessageService
     return if @contact.avatar.attached?
 
     avatar_url = inbox.channel.get_telegram_profile_image(params[:message][:from][:id])
-    ::ContactAvatarJob.perform_later(@contact, avatar_url) if avatar_url
+    ::Avatar::AvatarFromUrlJob.perform_later(@contact, avatar_url) if avatar_url
   end
 
   def conversation_params

app/services/twitter/webhooks_base_service.rb

@@ -30,6 +30,6 @@ class Twitter::WebhooksBaseService
       user['id'], user['name'], additional_contact_attributes(user)
     )
     @contact = @contact_inbox.contact
-    ContactAvatarJob.perform_later(@contact, user['profile_image_url']) if user['profile_image_url']
+    Avatar::AvatarFromUrlJob.perform_later(@contact, user['profile_image_url']) if user['profile_image_url']
   end
 end

db/migrate/20220712145440_sync_gravatar_for_existing_avatarables.rb

@@ -0,0 +1,26 @@
+class SyncGravatarForExistingAvatarables < ActiveRecord::Migration[6.1]
+  def change
+    return if GlobalConfigService.load('DISABLE_GRAVATAR', '').present?
+
+    sync_user_avatars
+    sync_contact_avatars
+  end
+
+  private
+
+  def sync_user_avatars
+    ::User.find_in_batches do |users_batch|
+      users_batch.each do |user|
+        Avatar::AvatarFromGravatarJob.perform_later(user, user.email)
+      end
+    end
+  end
+
+  def sync_contact_avatars
+    ::Contact.where.not(email: nil).find_in_batches do |contacts_batch|
+      contacts_batch.each do |contact|
+        Avatar::AvatarFromGravatarJob.perform_later(contact, contact.email)
+      end
+    end
+  end
+end

spec/actions/contact_identify_action_spec.rb

@@ -13,7 +13,7 @@ describe ::ContactIdentifyAction do
 
   describe '#perform' do
     it 'updates the contact' do
-      expect(ContactAvatarJob).not_to receive(:perform_later).with(contact, params[:avatar_url])
+      expect(Avatar::AvatarFromUrlJob).not_to receive(:perform_later).with(contact, params[:avatar_url])
       contact_identify
       expect(contact.reload.name).to eq 'test'
       # custom attributes are merged properly without overwriting existing ones
@@ -32,7 +32,7 @@ describe ::ContactIdentifyAction do
 
     it 'enques avatar job when avatar url parameter is passed' do
       params = { name: 'test', avatar_url: 'https://chatwoot-assets.local/sample.png' }
-      expect(ContactAvatarJob).to receive(:perform_later).with(contact, params[:avatar_url]).once
+      expect(Avatar::AvatarFromUrlJob).to receive(:perform_later).with(contact, params[:avatar_url]).once
       described_class.new(contact: contact, params: params).perform
     end
 

spec/builders/v2/report_builder_spec.rb

@@ -1,6 +1,7 @@
 require 'rails_helper'
 
 describe ::V2::ReportBuilder do
+  include ActiveJob::TestHelper
   let!(:account) { create(:account) }
   let!(:user) { create(:user, account: account) }
   let!(:inbox) { create(:inbox, account: account) }
@@ -8,49 +9,44 @@ describe ::V2::ReportBuilder do
   let!(:label_1) { create(:label, title: 'Label_1', account: account) }
   let!(:label_2) { create(:label, title: 'Label_2', account: account) }
 
-  # Running jobs inline to calculate the exact metrics
-  around do |test|
-    current_adapter = ActiveJob::Base.queue_adapter
-    ActiveJob::Base.queue_adapter = :inline
-
-    test.run
-  ensure
-    ActiveJob::Base.queue_adapter = current_adapter
-  end
-
   describe '#timeseries' do
     before do
-      10.times do
+      gravatar_url = 'https://www.gravatar.com'
-        conversation = create(:conversation, account: account,
+      stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404)
-                                             inbox: inbox, assignee: user,
-                                             created_at: Time.zone.today)
-        create_list(:message, 5, message_type: 'outgoing',
-                                 account: account, inbox: inbox,
-                                 conversation: conversation, created_at: Time.zone.today + 2.hours)
-        create_list(:message, 2, message_type: 'incoming',
-                                 account: account, inbox: inbox,
-                                 conversation: conversation,
-                                 created_at: Time.zone.today + 3.hours)
-        conversation.update_labels('label_1')
-        conversation.label_list
-        conversation.save!
-      end
 
-      5.times do
+      perform_enqueued_jobs do
-        conversation = create(:conversation, account: account,
+        10.times do
-                                             inbox: inbox, assignee: user,
+          conversation = create(:conversation, account: account,
-                                             created_at: (Time.zone.today - 2.days))
+                                               inbox: inbox, assignee: user,
-        create_list(:message, 3, message_type: 'outgoing',
+                                               created_at: Time.zone.today)
-                                 account: account, inbox: inbox,
+          create_list(:message, 5, message_type: 'outgoing',
-                                 conversation: conversation,
+                                   account: account, inbox: inbox,
-                                 created_at: (Time.zone.today - 2.days))
+                                   conversation: conversation, created_at: Time.zone.today + 2.hours)
-        create_list(:message, 1, message_type: 'incoming',
+          create_list(:message, 2, message_type: 'incoming',
-                                 account: account, inbox: inbox,
+                                   account: account, inbox: inbox,
-                                 conversation: conversation,
+                                   conversation: conversation,
-                                 created_at: (Time.zone.today - 2.days))
+                                   created_at: Time.zone.today + 3.hours)
-        conversation.update_labels('label_2')
+          conversation.update_labels('label_1')
-        conversation.label_list
+          conversation.label_list
-        conversation.save!
+          conversation.save!
+        end
+
+        5.times do
+          conversation = create(:conversation, account: account,
+                                               inbox: inbox, assignee: user,
+                                               created_at: (Time.zone.today - 2.days))
+          create_list(:message, 3, message_type: 'outgoing',
+                                   account: account, inbox: inbox,
+                                   conversation: conversation,
+                                   created_at: (Time.zone.today - 2.days))
+          create_list(:message, 1, message_type: 'incoming',
+                                   account: account, inbox: inbox,
+                                   conversation: conversation,
+                                   created_at: (Time.zone.today - 2.days))
+          conversation.update_labels('label_2')
+          conversation.label_list
+          conversation.save!
+        end
       end
     end
 

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

@@ -15,7 +15,7 @@ RSpec.describe 'Api::V1::Accounts::BulkActionsController', type: :request do
 
   describe 'POST /api/v1/accounts/{account.id}/bulk_action' do
     context 'when it is an unauthenticated user' do
-      let(:agent) { create(:user) }
+      let!(:agent) { create(:user) }
 
       it 'returns unauthorized' do
         post "/api/v1/accounts/#{account.id}/bulk_actions",
@@ -27,7 +27,7 @@ RSpec.describe 'Api::V1::Accounts::BulkActionsController', type: :request do
     end
 
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
+      let!(:agent) { create(:user, account: account, role: :agent) }
 
       it 'Ignores bulk_actions for wrong type' do
         post "/api/v1/accounts/#{account.id}/bulk_actions",
@@ -117,7 +117,7 @@ RSpec.describe 'Api::V1::Accounts::BulkActionsController', type: :request do
 
   describe 'POST /api/v1/accounts/{account.id}/bulk_actions' do
     context 'when it is an authenticated user' do
-      let(:agent) { create(:user, account: account, role: :agent) }
+      let!(:agent) { create(:user, account: account, role: :agent) }
 
       it 'Bulk delete conversation labels' do
         Conversation.first.add_labels(%w[support priority_customer])

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

@@ -487,6 +487,14 @@ RSpec.describe 'Contacts API', type: :request do
         contact.reload
         expect(contact.avatar.attached?).to be(true)
       end
+
+      it 'updated avatar with avatar_url' do
+        patch "/api/v1/accounts/#{account.id}/contacts/#{contact.id}",
+              params: valid_params.merge(avatar_url: 'http://example.com/avatar.png'),
+              headers: admin.create_new_auth_token
+        expect(response).to have_http_status(:success)
+        expect(Avatar::AvatarFromUrlJob).to have_been_enqueued.with(contact, 'http://example.com/avatar.png')
+      end
     end
   end
 

spec/factories/inbox_members.rb

@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :inbox_member do
-    user
+    user { create(:user, :with_avatar) }
     inbox
   end
 end

spec/jobs/avatar/avatar_from_gravatar_job_spec.rb

@@ -0,0 +1,29 @@
+require 'rails_helper'
+
+RSpec.describe Avatar::AvatarFromGravatarJob, type: :job do
+  let(:avatarable) { create(:contact) }
+  let(:email) { 'test@test.com' }
+  let(:gravatar_url) { "https://www.gravatar.com/avatar/#{Digest::MD5.hexdigest(email)}?d=404" }
+
+  it 'enqueues the job' do
+    expect { described_class.perform_later(avatarable, email) }.to have_enqueued_job(described_class)
+      .on_queue('low')
+  end
+
+  it 'will call AvatarFromUrlJob with gravatar url' do
+    expect(Avatar::AvatarFromUrlJob).to receive(:perform_later).with(avatarable, gravatar_url)
+    described_class.perform_now(avatarable, email)
+  end
+
+  it 'will not call AvatarFromUrlJob if DISABLE_GRAVATAR is configured' do
+    with_modified_env DISABLE_GRAVATAR: 'true' do
+      expect(Avatar::AvatarFromUrlJob).not_to receive(:perform_later).with(avatarable, gravatar_url)
+      described_class.perform_now(avatarable, '')
+    end
+  end
+
+  it 'will not call AvatarFromUrlJob if email is blank' do
+    expect(Avatar::AvatarFromUrlJob).not_to receive(:perform_later).with(avatarable, gravatar_url)
+    described_class.perform_now(avatarable, '')
+  end
+end

spec/jobs/avatar/avatar_from_url_job_spec.rb

@@ -0,0 +1,20 @@
+require 'rails_helper'
+
+RSpec.describe Avatar::AvatarFromUrlJob, type: :job do
+  let(:avatarable) { create(:contact) }
+  let(:avatar_url) { 'https://example.com/avatar.png' }
+
+  it 'enqueues the job' do
+    expect { described_class.perform_later(avatarable, avatar_url) }.to have_enqueued_job(described_class)
+      .on_queue('default')
+  end
+
+  it 'will attach avatar from url' do
+    expect(avatarable.avatar).not_to be_attached
+    expect(Down).to receive(:download).with(avatar_url,
+                                            max_size: 15 * 1024 * 1024).and_return(fixture_file_upload(Rails.root.join('spec/assets/avatar.png'),
+                                                                                                       'image/png'))
+    described_class.perform_now(avatarable, avatar_url)
+    expect(avatarable.avatar).to be_attached
+  end
+end

spec/models/agent_bot_spec.rb

@@ -1,5 +1,6 @@
 require 'rails_helper'
 require Rails.root.join 'spec/models/concerns/access_tokenable_shared.rb'
+require Rails.root.join 'spec/models/concerns/avatarable_shared.rb'
 
 RSpec.describe AgentBot, type: :model do
   describe 'associations' do
@@ -9,5 +10,6 @@ RSpec.describe AgentBot, type: :model do
 
   describe 'concerns' do
     it_behaves_like 'access_tokenable'
+    it_behaves_like 'avatarable'
   end
 end

spec/models/concerns/avatarable_shared.rb

@@ -0,0 +1,41 @@
+require 'rails_helper'
+
+shared_examples_for 'avatarable' do
+  let(:avatarable) { create(described_class.to_s.underscore) }
+
+  it { is_expected.to have_one_attached(:avatar) }
+
+  it 'add avatar_url method' do
+    expect(avatarable.respond_to?(:avatar_url)).to be true
+  end
+
+  context 'when avatarable has an email attribute' do
+    it 'enques job when email is changed on avatarable create' do
+      avatarable = build(described_class.to_s.underscore, account: create(:account))
+      if avatarable.respond_to?(:email)
+        avatarable.email = 'test@test.com'
+        avatarable.skip_reconfirmation! if avatarable.is_a? User
+        expect(Avatar::AvatarFromGravatarJob).to receive(:set).with(wait: 30.seconds).and_call_original
+      end
+      avatarable.save!
+      expect(Avatar::AvatarFromGravatarJob).to have_been_enqueued.with(avatarable, avatarable.email) if avatarable.respond_to?(:email)
+    end
+
+    it 'enques job when email is changes on avatarable update' do
+      if avatarable.respond_to?(:email)
+        avatarable.email = 'xyc@test.com'
+        avatarable.skip_reconfirmation! if avatarable.is_a? User
+        expect(Avatar::AvatarFromGravatarJob).to receive(:set).with(wait: 30.seconds).and_call_original
+      end
+      avatarable.save!
+      expect(Avatar::AvatarFromGravatarJob).to have_been_enqueued.with(avatarable, avatarable.email) if avatarable.respond_to?(:email)
+    end
+
+    it 'will not enqueu when email is not changed on avatarable update' do
+      avatarable.updated_at = Time.now.utc
+      expect do
+        avatarable.save!
+      end.not_to have_enqueued_job(Avatar::AvatarFromGravatarJob)
+    end
+  end
+end

spec/models/contact_spec.rb

@@ -2,6 +2,8 @@
 
 require 'rails_helper'
 
+require Rails.root.join 'spec/models/concerns/avatarable_shared.rb'
+
 RSpec.describe Contact do
   context 'validations' do
     it { is_expected.to validate_presence_of(:account_id) }
@@ -12,6 +14,10 @@ RSpec.describe Contact do
     it { is_expected.to have_many(:conversations).dependent(:destroy_async) }
   end
 
+  describe 'concerns' do
+    it_behaves_like 'avatarable'
+  end
+
   context 'prepare contact attributes before validation' do
     it 'sets email to lowercase' do
       contact = create(:contact, email: 'Test@test.com')

spec/models/inbox_spec.rb

@@ -2,6 +2,7 @@
 
 require 'rails_helper'
 require Rails.root.join 'spec/models/concerns/out_of_offisable_shared.rb'
+require Rails.root.join 'spec/models/concerns/avatarable_shared.rb'
 
 RSpec.describe Inbox do
   describe 'validations' do
@@ -37,6 +38,7 @@ RSpec.describe Inbox do
 
   describe 'concerns' do
     it_behaves_like 'out_of_offisable'
+    it_behaves_like 'avatarable'
   end
 
   describe '#add_member' do

spec/models/user_spec.rb

@@ -2,6 +2,7 @@
 
 require 'rails_helper'
 require Rails.root.join 'spec/models/concerns/access_tokenable_shared.rb'
+require Rails.root.join 'spec/models/concerns/avatarable_shared.rb'
 
 RSpec.describe User do
   let!(:user) { create(:user) }
@@ -25,6 +26,7 @@ RSpec.describe User do
 
   describe 'concerns' do
     it_behaves_like 'access_tokenable'
+    it_behaves_like 'avatarable'
   end
 
   describe 'pubsub_token' do