Commit 2d2ea563 authored by Mark Chao's avatar Mark Chao

Change project approval rule api

Now endpoints are under approval_settings scope.
Add PUT /projects/:id/approval_settings for updating
overall approvals_required.
parent 1458585d
...@@ -10,7 +10,7 @@ module API ...@@ -10,7 +10,7 @@ module API
requires :id, type: String, desc: 'The ID of a project' requires :id, type: String, desc: 'The ID of a project'
end end
resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
segment ':id/approval_rules' do segment ':id/approval_settings' do
desc 'Get all project approval rules' do desc 'Get all project approval rules' do
detail 'This feature was introduced in 11.6' detail 'This feature was introduced in 11.6'
success EE::API::Entities::ProjectApprovalRules success EE::API::Entities::ProjectApprovalRules
...@@ -21,47 +21,44 @@ module API ...@@ -21,47 +21,44 @@ module API
present user_project, with: EE::API::Entities::ProjectApprovalRules, current_user: current_user present user_project, with: EE::API::Entities::ProjectApprovalRules, current_user: current_user
end end
desc 'Create new approval rule' do desc 'Update fallback approvals required' do
detail 'This feature was introduced in 11.6' detail 'This feature was introduced in 11.8'
success EE::API::Entities::ApprovalRule success ::API::Entities::Project
end end
params do params do
requires :name, type: String, desc: 'The name of the approval rule' requires :fallback_approvals_required, as: :approvals_before_merge, type: Integer, desc: 'The total number of required approvals in case of fallback'
requires :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'
end end
post do put do
authorize! :admin_project, user_project authorize! :admin_project, user_project
result = ::ApprovalRules::CreateService.new(user_project, current_user, declared_params(include_missing: false)).execute result = ::Projects::UpdateService.new(user_project, current_user, declared_params).execute
if result[:status] == :success if result[:status] == :success
present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user present(
user_project,
with: ::API::Entities::Project,
user_can_admin_project: can?(current_user, :admin_project, user_project)
)
else else
render_api_error!(result[:message], 400) render_validation_error!(user_project)
end end
end end
segment ':approval_rule_id' do segment 'rules' do
desc 'Update approval rule' do desc 'Create new approval rule' do
detail 'This feature was introduced in 11.6' detail 'This feature was introduced in 11.6'
success EE::API::Entities::ApprovalRule success EE::API::Entities::ApprovalRule
end end
params do params do
requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule' requires :name, type: String, desc: 'The name of the approval rule'
optional :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 :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 :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 :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule'
end end
put do post do
authorize! :admin_project, user_project authorize! :admin_project, user_project
params = declared_params(include_missing: false) result = ::ApprovalRules::CreateService.new(user_project, current_user, declared_params(include_missing: false)).execute
puts params.inspect
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 if result[:status] == :success
present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user
...@@ -70,19 +67,47 @@ module API ...@@ -70,19 +67,47 @@ module API
end end
end end
desc 'Delete an approval rule' do segment ':approval_rule_id' do
detail 'This feature was introduced in 11.6' desc 'Update approval rule' do
end detail 'This feature was introduced in 11.6'
params do success EE::API::Entities::ApprovalRule
requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule' end
end params do
delete do requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule'
authorize! :admin_project, user_project 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'
end
put do
authorize! :admin_project, user_project
approval_rule = user_project.approval_rules.find(params[:approval_rule_id]) params = declared_params(include_missing: false)
destroy_conditionally!(approval_rule) puts params.inspect
approval_rule = user_project.approval_rules.find(params.delete(:approval_rule_id))
result = ::ApprovalRules::UpdateService.new(approval_rule, current_user, params).execute
no_content! 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 'This feature was introduced in 11.6'
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)
no_content!
end
end end
end end
end end
......
...@@ -269,6 +269,7 @@ module EE ...@@ -269,6 +269,7 @@ module EE
# Decorates Project # Decorates Project
class ProjectApprovalRules < Grape::Entity class ProjectApprovalRules < Grape::Entity
expose :approval_rules, as: :rules, using: ApprovalRule expose :approval_rules, as: :rules, using: ApprovalRule
expose :approvals_before_merge, as: :fallback_approvals_required
end end
# @deprecated # @deprecated
......
...@@ -4,7 +4,8 @@ ...@@ -4,7 +4,8 @@
"rules": { "rules": {
"type": "array", "type": "array",
"items": { "$ref": "project_approval_rule.json" } "items": { "$ref": "project_approval_rule.json" }
} },
"fallback_approvals_required": { "type": "integer" }
}, },
"additionalProperties": false "additionalProperties": false
} }
...@@ -10,9 +10,11 @@ describe API::ProjectApprovalRules do ...@@ -10,9 +10,11 @@ describe API::ProjectApprovalRules do
set(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) } set(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
set(:approver) { create(:user) } set(:approver) { create(:user) }
let(:url) { "/projects/#{project.id}/approval_rules" } 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
context 'when the request is correct' do context 'when the request is correct' do
let!(:rule) do let!(:rule) do
rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7) rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7)
...@@ -72,7 +74,43 @@ describe API::ProjectApprovalRules do ...@@ -72,7 +74,43 @@ describe API::ProjectApprovalRules do
end end
end end
describe 'POST /projects/:id/approval_rules' do describe 'PUT /projects/:id/approval_settings' do
let(:url) { "/projects/#{project.id}/approval_settings" }
shared_examples_for 'a user with access' do
context 'when sending json data' do
it 'updates approvals_before_merge' do
expect do
put api(url, current_user), params: { fallback_approvals_required: 1 }.to_json, headers: { CONTENT_TYPE: 'application/json' }
end.to change { project.reload.approvals_before_merge }.from(0).to(1)
expect(response).to have_gitlab_http_status(200)
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 random user' do
it 'returns 403' do
put api(url, user2), { fallback_approvals_required: 1 }.to_json, { CONTENT_TYPE: 'application/json' }
expect(response).to have_gitlab_http_status(403)
end
end
end
describe 'POST /projects/:id/approval_settings/rules' do
let(:current_user) { user } let(:current_user) { user }
let(:params) do let(:params) do
{ {
...@@ -119,9 +157,9 @@ describe API::ProjectApprovalRules do ...@@ -119,9 +157,9 @@ describe API::ProjectApprovalRules do
end end
end end
describe 'PUT /projects/:id/approval_rules/:approval_rule_id' do describe 'PUT /projects/:id/approval_settings/:approval_rule_id' do
let!(:approval_rule) { create(:approval_project_rule, project: project) } let!(:approval_rule) { create(:approval_project_rule, project: project) }
let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule.id}" } let(:url) { "/projects/#{project.id}/approval_settings/rules/#{approval_rule.id}" }
shared_examples_for 'a user with access' do shared_examples_for 'a user with access' do
before do before do
...@@ -185,9 +223,9 @@ describe API::ProjectApprovalRules do ...@@ -185,9 +223,9 @@ describe API::ProjectApprovalRules do
end end
end end
describe 'DELETE /projects/:id/approval_rules/:approval_rule_id' do describe 'DELETE /projects/:id/approval_settings/rules/:approval_rule_id' do
let!(:approval_rule) { create(:approval_project_rule, project: project) } let!(:approval_rule) { create(:approval_project_rule, project: project) }
let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule.id}" } let(:url) { "/projects/#{project.id}/approval_settings/rules/#{approval_rule.id}" }
it 'destroys' do it 'destroys' do
delete api(url, user) delete api(url, user)
...@@ -198,7 +236,7 @@ describe API::ProjectApprovalRules do ...@@ -198,7 +236,7 @@ describe API::ProjectApprovalRules do
context 'when approval rule not found' do context 'when approval rule not found' do
let!(:approval_rule_2) { create(:approval_project_rule) } let!(:approval_rule_2) { create(:approval_project_rule) }
let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule_2.id}" } let(:url) { "/projects/#{project.id}/approval_settings/#{approval_rule_2.id}" }
it 'returns not found' do it 'returns not found' do
delete api(url, user) delete api(url, user)
......
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