diff options
| -rw-r--r-- | app/models/concerns/mentionable.rb | 4 | ||||
| -rw-r--r-- | app/models/note.rb | 73 | ||||
| -rw-r--r-- | spec/models/commit_spec.rb | 14 | ||||
| -rw-r--r-- | spec/models/note_spec.rb | 4 | ||||
| -rw-r--r-- | spec/support/mentionable_shared_examples.rb | 40 |
5 files changed, 120 insertions, 15 deletions
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 8c31cdd12d0..5938d9cb28e 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -68,7 +68,9 @@ module Mentionable return [] if text.blank? ext = Gitlab::ReferenceExtractor.new ext.analyze(text, p) - (ext.issues_for(p) + ext.merge_requests_for(p) + ext.commits_for(p)).uniq - [local_reference] + (ext.issues_for + + ext.merge_requests_for + + ext.commits_for).uniq - [local_reference] end # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+. diff --git a/app/models/note.rb b/app/models/note.rb index fa5fdea4eb0..0c1d792ca9a 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -70,13 +70,17 @@ class Note < ActiveRecord::Base ) end - # +noteable+ was referenced from +mentioner+, by including GFM in either +mentioner+'s description or an associated Note. - # Create a system Note associated with +noteable+ with a GFM back-reference to +mentioner+. + # +noteable+ was referenced from +mentioner+, by including GFM in either + # +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) + note_options = { project: project, author: author, - note: "_mentioned in #{mentioner.gfm_reference}_", + note: "_mentioned in #{gfm_reference}_", system: true } @@ -163,12 +167,73 @@ class Note < ActiveRecord::Base # Determine whether or not a cross-reference note already exists. def cross_reference_exists?(noteable, mentioner) - where(noteable_id: noteable.id, system: true, note: "_mentioned in #{mentioner.gfm_reference}_").any? + gfm_reference = mentioner_gfm_ref(noteable, mentioner) + + where(['noteable_id = ? and system = ? and note like ?', + noteable.id, true, "_mentioned in #{gfm_reference}_"]).any? end def search(query) where("note like :query", query: "%#{query}%") end + + private + + # 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.repository.commit(noteable.id) + # The noteable commit belongs to the mentioner's project + mentioning_project.id + else + nil + end + else + noteable.project.id + end + 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 + mentioner.gfm_reference + else + if mentioner.is_a?(Commit) + mentioner.gfm_reference.sub( + /(commit )/, + "\\1#{mentioning_project.path_with_namespace}@" + ) + else + mentioner.gfm_reference.sub( + /(issue |merge request )/, + "\\1#{mentioning_project.path_with_namespace}" + ) + end + end + end end def commit_author diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 1673184cbe4..6f201adc4e8 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -53,11 +53,23 @@ eos describe '#closes_issues' do let(:issue) { create :issue, project: project } + let(:other_project) { create :project, :public } + let(:other_issue) { create :issue, project: other_project } it 'detects issues that this commit is marked as closing' do - commit.stub(issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/, safe_message: "Fixes ##{issue.iid}") + stub_const('Gitlab::ClosingIssueExtractor::ISSUE_CLOSING_REGEX', + /Fixes #\d+/) + commit.stub(safe_message: "Fixes ##{issue.iid}") commit.closes_issues(project).should == [issue] end + + it 'does not detect issues from other projects' do + ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}" + stub_const('Gitlab::ClosingIssueExtractor::ISSUE_CLOSING_REGEX', + /^([Cc]loses|[Ff]ixes)/) + commit.stub(safe_message: "Fixes #{ext_ref}") + commit.closes_issues(project).should be_empty + end end it_behaves_like 'a mentionable' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index da51100e0d7..c88a03beb0c 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -264,8 +264,8 @@ describe Note do let(:project) { create :project } let(:author) { create :user } let(:issue) { create :issue } - let(:commit0) { double 'commit0', gfm_reference: 'commit 123456' } - let(:commit1) { double 'commit1', gfm_reference: 'commit 654321' } + let(:commit0) { project.repository.commit } + let(:commit1) { project.repository.commit('HEAD~2') } before do Note.create_cross_reference_note(issue, commit0, author, project) diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 0d67e7ee4e6..692834c9f29 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -14,13 +14,23 @@ def common_mentionable_setup let(:mentioned_mr) { create :merge_request, :simple, source_project: mproject } let(:mentioned_commit) { double('commit', sha: '1234567890abcdef').as_null_object } + let(:ext_proj) { create :project, :public } + let(:ext_issue) { create :issue, project: ext_proj } + let(:other_ext_issue) { create :issue, project: ext_proj } + let(:ext_mr) { create :merge_request, :simple, source_project: ext_proj } + let(:ext_commit) { ext_proj.repository.commit } + # Override to add known commits to the repository stub. let(:extra_commits) { [] } # A string that mentions each of the +mentioned_.*+ objects above. Mentionables should add a self-reference # to this string and place it in their +mentionable_text+. let(:ref_string) do - "mentions ##{mentioned_issue.iid} twice ##{mentioned_issue.iid}, !#{mentioned_mr.iid}, " + + "mentions ##{mentioned_issue.iid} twice ##{mentioned_issue.iid}, " + + "!#{mentioned_mr.iid}, " + + "#{ext_proj.path_with_namespace}##{ext_issue.iid}, " + + "#{ext_proj.path_with_namespace}!#{ext_mr.iid}, " + + "#{ext_proj.path_with_namespace}@#{ext_commit.id[0..5]}, " + "#{mentioned_commit.sha[0..5]} and itself as #{backref_text}" end @@ -45,14 +55,20 @@ shared_examples 'a mentionable' do # De-duplicate and omit itself refs = subject.references(mproject) - refs.should have(3).items + refs.should have(6).items refs.should include(mentioned_issue) refs.should include(mentioned_mr) refs.should include(mentioned_commit) + refs.should include(ext_issue) + refs.should include(ext_mr) + refs.should include(ext_commit) end it 'creates cross-reference notes' do - [mentioned_issue, mentioned_mr, mentioned_commit].each do |referenced| + mentioned_objects = [mentioned_issue, mentioned_mr, mentioned_commit, + ext_issue, ext_mr, ext_commit] + + mentioned_objects.each do |referenced| Note.should_receive(:create_cross_reference_note).with(referenced, subject.local_reference, mauthor, mproject) end @@ -73,15 +89,25 @@ shared_examples 'an editable mentionable' do it_behaves_like 'a mentionable' it 'creates new cross-reference notes when the mentionable text is edited' do - new_text = "this text still mentions ##{mentioned_issue.iid} and #{mentioned_commit.sha[0..5]}, " + - "but now it mentions ##{other_issue.iid}, too." + new_text = "still mentions ##{mentioned_issue.iid}, " + + "#{mentioned_commit.sha[0..5]}, " + + "#{ext_issue.iid}, " + + "new refs: ##{other_issue.iid}, " + + "#{ext_proj.path_with_namespace}##{other_ext_issue.iid}" - [mentioned_issue, mentioned_commit].each do |oldref| + [mentioned_issue, mentioned_commit, ext_issue].each do |oldref| Note.should_not_receive(:create_cross_reference_note).with(oldref, subject.local_reference, mauthor, mproject) end - Note.should_receive(:create_cross_reference_note).with(other_issue, subject.local_reference, mauthor, mproject) + [other_issue, other_ext_issue].each do |newref| + Note.should_receive(:create_cross_reference_note).with( + newref, + subject.local_reference, + mauthor, + mproject + ) + end subject.save set_mentionable_text.call(new_text) |