Skip to content

G
gitlab-ce
Commit fadfe8e0 authored by Douwe Maan's avatar Douwe Maan
Browse files
Options

Merge branch 'fix-email-avatars' into 'master' 

Fix avatar images in pipeline and approval emails

See merge request !2182
parents 7834ae6b 02f866cc
Hide whitespace changes
Inline Side-by-side
Showing with 71 additions and 42 deletions
app/helpers/application_helper.rb
View file @ fadfe8e0
......@@ -68,7 +68,7 @@ module ApplicationHelper
end
end
def avatar_icon(user_or_email = nil, size = nil, scale = 2)
def avatar_icon(user_or_email = nil, size = nil, scale = 2, only_path: true)
user =
if user_or_email.is_a?(User)
user_or_email
......@@ -77,7 +77,7 @@ module ApplicationHelper
end
if user
user.avatar_url(size: size) || default_avatar
user.avatar_url(size: size, only_path: only_path) || default_avatar
else
gravatar_icon(user_or_email, size, scale)
end
......
app/views/notify/approved_merge_request_email.html.haml
View file @ fadfe8e0
......@@ -79,7 +79,7 @@
%span{ style: "font-weight: bold;color:#333333;" } Merge request
%a{ href: merge_request_url(@merge_request), style: "font-weight: bold;color:#3777b0;text-decoration:none" }= @merge_request.to_reference
%span was approved by
%img.avatar{ height: "24", src: avatar_icon(@approved_by, 24), style: "border-radius:12px;margin:-7px 0 -7px 3px;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(@approved_by, 24, only_path: false), style: "border-radius:12px;margin:-7px 0 -7px 3px;", width: "24", alt: "Avatar" }/
%a.muted{ href: user_url(@approved_by), style: "color:#333333;text-decoration:none;" }
= @approved_by.name
%tr.spacer
......@@ -117,7 +117,7 @@
%tbody
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon(@merge_request.author, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(@merge_request.author, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
%a.muted{ href: user_url(@merge_request.author), style: "color:#333333;text-decoration:none;" }
= @merge_request.author.name
......@@ -130,7 +130,7 @@
%tbody
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon(@merge_request.assignee, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(@merge_request.assignee, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
%a.muted{ href: user_url(@merge_request.assignee), style: "color:#333333;text-decoration:none;" }
= @merge_request.assignee.name
......
app/views/notify/pipeline_failed_email.html.haml
View file @ fadfe8e0
......@@ -60,7 +60,7 @@
%tbody
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
- if commit.author
%a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
......@@ -76,7 +76,7 @@
%tbody
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
- if commit.committer
%a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
......@@ -100,7 +100,7 @@
triggered by
- if @pipeline.user
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" }
%img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" }
%a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" }
= @pipeline.user.name
......
app/views/notify/pipeline_success_email.html.haml
View file @ fadfe8e0
......@@ -60,7 +60,7 @@
%tbody
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
- if commit.author
%a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" }
......@@ -76,7 +76,7 @@
%tbody
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
- if commit.committer
%a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" }
......@@ -100,7 +100,7 @@
triggered by
- if @pipeline.user
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" }
%img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" }
%a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" }
= @pipeline.user.name
......
spec/helpers/application_helper_spec.rb
View file @ fadfe8e0
......@@ -82,42 +82,71 @@ describe ApplicationHelper do
end
describe 'avatar_icon' do
it 'returns an url for the avatar' do
user = create(:user, avatar: File.open(uploaded_image_temp_path))
avatar_url = "/uploads/system/user/avatar/#{user.id}/banana_sample.gif"
expect(helper.avatar_icon(user.email).to_s).to match(avatar_url)
allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
avatar_url = "#{gitlab_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif"
expect(helper.avatar_icon(user.email).to_s).to match(avatar_url)
end
it 'returns an url for the avatar with relative url' do
stub_config_setting(relative_url_root: '/gitlab')
# Must be stubbed after the stub above, and separately
stub_config_setting(url: Settings.send(:build_gitlab_url))
user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user.email).to_s).
to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
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(user.email).to_s).
to eq("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
context 'when an asset_host is set in the config' do
let(:asset_host) { 'http://assets' }
before do
allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
end
it 'returns an absolute URL on that asset host' do
expect(helper.avatar_icon(user.email, only_path: false).to_s).
to eq("#{asset_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
context 'when only_path is set to false' do
it 'returns an absolute URL for the avatar' do
expect(helper.avatar_icon(user.email, only_path: false).to_s).
to eq("#{gitlab_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
context 'when the GitLab instance is at a relative URL' do
before do
stub_config_setting(relative_url_root: '/gitlab')
# Must be stubbed after the stub above, and separately
stub_config_setting(url: Settings.send(:build_gitlab_url))
end
it 'returns a relative URL with the correct prefix' do
expect(helper.avatar_icon(user.email).to_s).
to eq("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
end
it 'calls gravatar_icon when no User exists with the given email' do
expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
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('foo@example.com', 20, 2)
helper.avatar_icon('foo@example.com', 20, 2)
end
end
end
describe 'using a User' do
it 'returns an URL for the avatar' do
user = create(:user, avatar: File.open(uploaded_image_temp_path))
describe 'using a user' do
context 'when only_path is true' do
it 'returns a relative URL for the avatar' do
expect(helper.avatar_icon(user, only_path: true).to_s).
to eq("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
expect(helper.avatar_icon(user).to_s).
to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
context 'when only_path is false' do
it 'returns an absolute URL for the avatar' do
expect(helper.avatar_icon(user, only_path: false).to_s).
to eq("#{gitlab_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
end
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7