Commit 5b92d405 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'dm-commit-trailer-without-gravatar' into 'master'

Fix commit trailer rendering when Gravatar is disabled

Closes #45633

See merge request gitlab-org/gitlab-ce!18586
parents 110a3779 35227d3b
...@@ -17,7 +17,7 @@ export default { ...@@ -17,7 +17,7 @@ export default {
}, },
computed: { computed: {
/** /**
* This method is based on app/helpers/application_helper.rb#project_identicon * This method is based on app/helpers/avatars_helper.rb#project_identicon
*/ */
identiconStyles() { identiconStyles() {
const allowedColors = [ const allowedColors = [
......
...@@ -32,80 +32,6 @@ module ApplicationHelper ...@@ -32,80 +32,6 @@ module ApplicationHelper
args.any? { |v| v.to_s.downcase == action_name } args.any? { |v| v.to_s.downcase == action_name }
end end
def project_icon(project_id, options = {})
project =
if project_id.respond_to?(:avatar_url)
project_id
else
Project.find_by_full_path(project_id)
end
if project.avatar_url
image_tag project.avatar_url, options
else # generated icon
project_identicon(project, options)
end
end
def project_identicon(project, options = {})
allowed_colors = {
red: 'FFEBEE',
purple: 'F3E5F5',
indigo: 'E8EAF6',
blue: 'E3F2FD',
teal: 'E0F2F1',
orange: 'FBE9E7',
gray: 'EEEEEE'
}
options[:class] ||= ''
options[:class] << ' identicon'
bg_key = project.id % 7
style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555"
content_tag(:div, class: options[:class], style: style) do
project.name[0, 1].upcase
end
end
# Takes both user and email and returns the avatar_icon by
# user (preferred) or email.
def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
if user
avatar_icon_for_user(user, size, scale, only_path: only_path)
elsif email
avatar_icon_for_email(email, size, scale, only_path: only_path)
else
default_avatar
end
end
def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
user = User.find_by_any_email(email.try(:downcase))
if user
avatar_icon_for_user(user, size, scale, only_path: only_path)
else
gravatar_icon(email, size, scale)
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
end
def gravatar_icon(user_email = '', size = nil, scale = 2)
GravatarService.new.execute(user_email, size, scale) ||
default_avatar
end
def default_avatar
asset_path('no_avatar.png')
end
def last_commit(project) def last_commit(project)
if project.repo_exists? if project.repo_exists?
time_ago_with_tooltip(project.repository.commit.committed_date) time_ago_with_tooltip(project.repository.commit.committed_date)
......
module AvatarsHelper module AvatarsHelper
def project_icon(project_id, options = {})
project =
if project_id.respond_to?(:avatar_url)
project_id
else
Project.find_by_full_path(project_id)
end
if project.avatar_url
image_tag project.avatar_url, options
else # generated icon
project_identicon(project, options)
end
end
def project_identicon(project, options = {})
allowed_colors = {
red: 'FFEBEE',
purple: 'F3E5F5',
indigo: 'E8EAF6',
blue: 'E3F2FD',
teal: 'E0F2F1',
orange: 'FBE9E7',
gray: 'EEEEEE'
}
options[:class] ||= ''
options[:class] << ' identicon'
bg_key = project.id % 7
style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555"
content_tag(:div, class: options[:class], style: style) do
project.name[0, 1].upcase
end
end
# Takes both user and email and returns the avatar_icon by
# user (preferred) or email.
def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
if user
avatar_icon_for_user(user, size, scale, only_path: only_path)
elsif email
avatar_icon_for_email(email, size, scale, only_path: only_path)
else
default_avatar
end
end
def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
user = User.find_by_any_email(email.try(:downcase))
if user
avatar_icon_for_user(user, size, scale, only_path: only_path)
else
gravatar_icon(email, size, scale)
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
end
def gravatar_icon(user_email = '', size = nil, scale = 2)
GravatarService.new.execute(user_email, size, scale) ||
default_avatar
end
def default_avatar
ActionController::Base.helpers.image_path('no_avatar.png')
end
def author_avatar(commit_or_event, options = {}) def author_avatar(commit_or_event, options = {})
user_avatar(options.merge({ user_avatar(options.merge({
user: commit_or_event.author, user: commit_or_event.author,
......
...@@ -16,6 +16,7 @@ class Notify < BaseMailer ...@@ -16,6 +16,7 @@ class Notify < BaseMailer
helper BlobHelper helper BlobHelper
helper EmailsHelper helper EmailsHelper
helper MembersHelper helper MembersHelper
helper AvatarsHelper
helper GitlabRoutingHelper helper GitlabRoutingHelper
def test_email(recipient_email, subject, body) def test_email(recipient_email, subject, body)
......
---
title: Fix commit trailer rendering when Gravatar is disabled
merge_request:
author:
type: fixed
...@@ -13,7 +13,6 @@ module Banzai ...@@ -13,7 +13,6 @@ module Banzai
# * https://git.wiki.kernel.org/index.php/CommitMessageConventions # * https://git.wiki.kernel.org/index.php/CommitMessageConventions
class CommitTrailersFilter < HTML::Pipeline::Filter class CommitTrailersFilter < HTML::Pipeline::Filter
include ActionView::Helpers::TagHelper include ActionView::Helpers::TagHelper
include ApplicationHelper
include AvatarsHelper include AvatarsHelper
TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze
......
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
require 'spec_helper' require 'spec_helper'
describe ApplicationHelper do describe ApplicationHelper do
include UploadHelpers
describe 'current_controller?' do describe 'current_controller?' do
it 'returns true when controller matches argument' do it 'returns true when controller matches argument' do
stub_controller_name('foo') stub_controller_name('foo')
...@@ -54,143 +52,6 @@ describe ApplicationHelper do ...@@ -54,143 +52,6 @@ describe ApplicationHelper do
end end
end end
describe 'project_icon' do
it 'returns an url for the avatar' do
project = create(:project, :public, avatar: File.open(uploaded_image_temp_path))
expect(helper.project_icon(project.full_path).to_s)
.to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
end
end
describe 'avatar_icon_for' do
let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
let(:email) { 'foo@example.com' }
let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
it 'prefers the user to retrieve the avatar_url' do
expect(helper.avatar_icon_for(user, email).to_s)
.to eq(user.avatar.url)
end
it 'falls back to email lookup if no user given' do
expect(helper.avatar_icon_for(nil, email).to_s)
.to eq(another_user.avatar.url)
end
end
describe 'avatar_icon_for_email' do
let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
context 'using an email' do
context 'when there is a matching user' do
it 'returns a relative URL for the avatar' do
expect(helper.avatar_icon_for_email(user.email).to_s)
.to eq(user.avatar.url)
end
end
context 'when no user exists for the email' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
helper.avatar_icon_for_email('foo@example.com', 20, 2)
end
end
context 'without an email passed' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
helper.avatar_icon_for_email(nil, 20, 2)
end
end
end
end
describe 'avatar_icon_for_user' do
let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
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
end
context 'without a user object passed' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
helper.avatar_icon_for_user(nil, 20, 2)
end
end
end
describe 'gravatar_icon' do
let(:user_email) { 'user@email.com' }
context 'with Gravatar disabled' do
before do
stub_application_setting(gravatar_enabled?: false)
end
it 'returns a generic avatar' do
expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png')
end
end
context 'with Gravatar enabled' do
before do
stub_application_setting(gravatar_enabled?: true)
end
it 'returns a generic avatar when email is blank' do
expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png')
end
it 'returns a valid Gravatar URL' do
stub_config_setting(https: false)
expect(helper.gravatar_icon(user_email))
.to match('https://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118')
end
it 'uses HTTPs when configured' do
stub_config_setting(https: true)
expect(helper.gravatar_icon(user_email))
.to match('https://secure.gravatar.com')
end
it 'returns custom gravatar path when gravatar_url is set' do
stub_gravatar_setting(plain_url: 'http://example.local/?s=%{size}&hash=%{hash}')
expect(gravatar_icon(user_email, 20))
.to eq('http://example.local/?s=40&hash=b58c6f14d292556214bd64909bcdb118')
end
it 'accepts a custom size argument' do
expect(helper.gravatar_icon(user_email, 64)).to include '?s=128'
end
it 'defaults size to 40@2x when given an invalid size' do
expect(helper.gravatar_icon(user_email, nil)).to include '?s=80'
end
it 'accepts a scaling factor' do
expect(helper.gravatar_icon(user_email, 40, 3)).to include '?s=120'
end
it 'ignores case and surrounding whitespace' do
normal = helper.gravatar_icon('foo@example.com')
upcase = helper.gravatar_icon(' FOO@EXAMPLE.COM ')
expect(normal).to eq upcase
end
end
end
describe 'simple_sanitize' do describe 'simple_sanitize' do
let(:a_tag) { '<a href="#">Foo</a>' } let(:a_tag) { '<a href="#">Foo</a>' }
......
require 'rails_helper' require 'rails_helper'
describe AvatarsHelper do describe AvatarsHelper do
include ApplicationHelper include UploadHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
describe '#project_icon' do
it 'returns an url for the avatar' do
project = create(:project, :public, avatar: File.open(uploaded_image_temp_path))
expect(helper.project_icon(project.full_path).to_s)
.to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
end
end
describe '#avatar_icon_for' do
let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
let(:email) { 'foo@example.com' }
let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
it 'prefers the user to retrieve the avatar_url' do
expect(helper.avatar_icon_for(user, email).to_s)
.to eq(user.avatar.url)
end
it 'falls back to email lookup if no user given' do
expect(helper.avatar_icon_for(nil, email).to_s)
.to eq(another_user.avatar.url)
end
end
describe '#avatar_icon_for_email' do
let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
context 'using an email' do
context 'when there is a matching user' do
it 'returns a relative URL for the avatar' do
expect(helper.avatar_icon_for_email(user.email).to_s)
.to eq(user.avatar.url)
end
end
context 'when no user exists for the email' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
helper.avatar_icon_for_email('foo@example.com', 20, 2)
end
end
context 'without an email passed' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
helper.avatar_icon_for_email(nil, 20, 2)
end
end
end
end
describe '#avatar_icon_for_user' do
let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
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
end
context 'without a user object passed' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
helper.avatar_icon_for_user(nil, 20, 2)
end
end
end
describe '#gravatar_icon' do
let(:user_email) { 'user@email.com' }
context 'with Gravatar disabled' do
before do
stub_application_setting(gravatar_enabled?: false)
end
it 'returns a generic avatar' do
expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png')
end
end
context 'with Gravatar enabled' do
before do
stub_application_setting(gravatar_enabled?: true)
end
it 'returns a generic avatar when email is blank' do
expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png')
end
it 'returns a valid Gravatar URL' do
stub_config_setting(https: false)
expect(helper.gravatar_icon(user_email))
.to match('https://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118')
end
it 'uses HTTPs when configured' do
stub_config_setting(https: true)
expect(helper.gravatar_icon(user_email))
.to match('https://secure.gravatar.com')
end
it 'returns custom gravatar path when gravatar_url is set' do
stub_gravatar_setting(plain_url: 'http://example.local/?s=%{size}&hash=%{hash}')
expect(gravatar_icon(user_email, 20))
.to eq('http://example.local/?s=40&hash=b58c6f14d292556214bd64909bcdb118')
end
it 'accepts a custom size argument' do
expect(helper.gravatar_icon(user_email, 64)).to include '?s=128'
end
it 'defaults size to 40@2x when given an invalid size' do
expect(helper.gravatar_icon(user_email, nil)).to include '?s=80'
end
it 'accepts a scaling factor' do
expect(helper.gravatar_icon(user_email, 40, 3)).to include '?s=120'
end
it 'ignores case and surrounding whitespace' do
normal = helper.gravatar_icon('foo@example.com')
upcase = helper.gravatar_icon(' FOO@EXAMPLE.COM ')
expect(normal).to eq upcase
end
end
end
describe '#user_avatar' do describe '#user_avatar' do
subject { helper.user_avatar(user: user) } subject { helper.user_avatar(user: user) }
......
...@@ -47,7 +47,9 @@ describe Banzai::Filter::CommitTrailersFilter do ...@@ -47,7 +47,9 @@ describe Banzai::Filter::CommitTrailersFilter do
) )
end end
it 'non GitLab users and replaces them with mailto links' do context 'non GitLab users' do
shared_examples 'mailto links' do
it 'replaces them with mailto links' do
_, message_html = build_commit_message( _, message_html = build_commit_message(
trailer: trailer, trailer: trailer,
name: FFaker::Name.name, name: FFaker::Name.name,
...@@ -56,7 +58,25 @@ describe Banzai::Filter::CommitTrailersFilter do ...@@ -56,7 +58,25 @@ describe Banzai::Filter::CommitTrailersFilter do
doc = filter(message_html) doc = filter(message_html)
expect_to_have_mailto_link(doc, email: email, trailer: trailer) expect_to_have_mailto_link_with_avatar(doc, email: email, trailer: trailer)
end
end
context 'when Gravatar is disabled' do
before do
stub_application_setting(gravatar_enabled: false)
end
it_behaves_like 'mailto links'
end
context 'when Gravatar is enabled' do
before do
stub_application_setting(gravatar_enabled: true)
end
it_behaves_like 'mailto links'
end
end end
it 'multiple trailers in the same message' do it 'multiple trailers in the same message' do
...@@ -69,7 +89,7 @@ describe Banzai::Filter::CommitTrailersFilter do ...@@ -69,7 +89,7 @@ describe Banzai::Filter::CommitTrailersFilter do
doc = filter(message) doc = filter(message)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
expect_to_have_mailto_link(doc, email: email, trailer: different_trailer) expect_to_have_mailto_link_with_avatar(doc, email: email, trailer: different_trailer)
end end
context 'special names' do context 'special names' do
...@@ -90,7 +110,7 @@ describe Banzai::Filter::CommitTrailersFilter do ...@@ -90,7 +110,7 @@ describe Banzai::Filter::CommitTrailersFilter do
doc = filter(message_html) doc = filter(message_html)
expect_to_have_mailto_link(doc, email: email, trailer: trailer) expect_to_have_mailto_link_with_avatar(doc, email: email, trailer: trailer)
expect(doc.text).to match Regexp.escape(message) expect(doc.text).to match Regexp.escape(message)
end end
end end
......
...@@ -8,7 +8,7 @@ module CommitTrailersSpecHelper ...@@ -8,7 +8,7 @@ module CommitTrailersSpecHelper
expect(wrapper.attribute('data-user').value).to eq user.id.to_s expect(wrapper.attribute('data-user').value).to eq user.id.to_s
end end
def expect_to_have_mailto_link(doc, email:, trailer:) def expect_to_have_mailto_link_with_avatar(doc, email:, trailer:)
wrapper = find_user_wrapper(doc, trailer) wrapper = find_user_wrapper(doc, trailer)
expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email) expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment