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.

3 files changed, 36 insertions, 19 deletions

diff --git a/lib/api/merge_request_diffs.rb b/lib/api/merge_request_diffs.rb
index 4901a7cfea6..a59e39cca26 100644
--- a/lib/api/merge_request_diffs.rb
+++ b/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
diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb
index 1d02e827183..79f3151ba52 100644
--- a/spec/requests/api/merge_request_diffs_spec.rb
+++ b/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
diff --git a/spec/requests/api/v3/merge_request_diffs_spec.rb b/spec/requests/api/v3/merge_request_diffs_spec.rb
index e1887138aab..c53800eef30 100644
--- a/spec/requests/api/v3/merge_request_diffs_spec.rb
+++ b/spec/requests/api/v3/merge_request_diffs_spec.rb
@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
+describe API::V3::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
   include ApiHelpers
 
   let!(:user)          { create(:user) }
@@ -15,7 +15,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
 
   describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do
     it 'returns 200 for a valid merge request' do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user)
+      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
@@ -25,7 +25,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
     end
 
     it 'returns a 404 when merge_request_id not found' do
-      get api("/projects/#{project.id}/merge_requests/999/versions", user)
+      get v3_api("/projects/#{project.id}/merge_requests/999/versions", user)
       expect(response).to have_http_status(404)
     end
   end
@@ -33,7 +33,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
   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 api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user)
+      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)
@@ -42,7 +42,8 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true  do
     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)
+      get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/999", user)
+
       expect(response).to have_http_status(404)
     end
   end