Merge branch 'dz-merge-request-version'

5 files changed, 143 insertions, 14 deletions

diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb
new file mode 100644
index 00000000000..577c910f11b
--- /dev/null
+++ b/spec/features/merge_requests/merge_request_versions_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+feature 'Merge Request versions', js: true, feature: true do
+  before do
+    login_as :admin
+    merge_request = create(:merge_request, importing: true)
+    merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+    merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e')
+    project = merge_request.source_project
+    visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
+  end
+
+  it 'show the latest version of the diff' do
+    page.within '.mr-version-switch' do
+      expect(page).to have_content 'Version: latest'
+    end
+
+    expect(page).to have_content '8 changed files'
+  end
+
+  describe 'switch between versions' do
+    before do
+      page.within '.mr-version-switch' do
+        find('.btn-link').click
+        click_link '6f6d7e7e'
+      end
+    end
+
+    it 'should show older version' do
+      page.within '.mr-version-switch' do
+        expect(page).to have_content 'Version: 6f6d7e7e'
+      end
+
+      expect(page).to have_content '5 changed files'
+    end
+  end
+end
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 29f7396f862..e5b185dc3f6 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -1,6 +1,27 @@
 require 'spec_helper'
 
 describe MergeRequestDiff, models: true do
+  describe 'create new record' do
+    subject { create(:merge_request).merge_request_diff }
+
+    it { expect(subject).to be_valid }
+    it { expect(subject).to be_persisted }
+    it { expect(subject.commits.count).to eq(5) }
+    it { expect(subject.diffs.count).to eq(8) }
+    it { expect(subject.head_commit_sha).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
+    it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
+    it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
+  end
+
+  describe '#latest' do
+    let!(:mr) { create(:merge_request, :with_diffs) }
+    let!(:first_diff) { mr.merge_request_diff }
+    let!(:last_diff) { mr.create_merge_request_diff }
+
+    it { expect(last_diff.latest?).to be_truthy }
+    it { expect(first_diff.latest?).to be_falsey }
+  end
+
   describe '#diffs' do
     let(:mr) { create(:merge_request, :with_diffs) }
     let(:mr_diff) { mr.merge_request_diff }
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 64c56d922ff..18d178be49c 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -9,7 +9,7 @@ describe MergeRequest, models: true do
     it { is_expected.to belong_to(:target_project).with_foreign_key(:target_project_id).class_name('Project') }
     it { is_expected.to belong_to(:source_project).with_foreign_key(:source_project_id).class_name('Project') }
     it { is_expected.to belong_to(:merge_user).class_name("User") }
-    it { is_expected.to have_one(:merge_request_diff).dependent(:destroy) }
+    it { is_expected.to have_many(:merge_request_diffs).dependent(:destroy) }
   end
 
   describe 'modules' do
@@ -159,7 +159,7 @@ describe MergeRequest, models: true do
 
     context 'when there are MR diffs' do
       it 'delegates to the MR diffs' do
-        merge_request.merge_request_diff = MergeRequestDiff.new
+        merge_request.save
 
         expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options))
 
@@ -316,7 +316,7 @@ describe MergeRequest, models: true do
     end
 
     it "can be removed if the last commit is the head of the source branch" do
-      allow(subject.source_project).to receive(:commit).and_return(subject.diff_head_commit)
+      allow(subject).to receive(:source_branch_head).and_return(subject.diff_head_commit)
 
       expect(subject.can_remove_source_branch?(user)).to be_truthy
     end
@@ -721,12 +721,15 @@ describe MergeRequest, models: true do
 
     let(:commit) { subject.project.commit(sample_commit.id) }
 
-    it "reloads the diff content" do
-      expect(subject.merge_request_diff).to receive(:reload_content)
-
+    it "does not change existing merge request diff" do
+      expect(subject.merge_request_diff).not_to receive(:save_git_content)
       subject.reload_diff
     end
 
+    it "creates new merge request diff" do
+      expect { subject.reload_diff }.to change { subject.merge_request_diffs.count }.by(1)
+    end
+
     it "executs diff cache service" do
       expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject)
 
@@ -736,13 +739,15 @@ describe MergeRequest, models: true do
     it "updates diff note positions" do
       old_diff_refs = subject.diff_refs
 
-      merge_request_diff = subject.merge_request_diff
-
       # Update merge_request_diff so that #diff_refs will return commit.diff_refs
-      allow(merge_request_diff).to receive(:reload_content) do
-        merge_request_diff.base_commit_sha = commit.parent_id
-        merge_request_diff.start_commit_sha = commit.parent_id
-        merge_request_diff.head_commit_sha = commit.sha
+      allow(subject).to receive(:create_merge_request_diff) do
+        subject.merge_request_diffs.create(
+          base_commit_sha: commit.parent_id,
+          start_commit_sha: commit.parent_id,
+          head_commit_sha: commit.sha
+        )
+
+        subject.merge_request_diff(true)
       end
 
       expect(Notes::DiffPositionUpdateService).to receive(:new).with(
@@ -752,14 +757,31 @@ describe MergeRequest, models: true do
         new_diff_refs: commit.diff_refs,
         paths: note.position.paths
       ).and_call_original
-      expect_any_instance_of(Notes::DiffPositionUpdateService).to receive(:execute).with(note)
 
+      expect_any_instance_of(Notes::DiffPositionUpdateService).to receive(:execute).with(note)
       expect_any_instance_of(DiffNote).to receive(:save).once
 
       subject.reload_diff
     end
   end
 
+  describe '#branch_merge_base_commit' do
+    context 'source and target branch exist' do
+      it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
+      it { expect(subject.branch_merge_base_commit).to be_a(Commit) }
+    end
+
+    context 'when the target branch does not exist' do
+      before do
+        subject.project.repository.raw_repository.delete_branch(subject.target_branch)
+      end
+
+      it 'returns nil' do
+        expect(subject.branch_merge_base_commit).to be_nil
+      end
+    end
+  end
+
   describe "#diff_sha_refs" do
     context "with diffs" do
       subject { create(:merge_request, :with_diffs) }
diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb
new file mode 100644
index 00000000000..8f1e5ac9891
--- /dev/null
+++ b/spec/requests/api/merge_request_diffs_spec.rb
@@ -0,0 +1,49 @@
+require "spec_helper"
+
+describe API::API, 'MergeRequestDiffs', api: true  do
+  include ApiHelpers
+
+  let!(:user)          { create(:user) }
+  let!(:merge_request) { create(:merge_request, importing: true) }
+  let!(:project)       { merge_request.target_project }
+
+  before do
+    merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+    merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e')
+    project.team << [user, :master]
+  end
+
+  describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do
+    context 'valid merge request' do
+      before { get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) }
+      let(:merge_request_diff) { merge_request.merge_request_diffs.first }
+
+      it { expect(response.status).to eq 200 }
+      it { expect(json_response.size).to eq(merge_request.merge_request_diffs.size) }
+      it { expect(json_response.first['id']).to eq(merge_request_diff.id) }
+      it { expect(json_response.first['head_commit_sha']).to eq(merge_request_diff.head_commit_sha) }
+    end
+
+    it 'returns a 404 when merge_request_id not found' do
+      get api("/projects/#{project.id}/merge_requests/999/versions", user)
+      expect(response).to have_http_status(404)
+    end
+  end
+
+  describe 'GET /projects/:id/merge_requests/:merge_request_id/versions/:version_id' do
+    context 'valid merge request' do
+      before { get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) }
+      let(:merge_request_diff) { merge_request.merge_request_diffs.first }
+
+      it { expect(response.status).to eq 200 }
+      it { expect(json_response['id']).to eq(merge_request_diff.id) }
+      it { expect(json_response['head_commit_sha']).to eq(merge_request_diff.head_commit_sha) }
+      it { expect(json_response['diffs'].size).to eq(merge_request_diff.diffs.size) }
+    end
+
+    it 'returns a 404 when merge_request_id not found' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/999", user)
+      expect(response).to have_http_status(404)
+    end
+  end
+end
diff --git a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb
index c4b87468275..807f89e80b7 100644
--- a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb
+++ b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb
@@ -6,7 +6,7 @@ describe MergeRequests::MergeRequestDiffCacheService do
   describe '#execute' do
     it 'retrieves the diff files to cache the highlighted result' do
       merge_request = create(:merge_request)
-      cache_key = [merge_request.merge_request_diff, 'highlighted-diff-files', Gitlab::Diff::FileCollection::MergeRequest.default_options]
+      cache_key = [merge_request.merge_request_diff, 'highlighted-diff-files', Gitlab::Diff::FileCollection::MergeRequestDiff.default_options]
 
       expect(Rails.cache).to receive(:read).with(cache_key).and_return({})
       expect(Rails.cache).to receive(:write).with(cache_key, anything)