Commit d2f9a901 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'link-refs' into 'master'

Recognize issue/MR/snippet/commit links as references.

Fixes #3744 and #3745

See merge request !1933
parents 3c805177 f9d954fa
...@@ -4,6 +4,7 @@ v 8.3.0 (unreleased) ...@@ -4,6 +4,7 @@ v 8.3.0 (unreleased)
- Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera) - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera)
- Fix 500 error when update group member permission - Fix 500 error when update group member permission
- Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera) - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
- Recognize issue/MR/snippet/commit links as references
- Add ignore whitespace change option to commit view - Add ignore whitespace change option to commit view
- Fire update hook from GitLab - Fire update hook from GitLab
- Don't show project fork event as "imported" - Don't show project fork event as "imported"
......
...@@ -87,7 +87,11 @@ def issue_to_atom(xml, issue) ...@@ -87,7 +87,11 @@ def issue_to_atom(xml, issue)
end end
def merge_requests_sentence(merge_requests) def merge_requests_sentence(merge_requests)
merge_requests.map(&:to_reference).to_sentence(last_word_connector: ', or ') # Sorting based on the `!123` or `group/project!123` reference will sort
# local merge requests first.
merge_requests.map do |merge_request|
merge_request.to_reference(@project)
end.sort.to_sentence(last_word_connector: ', or ')
end end
def url_to_emoji(name) def url_to_emoji(name)
......
...@@ -39,7 +39,11 @@ def merge_path_description(merge_request, separator) ...@@ -39,7 +39,11 @@ def merge_path_description(merge_request, separator)
end end
def issues_sentence(issues) def issues_sentence(issues)
issues.map(&:to_reference).to_sentence # Sorting based on the `#123` or `group/project#123` reference will sort
# local issues first.
issues.map do |issue|
issue.to_reference(@project)
end.sort.to_sentence
end end
def mr_change_branches_path(merge_request) def mr_change_branches_path(merge_request)
......
...@@ -78,11 +78,23 @@ def self.reference_pattern ...@@ -78,11 +78,23 @@ def self.reference_pattern
}x }x
end end
def self.link_reference_pattern
super("commit", /(?<commit>\h{6,40})/)
end
def to_reference(from_project = nil) def to_reference(from_project = nil)
if cross_project_reference?(from_project) if cross_project_reference?(from_project)
"#{project.to_reference}@#{id}" project.to_reference + self.class.reference_prefix + self.id
else
self.id
end
end
def reference_link_text(from_project = nil)
if cross_project_reference?(from_project)
project.to_reference + self.class.reference_prefix + self.short_id
else else
id self.short_id
end end
end end
......
...@@ -2,36 +2,38 @@ ...@@ -2,36 +2,38 @@
# #
# Examples: # Examples:
# #
# range = CommitRange.new('f3f85602...e86e1013') # range = CommitRange.new('f3f85602...e86e1013', project)
# range.exclude_start? # => false # range.exclude_start? # => false
# range.reference_title # => "Commits f3f85602 through e86e1013" # range.reference_title # => "Commits f3f85602 through e86e1013"
# range.to_s # => "f3f85602...e86e1013" # range.to_s # => "f3f85602...e86e1013"
# #
# range = CommitRange.new('f3f856029bc5f966c5a7ee24cf7efefdd20e6019..e86e1013709735be5bb767e2b228930c543f25ae') # range = CommitRange.new('f3f856029bc5f966c5a7ee24cf7efefdd20e6019..e86e1013709735be5bb767e2b228930c543f25ae', project)
# range.exclude_start? # => true # range.exclude_start? # => true
# range.reference_title # => "Commits f3f85602^ through e86e1013" # range.reference_title # => "Commits f3f85602^ through e86e1013"
# range.to_param # => {from: "f3f856029bc5f966c5a7ee24cf7efefdd20e6019^", to: "e86e1013709735be5bb767e2b228930c543f25ae"} # range.to_param # => {from: "f3f856029bc5f966c5a7ee24cf7efefdd20e6019^", to: "e86e1013709735be5bb767e2b228930c543f25ae"}
# range.to_s # => "f3f85602..e86e1013" # range.to_s # => "f3f85602..e86e1013"
# #
# # Assuming `project` is a Project with a repository containing both commits: # # Assuming the specified project has a repository containing both commits:
# range.project = project
# range.valid_commits? # => true # range.valid_commits? # => true
# #
class CommitRange class CommitRange
include ActiveModel::Conversion include ActiveModel::Conversion
include Referable include Referable
attr_reader :sha_from, :notation, :sha_to attr_reader :commit_from, :notation, :commit_to
attr_reader :ref_from, :ref_to
# Optional Project model # Optional Project model
attr_accessor :project attr_accessor :project
# See `exclude_start?` # The beginning and ending refs can be named or SHAs, and
attr_reader :exclude_start
# The beginning and ending SHAs can be between 6 and 40 hex characters, and
# the range notation can be double- or triple-dot. # the range notation can be double- or triple-dot.
PATTERN = /\h{6,40}\.{2,3}\h{6,40}/ REF_PATTERN = /[0-9a-zA-Z][0-9a-zA-Z_.-]*[0-9a-zA-Z\^]/
PATTERN = /#{REF_PATTERN}\.{2,3}#{REF_PATTERN}/
# In text references, the beginning and ending refs can only be SHAs
# between 6 and 40 hex characters.
STRICT_PATTERN = /\h{6,40}\.{2,3}\h{6,40}/
def self.reference_prefix def self.reference_prefix
'@' '@'
...@@ -43,27 +45,40 @@ def self.reference_prefix ...@@ -43,27 +45,40 @@ def self.reference_prefix
def self.reference_pattern def self.reference_pattern
%r{ %r{
(?:#{Project.reference_pattern}#{reference_prefix})? (?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit_range>#{PATTERN}) (?<commit_range>#{STRICT_PATTERN})
}x }x
end end
def self.link_reference_pattern
super("compare", /(?<commit_range>#{PATTERN})/)
end
# Initialize a CommitRange # Initialize a CommitRange
# #
# range_string - The String commit range. # range_string - The String commit range.
# project - An optional Project model. # project - An optional Project model.
# #
# Raises ArgumentError if `range_string` does not match `PATTERN`. # Raises ArgumentError if `range_string` does not match `PATTERN`.
def initialize(range_string, project = nil) def initialize(range_string, project)
@project = project
range_string.strip! range_string.strip!
unless range_string.match(/\A#{PATTERN}\z/) unless range_string =~ /\A#{PATTERN}\z/
raise ArgumentError, "invalid CommitRange string format: #{range_string}" raise ArgumentError, "invalid CommitRange string format: #{range_string}"
end end
@exclude_start = !range_string.include?('...') @ref_from, @notation, @ref_to = range_string.split(/(\.{2,3})/, 2)
@sha_from, @notation, @sha_to = range_string.split(/(\.{2,3})/, 2)
@project = project if project.valid_repo?
@commit_from = project.commit(@ref_from)
@commit_to = project.commit(@ref_to)
end
if valid_commits?
@ref_from = Commit.truncate_sha(sha_from) if sha_from.start_with?(@ref_from)
@ref_to = Commit.truncate_sha(sha_to) if sha_to.start_with?(@ref_to)
end
end end
def inspect def inspect
...@@ -71,15 +86,24 @@ def inspect ...@@ -71,15 +86,24 @@ def inspect
end end
def to_s def to_s
"#{sha_from[0..7]}#{notation}#{sha_to[0..7]}" sha_from + notation + sha_to
end end
alias_method :id, :to_s
def to_reference(from_project = nil) def to_reference(from_project = nil)
# Not using to_s because we want the full SHAs if cross_project_reference?(from_project)
reference = sha_from + notation + sha_to project.to_reference + self.class.reference_prefix + self.id
else
self.id
end
end
def reference_link_text(from_project = nil)
reference = ref_from + notation + ref_to
if cross_project_reference?(from_project) if cross_project_reference?(from_project)
reference = project.to_reference + '@' + reference reference = project.to_reference + self.class.reference_prefix + reference
end end
reference reference
...@@ -87,46 +111,58 @@ def to_reference(from_project = nil) ...@@ -87,46 +111,58 @@ def to_reference(from_project = nil)
# Returns a String for use in a link's title attribute # Returns a String for use in a link's title attribute
def reference_title def reference_title
"Commits #{suffixed_sha_from} through #{sha_to}" "Commits #{sha_start} through #{sha_to}"
end end
# Return a Hash of parameters for passing to a URL helper # Return a Hash of parameters for passing to a URL helper
# #
# See `namespace_project_compare_url` # See `namespace_project_compare_url`
def to_param def to_param
{ from: suffixed_sha_from, to: sha_to } { from: sha_start, to: sha_to }
end end
def exclude_start? def exclude_start?
exclude_start @notation == '..'
end end
# Check if both the starting and ending commit IDs exist in a project's # Check if both the starting and ending commit IDs exist in a project's
# repository # repository
# def valid_commits?
# project - An optional Project to check (default: `project`) commit_start.present? && commit_end.present?
def valid_commits?(project = project)
return nil unless project.present?
return false unless project.valid_repo?
commit_from.present? && commit_to.present?
end end
def persisted? def persisted?
true true
end end
def commit_from def sha_from
@commit_from ||= project.repository.commit(suffixed_sha_from) return nil unless @commit_from
@commit_from.id
end
def sha_to
return nil unless @commit_to
@commit_to.id
end end
def commit_to def sha_start
@commit_to ||= project.repository.commit(sha_to) return nil unless sha_from
exclude_start? ? sha_from + '^' : sha_from
end end
private def commit_start
return nil unless sha_start
def suffixed_sha_from if exclude_start?
sha_from + (exclude_start? ? '^' : '') @commit_start ||= project.commit(sha_start)
else
commit_from
end
end end
alias_method :sha_end, :sha_to
alias_method :commit_end, :commit_to
end end
...@@ -62,13 +62,18 @@ def referenced_mentionables(current_user = self.author, text = self.mentionable_ ...@@ -62,13 +62,18 @@ def referenced_mentionables(current_user = self.author, text = self.mentionable_
return [] if text.blank? return [] if text.blank?
refs = all_references(current_user, text, load_lazy_references: load_lazy_references) refs = all_references(current_user, text, load_lazy_references: load_lazy_references)
(refs.issues + refs.merge_requests + refs.commits) - [local_reference] refs = (refs.issues + refs.merge_requests + refs.commits)
# We're using this method instead of Array diffing because that requires
# both of the object's `hash` values to be the same, which may not be the
# case for otherwise identical Commit objects.
refs.reject { |ref| ref == local_reference }
end end
# Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+. # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.
def create_cross_references!(author = self.author, without = [], text = self.mentionable_text) def create_cross_references!(author = self.author, without = [], text = self.mentionable_text)
refs = referenced_mentionables(author, text) refs = referenced_mentionables(author, text)
# We're using this method instead of Array diffing because that requires # We're using this method instead of Array diffing because that requires
# both of the object's `hash` values to be the same, which may not be the # both of the object's `hash` values to be the same, which may not be the
# case for otherwise identical Commit objects. # case for otherwise identical Commit objects.
...@@ -111,7 +116,7 @@ def detect_mentionable_changes ...@@ -111,7 +116,7 @@ def detect_mentionable_changes
# Only include changed fields that are mentionable # Only include changed fields that are mentionable
source.select { |key, val| mentionable.include?(key) } source.select { |key, val| mentionable.include?(key) }
end end
# Determine whether or not a cross-reference Note has already been created between this Mentionable and # Determine whether or not a cross-reference Note has already been created between this Mentionable and
# the specified target. # the specified target.
def cross_reference_exists?(target) def cross_reference_exists?(target)
......
...@@ -21,6 +21,10 @@ def to_reference(_from_project = nil) ...@@ -21,6 +21,10 @@ def to_reference(_from_project = nil)
'' ''
end end
def reference_link_text(from_project = nil)
to_reference(from_project)
end
module ClassMethods module ClassMethods
# The character that prefixes the actual reference identifier # The character that prefixes the actual reference identifier
# #
...@@ -44,6 +48,25 @@ def reference_prefix ...@@ -44,6 +48,25 @@ def reference_prefix
def reference_pattern def reference_pattern
raise NotImplementedError, "#{self} does not implement #{__method__}" raise NotImplementedError, "#{self} does not implement #{__method__}"
end end
def link_reference_pattern(route, pattern)
%r{
(?<url>
#{Regexp.escape(Gitlab.config.gitlab.url)}
\/#{Project.reference_pattern}
\/#{Regexp.escape(route)}
\/#{pattern}
(?<path>
(\/[a-z0-9_=-]+)*
)?
(?<query>
\?[a-z0-9_=-]+
(&[a-z0-9_=-]+)*
)?
(?<anchor>\#[a-z0-9_-]+)?
)
}x
end
end end
private private
......
...@@ -69,6 +69,10 @@ def self.reference_pattern ...@@ -69,6 +69,10 @@ def self.reference_pattern
}x }x
end end
def self.link_reference_pattern
super("issues", /(?<issue>\d+)/)
end
def to_reference(from_project = nil) def to_reference(from_project = nil)
reference = "#{self.class.reference_prefix}#{iid}" reference = "#{self.class.reference_prefix}#{iid}"
......
...@@ -151,6 +151,10 @@ def self.reference_pattern ...@@ -151,6 +151,10 @@ def self.reference_pattern
}x }x
end end
def self.link_reference_pattern
super("merge_requests", /(?<merge_request>\d+)/)
end
def to_reference(from_project = nil) def to_reference(from_project = nil)
reference = "#{self.class.reference_prefix}#{iid}" reference = "#{self.class.reference_prefix}#{iid}"
...@@ -316,7 +320,7 @@ def closes_issues(current_user = self.author) ...@@ -316,7 +320,7 @@ def closes_issues(current_user = self.author)
issues = commits.flat_map { |c| c.closes_issues(current_user) } issues = commits.flat_map { |c| c.closes_issues(current_user) }
issues.push(*Gitlab::ClosingIssueExtractor.new(project, current_user). issues.push(*Gitlab::ClosingIssueExtractor.new(project, current_user).
closed_by_message(description)) closed_by_message(description))
issues.uniq.sort_by(&:id) issues.uniq
else else
[] []
end end
......
...@@ -65,6 +65,10 @@ def self.reference_pattern ...@@ -65,6 +65,10 @@ def self.reference_pattern
}x }x
end end
def self.link_reference_pattern
super("snippets", /(?<snippet>\d+)/)
end
def to_reference(from_project = nil) def to_reference(from_project = nil)
reference = "#{self.class.reference_prefix}#{id}" reference = "#{self.class.reference_prefix}#{id}"
......
...@@ -125,7 +125,7 @@ def self.change_milestone(noteable, project, author, milestone) ...@@ -125,7 +125,7 @@ def self.change_milestone(noteable, project, author, milestone)
# Returns the created Note object # Returns the created Note object
def self.change_status(noteable, project, author, status, source) def self.change_status(noteable, project, author, status, source)
body = "Status changed to #{status}" body = "Status changed to #{status}"
body += " by #{source.gfm_reference}" if source body += " by #{source.gfm_reference(project)}" if source
create_note(noteable: noteable, project: project, author: author, note: body) create_note(noteable: noteable, project: project, author: author, note: body)
end end
......
.issue-closed-by-widget .issue-closed-by-widget
= icon('check') = icon('check')
This issue will be closed automatically when merge request #{gfm(merge_requests_sentence(@closed_by_merge_requests.sort))} is accepted. This issue will be closed automatically when merge request #{gfm(merge_requests_sentence(@closed_by_merge_requests))} is accepted.
...@@ -164,7 +164,7 @@ def base_gitlab_url ...@@ -164,7 +164,7 @@ def base_gitlab_url
Settings.gitlab['twitter_sharing_enabled'] ||= true if Settings.gitlab['twitter_sharing_enabled'].nil? Settings.gitlab['twitter_sharing_enabled'] ||= true if Settings.gitlab['twitter_sharing_enabled'].nil?
Settings.gitlab['restricted_visibility_levels'] = Settings.send(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], []) Settings.gitlab['restricted_visibility_levels'] = Settings.send(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], [])
Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil? Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil?
Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' if Settings.gitlab['issue_closing_pattern'].nil? Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?))+)' if Settings.gitlab['issue_closing_pattern'].nil?
Settings.gitlab['default_projects_features'] ||= {} Settings.gitlab['default_projects_features'] ||= {}
Settings.gitlab['webhook_timeout'] ||= 10 Settings.gitlab['webhook_timeout'] ||= 10
Settings.gitlab['max_attachment_size'] ||= 10 Settings.gitlab['max_attachment_size'] ||= 10
......
module Gitlab module Gitlab
class ClosingIssueExtractor class ClosingIssueExtractor
ISSUE_CLOSING_REGEX = Regexp.new(Gitlab.config.gitlab.issue_closing_pattern) ISSUE_CLOSING_REGEX = begin
link_pattern = URI.regexp(%w(http https))
pattern = Gitlab.config.gitlab.issue_closing_pattern
pattern = pattern.sub('%{issue_ref}', "(?:(?:#{link_pattern})|(?:#{Issue.reference_pattern}))")
Regexp.new(pattern).freeze
end
def initialize(project, current_user = nil) def initialize(project, current_user = nil)
@extractor = Gitlab::ReferenceExtractor.new(project, current_user) @extractor = Gitlab::ReferenceExtractor.new(project, current_user)
...@@ -9,10 +15,12 @@ def initialize(project, current_user = nil) ...@@ -9,10 +15,12 @@ def initialize(project, current_user = nil)
def closed_by_message(message) def closed_by_message(message)
return [] if message.nil? return [] if message.nil?
closing_statements = message.scan(ISSUE_CLOSING_REGEX). closing_statements = []
map { |ref| ref[0] }.join(" ") message.scan(ISSUE_CLOSING_REGEX) do
closing_statements << Regexp.last_match[0]
end
@extractor.analyze(closing_statements) @extractor.analyze(closing_statements.join(" "))
@extractor.issues @extractor.issues
end end
......
...@@ -178,7 +178,6 @@ def self.filters ...@@ -178,7 +178,6 @@ def self.filters
Gitlab::Markdown::SanitizationFilter, Gitlab::Markdown::SanitizationFilter,
Gitlab::Markdown::UploadLinkFilter, Gitlab::Markdown::UploadLinkFilter,
Gitlab::Markdown::RelativeLinkFilter,
Gitlab::Markdown::EmojiFilter, Gitlab::Markdown::EmojiFilter,
Gitlab::Markdown::TableOfContentsFilter, Gitlab::Markdown::TableOfContentsFilter,
Gitlab::Markdown::AutolinkFilter, Gitlab::Markdown::AutolinkFilter,
...@@ -193,6 +192,8 @@ def self.filters ...@@ -193,6 +192,8 @@ def self.filters
Gitlab::Markdown::CommitReferenceFilter, Gitlab::Markdown::CommitReferenceFilter,
Gitlab::Markdown::LabelReferenceFilter, Gitlab::Markdown::LabelReferenceFilter,
Gitlab::Markdown::RelativeLinkFilter,
Gitlab::Markdown::TaskListFilter Gitlab::Markdown::TaskListFilter
] ]
end end
......