Skip to content

G
gitlab-ce
Commit 07060a67 authored by Michelle Gill's avatar Michelle Gill
Browse files
Options

Remove deprecated /approvers endpoint 

The /projects/:id/merge_requests/:merge_request_iid/approvers endpoint
was deprecated and scheduled to be removed in 12.0. It is time to drop
support for it as almost all users have stopped using it.
parent d86e7c7f
Show whitespace changes
Inline Side-by-side
Showing with 5 additions and 204 deletions
doc/api/merge_request_approvals.md
View file @ 07060a67
...@@ -769,81 +769,6 @@ POST /projects/:id/merge_requests/:merge_request_iid/approvals ...@@ -769,81 +769,6 @@ POST /projects/:id/merge_requests/:merge_request_iid/approvals
} }
``` ```
### Change allowed approvers for Merge Request
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/183) in GitLab 10.6.
> - Moved to GitLab Premium in 13.9.
NOTE:
This API endpoint has been deprecated. Please use Approval Rule API instead.
If you are allowed to, you can change approvers and approver groups using
the following endpoint:
```plaintext
PUT /projects/:id/merge_requests/:merge_request_iid/approvers
```
**Important:** Approvers and groups not in the request are **removed**
**Parameters:**
| Attribute | Type | Required | Description |
|----------------------|---------|----------|-----------------------------------------------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of MR |
| `approver_ids` | Array | yes | An array of User IDs that can approve the MR |
| `approver_group_ids` | Array | yes | An array of Group IDs whose members can approve the MR |
```json
{
"id": 5,
"iid": 5,
"project_id": 1,
"title": "Approvals API",
"description": "Test",
"state": "opened",
"created_at": "2016-06-08T00:19:52.638Z",
"updated_at": "2016-06-08T21:20:42.470Z",
"merge_status": "cannot_be_merged",
"approvals_required": 2,
"approvals_left": 2,
"approved_by": [],
"approvers": [
{
"user": {
"name": "Administrator",
"username": "root",
"id": 1,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon",
"web_url": "http://localhost:3000/root"
}
}
],
"approver_groups": [
{
"group": {
"id": 5,
"name": "group1",
"path": "group1",
"description": "",
"visibility": "public",
"lfs_enabled": false,
"avatar_url": null,
"web_url": "http://localhost/groups/group1",
"request_access_enabled": false,
"full_name": "group1",
"full_path": "group1",
"parent_id": null,
"ldap_cn": null,
"ldap_access": null
}
}
]
}
```
### Get the approval state of merge requests ### Get the approval state of merge requests
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13712) in GitLab 12.3. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13712) in GitLab 12.3.
......
ee/changelogs/unreleased/mgill-deprecate-mr-approvers.yml 0 → 100644
View file @ 07060a67
---
title: Remove /projects/:id/merge_requests/:merge_request_iid/approvers endpoint
merge_request: 57344
author:
type: removed
ee/lib/ee/api/merge_request_approvals.rb
View file @ 07060a67
...@@ -77,30 +77,6 @@ module EE ...@@ -77,30 +77,6 @@ module EE
present_approval(merge_request) present_approval(merge_request)
end end
# Deprecated in favor of approval rules API
desc 'Update approvers and approver groups' do
detail 'This feature was introduced in 10.6'
success ::EE::API::Entities::ApprovalState
end
params do
requires :approver_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce,
desc: 'Array of User IDs to set as approvers.'
requires :approver_group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce,
desc: 'Array of Group IDs to set as approvers.'
end
put 'approvers' do
::Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/issues/8883')
merge_request = find_merge_request_with_access(params[:merge_request_iid], :update_approvers)
merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, declared(params, include_parent_namespaces: false).merge(remove_old_approvers: true)).execute(merge_request)
# Merge request shouldn't be in an invalid state after the changes, but handling errors to be safe
handle_merge_request_errors!(merge_request)
present_approval(merge_request)
end
end end
end end
end end
......
ee/spec/requests/api/merge_request_approvals_spec.rb
View file @ 07060a67
...@@ -333,111 +333,6 @@ RSpec.describe API::MergeRequestApprovals do ...@@ -333,111 +333,6 @@ RSpec.describe API::MergeRequestApprovals do
end end
end end
describe 'PUT :id/merge_requests/:merge_request_iid/approvers' do
shared_examples_for 'user allowed to change approvers' do
context 'when disable_overriding_approvers_per_merge_request is true on the project' do
before do
project.update(disable_overriding_approvers_per_merge_request: true)
end
it 'does not allow overriding approvers' do
expect do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", current_user),
params: { approver_ids: approver.id.to_s, approver_group_ids: group.id.to_s }
end.to not_change { merge_request.approvers.count }.and not_change { merge_request.approver_groups.count }
end
end
context 'when disable_overriding_approvers_per_merge_request is false on the project' do
before do
project.update(disable_overriding_approvers_per_merge_request: false)
end
it 'allows overriding approvers' do
expect do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", current_user),
params: { approver_ids: "#{approver.id},#{user2.id}", approver_group_ids: "#{group.id}" }
end.to change { merge_request.approvers.count }.from(0).to(2)
.and change { merge_request.approver_groups.count }.from(0).to(1)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['approvers'].map { |approver| approver['user'] }.map { |user| user['username'] }).to contain_exactly(approver.username, user2.username)
expect(json_response['approver_groups'][0]['group']['name']).to eq(group.name)
end
it 'removes approvers not in the payload' do
merge_request.approvers.create(user: approver)
merge_request.approver_groups.create(group: group)
expect do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", current_user),
params: { approver_ids: '', approver_group_ids: '' }.to_json, headers: { CONTENT_TYPE: 'application/json' }
end.to change { merge_request.approvers.count }.from(1).to(0)
.and change { merge_request.approver_groups.count }.from(1).to(0)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['approvers']).to eq([])
end
context 'when sending form-encoded data' do
it 'removes approvers not in the payload' do
merge_request.approvers.create(user: approver)
merge_request.approver_groups.create(group: group)
expect do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", current_user),
params: { approver_ids: '', approver_group_ids: '' }
end.to change { merge_request.approvers.count }.from(1).to(0)
.and change { merge_request.approver_groups.count }.from(1).to(0)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['approvers']).to eq([])
end
end
end
it 'only shows approver groups that are visible to current user' do
private_group = create(:group, :private)
merge_request.approver_groups.create(group: private_group)
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", current_user),
params: { approver_ids: [approver.id], approver_group_ids: [private_group.id, group.id] }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['approver_groups'].size).to eq(expected_group_size)
end
end
context 'as a project admin' do
it_behaves_like 'user allowed to change approvers' do
let(:current_user) { user }
let(:expected_group_size) { 1 }
end
end
context 'as a global admin' do
it_behaves_like 'user allowed to change approvers' do
let(:current_user) { admin }
let(:expected_group_size) { 2 }
end
end
context 'as a random user' do
before do
project.update(disable_overriding_approvers_per_merge_request: false)
end
it 'does not allow overriding approvers' do
expect do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", user2),
params: { approver_ids: [approver.id], approver_group_ids: [group.id] }
end.to not_change { merge_request.approvers.count }.and not_change { merge_request.approver_groups.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
describe 'POST :id/merge_requests/:merge_request_iid/approve' do describe 'POST :id/merge_requests/:merge_request_iid/approve' do
let!(:rule) { create(:approval_merge_request_rule, merge_request: merge_request, approvals_required: 2) } let!(:rule) { create(:approval_merge_request_rule, merge_request: merge_request, approvals_required: 2) }
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7