Commit d071f965 authored by Toon Claes's avatar Toon Claes

API: make Merge Request approve/unapprove endpoints work with iid

All the other endpoints of `/project/:id/merge_request` work with
`:merge_request_iid`, except for `approvals`, `approve`, and
`unapprove`. This commit fixes that.

Closes gitlab-org/gitlab-ee#1875
parent 78582eee
...@@ -546,15 +546,15 @@ You can request information about a merge request's approval status using the ...@@ -546,15 +546,15 @@ You can request information about a merge request's approval status using the
following endpoint: following endpoint:
``` ```
GET /projects/:id/merge_requests/:merge_request_id/approvals GET /projects/:id/merge_requests/:merge_request_iid/approvals
``` ```
**Parameters:** **Parameters:**
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
|--------------------|---------|----------|---------------------| |---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project | | `id` | integer | yes | The ID of a project |
| `merge_request_id` | integer | yes | The ID of MR | | `merge_request_iid` | integer | yes | The IID of MR |
```json ```json
{ {
...@@ -592,15 +592,15 @@ If you are allowed to, you can approve a merge request using the following ...@@ -592,15 +592,15 @@ If you are allowed to, you can approve a merge request using the following
endpoint: endpoint:
``` ```
POST /projects/:id/merge_requests/:merge_request_id/approve POST /projects/:id/merge_requests/:merge_request_iid/approve
``` ```
**Parameters:** **Parameters:**
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
|--------------------|---------|----------|---------------------| |---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project | | `id` | integer | yes | The ID of a project |
| `merge_request_id` | integer | yes | The ID of MR | | `merge_request_iid` | integer | yes | The IID of MR |
```json ```json
{ {
......
...@@ -249,14 +249,13 @@ module API ...@@ -249,14 +249,13 @@ module API
# #
# Parameters: # Parameters:
# id (required) - The ID of a project # id (required) - The ID of a project
# merge_request_id (required) - ID of MR # merge_request_idd (required) - IID of MR
# Examples: # Examples:
# GET /projects/:id/merge_requests/:merge_request_id/approvals # GET /projects/:id/merge_requests/:merge_request_iid/approvals
# #
get ':id/merge_requests/:merge_request_id/approvals' do get ':id/merge_requests/:merge_request_iid/approvals' do
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
authorize! :read_merge_request, merge_request
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
end end
...@@ -264,12 +263,12 @@ module API ...@@ -264,12 +263,12 @@ module API
# #
# Parameters: # Parameters:
# id (required) - The ID of a project # id (required) - The ID of a project
# merge_request_id (required) - ID of MR # merge_request_iid (required) - IID of MR
# Examples: # Examples:
# POST /projects/:id/merge_requests/:merge_request_id/approvals # POST /projects/:id/merge_requests/:merge_request_iid/approve
# #
post ':id/merge_requests/:merge_request_id/approve' do post ':id/merge_requests/:merge_request_iid/approve' do
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = find_project_merge_request(params[:merge_request_iid])
unauthorized! unless merge_request.can_approve?(current_user) unauthorized! unless merge_request.can_approve?(current_user)
...@@ -280,8 +279,8 @@ module API ...@@ -280,8 +279,8 @@ module API
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
end end
delete ':id/merge_requests/:merge_request_id/unapprove' do delete ':id/merge_requests/:merge_request_iid/unapprove' do
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = find_project_merge_request(params[:merge_request_iid])
not_found! unless merge_request.has_approved?(current_user) not_found! unless merge_request.has_approved?(current_user)
...@@ -298,8 +297,8 @@ module API ...@@ -298,8 +297,8 @@ module API
params do params do
use :pagination use :pagination
end end
get ':id/merge_requests/:merge_request_id/closes_issues' do get ':id/merge_requests/:merge_request_iid/closes_issues' do
merge_request = find_merge_request_with_access(params[:merge_request_id]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user)) issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user))
present paginate(issues), with: issue_entity(user_project), current_user: current_user present paginate(issues), with: issue_entity(user_project), current_user: current_user
end end
......
...@@ -838,7 +838,7 @@ describe API::MergeRequests, api: true do ...@@ -838,7 +838,7 @@ describe API::MergeRequests, api: true do
end end
end end
describe 'GET :id/merge_requests/:merge_request_id/approvals' do describe 'GET :id/merge_requests/:merge_request_iid/approvals' do
it 'retrieves the approval status' do it 'retrieves the approval status' do
approver = create :user approver = create :user
project.update_attribute(:approvals_before_merge, 2) project.update_attribute(:approvals_before_merge, 2)
...@@ -846,7 +846,7 @@ describe API::MergeRequests, api: true do ...@@ -846,7 +846,7 @@ describe API::MergeRequests, api: true do
project.team << [create(:user), :developer] project.team << [create(:user), :developer]
merge_request.approvals.create(user: approver) merge_request.approvals.create(user: approver)
get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/approvals", user) get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", user)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response['approvals_required']).to eq 2 expect(json_response['approvals_required']).to eq 2
...@@ -857,11 +857,11 @@ describe API::MergeRequests, api: true do ...@@ -857,11 +857,11 @@ describe API::MergeRequests, api: true do
end end
end end
describe 'POST :id/merge_requests/:merge_request_id/approve' do describe 'POST :id/merge_requests/:merge_request_iid/approve' do
before { project.update_attribute(:approvals_before_merge, 2) } before { project.update_attribute(:approvals_before_merge, 2) }
context 'as the author of the merge request' do context 'as the author of the merge request' do
before { post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/approve", user) } before { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approve", user) }
it 'returns a 401' do it 'returns a 401' do
expect(response).to have_http_status(401) expect(response).to have_http_status(401)
...@@ -875,7 +875,7 @@ describe API::MergeRequests, api: true do ...@@ -875,7 +875,7 @@ describe API::MergeRequests, api: true do
project.team << [approver, :developer] project.team << [approver, :developer]
project.team << [create(:user), :developer] project.team << [create(:user), :developer]
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/approve", approver) post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approve", approver)
end end
it 'approves the merge request' do it 'approves the merge request' do
...@@ -887,7 +887,7 @@ describe API::MergeRequests, api: true do ...@@ -887,7 +887,7 @@ describe API::MergeRequests, api: true do
end end
end end
describe 'DELETE :id/merge_requests/:merge_request_id/unapprove' do describe 'DELETE :id/merge_requests/:merge_request_iid/unapprove' do
before { project.update_attribute(:approvals_before_merge, 2) } before { project.update_attribute(:approvals_before_merge, 2) }
context 'as a user who has approved the merge request' do context 'as a user who has approved the merge request' do
...@@ -901,7 +901,7 @@ describe API::MergeRequests, api: true do ...@@ -901,7 +901,7 @@ describe API::MergeRequests, api: true do
merge_request.approvals.create(user: approver) merge_request.approvals.create(user: approver)
merge_request.approvals.create(user: unapprover) merge_request.approvals.create(user: unapprover)
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unapprove", unapprover) delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unapprove", unapprover)
end end
it 'unapproves the merge request' do it 'unapproves the merge request' do
......
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