Merge branch 'link-refs' into 'master'

Recognize issue/MR/snippet/commit links as references.

Fixes #3744 and #3745

See merge request !1933

2 files changed, 75 insertions, 73 deletions

diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb
index 1031af097bd..3c1009a2eb0 100644
--- a/spec/models/commit_range_spec.rb
+++ b/spec/models/commit_range_spec.rb
@@ -7,50 +7,72 @@ describe CommitRange do
     it { is_expected.to include_module(Referable) }
   end
 
-  let(:sha_from) { 'f3f85602' }
-  let(:sha_to)   { 'e86e1013' }
+  let!(:project) { create(:project, :public) }
+  let!(:commit1) { project.commit("HEAD~2") }
+  let!(:commit2) { project.commit }
 
-  let(:range)  { described_class.new("#{sha_from}...#{sha_to}") }
-  let(:range2) { described_class.new("#{sha_from}..#{sha_to}") }
+  let(:sha_from) { commit1.short_id }
+  let(:sha_to)   { commit2.short_id }
+
+  let(:full_sha_from) { commit1.id }
+  let(:full_sha_to)   { commit2.id }
+
+  let(:range)  { described_class.new("#{sha_from}...#{sha_to}", project) }
+  let(:range2) { described_class.new("#{sha_from}..#{sha_to}", project) }
 
   it 'raises ArgumentError when given an invalid range string' do
-    expect { described_class.new("Foo") }.to raise_error(ArgumentError)
+    expect { described_class.new("Foo", project) }.to raise_error(ArgumentError)
   end
 
   describe '#to_s' do
     it 'is correct for three-dot syntax' do
-      expect(range.to_s).to eq "#{sha_from[0..7]}...#{sha_to[0..7]}"
+      expect(range.to_s).to eq "#{full_sha_from}...#{full_sha_to}"
     end
 
     it 'is correct for two-dot syntax' do
-      expect(range2.to_s).to eq "#{sha_from[0..7]}..#{sha_to[0..7]}"
+      expect(range2.to_s).to eq "#{full_sha_from}..#{full_sha_to}"
     end
   end
 
   describe '#to_reference' do
-    let(:project) { double('project', to_reference: 'namespace1/project') }
+    let(:cross) { create(:project) }
+
+    it 'returns a String reference to the object' do
+      expect(range.to_reference).to eq "#{full_sha_from}...#{full_sha_to}"
+    end
+
+    it 'returns a String reference to the object' do
+      expect(range2.to_reference).to eq "#{full_sha_from}..#{full_sha_to}"
+    end
+
+    it 'supports a cross-project reference' do
+      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{full_sha_from}...#{full_sha_to}"
+    end
+  end
 
-    before do
-      range.project = project
+  describe '#reference_link_text' do
+    let(:cross) { create(:project) }
+
+    it 'returns a String reference to the object' do
+      expect(range.reference_link_text).to eq "#{sha_from}...#{sha_to}"
     end
 
     it 'returns a String reference to the object' do
-      expect(range.to_reference).to eq range.to_s
+      expect(range2.reference_link_text).to eq "#{sha_from}..#{sha_to}"
     end
 
     it 'supports a cross-project reference' do
-      cross = double('project')
-      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{range.to_s}"
+      expect(range.reference_link_text(cross)).to eq "#{project.to_reference}@#{sha_from}...#{sha_to}"
     end
   end
 
   describe '#reference_title' do
     it 'returns the correct String for three-dot ranges' do
-      expect(range.reference_title).to eq "Commits #{sha_from} through #{sha_to}"
+      expect(range.reference_title).to eq "Commits #{full_sha_from} through #{full_sha_to}"
     end
 
     it 'returns the correct String for two-dot ranges' do
-      expect(range2.reference_title).to eq "Commits #{sha_from}^ through #{sha_to}"
+      expect(range2.reference_title).to eq "Commits #{full_sha_from}^ through #{full_sha_to}"
     end
   end
 
@@ -60,11 +82,11 @@ describe CommitRange do
     end
 
     it 'includes the correct values for a three-dot range' do
-      expect(range.to_param).to eq({ from: sha_from, to: sha_to })
+      expect(range.to_param).to eq({ from: full_sha_from, to: full_sha_to })
     end
 
     it 'includes the correct values for a two-dot range' do
-      expect(range2.to_param).to eq({ from: sha_from + '^', to: sha_to })
+      expect(range2.to_param).to eq({ from: full_sha_from + '^', to: full_sha_to })
     end
   end
 
@@ -79,64 +101,37 @@ describe CommitRange do
   end
 
   describe '#valid_commits?' do
-    context 'without a project' do
-      it 'returns nil' do
-        expect(range.valid_commits?).to be_nil
+    context 'with a valid repo' do
+      before do
+        expect(project).to receive(:valid_repo?).and_return(true)
       end
-    end
-
-    it 'accepts an optional project argument' do
-      project1 = double('project1').as_null_object
-      project2 = double('project2').as_null_object
-
-      # project1 gets assigned through the accessor, but ignored when not given
-      # as an argument to `valid_commits?`
-      expect(project1).not_to receive(:present?)
-      range.project = project1
-
-      # project2 gets passed to `valid_commits?`
-      expect(project2).to receive(:present?).and_return(false)
 
-      range.valid_commits?(project2)
-    end
-
-    context 'with a project' do
-      let(:project) { double('project', repository: double('repository')) }
+      it 'is false when `sha_from` is invalid' do
+        expect(project).to receive(:commit).with(sha_from).and_return(nil)
+        expect(project).to receive(:commit).with(sha_to).and_call_original
 
-      context 'with a valid repo' do
-        before do
-          expect(project).to receive(:valid_repo?).and_return(true)
-          range.project = project
-        end
+        expect(range).not_to be_valid_commits
+      end
 
-        it 'is false when `sha_from` is invalid' do
-          expect(project.repository).to receive(:commit).with(sha_from).and_return(false)
-          expect(project.repository).not_to receive(:commit).with(sha_to)
-          expect(range).not_to be_valid_commits
-        end
+      it 'is false when `sha_to` is invalid' do
+        expect(project).to receive(:commit).with(sha_from).and_call_original
+        expect(project).to receive(:commit).with(sha_to).and_return(nil)
 
-        it 'is false when `sha_to` is invalid' do
-          expect(project.repository).to receive(:commit).with(sha_from).and_return(true)
-          expect(project.repository).to receive(:commit).with(sha_to).and_return(false)
-          expect(range).not_to be_valid_commits
-        end
+        expect(range).not_to be_valid_commits
+      end
 
-        it 'is true when both `sha_from` and `sha_to` are valid' do
-          expect(project.repository).to receive(:commit).with(sha_from).and_return(true)
-          expect(project.repository).to receive(:commit).with(sha_to).and_return(true)
-          expect(range).to be_valid_commits
-        end
+      it 'is true when both `sha_from` and `sha_to` are valid' do
+        expect(range).to be_valid_commits
       end
+    end
 
-      context 'without a valid repo' do
-        before do
-          expect(project).to receive(:valid_repo?).and_return(false)
-          range.project = project
-        end
+    context 'without a valid repo' do
+      before do
+        expect(project).to receive(:valid_repo?).and_return(false)
+      end
 
-        it 'returns false' do
-          expect(range).not_to be_valid_commits
-        end
+      it 'returns false' do
+        expect(range).not_to be_valid_commits
       end
     end
   end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 90be9324951..974b52c1833 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -24,6 +24,17 @@ describe Commit do
     end
   end
 
+  describe '#reference_link_text' do
+    it 'returns a String reference to the object' do
+      expect(commit.reference_link_text).to eq commit.short_id
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(commit.reference_link_text(cross)).to eq "#{project.to_reference}@#{commit.short_id}"
+    end
+  end
+
   describe '#title' do
     it "returns no_commit_message when safe_message is blank" do
       allow(commit).to receive(:safe_message).and_return('')
@@ -77,14 +88,10 @@ eos
     let(:other_issue) { create :issue, project: other_project }
 
     it 'detects issues that this commit is marked as closing' do
-      allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid}")
-      expect(commit.closes_issues).to eq([issue])
-    end
-
-    it 'does not detect issues from other projects' do
       ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}"
-      allow(commit).to receive(:safe_message).and_return("Fixes #{ext_ref}")
-      expect(commit.closes_issues).to be_empty
+      allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid} and #{ext_ref}")
+      expect(commit.closes_issues).to include(issue)
+      expect(commit.closes_issues).to include(other_issue)
     end
   end