Skip to content

G
gitlab-ce
Commit 9c000451 authored by Mario de la Ossa's avatar Mario de la Ossa
Browse files
Options

API - add approval-related endpoints for MRs and Projects 

allow changing of approval related configuration as well as adding and
removing approvers and approver groups for both Projects and MRs
parent b7fa60d3
Show whitespace changes
Inline Side-by-side
Showing with 1200 additions and 250 deletions
app/services/merge_requests/update_service.rb
View file @ 9c000451
module MergeRequests
class UpdateService < MergeRequests::BaseService
prepend ::EE::MergeRequests::UpdateService
def execute(merge_request)
# We don't allow change of source/target projects and source branch
# after merge request was created
......
doc/api/README.md
View file @ 9c000451
......@@ -37,6 +37,7 @@ following locations:
- [Labels](labels.md)
- [License](license.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)
- [Group milestones](group_milestones.md)
- [Namespaces](namespaces.md)
......@@ -48,6 +49,7 @@ following locations:
- [Pipeline Triggers](pipeline_triggers.md)
- [Pipeline Schedules](pipeline_schedules.md)
- [Projects](projects.md) including setting Webhooks
- [Project Approvals](project_approvals.md)
- [Project Access Requests](access_requests.md)
- [Project Badges](project_badges.md)
- [Project import/export](project_import_export.md)
......
doc/api/merge_request_approvals.md 0 → 100644
View file @ 9c000451
# 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 @ 9c000451
......@@ -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
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 @ 9c000451
# 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 @ 9c000451
......@@ -151,7 +151,7 @@ module Approvable
end
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
approvers.find_or_initialize_by(user_id: user_id, target_id: id)
......@@ -159,7 +159,7 @@ module Approvable
end
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)
end
end
......
ee/app/models/ee/project.rb
View file @ 9c000451
......@@ -317,13 +317,13 @@ module EE
alias_method :reset_approvals_on_push?, :reset_approvals_on_push
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)
end
end
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)
end
end
......
ee/app/services/concerns/cleanup_approvers.rb 0 → 100644
View file @ 9c000451
# 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 @ 9c000451
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 @ 9c000451
......@@ -3,6 +3,8 @@ module EE
module UpdateService
extend ::Gitlab::Utils::Override
include CleanupApprovers
override :execute
def execute
unless project.feature_available?(:repository_mirrors)
......@@ -11,8 +13,12 @@ module EE
params.delete(:mirror_trigger_builds)
end
should_remove_old_approvers = params.delete(:remove_old_approvers)
result = super
cleanup_approvers(project) if should_remove_old_approvers && result[:status] == :success
log_audit_events if result[:status] == :success
result
......
ee/changelogs/unreleased/183-approvers-api.yml 0 → 100644
View file @ 9c000451
---
title: Projects and MRs Approvers API
merge_request: 4636
author:
type: added
ee/lib/api/merge_request_approvals.rb 0 → 100644
View file @ 9c000451
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 @ 9c000451
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 @ 9c000451
......@@ -20,6 +20,12 @@ module EE
def check_project_feature_available!(feature)
not_found! unless user_project.feature_available?(feature)
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
ee/spec/fixtures/api/schemas/public_api/v4/project_approvers.json 0 → 100644
View file @ 9c000451
{
"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 @ 9c000451
require 'spec_helper'
describe API::MergeRequestApprovals do
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(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: Time.now) }
before do
project.update_attribute(:approvals_before_merge, 2)
end
describe 'GET :id/merge_requests/:merge_request_iid/approvals' do
it 'retrieves the approval status' do
approver = create :user
group = create :group
project.add_developer(approver)
project.add_developer(create(:user))
merge_request.approvals.create(user: approver)
merge_request.approvers.create(user: approver)
merge_request.approver_groups.create(group: group)
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
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
describe 'POST :id/merge_requests/:merge_request_iid/approvals' do
shared_examples_for 'user allowed to override approvals required' do
context 'when disable_overriding_approvers_per_merge_request is false on the project' do
before do
project.update_attribute(:disable_overriding_approvers_per_merge_request, false)
end
it 'allows you to override approvals required' do
expect do
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", current_user), approvals_required: 5
end.to change { merge_request.reload.approvals_before_merge }.from(nil).to(5)
expect(response).to have_gitlab_http_status(201)
expect(json_response['approvals_required']).to eq(5)
end
it 'does not allow approvals required under what the project requires' do
expect do
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", current_user), approvals_required: 1
end.not_to change { merge_request.reload.approvals_before_merge }
expect(response).to have_gitlab_http_status(400)
end
context 'when project approvals are not enabled' do
before do
project.update_attribute(:approvals_before_merge, 0)
end
it 'does not allow you to override approvals required' do
expect do
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", current_user), approvals_required: 5
end.not_to change { merge_request.reload.approvals_before_merge }
expect(response).to have_gitlab_http_status(400)
end
end
end
context 'when disable_overriding_approvers_per_merge_request is true on the project' do
before do
project.update_attribute(:disable_overriding_approvers_per_merge_request, true)
end
it 'does not allow you to override approvals required' do
expect do
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", current_user), approvals_required: 5
end.not_to change { merge_request.reload.approvals_before_merge }
expect(response).to have_gitlab_http_status(422)
end
end
end
context 'as a project admin' do
it_behaves_like 'user allowed to override approvals required' do
let(:current_user) { user }
end
end
context 'as a global admin' do
it_behaves_like 'user allowed to override approvals required' do
let(:current_user) { admin }
end
end
context 'as a random user' do
before do
project.update_attribute(:disable_overriding_approvers_per_merge_request, false)
end
it 'does not allow you to override approvals required' do
expect do
post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", user2), approvals_required: 5
end.not_to change { merge_request.reload.approvals_before_merge }
expect(response).to have_gitlab_http_status(403)
end
end
end
describe 'PUT :id/merge_requests/:merge_request_iid/approvers' do
set(:approver) { create(:user) }
set(:approver_group) { create(:group) }
RSpec::Matchers.define_negated_matcher :not_change, :change
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_attribute(: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),
approver_ids: [approver.id], approver_group_ids: [approver_group.id]
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_attribute(: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),
approver_ids: [approver.id], approver_group_ids: [approver_group.id]
end.to change { merge_request.approvers.count }.from(0).to(1)
.and change { merge_request.approver_groups.count }.from(0).to(1)
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(approver_group.name)
end
it 'removes approvers not in the payload' do
merge_request.approvers.create(user: approver)
merge_request.approver_groups.create(group: approver_group)
expect do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvers", current_user),
{ approver_ids: [], approver_group_ids: [] }.to_json, { 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(200)
expect(json_response['approvers']).to eq([])
end
end
end
context 'as a project admin' do
it_behaves_like 'user allowed to change approvers' do
let(:current_user) { user }
end
end
context 'as a global admin' do
it_behaves_like 'user allowed to change approvers' do
let(:current_user) { admin }
end
end
context 'as a random user' do
before do
project.update_attribute(: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),
approver_ids: [approver.id], approver_group_ids: [approver_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(403)
end
end
end
describe 'POST :id/merge_requests/:merge_request_iid/approve' do
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
set(: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
context 'as a user who has approved the merge request' do
set(:approver) { create(:user) }
set(: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
end
ee/spec/requests/api/project_approvals_spec.rb 0 → 100644
View file @ 9c000451
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 @ 9c000451
......@@ -143,6 +143,7 @@ module API
mount ::API::Labels
mount ::API::Lint
mount ::API::Members
mount ::API::MergeRequestApprovals
mount ::API::MergeRequestDiffs
mount ::API::MergeRequests
mount ::API::Namespaces
......@@ -151,6 +152,7 @@ module API
mount ::API::PagesDomains
mount ::API::Pipelines
mount ::API::PipelineSchedules
mount ::API::ProjectApprovals
mount ::API::ProjectExport
mount ::API::ProjectImport
mount ::API::ProjectHooks
......
lib/api/entities.rb
View file @ 9c000451
......@@ -611,6 +611,32 @@ module API
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
expose :id, :head_commit_sha, :base_commit_sha, :start_commit_sha,
:created_at, :merge_request_id, :state, :real_size
......@@ -1279,6 +1305,14 @@ module API
klass.descendants.each { |descendant| descendant.prepend(with) }
klass.prepend(with)
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
......
lib/gitlab/utils.rb
View file @ 9c000451
......@@ -74,5 +74,12 @@ module Gitlab
rescue ArgumentError
size
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
spec/lib/gitlab/utils_spec.rb
View file @ 9c000451
require 'spec_helper'
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
{
......@@ -83,4 +83,18 @@ describe Gitlab::Utils do
expect(which('sh', 'PATH' => '/bin')).to eq('/bin/sh')
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
spec/requests/api/merge_requests_spec.rb
View file @ 9c000451
......@@ -1378,125 +1378,6 @@ describe API::MergeRequests do
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
before do
::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