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

9 files changed, 36 insertions, 61 deletions

diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index aad5e514793..3ef3e8b67d8 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -64,7 +64,7 @@ module Mentionable
   def create_cross_references!(p = project, a = author, without = [])
     refs = references(p) - without
     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
 
diff --git a/app/models/note.rb b/app/models/note.rb
index 99e86ac0250..26739426a88 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -77,11 +77,11 @@ class Note < ActiveRecord::Base
     # +mentioner+'s description or an associated Note.
     # Create a system Note associated with +noteable+ with a GFM back-reference
     # to +mentioner+.
-    def create_cross_reference_note(noteable, mentioner, author, project)
-      gfm_reference = mentioner_gfm_ref(noteable, mentioner, project)
+    def create_cross_reference_note(noteable, mentioner, author)
+      gfm_reference = mentioner_gfm_ref(noteable, mentioner)
 
       note_options = {
-        project: project,
+        project: noteable.project,
         author: author,
         note: cross_reference_note_content(gfm_reference),
         system: true
@@ -236,7 +236,7 @@ class Note < ActiveRecord::Base
 
     # Determine whether or not a cross-reference note already exists.
     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)
                 where(commit_id: noteable.id, noteable_type: 'Commit')
               else
@@ -269,43 +269,19 @@ class Note < ActiveRecord::Base
     # Prepend the mentioner's namespaced project path to the GFM reference for
     # cross-project references.  For same-project references, return the
     # unmodified GFM reference.
-    def mentioner_gfm_ref(noteable, mentioner, project = nil)
-      if mentioner.is_a?(Commit)
-        if project.nil?
-          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
+    def mentioner_gfm_ref(noteable, mentioner, mentioner_project = mentioner.project)
+      if mentioner.is_a?(Commit) && project.nil?
+        return mentioner.gfm_reference.sub('commit ', 'commit %')
       end
+      
+      full_gfm_reference(mentioner_project, noteable.project, mentioner)
     end
 
     # Return the +mentioner+ GFM reference.  If the mentioner and noteable
     # projects are not the same, add the mentioning project's path to the
     # returned value.
-    def full_gfm_reference(mentioning_project, noteable_project_id, mentioner)
-      if mentioning_project.id == noteable_project_id
+    def full_gfm_reference(mentioning_project, noteable_project, mentioner)
+      if mentioning_project.id == noteable_project
         mentioner.gfm_reference
       else
         if mentioner.is_a?(Commit)
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 3affd6d6463..1b889e0da8b 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -94,7 +94,7 @@ class GitPushService
         author ||= commit_user(commit)
 
         refs.each do |r|
-          Note.create_cross_reference_note(r, commit, author, project)
+          Note.create_cross_reference_note(r, commit, author)
         end
       end
     end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index e969061f229..d19a6c2eca3 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -15,7 +15,7 @@ module Notes
           # Create a cross-reference note if this Note contains GFM that names an
           # issue, merge request, or commit.
           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
 
           execute_hooks(note)
diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb
index 63431b82471..45a0db761ec 100644
--- a/app/services/notes/update_service.rb
+++ b/app/services/notes/update_service.rb
@@ -13,8 +13,7 @@ module Notes
           # Create a cross-reference note if this Note contains GFM that
           # names an issue, merge request, or commit.
           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
       end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 095eb1500a9..4a6bfdb2910 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -335,7 +335,7 @@ describe Note do
     # roles, to ensure that the correct information can be inferred from any argument.
 
     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 }
 
@@ -361,7 +361,7 @@ describe Note do
     end
 
     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 }
 
@@ -377,7 +377,7 @@ describe Note do
     end
 
     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 }
 
@@ -398,7 +398,7 @@ describe Note do
     end
 
     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 }
 
@@ -419,13 +419,13 @@ describe Note do
     end
 
     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 }
     end
 
     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 }
 
@@ -452,7 +452,7 @@ describe Note do
 
     context 'commit from commit' do
       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 }
 
@@ -486,7 +486,7 @@ describe Note do
     let(:commit1) { project.commit('HEAD~2') }
 
     before do
-      Note.create_cross_reference_note(issue, commit0, author, project)
+      Note.create_cross_reference_note(issue, commit0, author)
     end
 
     it 'detects if a mentionable has already been mentioned' do
@@ -499,7 +499,7 @@ describe Note do
 
     context 'commit on commit' do
       before do
-        Note.create_cross_reference_note(commit0, commit1, author, project)
+        Note.create_cross_reference_note(commit0, commit1, author)
       end
 
       it { expect(Note.cross_reference_exists?(commit0, commit1)).to be_truthy }
@@ -527,7 +527,7 @@ describe Note do
     let(:author) { create :user }
     let(:issue0) { create :issue, project: 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
       expect(Note.cross_reference_exists?(issue0, issue1)).to be_truthy
@@ -562,7 +562,7 @@ describe Note do
     end
 
     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
     end
 
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 246bc1cc33d..af37e8319a4 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -164,22 +164,22 @@ describe GitPushService do
     end
 
     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)
     end
 
     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)
     end
 
     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')
-      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)
     end
@@ -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("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')
     end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 6d2bc41c2b9..2a54b2e920a 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -57,7 +57,7 @@ describe NotificationService do
         end
 
         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)
           notification.new_note(mentioned_note)
@@ -128,7 +128,7 @@ describe NotificationService do
         end
 
         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)
           notification.new_note(mentioned_note)
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index 25ec5cbe6f2..dde80b1e1dd 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -83,14 +83,14 @@ shared_examples 'a mentionable' do
 
     mentioned_objects.each do |referenced|
       expect(Note).to receive(:create_cross_reference_note).
-        with(referenced, subject.local_reference, author, project)
+        with(referenced, subject.local_reference, author)
     end
 
     subject.create_cross_references!(project, author)
   end
 
   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).not_to have_mentioned(mentioned_mr)
@@ -132,7 +132,7 @@ shared_examples 'an editable mentionable' do
     # These two issues are new and should receive reference notes
     new_issues.each do |newref|
       expect(Note).to receive(:create_cross_reference_note).
-        with(newref, subject.local_reference, author, project)
+        with(newref, subject.local_reference, author)
     end
 
     subject.save