Merge branch 'dblessing_fix_avatar_for_blocked_users' into 'master'

Hide user avatar for blocked and unconfirmed users 2.0

See merge request gitlab-org/gitlab!77250

app/helpers/avatars_helper.rb

 # frozen_string_literal: true
 
 module AvatarsHelper
+  DEFAULT_AVATAR_PATH = 'no_avatar.png'
+
   def project_icon(project, options = {})
     source_icon(project, options)
   end
@@ -33,12 +35,12 @@ module AvatarsHelper
     end
   end
 
-  def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
-    if user
-      user.avatar_url(size: size, only_path: only_path) || default_avatar
-    else
-      gravatar_icon(nil, size, scale)
-    end
+  def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true, current_user: nil)
+    return gravatar_icon(nil, size, scale) unless user
+    return default_avatar if blocked_or_unconfirmed?(user) && !can_admin?(current_user)
+
+    user_avatar = user.avatar_url(size: size, only_path: only_path)
+    user_avatar || default_avatar
   end
 
   def gravatar_icon(user_email = '', size = nil, scale = 2)
@@ -47,7 +49,7 @@ module AvatarsHelper
   end
 
   def default_avatar
-    ActionController::Base.helpers.image_path('no_avatar.png')
+    ActionController::Base.helpers.image_path(DEFAULT_AVATAR_PATH)
   end
 
   def author_avatar(commit_or_event, options = {})
@@ -157,4 +159,14 @@ module AvatarsHelper
       source.name[0, 1].upcase
     end
   end
+
+  def blocked_or_unconfirmed?(user)
+    user.blocked? || !user.confirmed?
+  end
+
+  def can_admin?(user)
+    return false unless user
+
+    user.can_admin_all_resources?
+  end
 end

app/views/admin/users/show.html.haml

@@ -10,7 +10,7 @@
         = @user.name
       %ul.content-list
         %li
-          = image_tag avatar_icon_for_user(@user, 60), class: "avatar s60"
+          = image_tag avatar_icon_for_user(@user, 60, current_user: current_user), class: "avatar s60"
         %li
           %span.light= _('Profile page:')
           %strong

app/views/users/show.html.haml

@@ -50,8 +50,8 @@
 
     .profile-header{ class: [('with-no-profile-tabs' if profile_tabs.empty?)] }
       .avatar-holder
-        = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
-          = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: '', itemprop: 'image'
+        = link_to avatar_icon_for_user(@user, 400, current_user: current_user), target: '_blank', rel: 'noopener noreferrer' do
+          = image_tag avatar_icon_for_user(@user, 90, current_user: current_user), class: "avatar s90", alt: '', itemprop: 'image'
 
       - if @user.blocked? || !@user.confirmed?
         .user-info

spec/helpers/avatars_helper_spec.rb

@@ -146,11 +146,52 @@ RSpec.describe AvatarsHelper do
   describe '#avatar_icon_for_user' do
     let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
 
+    shared_examples 'blocked or unconfirmed user with avatar' do
+      context 'when the viewer is not an admin' do
+        let!(:viewing_user) { create(:user) }
+
+        it 'returns the default avatar' do
+          expect(helper.avatar_icon_for_user(user, current_user: viewing_user).to_s)
+            .to match_asset_path(described_class::DEFAULT_AVATAR_PATH)
+        end
+      end
+
+      context 'when the viewer is an admin', :enable_admin_mode do
+        let!(:viewing_user) { create(:user, :admin) }
+
+        it 'returns the default avatar when the user is not passed' do
+          expect(helper.avatar_icon_for_user(user).to_s)
+            .to match_asset_path(described_class::DEFAULT_AVATAR_PATH)
+        end
+
+        it 'returns the user avatar when the user is passed' do
+          expect(helper.avatar_icon_for_user(user, current_user: viewing_user).to_s)
+            .to eq(user.avatar.url)
+        end
+      end
+    end
+
     context 'with a user object passed' do
       it 'returns a relative URL for the avatar' do
         expect(helper.avatar_icon_for_user(user).to_s)
           .to eq(user.avatar.url)
       end
+
+      context 'when the user is blocked' do
+        before do
+          user.block!
+        end
+
+        it_behaves_like 'blocked or unconfirmed user with avatar'
+      end
+
+      context 'when the user is unconfirmed' do
+        before do
+          user.update!(confirmed_at: nil)
+        end
+
+        it_behaves_like 'blocked or unconfirmed user with avatar'
+      end
     end
 
     context 'without a user object passed' do
@@ -171,7 +212,7 @@ RSpec.describe AvatarsHelper do
       end
 
       it 'returns a generic avatar' do
-        expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png')
+        expect(helper.gravatar_icon(user_email)).to match_asset_path(described_class::DEFAULT_AVATAR_PATH)
       end
     end
 
@@ -181,7 +222,7 @@ RSpec.describe AvatarsHelper do
       end
 
       it 'returns a generic avatar when email is blank' do
-        expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png')
+        expect(helper.gravatar_icon('')).to match_asset_path(described_class::DEFAULT_AVATAR_PATH)
       end
 
       it 'returns a valid Gravatar URL' do