From e117deb2bafd52e55704f85455a32bb660e7961d Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Tue, 4 Dec 2018 15:09:32 +0000
Subject: [PATCH] FIX: Improve avatar loading, and add tests

Follow-up from 4e2cc9c
---
 lib/auth/managed_authenticator.rb             |  4 +--
 .../auth/managed_authenticator_spec.rb        | 34 +++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb
index 9df6151f787..9805623ccf4 100644
--- a/lib/auth/managed_authenticator.rb
+++ b/lib/auth/managed_authenticator.rb
@@ -72,7 +72,7 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
         credentials: auth_token[:credentials] || {},
         extra: auth_token[:extra] || {}
       )
-      retrieve_avatar(result.user, auth_token[:info][:image])
+      retrieve_avatar(result.user, auth_token.dig(:info, :image))
     end
 
     result.email_valid = true if result.email
@@ -94,7 +94,7 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
   def after_create_account(user, auth)
     data = auth[:extra_data]
     create_association!(data.merge(user: user))
-    retrieve_avatar(user, data&.[]("info")&.[]("image"))
+    retrieve_avatar(user, data.dig(:info, :image))
   end
 
   def retrieve_avatar(user, url)
diff --git a/spec/components/auth/managed_authenticator_spec.rb b/spec/components/auth/managed_authenticator_spec.rb
index da02c870521..6fed762f6f9 100644
--- a/spec/components/auth/managed_authenticator_spec.rb
+++ b/spec/components/auth/managed_authenticator_spec.rb
@@ -114,6 +114,40 @@ describe Auth::ManagedAuthenticator do
         expect(result.user.id).to eq(user.id)
       end
     end
+
+    describe "avatar on update" do
+      let(:user) { Fabricate(:user) }
+      let!(:associated) { UserAssociatedAccount.create!(user: user, provider_name: 'myauth', provider_uid: "1234") }
+
+      it "schedules the job upon update correctly" do
+        # No image supplied, do not schedule
+        expect { result = authenticator.after_authenticate(hash) }
+          .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(0)
+
+        # Image supplied, schedule
+        expect { result = authenticator.after_authenticate(hash.deep_merge(info: { image: "https://some.domain/image.jpg" })) }
+          .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(1)
+
+        # User already has profile picture, don't schedule
+        user.user_avatar = Fabricate(:user_avatar, custom_upload: Fabricate(:upload))
+        user.save!
+        expect { result = authenticator.after_authenticate(hash.deep_merge(info: { image: "https://some.domain/image.jpg" })) }
+          .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(0)
+      end
+    end
+
+    describe "avatar on create" do
+      let(:user) { Fabricate(:user) }
+      it "doesn't schedule with no image" do
+        expect { result = authenticator.after_create_account(user, extra_data: hash) }
+          .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(0)
+      end
+
+      it "schedules with image" do
+        expect { result = authenticator.after_create_account(user, extra_data: hash.deep_merge(info: { image: "https://some.domain/image.jpg" })) }
+          .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(1)
+      end
+    end
   end
 
   describe 'description_for_user' do