Commit 402e0a2d authored by Timothy Andrew's avatar Timothy Andrew

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.
parent 71932711
...@@ -13,11 +13,11 @@ module API ...@@ -13,11 +13,11 @@ module API
params do params do
requires :id, type: String, desc: 'The ID of a project' 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 use :pagination
end end
get ":id/merge_requests/:merge_request_id/versions" do get ":id/merge_requests/:merge_request_iid/versions" do
merge_request = find_merge_request_with_access(params[:merge_request_id]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
present paginate(merge_request.merge_request_diffs), with: Entities::MergeRequestDiff present paginate(merge_request.merge_request_diffs), with: Entities::MergeRequestDiff
end end
...@@ -29,12 +29,12 @@ module API ...@@ -29,12 +29,12 @@ module API
params do params do
requires :id, type: String, desc: 'The ID of a project' 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' requires :version_id, type: Integer, desc: 'The ID of a merge request diff version'
end end
get ":id/merge_requests/:merge_request_id/versions/:version_id" do get ":id/merge_requests/:merge_request_iid/versions/:version_id" do
merge_request = find_merge_request_with_access(params[:merge_request_id]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull
end end
......
...@@ -13,9 +13,9 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -13,9 +13,9 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
project.team << [user, :master] project.team << [user, :master]
end 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 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 merge_request_diff = merge_request.merge_request_diffs.first
expect(response.status).to eq 200 expect(response.status).to eq 200
...@@ -26,16 +26,22 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -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) expect(json_response.first['head_commit_sha']).to eq(merge_request_diff.head_commit_sha)
end 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) get api("/projects/#{project.id}/merge_requests/999/versions", user)
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
end end
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 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.iid}/versions/#{merge_request_diff.id}", user)
get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user)
expect(response.status).to eq 200 expect(response.status).to eq 200
expect(json_response['id']).to eq(merge_request_diff.id) expect(json_response['id']).to eq(merge_request_diff.id)
...@@ -43,8 +49,18 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -43,8 +49,18 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
expect(json_response['diffs'].size).to eq(merge_request_diff.diffs.size) expect(json_response['diffs'].size).to eq(merge_request_diff.diffs.size)
end 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/999", user) 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) expect(response).to have_http_status(404)
end end
end end
......
require "spec_helper" require "spec_helper"
describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do describe API::V3::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
include ApiHelpers include ApiHelpers
let!(:user) { create(:user) } let!(:user) { create(:user) }
...@@ -15,7 +15,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -15,7 +15,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do
it 'returns 200 for a valid merge request' 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 merge_request_diff = merge_request.merge_request_diffs.first
expect(response.status).to eq 200 expect(response.status).to eq 200
...@@ -25,7 +25,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -25,7 +25,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
end end
it 'returns a 404 when merge_request_id not found' do 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) expect(response).to have_http_status(404)
end end
end end
...@@ -33,7 +33,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -33,7 +33,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
describe 'GET /projects/:id/merge_requests/:merge_request_id/versions/:version_id' do describe 'GET /projects/:id/merge_requests/:merge_request_id/versions/:version_id' do
it 'returns a 200 for a valid merge request' do it 'returns a 200 for a valid merge request' do
merge_request_diff = merge_request.merge_request_diffs.first 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(response.status).to eq 200
expect(json_response['id']).to eq(merge_request_diff.id) expect(json_response['id']).to eq(merge_request_diff.id)
...@@ -42,7 +42,8 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do ...@@ -42,7 +42,8 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do
end end
it 'returns a 404 when merge_request_id not found' do 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) expect(response).to have_http_status(404)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment