Commit 0ff778c0 authored by Douwe Maan's avatar Douwe Maan

Link cross-project cross-reference notes to correct project.

parent 27af24c1
...@@ -64,7 +64,7 @@ module Mentionable ...@@ -64,7 +64,7 @@ module Mentionable
def create_cross_references!(p = project, a = author, without = []) def create_cross_references!(p = project, a = author, without = [])
refs = references(p) - without refs = references(p) - without
refs.each do |ref| refs.each do |ref|
Note.create_cross_reference_note(ref, local_reference, a, p) Note.create_cross_reference_note(ref, local_reference, a)
end end
end end
......
...@@ -77,11 +77,11 @@ class Note < ActiveRecord::Base ...@@ -77,11 +77,11 @@ class Note < ActiveRecord::Base
# +mentioner+'s description or an associated Note. # +mentioner+'s description or an associated Note.
# Create a system Note associated with +noteable+ with a GFM back-reference # Create a system Note associated with +noteable+ with a GFM back-reference
# to +mentioner+. # to +mentioner+.
def create_cross_reference_note(noteable, mentioner, author, project) def create_cross_reference_note(noteable, mentioner, author)
gfm_reference = mentioner_gfm_ref(noteable, mentioner, project) gfm_reference = mentioner_gfm_ref(noteable, mentioner)
note_options = { note_options = {
project: project, project: noteable.project,
author: author, author: author,
note: cross_reference_note_content(gfm_reference), note: cross_reference_note_content(gfm_reference),
system: true system: true
...@@ -236,7 +236,7 @@ class Note < ActiveRecord::Base ...@@ -236,7 +236,7 @@ class Note < ActiveRecord::Base
# Determine whether or not a cross-reference note already exists. # Determine whether or not a cross-reference note already exists.
def cross_reference_exists?(noteable, mentioner) def cross_reference_exists?(noteable, mentioner)
gfm_reference = mentioner_gfm_ref(noteable, mentioner) gfm_reference = mentioner_gfm_ref(noteable, mentioner, nil)
notes = if noteable.is_a?(Commit) notes = if noteable.is_a?(Commit)
where(commit_id: noteable.id, noteable_type: 'Commit') where(commit_id: noteable.id, noteable_type: 'Commit')
else else
...@@ -269,43 +269,19 @@ class Note < ActiveRecord::Base ...@@ -269,43 +269,19 @@ class Note < ActiveRecord::Base
# Prepend the mentioner's namespaced project path to the GFM reference for # Prepend the mentioner's namespaced project path to the GFM reference for
# cross-project references. For same-project references, return the # cross-project references. For same-project references, return the
# unmodified GFM reference. # unmodified GFM reference.
def mentioner_gfm_ref(noteable, mentioner, project = nil) def mentioner_gfm_ref(noteable, mentioner, mentioner_project = mentioner.project)
if mentioner.is_a?(Commit) if mentioner.is_a?(Commit) && project.nil?
if project.nil? return mentioner.gfm_reference.sub('commit ', 'commit %')
return mentioner.gfm_reference.sub('commit ', 'commit %')
else
mentioning_project = project
end
else
mentioning_project = mentioner.project
end
noteable_project_id = noteable_project_id(noteable, mentioning_project)
full_gfm_reference(mentioning_project, noteable_project_id, mentioner)
end
# Return the ID of the project that +noteable+ belongs to, or nil if
# +noteable+ is a commit and is not part of the project that owns
# +mentioner+.
def noteable_project_id(noteable, mentioning_project)
if noteable.is_a?(Commit)
if mentioning_project.commit(noteable.id)
# The noteable commit belongs to the mentioner's project
mentioning_project.id
else
nil
end
else
noteable.project.id
end end
full_gfm_reference(mentioner_project, noteable.project, mentioner)
end end
# Return the +mentioner+ GFM reference. If the mentioner and noteable # Return the +mentioner+ GFM reference. If the mentioner and noteable
# projects are not the same, add the mentioning project's path to the # projects are not the same, add the mentioning project's path to the
# returned value. # returned value.
def full_gfm_reference(mentioning_project, noteable_project_id, mentioner) def full_gfm_reference(mentioning_project, noteable_project, mentioner)
if mentioning_project.id == noteable_project_id if mentioning_project.id == noteable_project
mentioner.gfm_reference mentioner.gfm_reference
else else
if mentioner.is_a?(Commit) if mentioner.is_a?(Commit)
......
...@@ -94,7 +94,7 @@ class GitPushService ...@@ -94,7 +94,7 @@ class GitPushService
author ||= commit_user(commit) author ||= commit_user(commit)
refs.each do |r| refs.each do |r|
Note.create_cross_reference_note(r, commit, author, project) Note.create_cross_reference_note(r, commit, author)
end end
end end
end end
......
...@@ -15,7 +15,7 @@ module Notes ...@@ -15,7 +15,7 @@ module Notes
# Create a cross-reference note if this Note contains GFM that names an # Create a cross-reference note if this Note contains GFM that names an
# issue, merge request, or commit. # issue, merge request, or commit.
note.references.each do |mentioned| note.references.each do |mentioned|
Note.create_cross_reference_note(mentioned, note.noteable, note.author, note.project) Note.create_cross_reference_note(mentioned, note.noteable, note.author)
end end
execute_hooks(note) execute_hooks(note)
......
...@@ -13,8 +13,7 @@ module Notes ...@@ -13,8 +13,7 @@ module Notes
# Create a cross-reference note if this Note contains GFM that # Create a cross-reference note if this Note contains GFM that
# names an issue, merge request, or commit. # names an issue, merge request, or commit.
note.references.each do |mentioned| note.references.each do |mentioned|
Note.create_cross_reference_note(mentioned, note.noteable, Note.create_cross_reference_note(mentioned, note.noteable, note.author)
note.author, note.project)
end end
end end
end end
......
...@@ -335,7 +335,7 @@ describe Note do ...@@ -335,7 +335,7 @@ describe Note do
# roles, to ensure that the correct information can be inferred from any argument. # roles, to ensure that the correct information can be inferred from any argument.
context 'issue from a merge request' do context 'issue from a merge request' do
subject { Note.create_cross_reference_note(issue, mergereq, author, project) } subject { Note.create_cross_reference_note(issue, mergereq, author) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -361,7 +361,7 @@ describe Note do ...@@ -361,7 +361,7 @@ describe Note do
end end
context 'issue from a commit' do context 'issue from a commit' do
subject { Note.create_cross_reference_note(issue, commit, author, project) } subject { Note.create_cross_reference_note(issue, commit, author) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -377,7 +377,7 @@ describe Note do ...@@ -377,7 +377,7 @@ describe Note do
end end
context 'merge request from an issue' do context 'merge request from an issue' do
subject { Note.create_cross_reference_note(mergereq, issue, author, project) } subject { Note.create_cross_reference_note(mergereq, issue, author) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -398,7 +398,7 @@ describe Note do ...@@ -398,7 +398,7 @@ describe Note do
end end
context 'commit from a merge request' do context 'commit from a merge request' do
subject { Note.create_cross_reference_note(commit, mergereq, author, project) } subject { Note.create_cross_reference_note(commit, mergereq, author) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -419,13 +419,13 @@ describe Note do ...@@ -419,13 +419,13 @@ describe Note do
end end
context 'commit contained in a merge request' do context 'commit contained in a merge request' do
subject { Note.create_cross_reference_note(mergereq.commits.first, mergereq, author, project) } subject { Note.create_cross_reference_note(mergereq.commits.first, mergereq, author) }
it { is_expected.to be_nil } it { is_expected.to be_nil }
end end
context 'commit from issue' do context 'commit from issue' do
subject { Note.create_cross_reference_note(commit, issue, author, project) } subject { Note.create_cross_reference_note(commit, issue, author) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -452,7 +452,7 @@ describe Note do ...@@ -452,7 +452,7 @@ describe Note do
context 'commit from commit' do context 'commit from commit' do
let(:parent_commit) { commit.parents.first } let(:parent_commit) { commit.parents.first }
subject { Note.create_cross_reference_note(commit, parent_commit, author, project) } subject { Note.create_cross_reference_note(commit, parent_commit, author) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -486,7 +486,7 @@ describe Note do ...@@ -486,7 +486,7 @@ describe Note do
let(:commit1) { project.commit('HEAD~2') } let(:commit1) { project.commit('HEAD~2') }
before do before do
Note.create_cross_reference_note(issue, commit0, author, project) Note.create_cross_reference_note(issue, commit0, author)
end end
it 'detects if a mentionable has already been mentioned' do it 'detects if a mentionable has already been mentioned' do
...@@ -499,7 +499,7 @@ describe Note do ...@@ -499,7 +499,7 @@ describe Note do
context 'commit on commit' do context 'commit on commit' do
before do before do
Note.create_cross_reference_note(commit0, commit1, author, project) Note.create_cross_reference_note(commit0, commit1, author)
end end
it { expect(Note.cross_reference_exists?(commit0, commit1)).to be_truthy } it { expect(Note.cross_reference_exists?(commit0, commit1)).to be_truthy }
...@@ -527,7 +527,7 @@ describe Note do ...@@ -527,7 +527,7 @@ describe Note do
let(:author) { create :user } let(:author) { create :user }
let(:issue0) { create :issue, project: project } let(:issue0) { create :issue, project: project }
let(:issue1) { create :issue, project: second_project } let(:issue1) { create :issue, project: second_project }
let!(:note) { Note.create_cross_reference_note(issue0, issue1, author, project) } let!(:note) { Note.create_cross_reference_note(issue0, issue1, author) }
it 'detects if a mentionable has already been mentioned' do it 'detects if a mentionable has already been mentioned' do
expect(Note.cross_reference_exists?(issue0, issue1)).to be_truthy expect(Note.cross_reference_exists?(issue0, issue1)).to be_truthy
...@@ -562,7 +562,7 @@ describe Note do ...@@ -562,7 +562,7 @@ describe Note do
end end
it 'should identify cross-reference notes as system notes' do it 'should identify cross-reference notes as system notes' do
@note = Note.create_cross_reference_note(issue, other, author, project) @note = Note.create_cross_reference_note(issue, other, author)
expect(@note).to be_system expect(@note).to be_system
end end
......
...@@ -164,22 +164,22 @@ describe GitPushService do ...@@ -164,22 +164,22 @@ describe GitPushService do
end end
it "creates a note if a pushed commit mentions an issue" do it "creates a note if a pushed commit mentions an issue" do
expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author, project) expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author)
service.execute(project, user, @oldrev, @newrev, @ref) service.execute(project, user, @oldrev, @newrev, @ref)
end end
it "only creates a cross-reference note if one doesn't already exist" do it "only creates a cross-reference note if one doesn't already exist" do
Note.create_cross_reference_note(issue, commit, user, project) Note.create_cross_reference_note(issue, commit, user)
expect(Note).not_to receive(:create_cross_reference_note).with(issue, commit, commit_author, project) expect(Note).not_to receive(:create_cross_reference_note).with(issue, commit, commit_author)
service.execute(project, user, @oldrev, @newrev, @ref) service.execute(project, user, @oldrev, @newrev, @ref)
end end
it "defaults to the pushing user if the commit's author is not known" do it "defaults to the pushing user if the commit's author is not known" do
commit.stub(author_name: 'unknown name', author_email: 'unknown@email.com') commit.stub(author_name: 'unknown name', author_email: 'unknown@email.com')
expect(Note).to receive(:create_cross_reference_note).with(issue, commit, user, project) expect(Note).to receive(:create_cross_reference_note).with(issue, commit, user)
service.execute(project, user, @oldrev, @newrev, @ref) service.execute(project, user, @oldrev, @newrev, @ref)
end end
...@@ -188,7 +188,7 @@ describe GitPushService do ...@@ -188,7 +188,7 @@ describe GitPushService do
allow(project.repository).to receive(:commits_between).with(@blankrev, @newrev).and_return([]) allow(project.repository).to receive(:commits_between).with(@blankrev, @newrev).and_return([])
allow(project.repository).to receive(:commits_between).with("master", @newrev).and_return([commit]) allow(project.repository).to receive(:commits_between).with("master", @newrev).and_return([commit])
expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author, project) expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author)
service.execute(project, user, @blankrev, @newrev, 'refs/heads/other') service.execute(project, user, @blankrev, @newrev, 'refs/heads/other')
end end
......
...@@ -57,7 +57,7 @@ describe NotificationService do ...@@ -57,7 +57,7 @@ describe NotificationService do
end end
it 'filters out "mentioned in" notes' do it 'filters out "mentioned in" notes' do
mentioned_note = Note.create_cross_reference_note(mentioned_issue, issue, issue.author, issue.project) mentioned_note = Note.create_cross_reference_note(mentioned_issue, issue, issue.author)
expect(Notify).not_to receive(:note_issue_email) expect(Notify).not_to receive(:note_issue_email)
notification.new_note(mentioned_note) notification.new_note(mentioned_note)
...@@ -128,7 +128,7 @@ describe NotificationService do ...@@ -128,7 +128,7 @@ describe NotificationService do
end end
it 'filters out "mentioned in" notes' do it 'filters out "mentioned in" notes' do
mentioned_note = Note.create_cross_reference_note(mentioned_issue, issue, issue.author, issue.project) mentioned_note = Note.create_cross_reference_note(mentioned_issue, issue, issue.author)
expect(Notify).not_to receive(:note_issue_email) expect(Notify).not_to receive(:note_issue_email)
notification.new_note(mentioned_note) notification.new_note(mentioned_note)
......
...@@ -83,14 +83,14 @@ shared_examples 'a mentionable' do ...@@ -83,14 +83,14 @@ shared_examples 'a mentionable' do
mentioned_objects.each do |referenced| mentioned_objects.each do |referenced|
expect(Note).to receive(:create_cross_reference_note). expect(Note).to receive(:create_cross_reference_note).
with(referenced, subject.local_reference, author, project) with(referenced, subject.local_reference, author)
end end
subject.create_cross_references!(project, author) subject.create_cross_references!(project, author)
end end
it 'detects existing cross-references' do it 'detects existing cross-references' do
Note.create_cross_reference_note(mentioned_issue, subject.local_reference, author, project) Note.create_cross_reference_note(mentioned_issue, subject.local_reference, author)
expect(subject).to have_mentioned(mentioned_issue) expect(subject).to have_mentioned(mentioned_issue)
expect(subject).not_to have_mentioned(mentioned_mr) expect(subject).not_to have_mentioned(mentioned_mr)
...@@ -132,7 +132,7 @@ shared_examples 'an editable mentionable' do ...@@ -132,7 +132,7 @@ shared_examples 'an editable mentionable' do
# These two issues are new and should receive reference notes # These two issues are new and should receive reference notes
new_issues.each do |newref| new_issues.each do |newref|
expect(Note).to receive(:create_cross_reference_note). expect(Note).to receive(:create_cross_reference_note).
with(newref, subject.local_reference, author, project) with(newref, subject.local_reference, author)
end end
subject.save subject.save
......
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