From 402e0a2dd7cd9aa6766500b9b6809c97dfd40912 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Tue, 28 Feb 2017 10:56:09 +0530
Subject: [PATCH] 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 | 12 +++----
spec/requests/api/merge_request_diffs_spec.rb | 32 ++++++++++++++-----
.../api/v3/merge_request_diffs_spec.rb | 11 ++++---
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
--
GitLab