Commit b0164771 authored by Douwe Maan's avatar Douwe Maan

Simplify code around (cross)-references

parent 03b7fe71
...@@ -55,7 +55,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -55,7 +55,7 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def show def show
@participants = @issue.participants(current_user, @project) @participants = @issue.participants(current_user)
@note = @project.notes.new(noteable: @issue) @note = @project.notes.new(noteable: @issue)
@notes = @issue.notes.inc_author.fresh @notes = @issue.notes.inc_author.fresh
@noteable = @issue @noteable = @issue
......
...@@ -246,7 +246,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -246,7 +246,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
def define_show_vars def define_show_vars
@participants = @merge_request.participants(current_user, @project) @participants = @merge_request.participants(current_user)
# Build a note object for comment form # Build a note object for comment form
@note = @project.notes.new(noteable: @merge_request) @note = @project.notes.new(noteable: @merge_request)
......
...@@ -43,53 +43,53 @@ module Mentionable ...@@ -43,53 +43,53 @@ module Mentionable
# 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 has_mentioned?(target) def cross_reference_exists?(target)
SystemNoteService.cross_reference_exists?(target, local_reference) SystemNoteService.cross_reference_exists?(target, local_reference)
end end
def mentioned_users(current_user = nil) def all_references(current_user = self.author, text = self.mentionable_text)
return [] if mentionable_text.blank?
ext = Gitlab::ReferenceExtractor.new(self.project, current_user) ext = Gitlab::ReferenceExtractor.new(self.project, current_user)
ext.analyze(mentionable_text) ext.analyze(text)
ext.users.uniq ext
end
def mentioned_users(current_user = nil)
all_references(current_user).users.uniq
end end
# Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
def references(p = project, current_user = self.author, text = mentionable_text) def referenced_mentionables(current_user = self.author, text = self.mentionable_text)
return [] if text.blank? return [] if text.blank?
ext = Gitlab::ReferenceExtractor.new(p, current_user) refs = all_references(current_user, text)
ext.analyze(text) (refs.issues + refs.merge_requests + refs.commits).uniq - [local_reference]
(ext.issues + ext.merge_requests + ext.commits).uniq - [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!(p = project, a = author, without = []) def create_cross_references!(author = self.author, without = [])
refs = references(p) refs = referenced_mentionables(author)
# 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.
refs.reject! { |ref| without.include?(ref) } refs.reject! { |ref| without.include?(ref) || cross_reference_exists?(ref) }
refs.each do |ref| refs.each do |ref|
SystemNoteService.cross_reference(ref, local_reference, a) SystemNoteService.cross_reference(ref, local_reference, author)
end end
end end
# When a mentionable field is changed, creates cross-reference notes that # When a mentionable field is changed, creates cross-reference notes that
# don't already exist # don't already exist
def create_new_cross_references!(p = project, a = author) def create_new_cross_references!(author = self.author)
changes = detect_mentionable_changes changes = detect_mentionable_changes
return if changes.empty? return if changes.empty?
original_text = changes.collect { |_, vals| vals.first }.join(' ') original_text = changes.collect { |_, vals| vals.first }.join(' ')
preexisting = references(p, self.author, original_text) preexisting = referenced_mentionables(author, original_text)
create_cross_references!(p, a, preexisting) create_cross_references!(author, preexisting)
end end
private private
......
...@@ -37,7 +37,7 @@ module Participable ...@@ -37,7 +37,7 @@ module Participable
# Be aware that this method makes a lot of sql queries. # Be aware that this method makes a lot of sql queries.
# Save result into variable if you are going to reuse it inside same request # Save result into variable if you are going to reuse it inside same request
def participants(current_user = self.author, project = self.project) def participants(current_user = self.author)
participants = self.class.participant_attrs.flat_map do |attr| participants = self.class.participant_attrs.flat_map do |attr|
meth = method(attr) meth = method(attr)
...@@ -48,13 +48,11 @@ module Participable ...@@ -48,13 +48,11 @@ module Participable
meth.call meth.call
end end
participants_for(value, current_user, project) participants_for(value, current_user)
end.compact.uniq end.compact.uniq
if project
participants.select! do |user| participants.select! do |user|
user.can?(:read_project, project) user.can?(:read_project, self.project)
end
end end
participants participants
...@@ -62,14 +60,14 @@ module Participable ...@@ -62,14 +60,14 @@ module Participable
private private
def participants_for(value, current_user = nil, project = nil) def participants_for(value, current_user = nil)
case value case value
when User when User
[value] [value]
when Enumerable, ActiveRecord::Relation when Enumerable, ActiveRecord::Relation
value.flat_map { |v| participants_for(v, current_user, project) } value.flat_map { |v| participants_for(v, current_user) }
when Participable when Participable
value.participants(current_user, project) value.participants(current_user)
end end
end end
end end
...@@ -358,7 +358,7 @@ class Note < ActiveRecord::Base ...@@ -358,7 +358,7 @@ class Note < ActiveRecord::Base
end end
def set_references def set_references
create_new_cross_references!(project, author) create_new_cross_references!
end end
def system? def system?
......
...@@ -74,48 +74,30 @@ class GitPushService ...@@ -74,48 +74,30 @@ class GitPushService
def process_commit_messages(ref) def process_commit_messages(ref)
is_default_branch = is_default_branch?(ref) is_default_branch = is_default_branch?(ref)
@push_commits.each do |commit| authors = Hash.new do |hash, commit|
# Close issues if these commits were pushed to the project's default branch and the commit message matches the email = commit.author_email
# closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to return hash[email] if hash.has_key?(email)
# a different branch.
issues_to_close = commit.closes_issues(user)
# Load commit author only if needed. hash[email] = commit_user(commit)
# For push with 1k commits it prevents 900+ requests in database end
author = nil
@push_commits.each do |commit|
# Keep track of the issues that will be actually closed because they are on a default branch. # Keep track of the issues that will be actually closed because they are on a default branch.
# Hence, when creating cross-reference notes, the not-closed issues (on non-default branches) # Hence, when creating cross-reference notes, the not-closed issues (on non-default branches)
# will also have cross-reference. # will also have cross-reference.
actually_closed_issues = [] closed_issues = []
if issues_to_close.present? && is_default_branch
author ||= commit_user(commit)
actually_closed_issues = issues_to_close
issues_to_close.each do |issue|
Issues::CloseService.new(project, author, {}).execute(issue, commit)
end
end
if project.default_issues_tracker? if is_default_branch
create_cross_reference_notes(commit, actually_closed_issues) # Close issues if these commits were pushed to the project's default branch and the commit message matches the
end # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
# a different branch.
closed_issues = commit.closes_issues(user)
closed_issues.each do |issue|
Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit)
end end
end end
def create_cross_reference_notes(commit, issues_to_close) commit.create_cross_references!(authors[commit], closed_issues)
# Create cross-reference notes for any other references than those given in issues_to_close.
# Omit any issues that were referenced in an issue-closing phrase, or have already been
# mentioned from this commit (probably from this commit being pushed to a different branch).
refs = commit.references(project, user) - issues_to_close
refs.reject! { |r| commit.has_mentioned?(r) }
if refs.present?
author ||= commit_user(commit)
refs.each do |r|
SystemNoteService.cross_reference(r, commit, author)
end
end end
end end
......
...@@ -10,7 +10,7 @@ module Issues ...@@ -10,7 +10,7 @@ module Issues
issue.update_attributes(label_ids: label_params) issue.update_attributes(label_ids: label_params)
notification_service.new_issue(issue, current_user) notification_service.new_issue(issue, current_user)
event_service.open_issue(issue, current_user) event_service.open_issue(issue, current_user)
issue.create_cross_references!(issue.project, current_user) issue.create_cross_references!(current_user)
execute_hooks(issue, 'open') execute_hooks(issue, 'open')
end end
......
...@@ -35,7 +35,7 @@ module Issues ...@@ -35,7 +35,7 @@ module Issues
create_title_change_note(issue, issue.previous_changes['title'].first) create_title_change_note(issue, issue.previous_changes['title'].first)
end end
issue.create_new_cross_references!(issue.project, current_user) issue.create_new_cross_references!
execute_hooks(issue, 'update') execute_hooks(issue, 'update')
end end
......
...@@ -18,7 +18,7 @@ module MergeRequests ...@@ -18,7 +18,7 @@ module MergeRequests
merge_request.update_attributes(label_ids: label_params) merge_request.update_attributes(label_ids: label_params)
event_service.open_mr(merge_request, current_user) event_service.open_mr(merge_request, current_user)
notification_service.new_merge_request(merge_request, current_user) notification_service.new_merge_request(merge_request, current_user)
merge_request.create_cross_references!(merge_request.project, current_user) merge_request.create_cross_references!(current_user)
execute_hooks(merge_request) execute_hooks(merge_request)
end end
......
...@@ -59,7 +59,7 @@ module MergeRequests ...@@ -59,7 +59,7 @@ module MergeRequests
merge_request.mark_as_unchecked merge_request.mark_as_unchecked
end end
merge_request.create_new_cross_references!(merge_request.project, current_user) merge_request.create_new_cross_references!
execute_hooks(merge_request, 'update') execute_hooks(merge_request, 'update')
end end
......
...@@ -39,6 +39,8 @@ module Gitlab ...@@ -39,6 +39,8 @@ module Gitlab
# #
# Returns the results Array for the requested filter type # Returns the results Array for the requested filter type
def pipeline_result(filter_type) def pipeline_result(filter_type)
return [] if @text.blank?
klass = filter_type.to_s.camelize + 'ReferenceFilter' klass = filter_type.to_s.camelize + 'ReferenceFilter'
filter = Gitlab::Markdown.const_get(klass) filter = Gitlab::Markdown.const_get(klass)
......
...@@ -25,7 +25,7 @@ describe Issue, "Mentionable" do ...@@ -25,7 +25,7 @@ describe Issue, "Mentionable" do
it 'correctly removes already-mentioned Commits' do it 'correctly removes already-mentioned Commits' do
expect(SystemNoteService).not_to receive(:cross_reference) expect(SystemNoteService).not_to receive(:cross_reference)
issue.create_cross_references!(project, author, [commit2]) issue.create_cross_references!(author, [commit2])
end end
end end
......
...@@ -155,7 +155,7 @@ describe GitPushService do ...@@ -155,7 +155,7 @@ describe GitPushService do
before do before do
allow(commit).to receive_messages( allow(commit).to receive_messages(
safe_message: "this commit \n mentions ##{issue.id}", safe_message: "this commit \n mentions ##{issue.iid}",
references: [issue], references: [issue],
author_name: commit_author.name, author_name: commit_author.name,
author_email: commit_author.email author_email: commit_author.email
......
...@@ -65,7 +65,7 @@ shared_examples 'a mentionable' do ...@@ -65,7 +65,7 @@ shared_examples 'a mentionable' do
it "extracts references from its reference property" do it "extracts references from its reference property" do
# De-duplicate and omit itself # De-duplicate and omit itself
refs = subject.references(project) refs = subject.referenced_mentionables
expect(refs.size).to eq(6) expect(refs.size).to eq(6)
expect(refs).to include(mentioned_issue) expect(refs).to include(mentioned_issue)
expect(refs).to include(mentioned_mr) expect(refs).to include(mentioned_mr)
...@@ -84,14 +84,14 @@ shared_examples 'a mentionable' do ...@@ -84,14 +84,14 @@ shared_examples 'a mentionable' do
with(referenced, subject.local_reference, author) with(referenced, subject.local_reference, author)
end end
subject.create_cross_references!(project, author) subject.create_cross_references!
end end
it 'detects existing cross-references' do it 'detects existing cross-references' do
SystemNoteService.cross_reference(mentioned_issue, subject.local_reference, author) SystemNoteService.cross_reference(mentioned_issue, subject.local_reference, author)
expect(subject).to have_mentioned(mentioned_issue) expect(subject.cross_reference_exists?(mentioned_issue)).to be_truthy
expect(subject).not_to have_mentioned(mentioned_mr) expect(subject.cross_reference_exists?(mentioned_mr)).to be_falsey
end end
end end
...@@ -143,6 +143,6 @@ shared_examples 'an editable mentionable' do ...@@ -143,6 +143,6 @@ shared_examples 'an editable mentionable' do
end end
set_mentionable_text.call(new_text) set_mentionable_text.call(new_text)
subject.create_new_cross_references!(project, author) subject.create_new_cross_references!(author)
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