From 05a37f69e13250abe291b39f5a5f8926a717b5e6 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Fri, 30 Aug 2019 17:26:10 +0000 Subject: Fix for #56295, https://gitlab.com/gitlab-org/gitlab-ce/issues/56295. All avatars now visible in commit trailers. --- app/helpers/avatars_helper.rb | 9 ++--- ...some-avatars-not-visible-in-commit-trailers.yml | 5 +++ lib/banzai/filter/commit_trailers_filter.rb | 3 +- spec/helpers/avatars_helper_spec.rb | 42 ++++++++++++++++++++++ .../banzai/filter/commit_trailers_filter_spec.rb | 21 +++++++++++ 5 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/56295-some-avatars-not-visible-in-commit-trailers.yml diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 81ff359556d..b7f7e617825 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -56,13 +56,13 @@ module AvatarsHelper })) end - def user_avatar_url_for(options = {}) + def user_avatar_url_for(only_path: true, **options) if options[:url] options[:url] elsif options[:user] - avatar_icon_for_user(options[:user], options[:size]) + avatar_icon_for_user(options[:user], options[:size], only_path: only_path) else - avatar_icon_for_email(options[:user_email], options[:size]) + avatar_icon_for_email(options[:user_email], options[:size], only_path: only_path) end end @@ -75,6 +75,7 @@ module AvatarsHelper has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip] data_attributes = options[:data] || {} css_class = %W[avatar s#{avatar_size}].push(*options[:css_class]) + alt_text = user_name ? "#{user_name}'s avatar" : "default avatar" if has_tooltip css_class.push('has-tooltip') @@ -88,7 +89,7 @@ module AvatarsHelper end image_options = { - alt: "#{user_name}'s avatar", + alt: alt_text, src: avatar_url, data: data_attributes, class: css_class, diff --git a/changelogs/unreleased/56295-some-avatars-not-visible-in-commit-trailers.yml b/changelogs/unreleased/56295-some-avatars-not-visible-in-commit-trailers.yml new file mode 100644 index 00000000000..23450c0fdc3 --- /dev/null +++ b/changelogs/unreleased/56295-some-avatars-not-visible-in-commit-trailers.yml @@ -0,0 +1,5 @@ +--- +title: Fix for missing avatar images dislpayed in commit trailers. +merge_request: 32374 +author: Jesse Hall @jessehall3 +type: fixed diff --git a/lib/banzai/filter/commit_trailers_filter.rb b/lib/banzai/filter/commit_trailers_filter.rb index f49c4b403db..02a47556151 100644 --- a/lib/banzai/filter/commit_trailers_filter.rb +++ b/lib/banzai/filter/commit_trailers_filter.rb @@ -88,7 +88,8 @@ module Banzai user: user, user_email: email, css_class: 'avatar-inline', - has_tooltip: false + has_tooltip: false, + only_path: false ) link_href = user.nil? ? "mailto:#{email}" : urls.user_url(user) diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index 94998d302f9..6fbb6147d84 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -324,5 +324,47 @@ describe AvatarsHelper do ) end end + + context 'with only_path parameter set to false' do + let(:user_with_avatar) { create(:user, :with_avatar, username: 'foobar') } + + context 'with user parameter' do + let(:options) { { user: user_with_avatar, only_path: false } } + + it 'will return avatar with a full path' do + is_expected.to eq tag( + :img, + alt: "#{user_with_avatar.name}'s avatar", + src: avatar_icon_for_user(user_with_avatar, 16, only_path: false), + data: { container: 'body' }, + class: "avatar s16 has-tooltip", + title: user_with_avatar.name + ) + end + end + + context 'with user_name and user_email' do + let(:options) { { user_email: user_with_avatar.email, user_name: user_with_avatar.username, only_path: false } } + + it 'will return avatar with a full path' do + is_expected.to eq tag( + :img, + alt: "#{user_with_avatar.username}'s avatar", + src: avatar_icon_for_email(user_with_avatar.email, 16, only_path: false), + data: { container: 'body' }, + class: "avatar s16 has-tooltip", + title: user_with_avatar.username + ) + end + end + end + + context 'with unregistered email address' do + let(:options) { { user_email: "unregistered_email@example.com" } } + + it 'will return default alt text for avatar' do + expect(subject).to include("default avatar") + end + end end end diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb index bcb74be1034..192d00805e0 100644 --- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -189,5 +189,26 @@ describe Banzai::Filter::CommitTrailersFilter do expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) expect(doc.text).to include(commit_body) end + + context 'with Gitlab-hosted avatars in commit trailers' do + # Because commit trailers are contained within markdown, + # any path-only link will automatically be prefixed + # with the path of its repository. + # See: "build_relative_path" in "lib/banzai/filter/relative_link_filter.rb" + let(:user_with_avatar) { create(:user, :with_avatar, username: 'foobar') } + + it 'returns a full path for avatar urls' do + _, message_html = build_commit_message( + trailer: trailer, + name: user_with_avatar.name, + email: user_with_avatar.email + ) + + doc = filter(message_html) + expected = "#{Gitlab.config.gitlab.url}#{user_with_avatar.avatar_url}" + + expect(doc.css('img')[0].attr('src')).to start_with expected + end + end end end -- cgit v1.2.1