diff --git a/doc/api/merge_request_approvals.md b/doc/api/merge_request_approvals.md index cc95689a65f62aee79fb511fbb1b522f79b8608c..bf83bfd7e6ace1e630bb2a31b7155d1c6a16bbc3 100644 --- a/doc/api/merge_request_approvals.md +++ b/doc/api/merge_request_approvals.md @@ -23,36 +23,6 @@ GET /projects/:id/approvals ```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 @@ -75,7 +45,7 @@ POST /projects/:id/approvals | 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 | +| `approvals_before_merge` | integer | no | How many approvals are required before an MR can be merged. Deprecated in 12.0 in favor of Approval Rules API. | | `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 | | `merge_requests_author_approval` | boolean | no | Allow/Disallow authors from self approving merge requests; `true` means authors cannot self approve | @@ -83,20 +53,68 @@ POST /projects/:id/approvals ```json { - "approvers": [ - { - "user": { + "approvals_before_merge": 2, + "reset_approvals_on_push": true, + "disable_overriding_approvers_per_merge_request": false, + "merge_requests_author_approval": false, + "merge_requests_disable_committers_approval": false +} +``` + +### Get project-level rules + +>**Note:** This API endpoint is only available on 12.3 Starter and above. + +You can request information about a project's approval rules using the following endpoint: + +``` +GET /projects/:id/approval_rules +``` + +**Parameters:** + +| Attribute | Type | Required | Description | +|----------------------|---------|----------|-----------------------------------------------------------| +| `id` | integer | yes | The ID of a project | + +```json +[ + { + "id": 1, + "name": "security", + "rule_type": "regular", + "eligible_approvers": [ + { "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" + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + }, + { + "id": 50, + "name": "Group Member 1", + "username": "group_member_1", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/group_member_1" } - } - ], - "approver_groups": [ - { - "group": { - "id": 1, + ], + "approvals_required": 3, + "users": [ + { + "id": 5, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "groups": [ + { + "id": 5, "name": "group1", "path": "group1", "description": "", @@ -111,18 +129,187 @@ POST /projects/:id/approvals "ldap_cn": null, "ldap_access": null } + ], + "contains_hidden_groups": false + } +] +``` + +### Create project-level rule + +>**Note:** This API endpoint is only available on 12.3 Starter and above. + +You can create project approval rules using the following endpoint: + +``` +POST /projects/:id/approval_rules +``` + +**Parameters:** + +| Attribute | Type | Required | Description | +|----------------------|---------|----------|-----------------------------------------------------------| +| `id` | integer | yes | The ID of a project | +| `name` | string | yes | The name of the approval rule | +| `approvals_required` | integer | yes | The number of required approvals for this rule | +| `users` | integer | no | The ids of users as approvers | +| `groups` | integer | no | The ids of groups as approvers | + +```json +{ + "id": 1, + "name": "security", + "rule_type": "regular", + "eligible_approvers": [ + { + "id": 2, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + }, + { + "id": 50, + "name": "Group Member 1", + "username": "group_member_1", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/group_member_1" } ], - "approvals_before_merge": 2, - "reset_approvals_on_push": true, - "disable_overriding_approvers_per_merge_request": false, - "merge_requests_author_approval": false, - "merge_requests_disable_committers_approval": false + "approvals_required": 1, + "users": [ + { + "id": 2, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "groups": [ + { + "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 + } + ], + "contains_hidden_groups": false +} +``` + +### Update project-level rule + +>**Note:** This API endpoint is only available on 12.3 Starter and above. + +You can update project approval rules using the following endpoint: + +``` +PUT /projects/:id/approval_rules/:approval_rule_id +``` + +**Important:** Approvers and groups not in the `users`/`groups` param will be **removed** + +**Parameters:** + +| Attribute | Type | Required | Description | +|----------------------|---------|----------|-----------------------------------------------------------| +| `id` | integer | yes | The ID of a project | +| `approval_rule_id` | integer | yes | The ID of a approval rule | +| `name` | string | yes | The name of the approval rule | +| `approvals_required` | integer | yes | The number of required approvals for this rule | +| `users` | integer | no | The ids of users as approvers | +| `groups` | integer | no | The ids of groups as approvers | + +```json +{ + "id": 1, + "name": "security", + "rule_type": "regular", + "eligible_approvers": [ + { + "id": 2, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + }, + { + "id": 50, + "name": "Group Member 1", + "username": "group_member_1", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/group_member_1" + } + ], + "approvals_required": 1, + "users": [ + { + "id": 2, + "name": "John Doe", + "username": "jdoe", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/0?s=80&d=identicon", + "web_url": "http://localhost/jdoe" + } + ], + "groups": [ + { + "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 + } + ], + "contains_hidden_groups": false } ``` +### Delete project-level rule + +>**Note:** This API endpoint is only available on 12.3 Starter and above. + +You can delete project approval rules using the following endpoint: + +``` +DELETE /projects/:id/approval_rules/:approval_rule_id +``` + +**Parameters:** + +| Attribute | Type | Required | Description | +|----------------------|---------|----------|-----------------------------------------------------------| +| `id` | integer | yes | The ID of a project | +| `approval_rule_id` | integer | yes | The ID of a approval rule + ### Change allowed approvers +>**Note:** This API endpoint has been deprecated. Please use Approval Rule API instead. >**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 @@ -227,8 +414,6 @@ GET /projects/:id/merge_requests/:merge_request_iid/approvals } } ], - "approvers": [], - "approver_groups": [] } ``` @@ -249,7 +434,7 @@ POST /projects/:id/merge_requests/:merge_request_iid/approvals |----------------------|---------|----------|--------------------------------------------| | `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 | +| `approvals_required` | integer | yes | Approvals required before MR can be merged. Deprecated in 12.0 in favor of Approval Rules API. | ```json { @@ -264,14 +449,13 @@ POST /projects/:id/merge_requests/:merge_request_iid/approvals "merge_status": "cannot_be_merged", "approvals_required": 2, "approvals_left": 2, - "approved_by": [], - "approvers": [], - "approver_groups": [] + "approved_by": [] } ``` ### Change allowed approvers for Merge Request +>**Note:** This API endpoint has been deprecated. Please use Approval Rule API instead. >**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 @@ -401,8 +585,6 @@ does not match, the response code will be `409`. } } ], - "approvers": [], - "approver_groups": [] } ``` diff --git a/ee/changelogs/unreleased/11877-public-approval-api.yml b/ee/changelogs/unreleased/11877-public-approval-api.yml new file mode 100644 index 0000000000000000000000000000000000000000..ba35851b1dac771100e51d313fa9fa43cc682ec5 --- /dev/null +++ b/ee/changelogs/unreleased/11877-public-approval-api.yml @@ -0,0 +1,5 @@ +--- +title: Public project-level approval rule API +merge_request: 13895 +author: +type: added diff --git a/ee/lib/api/helpers/project_approval_rules_helpers.rb b/ee/lib/api/helpers/project_approval_rules_helpers.rb new file mode 100644 index 0000000000000000000000000000000000000000..0822a7e0a9f400b6cab0fbccbe9b8c4b04ecb3d6 --- /dev/null +++ b/ee/lib/api/helpers/project_approval_rules_helpers.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module API + module Helpers + module ProjectApprovalRulesHelpers + extend Grape::API::Helpers + + ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) } + + params :create_project_approval_rule do + requires :name, type: String, desc: 'The name of the approval rule' + requires :approvals_required, type: Integer, desc: 'The number of required approvals for this rule' + optional :rule_type, type: String, desc: 'The type of approval rule' + optional :users, as: :user_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The user ids for this rule' + optional :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule' + end + + params :update_project_approval_rule do + requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule' + optional :name, type: String, desc: 'The name of the approval rule' + optional :approvals_required, type: Integer, desc: 'The number of required approvals for this rule' + optional :users, as: :user_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The user ids for this rule' + optional :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule' + optional :remove_hidden_groups, type: Boolean, desc: 'Whether hidden groups should be removed' + end + + params :delete_project_approval_rule do + requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule' + end + + def authorize_create_merge_request_in_project + authorize! :create_merge_request_in, user_project + end + + def create_project_approval_rule(present_with:) + authorize! :admin_project, user_project + + result = ::ApprovalRules::CreateService.new(user_project, current_user, declared_params(include_missing: false)).execute + + if result[:status] == :success + present result[:rule], with: present_with, current_user: current_user + else + render_api_error!(result[:message], 400) + end + end + + def update_project_approval_rule(present_with:) + authorize! :admin_project, user_project + + params = declared_params(include_missing: false) + approval_rule = user_project.approval_rules.find(params.delete(:approval_rule_id)) + result = ::ApprovalRules::UpdateService.new(approval_rule, current_user, params).execute + + if result[:status] == :success + present result[:rule], with: present_with, current_user: current_user + else + render_api_error!(result[:message], 400) + end + end + + def destroy_project_approval_rule + authorize! :admin_project, user_project + + approval_rule = user_project.approval_rules.find(params[:approval_rule_id]) + + destroy_conditionally!(approval_rule) do |rule| + ::ApprovalRules::ProjectRuleDestroyService.new(rule).execute + end + end + end + end +end diff --git a/ee/lib/api/project_approval_rules.rb b/ee/lib/api/project_approval_rules.rb index a40af59c796a291bbfb2ee44363bd01912d2fa53..a251e568e98a300f63db558154d9ae16075ed2db 100644 --- a/ee/lib/api/project_approval_rules.rb +++ b/ee/lib/api/project_approval_rules.rb @@ -4,89 +4,49 @@ module API class ProjectApprovalRules < ::Grape::API before { authenticate! } - ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) } + helpers ::API::Helpers::ProjectApprovalRulesHelpers params do requires :id, type: String, desc: 'The ID of a project' end resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - segment ':id/approval_settings' do + segment ':id/approval_rules' do desc 'Get all project approval rules' do - detail 'Private API subject to change' - success EE::API::Entities::ProjectApprovalRules + success EE::API::Entities::ApprovalRule end get do - authorize! :create_merge_request_in, user_project + authorize_create_merge_request_in_project - present user_project, with: EE::API::Entities::ProjectApprovalRules, current_user: current_user + present user_project.visible_approval_rules, with: EE::API::Entities::ApprovalRule, current_user: current_user end - segment 'rules' do - desc 'Create new approval rule' do - detail 'Private API subject to change' + desc 'Create new project approval rule' do + success EE::API::Entities::ApprovalRule + end + params do + use :create_project_approval_rule + end + post do + create_project_approval_rule(present_with: EE::API::Entities::ApprovalRule) + end + + segment ':approval_rule_id' do + desc 'Update project approval rule' do success EE::API::Entities::ApprovalRule end params do - requires :name, type: String, desc: 'The name of the approval rule' - requires :approvals_required, type: Integer, desc: 'The number of required approvals for this rule' - optional :rule_type, type: String, desc: 'The type of approval rule' - optional :users, as: :user_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The user ids for this rule' - optional :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule' + use :update_project_approval_rule end - post do - authorize! :admin_project, user_project - - result = ::ApprovalRules::CreateService.new(user_project, current_user, declared_params(include_missing: false)).execute - - if result[:status] == :success - present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user - else - render_api_error!(result[:message], 400) - end + put do + update_project_approval_rule(present_with: EE::API::Entities::ApprovalRule) end - segment ':approval_rule_id' do - desc 'Update approval rule' do - detail 'Private API subject to change' - success EE::API::Entities::ApprovalRule - end - params do - requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule' - optional :name, type: String, desc: 'The name of the approval rule' - optional :approvals_required, type: Integer, desc: 'The number of required approvals for this rule' - optional :users, as: :user_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The user ids for this rule' - optional :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule' - optional :remove_hidden_groups, type: Boolean, desc: 'Whether hidden groups should be removed' - end - put do - authorize! :admin_project, user_project - - params = declared_params(include_missing: false) - approval_rule = user_project.approval_rules.find(params.delete(:approval_rule_id)) - result = ::ApprovalRules::UpdateService.new(approval_rule, current_user, params).execute - - if result[:status] == :success - present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user - else - render_api_error!(result[:message], 400) - end - end - - desc 'Delete an approval rule' do - detail 'Private API subject to change' - end - params do - requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule' - end - delete do - authorize! :admin_project, user_project - - approval_rule = user_project.approval_rules.find(params[:approval_rule_id]) - - destroy_conditionally!(approval_rule) do |rule| - ::ApprovalRules::ProjectRuleDestroyService.new(rule).execute - end - end + desc 'Destroy project approval rule' + params do + use :delete_project_approval_rule + end + delete do + destroy_project_approval_rule end end end diff --git a/ee/lib/api/project_approval_settings.rb b/ee/lib/api/project_approval_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..1a2afe02b9ecdb915fa483dca66a7ee4838f5e2d --- /dev/null +++ b/ee/lib/api/project_approval_settings.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module API + class ProjectApprovalSettings < ::Grape::API + before { authenticate! } + + helpers ::API::Helpers::ProjectApprovalRulesHelpers + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + segment ':id/approval_settings' do + desc 'Get all project approval rules' do + detail 'Private API subject to change' + success EE::API::Entities::ProjectApprovalSettings + end + get do + authorize_create_merge_request_in_project + + present user_project, with: EE::API::Entities::ProjectApprovalSettings, current_user: current_user + end + + segment 'rules' do + desc 'Create new approval rule' do + detail 'Private API subject to change' + success EE::API::Entities::ApprovalSettingRule + end + params do + use :create_project_approval_rule + end + post do + create_project_approval_rule(present_with: EE::API::Entities::ApprovalSettingRule) + end + + segment ':approval_rule_id' do + desc 'Update approval rule' do + detail 'Private API subject to change' + success EE::API::Entities::ApprovalSettingRule + end + params do + use :update_project_approval_rule + end + put do + update_project_approval_rule(present_with: EE::API::Entities::ApprovalSettingRule) + end + + desc 'Delete an approval rule' do + detail 'Private API subject to change' + end + params do + use :delete_project_approval_rule + end + delete do + destroy_project_approval_rule + end + end + end + end + end + end +end diff --git a/ee/lib/ee/api/endpoints.rb b/ee/lib/ee/api/endpoints.rb index ac724008185c168373408d29c47aaedc0712deac..62c032c61b2f16643e0f2fc15e26ce7d97b5c95b 100644 --- a/ee/lib/ee/api/endpoints.rb +++ b/ee/lib/ee/api/endpoints.rb @@ -10,6 +10,7 @@ module Endpoints mount ::EE::API::GroupBoards mount ::API::ProjectApprovalRules + mount ::API::ProjectApprovalSettings mount ::API::Unleash mount ::API::EpicIssues mount ::API::EpicLinks diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index 0af462a241763e4ee3361a1990d2aa5ce799ea18..c48ffdd9bd266c108765a151d7d4493a8f2e4754 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -309,7 +309,7 @@ class ApprovalRuleShort < Grape::Entity expose :id, :name, :rule_type end - class ApprovalRule < ApprovalRuleShort + class ApprovalSettingRule < ApprovalRuleShort def initialize(object, options = {}) presenter = ::ApprovalRulePresenter.new(object, current_user: options[:current_user]) super(presenter, options) @@ -322,7 +322,11 @@ def initialize(object, options = {}) expose :contains_hidden_groups?, as: :contains_hidden_groups end - class MergeRequestApprovalRule < ApprovalRule + class ApprovalRule < ApprovalSettingRule + expose :approvers, as: :eligible_approvers, using: ::API::Entities::UserBasic, override: true + end + + class MergeRequestApprovalRule < ApprovalSettingRule class SourceRule < Grape::Entity expose :approvals_required end @@ -343,8 +347,8 @@ class MergeRequestApprovalRules < Grape::Entity end # Decorates Project - class ProjectApprovalRules < Grape::Entity - expose :visible_approval_rules, as: :rules, using: ApprovalRule + class ProjectApprovalSettings < Grape::Entity + expose :visible_approval_rules, as: :rules, using: ApprovalSettingRule expose :min_fallback_approvals, as: :fallback_approvals_required end diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rule.json b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rule.json index 226c692a2f3ee080f87a861a0dbad143dd087122..d27a218bf41556ae7c355a3ec5799ce600e9f6de 100644 --- a/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rule.json +++ b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rule.json @@ -6,7 +6,7 @@ "approvals_required": { "type": "integer" }, "contains_hidden_groups": { "type": "boolean" }, "rule_type": { "type": "tring" }, - "approvers": { + "eligible_approvers": { "type": "array", "items": { "type": "object", diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rules.json b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rules.json index 3753f8cf2700c74802523322aa7fd447d57f46b6..4460d376ea5d671dad8f64d142438a2d46186c83 100644 --- a/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rules.json +++ b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rules.json @@ -1,11 +1,4 @@ { - "type": "object", - "properties": { - "rules": { - "type": "array", - "items": { "$ref": "project_approval_rule.json" } - }, - "fallback_approvals_required": { "type": "integer" } - }, - "additionalProperties": false + "type": "array", + "items": { "$ref": "./project_approval_rule.json" } } diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_setting.json b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_setting.json new file mode 100644 index 0000000000000000000000000000000000000000..226c692a2f3ee080f87a861a0dbad143dd087122 --- /dev/null +++ b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_setting.json @@ -0,0 +1,32 @@ +{ + "type": "object", + "properties": { + "id": { "type": "integer" }, + "name": { "type": "string" }, + "approvals_required": { "type": "integer" }, + "contains_hidden_groups": { "type": "boolean" }, + "rule_type": { "type": "tring" }, + "approvers": { + "type": "array", + "items": { + "type": "object", + "properties": {} + } + }, + "groups": { + "type": "array", + "items": { + "type": "object", + "properties": {} + } + }, + "users": { + "type": "array", + "items": { + "type": "object", + "properties": {} + } + } + }, + "additionalProperties": false +} diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_settings.json b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_settings.json new file mode 100644 index 0000000000000000000000000000000000000000..5f37fe7289ad18eab66f9457b4036d274f5d78f5 --- /dev/null +++ b/ee/spec/fixtures/api/schemas/public_api/v4/project_approval_settings.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "properties": { + "rules": { + "type": "array", + "items": { "$ref": "project_approval_setting.json" } + }, + "fallback_approvals_required": { "type": "integer" } + }, + "additionalProperties": false +} diff --git a/ee/spec/requests/api/project_approval_rules_spec.rb b/ee/spec/requests/api/project_approval_rules_spec.rb index 5c86db2948d5da70d9a5c77cd49ed53ca0fe3a11..6d74931a18c492e7f9e299cf5b271faf9eeaf7ab 100644 --- a/ee/spec/requests/api/project_approval_rules_spec.rb +++ b/ee/spec/requests/api/project_approval_rules_spec.rb @@ -3,17 +3,15 @@ require 'spec_helper' describe API::ProjectApprovalRules 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(: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}/approval_settings/rules" } - - describe 'GET /projects/:id/approval_settings' do - let(:url) { "/projects/#{project.id}/approval_settings" } + describe 'GET /projects/:id/approval_rules' do + let(:url) { "/projects/#{project.id}/approval_rules" } context 'when the request is correct' do let!(:rule) do @@ -36,9 +34,9 @@ json = json_response - expect(json['rules'].size).to eq(1) + expect(json.size).to eq(1) - rule = json['rules'].first + rule = json.first expect(rule['approvals_required']).to eq(7) expect(rule['name']).to eq('security') @@ -55,7 +53,7 @@ get api(url, developer) json = json_response - rule = json['rules'].first + rule = json.first expect(rule['groups'].size).to eq(0) end @@ -66,7 +64,7 @@ get api(url, developer) json = json_response - rule = json['rules'].first + rule = json.first expect(rule['groups'].size).to eq(1) end @@ -82,162 +80,32 @@ json = json_response - expect(json['rules'].size).to eq(2) - expect(json['rules'].map { |rule| rule['name'] }).to contain_exactly(rule.name, report_approver_rule.name) + expect(json.size).to eq(2) + expect(json.map { |rule| rule['name'] }).to contain_exactly(rule.name, report_approver_rule.name) end end end end - describe 'POST /projects/:id/approval_settings/rules' do - let(:current_user) { user } - let(:params) do - { - name: 'security', - approvals_required: 10 - } - end - - 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 user is without access' do - it 'returns 403' do - post api(url, user2), params: params - - expect(response).to have_gitlab_http_status(403) - end - end - - context 'when the request is correct' do - it 'returns 201 status' do - post api(url, current_user), params: params - - expect(response).to have_gitlab_http_status(201) - expect(response).to match_response_schema('public_api/v4/project_approval_rule', dir: 'ee') - end - - it 'changes settings properly' do - create(:approval_project_rule, project: project, approvals_required: 2) + describe 'POST /projects/:id/approval_rules' do + let(:schema) { 'public_api/v4/project_approval_rule' } + let(:url) { "/projects/#{project.id}/approval_rules" } - project.reset_approvals_on_push = false - project.disable_overriding_approvers_per_merge_request = true - project.save - - post api(url, current_user), params: params - - expect(json_response.symbolize_keys).to include(params) - end - - it 'sets rule_type as report_approver if name matches default name for security reports' do - expect do - post api(url, current_user), params: params.merge(name: ApprovalProjectRule::DEFAULT_NAME_FOR_SECURITY_REPORT) - end.to change { ApprovalProjectRule.report_approver.count }.from(0).to(1) - - expect(response).to have_gitlab_http_status(201) - end - end + it_behaves_like 'an API endpoint for creating project approval rule' end - describe 'PUT /projects/:id/approval_settings/:approval_rule_id' do + describe 'PUT /projects/:id/approval_rules/:approval_rule_id' do let!(:approval_rule) { create(:approval_project_rule, project: project) } - let(:url) { "/projects/#{project.id}/approval_settings/rules/#{approval_rule.id}" } - - shared_examples_for 'a user with access' do - before do - project.add_developer(approver) - end - - context 'when approver already exists' do - before do - approval_rule.users << approver - approval_rule.groups << group - end - - context 'when sending json data' do - it 'removes all approvers if empty params are given' do - expect do - put api(url, current_user), params: { users: [], groups: [] }.to_json, headers: { CONTENT_TYPE: 'application/json' } - end.to change { approval_rule.users.count + approval_rule.groups.count }.from(2).to(0) - - expect(response).to have_gitlab_http_status(200) - end - end - end - - it 'sets approvers' do - expect do - put api(url, current_user), params: { users: [approver.id] } - end.to change { approval_rule.users.count }.from(0).to(1) - - expect(approval_rule.users).to contain_exactly(approver) - expect(approval_rule.groups).to be_empty - - expect(response).to have_gitlab_http_status(200) - expect(json_response['approvers'][0]['id']).to eq(approver.id) - end - end - - context 'as a project admin' do - it_behaves_like 'a user with access' do - let(:current_user) { user } - let(:visible_approver_groups_count) { 0 } - end - end - - context 'as a global admin' do - it_behaves_like 'a user with access' do - let(:current_user) { admin } - let(:visible_approver_groups_count) { 1 } - end - end - - context 'as a random user' do - it 'returns 403' do - project.approvers.create(user: approver) + let(:schema) { 'public_api/v4/project_approval_rule' } + let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule.id}" } - expect do - put api(url, user2), params: { users: [], groups: [] }.to_json, headers: { CONTENT_TYPE: 'application/json' } - end.not_to change { approval_rule.approvers.size } - - expect(response).to have_gitlab_http_status(403) - end - end + it_behaves_like 'an API endpoint for updating project approval rule' end - describe 'DELETE /projects/:id/approval_settings/rules/:approval_rule_id' do + describe 'DELETE /projects/:id/approval_rules/:approval_rule_id' do let!(:approval_rule) { create(:approval_project_rule, project: project) } - let(:url) { "/projects/#{project.id}/approval_settings/rules/#{approval_rule.id}" } - - it 'destroys' do - delete api(url, user) - - expect(ApprovalProjectRule.exists?(id: approval_rule.id)).to eq(false) - expect(response).to have_gitlab_http_status(204) - end - - context 'when approval rule not found' do - let!(:approval_rule_2) { create(:approval_project_rule) } - let(:url) { "/projects/#{project.id}/approval_settings/#{approval_rule_2.id}" } + let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule.id}" } - it 'returns not found' do - delete api(url, user) - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'when user is not eligible to delete' do - it 'returns forbidden' do - delete api(url, user2) - - expect(response).to have_gitlab_http_status(403) - end - end + it_behaves_like 'an API endpoint for deleting project approval rule' end end diff --git a/ee/spec/requests/api/project_approval_settings_spec.rb b/ee/spec/requests/api/project_approval_settings_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..dc103769a4f70808dee9632f7f1018fd31a573b4 --- /dev/null +++ b/ee/spec/requests/api/project_approval_settings_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::ProjectApprovalSettings 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) } + + describe 'GET /projects/:id/approval_settings' do + let(:url) { "/projects/#{project.id}/approval_settings" } + + context 'when the request is correct' do + let!(:rule) do + rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7) + rule.users << approver + rule + end + + let(:developer) do + user = create(:user) + project.add_guest(user) + user + end + + it 'matches the response schema' do + get api(url, developer) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/project_approval_settings', dir: 'ee') + + json = json_response + + expect(json['rules'].size).to eq(1) + + rule = json['rules'].first + + expect(rule['approvals_required']).to eq(7) + expect(rule['name']).to eq('security') + end + + context 'private group filtering' do + set(:private_group) { create :group, :private } + + before do + rule.groups << private_group + end + + it 'excludes private groups if user has no access' do + get api(url, developer) + + json = json_response + rule = json['rules'].first + + expect(rule['groups'].size).to eq(0) + end + + it 'includes private groups if user has access' do + private_group.add_owner(developer) + + get api(url, developer) + + json = json_response + rule = json['rules'].first + + expect(rule['groups'].size).to eq(1) + end + end + + context 'report_approver rules' do + let!(:report_approver_rule) do + create(:approval_project_rule, :security_report, project: project) + end + + it 'includes report_approver rules' do + get api(url, developer) + + json = json_response + + expect(json['rules'].size).to eq(2) + expect(json['rules'].map { |rule| rule['name'] }).to contain_exactly(rule.name, report_approver_rule.name) + end + end + end + end + + describe 'POST /projects/:id/approval_settings/rules' do + let(:schema) { 'public_api/v4/project_approval_setting' } + let(:url) { "/projects/#{project.id}/approval_settings/rules" } + + it_behaves_like 'an API endpoint for creating project approval rule' + end + + describe 'PUT /projects/:id/approval_settings/:approval_rule_id' do + let!(:approval_rule) { create(:approval_project_rule, project: project) } + let(:schema) { 'public_api/v4/project_approval_setting' } + let(:url) { "/projects/#{project.id}/approval_settings/rules/#{approval_rule.id}" } + + it_behaves_like 'an API endpoint for updating project approval rule' + end + + describe 'DELETE /projects/:id/approval_settings/rules/:approval_rule_id' do + let!(:approval_rule) { create(:approval_project_rule, project: project) } + let(:url) { "/projects/#{project.id}/approval_settings/rules/#{approval_rule.id}" } + + it_behaves_like 'an API endpoint for deleting project approval rule' + end +end diff --git a/ee/spec/support/shared_examples/project_approval_rules_api_shared_examples.rb b/ee/spec/support/shared_examples/project_approval_rules_api_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..b5f57f5d6aa12daaebb25eda632cab8a57d81d2c --- /dev/null +++ b/ee/spec/support/shared_examples/project_approval_rules_api_shared_examples.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +shared_examples_for 'an API endpoint for creating project approval rule' do + let(:current_user) { user } + let(:params) do + { + name: 'security', + approvals_required: 10 + } + end + + 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 user is without access' do + it 'returns 403' do + post api(url, user2), params: params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'when the request is correct' do + it 'returns 201 status' do + post api(url, current_user), params: params + + expect(response).to have_gitlab_http_status(201) + expect(response).to match_response_schema(schema, dir: 'ee') + end + + it 'changes settings properly' do + create(:approval_project_rule, project: project, approvals_required: 2) + + project.reset_approvals_on_push = false + project.disable_overriding_approvers_per_merge_request = true + project.save + + post api(url, current_user), params: params + + expect(json_response.symbolize_keys).to include(params) + end + + it 'sets rule_type as report_approver if name matches default name for security reports' do + expect do + post api(url, current_user), params: params.merge(name: ApprovalProjectRule::DEFAULT_NAME_FOR_SECURITY_REPORT) + end.to change { ApprovalProjectRule.report_approver.count }.from(0).to(1) + + expect(response).to have_gitlab_http_status(201) + end + end +end + +shared_examples_for 'an API endpoint for updating project approval rule' do + shared_examples_for 'a user with access' do + before do + project.add_developer(approver) + end + + context 'when approver already exists' do + before do + approval_rule.users << approver + approval_rule.groups << group + end + + context 'when sending json data' do + it 'removes all approvers if empty params are given' do + expect do + put api(url, current_user), params: { users: [], groups: [] }.to_json, headers: { CONTENT_TYPE: 'application/json' } + end.to change { approval_rule.users.count + approval_rule.groups.count }.from(2).to(0) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema(schema, dir: 'ee') + end + end + end + + it 'sets approvers' do + expect do + put api(url, current_user), params: { users: [approver.id] } + end.to change { approval_rule.users.count }.from(0).to(1) + + expect(approval_rule.users).to contain_exactly(approver) + expect(approval_rule.groups).to be_empty + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'as a project admin' do + it_behaves_like 'a user with access' do + let(:current_user) { user } + let(:visible_approver_groups_count) { 0 } + end + end + + context 'as a global admin' do + it_behaves_like 'a user with access' do + let(:current_user) { admin } + let(:visible_approver_groups_count) { 1 } + end + end + + context 'as a random user' do + it 'returns 403' do + project.approvers.create(user: approver) + + expect do + put api(url, user2), params: { users: [], groups: [] }.to_json, headers: { CONTENT_TYPE: 'application/json' } + end.not_to change { approval_rule.approvers.size } + + expect(response).to have_gitlab_http_status(403) + end + end +end + +shared_examples_for 'an API endpoint for deleting project approval rule' do + it 'destroys' do + delete api(url, user) + + expect(ApprovalProjectRule.exists?(id: approval_rule.id)).to eq(false) + expect(response).to have_gitlab_http_status(204) + end + + context 'when approval rule not found' do + let!(:approval_rule_2) { create(:approval_project_rule) } + let(:url) { "/projects/#{project.id}/approval_settings/#{approval_rule_2.id}" } + + it 'returns not found' do + delete api(url, user) + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when user is not eligible to delete' do + it 'returns forbidden' do + delete api(url, user2) + + expect(response).to have_gitlab_http_status(403) + end + end +end