Commit f8365c3d authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch 'feature_detect_co_authored_commits' into 'master'

Add banzai filter to detect commit message trailers and properly link the users

Closes #31640

See merge request gitlab-org/gitlab-ce!17919
parents b15dd5df a069aa49
......@@ -32,7 +32,8 @@ class Commit
COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze
def banzai_render_context(field)
context = { pipeline: :single_line, project: self.project }
pipeline = field == :description ? :commit_description : :single_line
context = { pipeline: pipeline, project: self.project }
context[:author] = self.author if self.author
context
......
......@@ -49,10 +49,10 @@
.commit-box{ data: { project_path: project_path(@project) } }
%h3.commit-title
= markdown(@commit.title, pipeline: :single_line, author: @commit.author)
= markdown_field(@commit, :title)
- if @commit.description.present?
%pre.commit-description
= preserve(markdown(@commit.description, pipeline: :single_line, author: @commit.author))
= preserve(markdown_field(@commit, :description))
.info-well
.well-segment.branch-info
......
......@@ -10,5 +10,5 @@ xml.entry do
xml.email commit.author_email
end
xml.summary markdown(commit.description, pipeline: :single_line), type: 'html'
xml.summary markdown_field(commit, :description), type: 'html'
end
---
title: Detect commit message trailers and link users properly to their accounts
on Gitlab
merge_request: 17919
author: cousine
type: added
module Banzai
module Filter
# HTML filter that replaces users' names and emails in commit trailers
# with links to their GitLab accounts or mailto links to their mentioned
# emails.
#
# Commit trailers are special labels in the form of `*-by:` and fall on a
# single line, ex:
#
# Reported-By: John S. Doe <john.doe@foo.bar>
#
# More info about this can be found here:
# * https://git.wiki.kernel.org/index.php/CommitMessageConventions
class CommitTrailersFilter < HTML::Pipeline::Filter
include ActionView::Helpers::TagHelper
include ApplicationHelper
include AvatarsHelper
TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze
AUTHOR_REGEXP = /(?<author_name>.+)/.freeze
# Devise.email_regexp wouldn't work here since its designed to match
# against strings that only contains email addresses; the \A and \z
# around the expression will only match if the string being matched
# contains just the email nothing else.
MAIL_REGEXP = /&lt;(?<author_email>[^@\s]+@[^@\s]+)&gt;/.freeze
FILTER_REGEXP = /(?<trailer>^\s*#{TRAILER_REGEXP}\s*#{AUTHOR_REGEXP}\s+#{MAIL_REGEXP}$)/mi.freeze
def call
doc.xpath('descendant-or-self::text()').each do |node|
content = node.to_html
next unless content.match(FILTER_REGEXP)
html = trailer_filter(content)
next if html == content
node.replace(html)
end
doc
end
private
# Replace trailer lines with links to GitLab users or mailto links to
# non GitLab users.
#
# text - String text to replace trailers in.
#
# Returns a String with all trailer lines replaced with links to GitLab
# users and mailto links to non GitLab users. All links have `data-trailer`
# and `data-user` attributes attached.
def trailer_filter(text)
text.gsub(FILTER_REGEXP) do |author_match|
label = $~[:label]
"#{label} #{parse_user($~[:author_name], $~[:author_email], label)}"
end
end
# Find a GitLab user using the supplied email and generate
# a valid link to them, otherwise, generate a mailto link.
#
# name - String name used in the commit message for the user
# email - String email used in the commit message for the user
# trailer - String trailer used in the commit message
#
# Returns a String with a link to the user.
def parse_user(name, email, trailer)
link_to_user User.find_by_any_email(email),
name: name,
email: email,
trailer: trailer
end
def urls
Gitlab::Routing.url_helpers
end
def link_to_user(user, name:, email:, trailer:)
wrapper = link_wrapper(data: {
trailer: trailer,
user: user.try(:id)
})
avatar = user_avatar_without_link(
user: user,
user_email: email,
css_class: 'avatar-inline',
has_tooltip: false
)
link_href = user.nil? ? "mailto:#{email}" : urls.user_url(user)
avatar_link = link_tag(
link_href,
content: avatar,
title: email
)
name_link = link_tag(
link_href,
content: name,
title: email
)
email_link = link_tag(
"mailto:#{email}",
content: email,
title: email
)
wrapper << "#{avatar_link}#{name_link} <#{email_link}>"
end
def link_wrapper(data: {})
data_attributes = data_attributes_from_hash(data)
doc.document.create_element(
'span',
data_attributes
)
end
def link_tag(url, title: "", content: "", data: {})
data_attributes = data_attributes_from_hash(data)
attributes = data_attributes.merge(
href: url,
title: title
)
link = doc.document.create_element('a', attributes)
if content.html_safe?
link << content
else
link.content = content # make sure we escape content using nokogiri's #content=
end
link
end
def data_attributes_from_hash(data = {})
data.reject! {|_, value| value.nil?}
data.map do |key, value|
[%(data-#{key.to_s.dasherize}), value]
end.to_h
end
end
end
end
module Banzai
module Pipeline
class CommitDescriptionPipeline < SingleLinePipeline
def self.filters
@filters ||= super.concat FilterArray[
Filter::CommitTrailersFilter,
]
end
end
end
end
require 'spec_helper'
require 'ffaker'
describe Banzai::Filter::CommitTrailersFilter do
include FilterSpecHelper
include CommitTrailersSpecHelper
let(:secondary_email) { create(:email, :confirmed) }
let(:user) { create(:user) }
let(:trailer) { "#{FFaker::Lorem.word}-by:"}
let(:commit_message) { trailer_line(trailer, user.name, user.email) }
let(:commit_message_html) { commit_html(commit_message) }
context 'detects' do
let(:email) { FFaker::Internet.email }
it 'trailers in the form of *-by and replace users with links' do
doc = filter(commit_message_html)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
end
it 'trailers prefixed with whitespaces' do
message_html = commit_html("\n\r #{commit_message}")
doc = filter(message_html)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
end
it 'GitLab users via a secondary email' do
_, message_html = build_commit_message(
trailer: trailer,
name: secondary_email.user.name,
email: secondary_email.email
)
doc = filter(message_html)
expect_to_have_user_link_with_avatar(
doc,
user: secondary_email.user,
trailer: trailer,
email: secondary_email.email
)
end
it 'non GitLab users and replaces them with mailto links' do
_, message_html = build_commit_message(
trailer: trailer,
name: FFaker::Name.name,
email: email
)
doc = filter(message_html)
expect_to_have_mailto_link(doc, email: email, trailer: trailer)
end
it 'multiple trailers in the same message' do
different_trailer = "#{FFaker::Lorem.word}-by:"
message = commit_html %(
#{commit_message}
#{trailer_line(different_trailer, FFaker::Name.name, email)}
)
doc = filter(message)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
expect_to_have_mailto_link(doc, email: email, trailer: different_trailer)
end
context 'special names' do
where(:name) do
[
'John S. Doe',
'L33t H@x0r'
]
end
with_them do
it do
message, message_html = build_commit_message(
trailer: trailer,
name: name,
email: email
)
doc = filter(message_html)
expect_to_have_mailto_link(doc, email: email, trailer: trailer)
expect(doc.text).to match Regexp.escape(message)
end
end
end
end
context "ignores" do
it 'commit messages without trailers' do
exp = message = commit_html(FFaker::Lorem.sentence)
doc = filter(message)
expect(doc.to_html).to match Regexp.escape(exp)
end
it 'trailers that are inline the commit message body' do
message = commit_html %(
#{FFaker::Lorem.sentence} #{commit_message} #{FFaker::Lorem.sentence}
)
doc = filter(message)
expect(doc.css('a').size).to eq 0
end
end
context "structure" do
it 'preserves the commit trailer structure' do
doc = filter(commit_message_html)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
expect(doc.text).to match Regexp.escape(commit_message)
end
it 'preserves the original name used in the commit message' do
message, message_html = build_commit_message(
trailer: trailer,
name: FFaker::Name.name,
email: user.email
)
doc = filter(message_html)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
expect(doc.text).to match Regexp.escape(message)
end
it 'preserves the original email used in the commit message' do
message, message_html = build_commit_message(
trailer: trailer,
name: secondary_email.user.name,
email: secondary_email.email
)
doc = filter(message_html)
expect_to_have_user_link_with_avatar(
doc,
user: secondary_email.user,
trailer: trailer,
email: secondary_email.email
)
expect(doc.text).to match Regexp.escape(message)
end
it 'only replaces trailer lines not the full commit message' do
commit_body = FFaker::Lorem.paragraph
message = commit_html %(
#{commit_body}
#{commit_message}
)
doc = filter(message)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
expect(doc.text).to include(commit_body)
end
end
end
module CommitTrailersSpecHelper
extend ActiveSupport::Concern
def expect_to_have_user_link_with_avatar(doc, user:, trailer:, email: nil)
wrapper = find_user_wrapper(doc, trailer)
expect_to_have_links_with_url_and_avatar(wrapper, urls.user_url(user), email || user.email)
expect(wrapper.attribute('data-user').value).to eq user.id.to_s
end
def expect_to_have_mailto_link(doc, email:, trailer:)
wrapper = find_user_wrapper(doc, trailer)
expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email)
end
def expect_to_have_links_with_url_and_avatar(doc, url, email)
expect(doc).not_to be_nil
expect(doc.xpath("a[position()<3 and @href='#{url}']").size).to eq 2
expect(doc.xpath("a[position()=3 and @href='mailto:#{CGI.escape_html(email)}']").size).to eq 1
expect(doc.css('img').size).to eq 1
end
def find_user_wrapper(doc, trailer)
doc.xpath("descendant-or-self::node()[@data-trailer='#{trailer}']").first
end
def build_commit_message(trailer:, name:, email:)
message = trailer_line(trailer, name, email)
[message, commit_html(message)]
end
def trailer_line(trailer, name, email)
"#{trailer} #{name} <#{email}>"
end
def commit_html(message)
"<pre>#{CGI.escape_html(message)}</pre>"
end
end
Supports Markdown
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