diff --git a/app/models/note.rb b/app/models/note.rb index 4b5fa7a2ab5..e3c571a1574 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -238,9 +238,9 @@ class Note < ActiveRecord::Base def cross_reference_exists?(noteable, mentioner) gfm_reference = mentioner_gfm_ref(noteable, mentioner) notes = if noteable.is_a?(Commit) - where(commit_id: noteable.id) + where(commit_id: noteable.id, noteable_type: 'Commit') else - where(noteable_id: noteable.id) + where(noteable_id: noteable.id, noteable_type: noteable.class) end notes.where('note like ?', cross_reference_note_pattern(gfm_reference)). diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 11cc7762ce4..506bc3339b6 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -69,8 +69,9 @@ eos end it_behaves_like 'a mentionable' do - let(:subject) { commit } - let(:mauthor) { create :user, email: commit.author_email } + subject { commit } + + let(:author) { create(:user, email: commit.author_email) } let(:backref_text) { "commit #{subject.id}" } let(:set_mentionable_text) { ->(txt){ subject.stub(safe_message: txt) } } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 087e40c3d84..20d823b40e5 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -56,7 +56,8 @@ describe Issue do end it_behaves_like 'an editable mentionable' do - let(:subject) { create :issue, project: mproject } + subject { create(:issue, project: project) } + let(:backref_text) { "issue ##{subject.iid}" } let(:set_mentionable_text) { ->(txt){ subject.description = txt } } end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d40503d791c..3fcd063efe8 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -116,12 +116,13 @@ describe MergeRequest do end it_behaves_like 'an editable mentionable' do - let(:subject) { create :merge_request, source_project: mproject, target_project: mproject } + subject { create(:merge_request, source_project: project, target_project: project) } + let(:backref_text) { "merge request !#{subject.iid}" } let(:set_mentionable_text) { ->(txt){ subject.title = txt } } end it_behaves_like 'a Taskable' do - let(:subject) { create :merge_request, :simple } + subject { create :merge_request, :simple } end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index a7bf5081d5b..e7e6717e775 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -629,8 +629,9 @@ describe Note do end it_behaves_like 'an editable mentionable' do + subject { create :note, noteable: issue, project: project } + let(:issue) { create :issue, project: project } - let(:subject) { create :note, noteable: issue, project: project } let(:backref_text) { issue.gfm_reference } let(:set_mentionable_text) { ->(txt) { subject.note = txt } } end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 305592fa5a6..25ec5cbe6f2 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -1,23 +1,20 @@ # Specifications for behavior common to all Mentionable implementations. # Requires a shared context containing: -# - let(:subject) { "the mentionable implementation" } +# - subject { "the mentionable implementation" } # - let(:backref_text) { "the way that +subject+ should refer to itself in backreferences " } # - let(:set_mentionable_text) { lambda { |txt| "block that assigns txt to the subject's mentionable_text" } } def common_mentionable_setup - # Avoid name collisions with let(:project) or let(:author) in the surrounding scope. - let(:mproject) { create :project } - let(:mauthor) { subject.author } + let(:project) { create :project } + let(:author) { subject.author } - let(:mentioned_issue) { create :issue, project: mproject } - let(:other_issue) { create :issue, project: mproject } - let(:mentioned_mr) { create :merge_request, :simple, source_project: mproject } - let(:mentioned_commit) { double('commit', sha: '1234567890abcdef').as_null_object } + let(:mentioned_issue) { create(:issue, project: project) } + let(:mentioned_mr) { create(:merge_request, :simple, source_project: project) } + let(:mentioned_commit) { project.repository.commit } - 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_proj) { create(:project, :public) } + let(: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. @@ -26,20 +23,37 @@ def common_mentionable_setup # 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}, " + - "#{ext_proj.path_with_namespace}##{ext_issue.iid}, " + - "#{ext_proj.path_with_namespace}!#{ext_mr.iid}, " + - "#{ext_proj.path_with_namespace}@#{ext_commit.short_id}, " + - "#{mentioned_commit.sha[0..10]} and itself as #{backref_text}" + cross = ext_proj.path_with_namespace + + <<-MSG.strip_heredoc + These references are new: + Issue: ##{mentioned_issue.iid} + Merge: !#{mentioned_mr.iid} + Commit: #{mentioned_commit.id} + + This reference is a repeat and should only be mentioned once: + Repeat: ##{mentioned_issue.iid} + + These references are cross-referenced: + Issue: #{cross}##{ext_issue.iid} + Merge: #{cross}!#{ext_mr.iid} + Commit: #{cross}@#{ext_commit.short_id} + + This is a self-reference and should not be mentioned at all: + Self: #{backref_text} + MSG end before do - # Wire the project's repository to return the mentioned commit, and +nil+ for any - # unrecognized commits. - commitmap = { '1234567890a' => mentioned_commit } + # Wire the project's repository to return the mentioned commit, and +nil+ + # for any unrecognized commits. + commitmap = { + mentioned_commit.id => mentioned_commit + } extra_commits.each { |c| commitmap[c.short_id] = c } - allow(mproject.repository).to receive(:commit) { |sha| commitmap[sha] } + + allow(project.repository).to receive(:commit) { |sha| commitmap[sha] } + set_mentionable_text.call(ref_string) end end @@ -53,7 +67,7 @@ shared_examples 'a mentionable' do it "extracts references from its reference property" do # De-duplicate and omit itself - refs = subject.references(mproject) + refs = subject.references(project) expect(refs.size).to eq(6) expect(refs).to include(mentioned_issue) expect(refs).to include(mentioned_mr) @@ -68,17 +82,18 @@ shared_examples 'a mentionable' do ext_issue, ext_mr, ext_commit] mentioned_objects.each do |referenced| - expect(Note).to receive(:create_cross_reference_note).with(referenced, subject.local_reference, mauthor, mproject) + expect(Note).to receive(:create_cross_reference_note). + with(referenced, subject.local_reference, author, project) end - subject.create_cross_references!(mproject, mauthor) + subject.create_cross_references!(project, author) end it 'detects existing cross-references' do - Note.create_cross_reference_note(mentioned_issue, subject.local_reference, mauthor, mproject) + Note.create_cross_reference_note(mentioned_issue, subject.local_reference, author, project) - expect(subject.has_mentioned?(mentioned_issue)).to be_truthy - expect(subject.has_mentioned?(mentioned_mr)).to be_falsey + expect(subject).to have_mentioned(mentioned_issue) + expect(subject).not_to have_mentioned(mentioned_mr) end end @@ -87,29 +102,41 @@ 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 = "still mentions ##{mentioned_issue.iid}, " + - "#{mentioned_commit.sha[0..10]}, " + - "#{ext_issue.iid}, " + - "new refs: ##{other_issue.iid}, " + - "#{ext_proj.path_with_namespace}##{other_ext_issue.iid}" + let(:new_issues) do + [create(:issue, project: project), create(:issue, project: ext_proj)] + end + it 'creates new cross-reference notes when the mentionable text is edited' do + cross = ext_proj.path_with_namespace + + new_text = <<-MSG + These references already existed: + Issue: ##{mentioned_issue.iid} + Commit: #{mentioned_commit.id} + + This cross-project reference already existed: + Issue: #{cross}##{ext_issue.iid} + + These two references are introduced in an edit: + Issue: ##{new_issues[0].iid} + Cross: #{cross}##{new_issues[1].iid} + MSG + + # These three objects were already referenced, and should not receive new + # notes [mentioned_issue, mentioned_commit, ext_issue].each do |oldref| - expect(Note).not_to receive(:create_cross_reference_note).with(oldref, subject.local_reference, - mauthor, mproject) + expect(Note).not_to receive(:create_cross_reference_note). + with(oldref, any_args) end - [other_issue, other_ext_issue].each do |newref| - expect(Note).to receive(:create_cross_reference_note).with( - newref, - subject.local_reference, - mauthor, - mproject - ) + # 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) end subject.save set_mentionable_text.call(new_text) - subject.notice_added_references(mproject, mauthor) + subject.notice_added_references(project, author) end end diff --git a/spec/support/taskable_shared_examples.rb b/spec/support/taskable_shared_examples.rb index 490f453d468..8e5e3a8aafc 100644 --- a/spec/support/taskable_shared_examples.rb +++ b/spec/support/taskable_shared_examples.rb @@ -1,7 +1,7 @@ # Specs for task state functionality for issues and merge requests. # # Requires a context containing: -# let(:subject) { Issue or MergeRequest } +# subject { Issue or MergeRequest } shared_examples 'a Taskable' do before do subject.description = <