Skip to content

G
gitlab-ce
Commit 7574be1c authored by Sean McGivern's avatar Sean McGivern
Browse files
Options

Merge branch '183-approvers-api' into 'master' 

Projects and MRs Approvers API

Closes #183

See merge request gitlab-org/gitlab-ee!4636
parents 25fb4798 9c000451
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 1200 additions and 250 deletions
app/services/merge_requests/update_service.rb
View file @ 7574be1c
module MergeRequests module MergeRequests
class UpdateService < MergeRequests::BaseService class UpdateService < MergeRequests::BaseService
prepend ::EE::MergeRequests::UpdateService
def execute(merge_request) def execute(merge_request)
# We don't allow change of source/target projects and source branch # We don't allow change of source/target projects and source branch
# after merge request was created # after merge request was created
......
doc/api/README.md
View file @ 7574be1c
...@@ -37,6 +37,7 @@ following locations: ...@@ -37,6 +37,7 @@ following locations:
- [Labels](labels.md) - [Labels](labels.md)
- [License](license.md) - [License](license.md)
- [Merge Requests](merge_requests.md) - [Merge Requests](merge_requests.md)
- [Merge Request Approvals](merge_request_approvals.md) - for project-level settings see [Project Approvals](project_approvals.md)
- [Project milestones](milestones.md) - [Project milestones](milestones.md)
- [Group milestones](group_milestones.md) - [Group milestones](group_milestones.md)
- [Namespaces](namespaces.md) - [Namespaces](namespaces.md)
...@@ -48,6 +49,7 @@ following locations: ...@@ -48,6 +49,7 @@ following locations:
- [Pipeline Triggers](pipeline_triggers.md) - [Pipeline Triggers](pipeline_triggers.md)
- [Pipeline Schedules](pipeline_schedules.md) - [Pipeline Schedules](pipeline_schedules.md)
- [Projects](projects.md) including setting Webhooks - [Projects](projects.md) including setting Webhooks
- [Project Approvals](project_approvals.md)
- [Project Access Requests](access_requests.md) - [Project Access Requests](access_requests.md)
- [Project Badges](project_badges.md) - [Project Badges](project_badges.md)
- [Project import/export](project_import_export.md) - [Project import/export](project_import_export.md)
......
doc/api/merge_request_approvals.md 0 → 100644
View file @ 7574be1c
# Merge request approvals API
Every API call must be authenticated
## Merge Request Approvals
>**Note:** This API endpoint is only available on 8.9 Starter and above.
You can request information about a merge request's approval status using the
following endpoint:
```
GET /projects/:id/merge_requests/:merge_request_iid/approvals
```
**Parameters:**
| Attribute | Type | Required | Description |
|---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of 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_missing": 1,
"approved_by": [
{
"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/u/root"
}
}
],
"approvers": [],
"approver_groups": []
}
```
## Change Merge Request approvals configuration
>**Note:** This API endpoint is only available on 10.6 Starter and above.
If you are allowed to, you can change `approvals_required` using the following
endpoint:
```
POST /projects/:id/merge_requests/:merge_request_iid/approvals
```
**Parameters:**
| Attribute | Type | Required | Description |
|----------------------|---------|----------|--------------------------------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of MR |
| `approvals_required` | integer | yes | Approvals required before MR can be merged |
```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_missing": 2,
"approved_by": [],
"approvers": [],
"approver_groups": []
}
```
## Change allowed approvers for Merge Request
>**Note:** This API endpoint is only available on 10.6 Starter and above.
If you are allowed to, you can change approvers and approver groups using
the following endpoint:
```
PUT /projects/:id/merge_requests/:merge_request_iid/approvers
```
**Important:** Approvers and groups not in the request will be **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_missing": 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/u/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
}
}
]
}
```
## Approve Merge Request
>**Note:** This API endpoint is only available on 8.9 Starter and above.
If you are allowed to, you can approve a merge request using the following
endpoint:
```
POST /projects/:id/merge_requests/:merge_request_iid/approve
```
**Parameters:**
| Attribute | Type | Required | Description |
|---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of MR |
| `sha` | string | no | The HEAD of the MR |
The `sha` parameter works in the same way as
when [accepting a merge request](#accept-mr): if it is passed, then it must
match the current HEAD of the merge request for the approval to be added. If it
does not match, the response code will be `409`.
```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-09T21:32:14.105Z",
"merge_status": "can_be_merged",
"approvals_required": 2,
"approvals_left": 0,
"approved_by": [
{
"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/u/root"
}
},
{
"user": {
"name": "Nico Cartwright",
"username": "ryley",
"id": 2,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/cf7ad14b34162a76d593e3affca2adca?s=80\u0026d=identicon",
"web_url": "http://localhost:3000/u/ryley"
}
}
],
"approvers": [],
"approver_groups": []
}
```
## Unapprove Merge Request
>**Note:** This API endpoint is only available on 9.0 Starter and above.
If you did approve a merge request, you can unapprove it using the following
endpoint:
```
POST /projects/:id/merge_requests/:merge_request_iid/unapprove
```
**Parameters:**
| Attribute | Type | Required | Description |
|---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of MR |
doc/api/merge_requests.md
View file @ 7574be1c
...@@ -821,132 +821,6 @@ Parameters: ...@@ -821,132 +821,6 @@ Parameters:
} }
``` ```
## Merge Request Approvals
>**Note:** This API endpoint is only available on 8.9 EE and above.
You can request information about a merge request's approval status using the
following endpoint:
```
GET /projects/:id/merge_requests/:merge_request_iid/approvals
```
**Parameters:**
| Attribute | Type | Required | Description |
|---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of 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": "can_be_merged",
"approvals_required": 2,
"approvals_missing": 1,
"approved_by": [
{
"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/u/root"
}
}
]
}
```
## Approve Merge Request
>**Note:** This API endpoint is only available on 8.9 EE and above.
If you are allowed to, you can approve a merge request using the following
endpoint:
```
POST /projects/:id/merge_requests/:merge_request_iid/approve
```
**Parameters:**
| Attribute | Type | Required | Description |
|---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of MR |
| `sha` | string | no | The HEAD of the MR |
The `sha` parameter works in the same way as
when [accepting a merge request](#accept-mr): if it is passed, then it must
match the current HEAD of the merge request for the approval to be added. If it
does not match, the response code will be `409`.
```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-09T21:32:14.105Z",
"merge_status": "can_be_merged",
"approvals_required": 2,
"approvals_left": 0,
"approved_by": [
{
"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/u/root"
}
},
{
"user": {
"name": "Nico Cartwright",
"username": "ryley",
"id": 2,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/cf7ad14b34162a76d593e3affca2adca?s=80\u0026d=identicon",
"web_url": "http://localhost:3000/u/ryley"
}
}
]
}
```
## Unapprove Merge Request
>**Note:** This API endpoint is only available on 9.0 EE and above.
If you did approve a merge request, you can unapprove it using the following
endpoint:
```
POST /projects/:id/merge_requests/:merge_request_iid/unapprove
```
**Parameters:**
| Attribute | Type | Required | Description |
|---------------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `merge_request_iid` | integer | yes | The IID of MR |
## Cancel Merge When Pipeline Succeeds ## Cancel Merge When Pipeline Succeeds
If you don't have permissions to accept this merge request - you'll get a `401` If you don't have permissions to accept this merge request - you'll get a `401`
......
doc/api/project_approvals.md 0 → 100644
View file @ 7574be1c
# Project approvals API
Global configuration for approvals on all Merge Requests in the project. Must be authenticated for all endpoints.
## Project Approvals
>**Note:** This API endpoint is only available on 10.6 Starter and above.
You can request information about a project's approval configuration using the
following endpoint:
```
GET /projects/:id/approvals
```
**Parameters:**
| Attribute | Type | Required | Description |
| --------- | ------- | -------- | ------------------- |
| `id` | integer | yes | The ID of a project |
```json
{
"approvers": [
{
"user": {
"id": 5,
"name": "John Doe6",
"username": "user5",
"state":"active","avatar_url":"https://www.gravatar.com/avatar/4aea8cf834ed91844a2da4ff7ae6b491?s=80\u0026d=identicon","web_url":"http://localhost/user5"
}
}
],
"approver_groups": [
{
"group": {
"id": 1,
"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
}
}
],
"approvals_before_merge": 2,
"reset_approvals_on_push": true,
"disable_overriding_approvers_per_merge_request": false
}
```
## Change project approvals configuration
>**Note:** This API endpoint is only available on 10.6 Starter and above.
If you are allowed to, you can change global approval configuration using the following
endpoint:
```
POST /projects/:id/approvals
```
**Parameters:**
| Attribute | Type | Required | Description |
| ------------------------------------------------ | ------- | -------- | ---------------------------------------------------------- |
| `id` | integer | yes | The ID of a project |
| `approvals_before_merge` | integer | no | How many approvals are required before an MR can be merged |
| `reset_approvals_on_push` | boolean | no | Reset approvals on a new push |
| `disable_overriding_approvers_per_merge_request` | boolean | no | Allow/Disallow overriding approvers per MR |
```json
{
"approvers": [
{
"user": {
"id": 5,
"name": "John Doe6",
"username": "user5",
"state":"active","avatar_url":"https://www.gravatar.com/avatar/4aea8cf834ed91844a2da4ff7ae6b491?s=80\u0026d=identicon","web_url":"http://localhost/user5"
}
}
],
"approver_groups": [
{
"group": {
"id": 1,
"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
}
}
],
"approvals_before_merge": 2,
"reset_approvals_on_push": true,
"disable_overriding_approvers_per_merge_request": false
}
```
## Change allowed approvers globally for Project
>**Note:** This API endpoint is only available on 10.6 Starter and above.
If you are allowed to, you can change global approvers and approver groups using
the following endpoint:
```
PUT /projects/:id/approvers
```
**Important:** Approvers and groups not in the request will be **removed**
**Parameters:**
| Attribute | Type | Required | Description |
| -------------------- | ------- | -------- | --------------------------------------------------- |
| `id` | integer | yes | The ID of a project |
| `approver_ids` | Array | yes | An array of User IDs that can approve MRs |
| `approver_group_ids` | Array | yes | An array of Group IDs whose members can approve MRs |
```json
{
"approvers": [
{
"user": {
"id": 5,
"name": "John Doe6",
"username": "user5",
"state":"active","avatar_url":"https://www.gravatar.com/avatar/4aea8cf834ed91844a2da4ff7ae6b491?s=80\u0026d=identicon","web_url":"http://localhost/user5"
}
}
],
"approver_groups": [
{
"group": {
"id": 1,
"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
}
}
],
"approvals_before_merge": 2,
"reset_approvals_on_push": true,
"disable_overriding_approvers_per_merge_request": false
}
```
ee/app/models/concerns/approvable.rb
View file @ 7574be1c
...@@ -151,7 +151,7 @@ module Approvable ...@@ -151,7 +151,7 @@ module Approvable
end end
def approver_ids=(value) def approver_ids=(value)
value.split(",").map(&:strip).each do |user_id| ::Gitlab::Utils.ensure_array_from_string(value).each do |user_id|
next if author && user_id == author.id next if author && user_id == author.id
approvers.find_or_initialize_by(user_id: user_id, target_id: id) approvers.find_or_initialize_by(user_id: user_id, target_id: id)
...@@ -159,7 +159,7 @@ module Approvable ...@@ -159,7 +159,7 @@ module Approvable
end end
def approver_group_ids=(value) def approver_group_ids=(value)
value.split(",").map(&:strip).each do |group_id| ::Gitlab::Utils.ensure_array_from_string(value).each do |group_id|
approver_groups.find_or_initialize_by(group_id: group_id, target_id: id) approver_groups.find_or_initialize_by(group_id: group_id, target_id: id)
end end
end end
......
ee/app/models/ee/project.rb
View file @ 7574be1c
...@@ -317,13 +317,13 @@ module EE ...@@ -317,13 +317,13 @@ module EE
alias_method :reset_approvals_on_push?, :reset_approvals_on_push alias_method :reset_approvals_on_push?, :reset_approvals_on_push
def approver_ids=(value) def approver_ids=(value)
value.split(",").map(&:strip).each do |user_id| ::Gitlab::Utils.ensure_array_from_string(value).each do |user_id|
approvers.find_or_create_by(user_id: user_id, target_id: id) approvers.find_or_create_by(user_id: user_id, target_id: id)
end end
end end
def approver_group_ids=(value) def approver_group_ids=(value)
value.split(",").map(&:strip).each do |group_id| ::Gitlab::Utils.ensure_array_from_string(value).each do |group_id|
approver_groups.find_or_initialize_by(group_id: group_id, target_id: id) approver_groups.find_or_initialize_by(group_id: group_id, target_id: id)
end end
end end
......
ee/app/services/concerns/cleanup_approvers.rb 0 → 100644
View file @ 7574be1c
# Concern that encapsulates logic to remove all
# approvers in a project that were not added during
# the current transaction
module CleanupApprovers
extend ActiveSupport::Concern
private
def cleanup_approvers(target, reload: false)
target.approvers.where.not(user_id: params[:approver_ids]).destroy_all
target.approver_groups.where.not(group_id: params[:approver_group_ids]).destroy_all
# If the target already has `approvers` and/or `approver_groups` loaded then we need to
# force a reload in order to not return stale information after the destroys above
if reload
target.approvers.reload
target.approver_groups.reload
end
target
end
end
ee/app/services/ee/merge_requests/update_service.rb 0 → 100644
View file @ 7574be1c
module EE
module MergeRequests
module UpdateService
extend ::Gitlab::Utils::Override
include CleanupApprovers
override :execute
def execute(merge_request)
should_remove_old_approvers = params.delete(:remove_old_approvers)
merge_request = super(merge_request)
cleanup_approvers(merge_request, reload: true) if should_remove_old_approvers && merge_request.valid?
merge_request
end
end
end
end
ee/app/services/ee/projects/update_service.rb
View file @ 7574be1c
...@@ -3,6 +3,8 @@ module EE ...@@ -3,6 +3,8 @@ module EE
module UpdateService module UpdateService
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
include CleanupApprovers
override :execute override :execute
def execute def execute
unless project.feature_available?(:repository_mirrors) unless project.feature_available?(:repository_mirrors)
...@@ -11,8 +13,12 @@ module EE ...@@ -11,8 +13,12 @@ module EE
params.delete(:mirror_trigger_builds) params.delete(:mirror_trigger_builds)
end end
should_remove_old_approvers = params.delete(:remove_old_approvers)
result = super result = super
cleanup_approvers(project) if should_remove_old_approvers && result[:status] == :success
log_audit_events if result[:status] == :success log_audit_events if result[:status] == :success
result result
......
ee/changelogs/unreleased/183-approvers-api.yml 0 → 100644
View file @ 7574be1c
---
title: Projects and MRs Approvers API
merge_request: 4636
author:
type: added
ee/lib/api/merge_request_approvals.rb 0 → 100644
View file @ 7574be1c
module API
class MergeRequestApprovals < ::Grape::API
before { authenticate_non_get! }
helpers do
def handle_merge_request_errors!(errors)
if errors.has_key? :project_access
error!(errors[:project_access], 422)
elsif errors.has_key? :branch_conflict
error!(errors[:branch_conflict], 422)
elsif errors.has_key? :validate_fork
error!(errors[:validate_fork], 422)
elsif errors.has_key? :validate_branches
conflict!(errors[:validate_branches])
elsif errors.has_key? :base
error!(errors[:base], 422)
end
render_api_error!(errors, 400)
end
end
params do
requires :id, type: String, desc: 'The ID of a project'
requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
end
resource :projects, requirements: ::API::API::PROJECT_ENDPOINT_REQUIREMENTS do
segment ':id/merge_requests/:merge_request_iid' do
# Get the status of the merge request's approvals
#
# Parameters:
# id (required) - The ID of a project
# merge_request_iid (required) - IID of MR
# Examples:
# GET /projects/:id/merge_requests/:merge_request_iid/approvals
#
desc 'List approvals for merge request' do
success Entities::MergeRequestApprovals
end
get 'approvals' do
merge_request = find_merge_request_with_access(params[:merge_request_iid])
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
end
desc 'Change approval-related configuration' do
detail 'This feature was introduced in 10.6'
success Entities::MergeRequestApprovals
end
params do
requires :approvals_required, type: Integer, desc: 'The amount of approvals required. Must be higher than the project approvals'
end
post 'approvals' do
merge_request = find_merge_request_with_access(params[:merge_request_iid], :update_merge_request)
error!('Overriding approvals is disabled', 422) if merge_request.project.disable_overriding_approvers_per_merge_request
merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, approvals_before_merge: params[:approvals_required]).execute(merge_request)
if merge_request.valid?
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
else
handle_merge_request_errors! merge_request.errors
end
end
desc 'Update approvers and approver groups' do
detail 'This feature was introduced in 10.6'
success Entities::MergeRequestApprovals
end
params do
requires :approver_ids, type: Array[String], allow_blank: true, desc: 'Array of User IDs to set as approvers.'
requires :approver_group_ids, type: Array[String], allow_blank: true, desc: 'Array of Group IDs to set as approvers.'
end
put 'approvers' do
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)
if merge_request.valid?
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
else
handle_merge_request_errors! merge_request.errors
end
end
# Approve a merge request
#
# Parameters:
# id (required) - The ID of a project
# merge_request_iid (required) - IID of MR
# Examples:
# POST /projects/:id/merge_requests/:merge_request_iid/approve
#
desc 'Approve a merge request' do
success Entities::MergeRequestApprovals
end
params do
optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
end
post 'approve' do
merge_request = find_project_merge_request(params[:merge_request_iid])
unauthorized! unless merge_request.can_approve?(current_user)
check_sha_param!(params, merge_request)
::MergeRequests::ApprovalService
.new(user_project, current_user)
.execute(merge_request)
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
end
desc 'Remove an approval from a merge request' do
success Entities::MergeRequestApprovals
end
post 'unapprove' do
merge_request = find_project_merge_request(params[:merge_request_iid])
not_found! unless merge_request.has_approved?(current_user)
::MergeRequests::RemoveApprovalService
.new(user_project, current_user)
.execute(merge_request)
present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user
end
end
end
end
end
ee/lib/api/project_approvals.rb 0 → 100644
View file @ 7574be1c
module API
class ProjectApprovals < ::Grape::API
before { authenticate! }
before { authorize! :update_approvers, user_project }
params do
requires :id, type: String, desc: 'The ID of a project'
end
resource :projects, requirements: ::API::API::PROJECT_ENDPOINT_REQUIREMENTS do
segment ':id/approvals' do
desc 'Get all project approvers and related configuration' do
detail 'This feature was introduced in 10.6'
success ::API::Entities::ApprovalSettings
end
get '/' do
present user_project, with: ::API::Entities::ApprovalSettings
end
desc 'Change approval-related configuration' do
detail 'This feature was introduced in 10.6'
success ::API::Entities::ApprovalSettings
end
params do
optional :approvals_before_merge, type: Integer, desc: 'The amount of approvals required before an MR can be merged'
optional :reset_approvals_on_push, type: Boolean, desc: 'Should the approval count be reset on a new push'
optional :disable_overriding_approvers_per_merge_request, type: Boolean, desc: 'Should MRs be able to override approvers and approval count'
at_least_one_of :approvals_before_merge, :reset_approvals_on_push, :disable_overriding_approvers_per_merge_request
end
post '/' do
project_params = declared(params, include_missing: false, include_parent_namespaces: false)
result = ::Projects::UpdateService.new(user_project, current_user, project_params).execute
if result[:status] == :success
present user_project, with: ::API::Entities::ApprovalSettings
else
render_validation_error!(user_project)
end
end
end
desc 'Update approvers and approver groups' do
detail 'This feature was introduced in 10.6'
success ::API::Entities::ApprovalSettings
end
params do
requires :approver_ids, type: Array[String], allow_blank: true, desc: 'Array of User IDs to set as approvers.'
requires :approver_group_ids, type: Array[String], allow_blank: true, desc: 'Array of Group IDs to set as approvers.'
end
put ':id/approvers' do
result = ::Projects::UpdateService.new(user_project, current_user, declared(params, include_parent_namespaces: false).merge(remove_old_approvers: true)).execute
if result[:status] == :success
present user_project, with: ::API::Entities::ApprovalSettings
else
render_validation_error!(user_project)
end
end
end
end
end
ee/lib/ee/api/helpers.rb
View file @ 7574be1c
...@@ -20,6 +20,12 @@ module EE ...@@ -20,6 +20,12 @@ module EE
def check_project_feature_available!(feature) def check_project_feature_available!(feature)
not_found! unless user_project.feature_available?(feature) not_found! unless user_project.feature_available?(feature)
end end
def check_sha_param!(params, merge_request)
if params[:sha] && merge_request.diff_head_sha != params[:sha]
render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
end
end
end end
end end
end end
ee/spec/fixtures/api/schemas/public_api/v4/project_approvers.json 0 → 100644
View file @ 7574be1c
{
"type": "object",
"properties": {
"approvals_before_merge": { "type": "integer" },
"disable_overriding_approvers_per_merge_request": { "type": ["boolean", "null"] },
"reset_approvals_on_push": { "type": "boolean" },
"approvers": {
"type": "array",
"items": {
"type": "object",
"properties": {}
}
},
"approver_groups": {
"type": "array",
"items": {
"type": "object",
"properties": {}
}
}
},
"additionalProperties": false
}
ee/spec/requests/api/merge_request_approvals_spec.rb 0 → 100644
View file @ 7574be1c
This diff is collapsed.
ee/spec/requests/api/project_approvals_spec.rb 0 → 100644
View file @ 7574be1c
require 'spec_helper'
describe API::ProjectApprovals do
set(:group) { create(:group_with_members) }
set(:user) { create(:user) }
set(:user2) { create(:user) }
set(:admin) { create(:user, :admin) }
set(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
set(:approver) { create(:user) }
let(:url) { "/projects/#{project.id}/approvals" }
describe 'GET /projects/:id/approvers' do
context 'when the request is correct' do
before do
project.approvers.create(user: approver)
project.approver_groups.create(group: group)
get api(url, user)
end
it 'returns 200 status' do
expect(response).to have_gitlab_http_status(200)
end
it 'matches the response schema' do
expect(response).to match_response_schema('public_api/v4/project_approvers', dir: 'ee')
end
end
end
describe 'POST /projects/:id/approvers' do
shared_examples_for 'a user with access' do
context 'when missing parameters' do
it 'returns 400 status' do
post api(url, current_user)
expect(response).to have_gitlab_http_status(400)
end
end
context 'when the request is correct' do
it 'returns 201 status' do
post api(url, current_user), approvals_before_merge: 3
expect(response).to have_gitlab_http_status(201)
end
it 'matches the response schema' do
post api(url, current_user), approvals_before_merge: 3
expect(response).to match_response_schema('public_api/v4/project_approvers', dir: 'ee')
end
it 'changes settings properly' do
project.approvals_before_merge = 2
project.reset_approvals_on_push = false
project.disable_overriding_approvers_per_merge_request = true
project.save
settings = {
approvals_before_merge: 4,
reset_approvals_on_push: true,
disable_overriding_approvers_per_merge_request: false
}
post api(url, current_user), settings
expect(JSON.parse(response.body).symbolize_keys).to include(settings)
end
end
end
context 'as a project admin' do
it_behaves_like 'a user with access' do
let(:current_user) { user }
end
end
context 'as a global admin' do
it_behaves_like 'a user with access' do
let(:current_user) { admin }
end
end
context 'as a user without access' do
it 'returns 403' do
post api(url, user2), approvals_before_merge: 4
expect(response).to have_gitlab_http_status(403)
end
end
end
describe 'PUT /projects/:id/approvers' do
let(:url) { "/projects/#{project.id}/approvers" }
shared_examples_for 'a user with access' do
it 'removes all approvers if no params are given' do
project.approvers.create(user: approver)
expect do
put api(url, current_user), { approver_ids: [], approver_group_ids: [] }.to_json, { CONTENT_TYPE: 'application/json' }
end.to change { project.approvers.count }.from(1).to(0)
expect(response).to have_gitlab_http_status(200)
expect(json_response['approvers']).to be_empty
expect(json_response['approver_groups']).to be_empty
end
it 'sets approvers and approver groups' do
project.approvers.create(user: approver)
expect do
put api(url, current_user), approver_ids: [approver.id], approver_group_ids: [group.id]
end.to change { project.approvers.count }.by(0).and change { project.approver_groups.count }.from(0).to(1)
expect(project.approvers.count).to eq(1)
expect(project.approvers.first.user_id).to eq(approver.id)
expect(project.approver_groups.first.group_id).to eq(group.id)
expect(response).to have_gitlab_http_status(200)
expect(json_response['approvers'][0]['user']['username']).to eq(approver.username)
expect(json_response['approver_groups'][0]['group']['name']).to eq(group.name)
end
end
context 'as a project admin' do
it_behaves_like 'a user with access' do
let(:current_user) { user }
end
end
context 'as a global admin' do
it_behaves_like 'a user with access' do
let(:current_user) { admin }
end
end
context 'as a random user' do
it 'returns 403' do
project.approvers.create(user: approver)
expect do
put api(url, user2), { approver_ids: [], approver_group_ids: [] }.to_json, { CONTENT_TYPE: 'application/json' }
end.not_to change { project.approvers.count }
expect(response).to have_gitlab_http_status(403)
end
end
end
end
lib/api/api.rb
View file @ 7574be1c
...@@ -143,6 +143,7 @@ module API ...@@ -143,6 +143,7 @@ module API
mount ::API::Labels mount ::API::Labels
mount ::API::Lint mount ::API::Lint
mount ::API::Members mount ::API::Members
mount ::API::MergeRequestApprovals
mount ::API::MergeRequestDiffs mount ::API::MergeRequestDiffs
mount ::API::MergeRequests mount ::API::MergeRequests
mount ::API::Namespaces mount ::API::Namespaces
...@@ -151,6 +152,7 @@ module API ...@@ -151,6 +152,7 @@ module API
mount ::API::PagesDomains mount ::API::PagesDomains
mount ::API::Pipelines mount ::API::Pipelines
mount ::API::PipelineSchedules mount ::API::PipelineSchedules
mount ::API::ProjectApprovals
mount ::API::ProjectExport mount ::API::ProjectExport
mount ::API::ProjectImport mount ::API::ProjectImport
mount ::API::ProjectHooks mount ::API::ProjectHooks
......
lib/api/entities.rb
View file @ 7574be1c
...@@ -611,6 +611,32 @@ module API ...@@ -611,6 +611,32 @@ module API
end end
end end
class Approver < Grape::Entity
expose :user, using: Entities::UserBasic
end
class ApproverGroup < Grape::Entity
expose :group, using: Entities::Group
end
class MergeRequestApprovals < ProjectEntity
expose :merge_status
expose :approvals_required
expose :approvals_left
expose :approvals, as: :approved_by, using: Entities::Approver
expose :approvers_left, as: :suggested_approvers, using: Entities::UserBasic
expose :approvers, using: Entities::Approver
expose :approver_groups, using: Entities::ApproverGroup
expose :user_has_approved do |merge_request, options|
merge_request.has_approved?(options[:current_user])
end
expose :user_can_approve do |merge_request, options|
merge_request.can_approve?(options[:current_user])
end
end
class MergeRequestDiff < Grape::Entity class MergeRequestDiff < Grape::Entity
expose :id, :head_commit_sha, :base_commit_sha, :start_commit_sha, expose :id, :head_commit_sha, :base_commit_sha, :start_commit_sha,
:created_at, :merge_request_id, :state, :real_size :created_at, :merge_request_id, :state, :real_size
...@@ -1279,6 +1305,14 @@ module API ...@@ -1279,6 +1305,14 @@ module API
klass.descendants.each { |descendant| descendant.prepend(with) } klass.descendants.each { |descendant| descendant.prepend(with) }
klass.prepend(with) klass.prepend(with)
end end
class ApprovalSettings < Grape::Entity
expose :approvers, using: Entities::Approver
expose :approver_groups, using: Entities::ApproverGroup
expose :approvals_before_merge
expose :reset_approvals_on_push
expose :disable_overriding_approvers_per_merge_request
end
end end
end end
......
lib/gitlab/utils.rb
View file @ 7574be1c
...@@ -74,5 +74,12 @@ module Gitlab ...@@ -74,5 +74,12 @@ module Gitlab
rescue ArgumentError rescue ArgumentError
size size
end end
# Accepts either an Array or a String and returns an array
def ensure_array_from_string(string_or_array)
return string_or_array if string_or_array.is_a?(Array)
string_or_array.split(',').map(&:strip)
end
end end
end end
spec/lib/gitlab/utils_spec.rb
View file @ 7574be1c
require 'spec_helper' require 'spec_helper'
describe Gitlab::Utils do describe Gitlab::Utils do
delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, to: :described_class delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string, to: :described_class
describe '.slugify' do describe '.slugify' do
{ {
...@@ -83,4 +83,18 @@ describe Gitlab::Utils do ...@@ -83,4 +83,18 @@ describe Gitlab::Utils do
expect(which('sh', 'PATH' => '/bin')).to eq('/bin/sh') expect(which('sh', 'PATH' => '/bin')).to eq('/bin/sh')
end end
end end
describe '.ensure_array_from_string' do
it 'returns the same array if given one' do
arr = ['a', 4, true, { test: 1 }]
expect(ensure_array_from_string(arr)).to eq(arr)
end
it 'turns comma-separated strings into arrays' do
str = 'seven, eight, 9, 10'
expect(ensure_array_from_string(str)).to eq(%w[seven eight 9 10])
end
end
end end
spec/requests/api/merge_requests_spec.rb
View file @ 7574be1c
...@@ -1378,125 +1378,6 @@ describe API::MergeRequests do ...@@ -1378,125 +1378,6 @@ describe API::MergeRequests do
end end
end end
describe 'GET :id/merge_requests/:merge_request_iid/approvals' do
it 'retrieves the approval status' do
approver = create :user
project.update_attribute(:approvals_before_merge, 2)
project.add_developer(approver)
project.add_developer(create(:user))
merge_request.approvals.create(user: approver)
get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", user)
expect(response).to have_gitlab_http_status(200)
expect(json_response['approvals_required']).to eq 2
expect(json_response['approvals_left']).to eq 1
expect(json_response['approved_by'][0]['user']['username']).to eq(approver.username)
expect(json_response['user_can_approve']).to be false
expect(json_response['user_has_approved']).to be false
end
end
describe 'POST :id/merge_requests/:merge_request_iid/approve' do
before do
project.update_attribute(:approvals_before_merge, 2)
end
context 'as the author of the merge request' do
before do
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approve", user)
end
it 'returns a 401' do
expect(response).to have_gitlab_http_status(401)
end
end
context 'as a valid approver' do
let(:approver) { create(:user) }
before do
project.add_developer(approver)
project.add_developer(create(:user))
end
def approve(extra_params = {})
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approve", approver), extra_params
end
context 'when the sha param is not set' do
before do
approve
end
it 'approves the merge request' do
expect(response).to have_gitlab_http_status(201)
expect(json_response['approvals_left']).to eq(1)
expect(json_response['approved_by'][0]['user']['username']).to eq(approver.username)
expect(json_response['user_has_approved']).to be true
end
end
context 'when the sha param is correct' do
before do
approve(sha: merge_request.diff_head_sha)
end
it 'approves the merge request' do
expect(response).to have_gitlab_http_status(201)
expect(json_response['approvals_left']).to eq(1)
expect(json_response['approved_by'][0]['user']['username']).to eq(approver.username)
expect(json_response['user_has_approved']).to be true
end
end
context 'when the sha param is incorrect' do
before do
approve(sha: merge_request.diff_head_sha.reverse)
end
it 'returns a 409' do
expect(response).to have_gitlab_http_status(409)
end
it 'does not approve the merge request' do
expect(merge_request.reload.approvals_left).to eq(2)
end
end
end
end
describe 'POST :id/merge_requests/:merge_request_iid/unapprove' do
before do
project.update_attribute(:approvals_before_merge, 2)
end
context 'as a user who has approved the merge request' do
let(:approver) { create(:user) }
let(:unapprover) { create(:user) }
before do
project.add_developer(approver)
project.add_developer(unapprover)
project.add_developer(create(:user))
merge_request.approvals.create(user: approver)
merge_request.approvals.create(user: unapprover)
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unapprove", unapprover)
end
it 'unapproves the merge request' do
expect(response).to have_gitlab_http_status(201)
expect(json_response['approvals_left']).to eq(1)
usernames = json_response['approved_by'].map { |u| u['user']['username'] }
expect(usernames).not_to include(unapprover.username)
expect(usernames.size).to be 1
expect(json_response['user_has_approved']).to be false
expect(json_response['user_can_approve']).to be true
end
end
end
describe 'POST :id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do describe 'POST :id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do
before do before do
::MergeRequests::MergeWhenPipelineSucceedsService.new(merge_request.target_project, user).execute(merge_request) ::MergeRequests::MergeWhenPipelineSucceedsService.new(merge_request.target_project, user).execute(merge_request)
......
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