Migrate the MergeRequestDiffs API to use `merge_request_iid`

- Instead of `merge_request_id`
- Duplicate the original `MergeRequestDiffs` API (and spec) for use with the V3
  API, since this is a breaking change.

lib/api/merge_request_diffs.rb

@@ -13,11 +13,11 @@ module API
 
       params do
         requires :id, type: String, desc: 'The ID of a project'
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
         use :pagination
       end
-      get ":id/merge_requests/:merge_request_id/versions" do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ":id/merge_requests/:merge_request_iid/versions" do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present paginate(merge_request.merge_request_diffs), with: Entities::MergeRequestDiff
       end
@@ -29,12 +29,12 @@ module API
 
       params do
         requires :id, type: String, desc: 'The ID of a project'
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
         requires :version_id, type: Integer, desc: 'The ID of a merge request diff version'
       end
 
-      get ":id/merge_requests/:merge_request_id/versions/:version_id" do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ":id/merge_requests/:merge_request_iid/versions/:version_id" do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull
       end

lib/api/v3/merge_request_diffs.rb

+module API
+  module V3
+    # MergeRequestDiff API
+    class MergeRequestDiffs < Grape::API
+      include PaginationParams
+
+      before { authenticate! }
+
+      resource :projects do
+        desc 'Get a list of merge request diff versions' do
+          detail 'This feature was introduced in GitLab 8.12.'
+          success ::API::Entities::MergeRequestDiff
+        end
+
+        params do
+          requires :id, type: String, desc: 'The ID of a project'
+          requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+          use :pagination
+        end
+        get ":id/merge_requests/:merge_request_id/versions" do
+          merge_request = find_merge_request_with_access(params[:merge_request_id])
+
+          present paginate(merge_request.merge_request_diffs), with: ::API::Entities::MergeRequestDiff
+        end
+
+        desc 'Get a single merge request diff version' do
+          detail 'This feature was introduced in GitLab 8.12.'
+          success ::API::Entities::MergeRequestDiffFull
+        end
+
+        params do
+          requires :id, type: String, desc: 'The ID of a project'
+          requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+          requires :version_id, type: Integer, desc: 'The ID of a merge request diff version'
+        end
+
+        get ":id/merge_requests/:merge_request_id/versions/:version_id" do
+          merge_request = find_merge_request_with_access(params[:merge_request_id])
+
+          present merge_request.merge_request_diffs.find(params[:version_id]), with: ::API::Entities::MergeRequestDiffFull
+        end
+      end
+    end
+  end
+end

spec/requests/api/merge_request_diffs_spec.rb

@@ -13,9 +13,9 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
     project.team << [user, :master]
   end
 
-  describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do
+  describe 'GET /projects/:id/merge_requests/:merge_request_iid/versions' do
     it 'returns 200 for a valid merge request' do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions", user)
       merge_request_diff = merge_request.merge_request_diffs.first
 
       expect(response.status).to eq 200
@@ -26,16 +26,22 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
       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
+    it 'returns a 404 when merge_request id is used instead of the iid' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user)
+      expect(response).to have_http_status(404)
+    end
+
+    it 'returns a 404 when merge_request_iid 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
+  describe 'GET /projects/:id/merge_requests/:merge_request_iid/versions/:version_id' do
+    let(:merge_request_diff) { merge_request.merge_request_diffs.first }
+
     it 'returns a 200 for a valid merge request' do
-      merge_request_diff = merge_request.merge_request_diffs.first
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}", user)
 
       expect(response.status).to eq 200
       expect(json_response['id']).to eq(merge_request_diff.id)
@@ -43,8 +49,18 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
       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)
+    it 'returns a 404 when merge_request id is used instead of the iid' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user)
+      expect(response).to have_http_status(404)
+    end
+
+    it 'returns a 404 when merge_request version_id is not found' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/999", user)
+      expect(response).to have_http_status(404)
+    end
+
+    it 'returns a 404 when merge_request_iid is not found' do
+      get api("/projects/#{project.id}/merge_requests/12345/versions/#{merge_request_diff.id}", user)
       expect(response).to have_http_status(404)
     end
   end

spec/requests/api/v3/merge_request_diffs_spec.rb

+require "spec_helper"
+
+describe API::V3::MergeRequestDiffs, '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
+    it 'returns 200 for a valid merge request' do
+      get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user)
+      merge_request_diff = merge_request.merge_request_diffs.first
+
+      expect(response.status).to eq 200
+      expect(response).to include_pagination_headers
+      expect(json_response).to be_an Array
+      expect(json_response.size).to eq(merge_request.merge_request_diffs.size)
+      expect(json_response.first['id']).to eq(merge_request_diff.id)
+      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 v3_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
+    it 'returns a 200 for a valid merge request' do
+      merge_request_diff = merge_request.merge_request_diffs.first
+      get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user)
+
+      expect(response.status).to eq 200
+      expect(json_response['id']).to eq(merge_request_diff.id)
+      expect(json_response['head_commit_sha']).to eq(merge_request_diff.head_commit_sha)
+      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 v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/999", user)
+      expect(response).to have_http_status(404)
+    end
+  end
+end