Commit 1d47ae13 authored by James Edwards-Jones's avatar James Edwards-Jones

CE backport of ProtectedBranches API changes

In EE we now allow individual users/groups to be set on POST, which required some refactoring.
See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3516
parent 29be9c1a
module ProtectedBranches
class AccessLevelParams
attr_reader :type, :params
def initialize(type, params)
@type = type
@params = params_with_default(params)
end
def access_levels
ce_style_access_level
end
private
def params_with_default(params)
params[:"#{type}_access_level"] ||= Gitlab::Access::MASTER if use_default_access_level?(params)
params
end
def use_default_access_level?(params)
true
end
def ce_style_access_level
access_level = params[:"#{type}_access_level"]
return [] unless access_level
[{ access_level: access_level }]
end
end
end
module ProtectedBranches
class ApiService < BaseService
def create
@push_params = AccessLevelParams.new(:push, params)
@merge_params = AccessLevelParams.new(:merge, params)
verify_params!
protected_branch_params = {
name: params[:name],
push_access_levels_attributes: @push_params.access_levels,
merge_access_levels_attributes: @merge_params.access_levels
}
::ProtectedBranches::CreateService.new(@project, @current_user, protected_branch_params).execute
end
private
def verify_params!
# EE-only
end
end
end
...@@ -136,7 +136,7 @@ DELETE /projects/:id/protected_branches/:name ...@@ -136,7 +136,7 @@ DELETE /projects/:id/protected_branches/:name
``` ```
```bash ```bash
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_branches/*-stable' curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_branches/*-stable'
``` ```
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
......
...@@ -39,10 +39,10 @@ module API ...@@ -39,10 +39,10 @@ module API
end end
params do params do
requires :name, type: String, desc: 'The name of the protected branch' requires :name, type: String, desc: 'The name of the protected branch'
optional :push_access_level, type: Integer, default: Gitlab::Access::MASTER, optional :push_access_level, type: Integer,
values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS,
desc: 'Access levels allowed to push (defaults: `40`, master access level)' desc: 'Access levels allowed to push (defaults: `40`, master access level)'
optional :merge_access_level, type: Integer, default: Gitlab::Access::MASTER, optional :merge_access_level, type: Integer,
values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS,
desc: 'Access levels allowed to merge (defaults: `40`, master access level)' desc: 'Access levels allowed to merge (defaults: `40`, master access level)'
end end
...@@ -52,15 +52,13 @@ module API ...@@ -52,15 +52,13 @@ module API
conflict!("Protected branch '#{params[:name]}' already exists") conflict!("Protected branch '#{params[:name]}' already exists")
end end
protected_branch_params = { # Replace with `declared(params)` after updating to grape v1.0.2
name: params[:name], # See https://github.com/ruby-grape/grape/pull/1710
push_access_levels_attributes: [{ access_level: params[:push_access_level] }], # and https://gitlab.com/gitlab-org/gitlab-ce/issues/40843
merge_access_levels_attributes: [{ access_level: params[:merge_access_level] }] declared_params = params.slice("name", "push_access_level", "merge_access_level", "allowed_to_push", "allowed_to_merge")
}
service_args = [user_project, current_user, protected_branch_params] api_service = ::ProtectedBranches::ApiService.new(user_project, current_user, declared_params)
protected_branch = api_service.create
protected_branch = ::ProtectedBranches::CreateService.new(*service_args).execute
if protected_branch.persisted? if protected_branch.persisted?
present protected_branch, with: Entities::ProtectedBranch, project: user_project present protected_branch, with: Entities::ProtectedBranch, project: user_project
......
...@@ -95,6 +95,12 @@ describe API::ProtectedBranches do ...@@ -95,6 +95,12 @@ describe API::ProtectedBranches do
describe 'POST /projects/:id/protected_branches' do describe 'POST /projects/:id/protected_branches' do
let(:branch_name) { 'new_branch' } let(:branch_name) { 'new_branch' }
let(:post_endpoint) { api("/projects/#{project.id}/protected_branches", user) }
def expect_protection_to_be_successful
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
end
context 'when authenticated as a master' do context 'when authenticated as a master' do
before do before do
...@@ -102,7 +108,7 @@ describe API::ProtectedBranches do ...@@ -102,7 +108,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch' do it 'protects a single branch' do
post api("/projects/#{project.id}/protected_branches", user), name: branch_name post post_endpoint, name: branch_name
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -111,8 +117,7 @@ describe API::ProtectedBranches do ...@@ -111,8 +117,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch and developers can push' do it 'protects a single branch and developers can push' do
post api("/projects/#{project.id}/protected_branches", user), post post_endpoint, name: branch_name, push_access_level: 30
name: branch_name, push_access_level: 30
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -121,8 +126,7 @@ describe API::ProtectedBranches do ...@@ -121,8 +126,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch and developers can merge' do it 'protects a single branch and developers can merge' do
post api("/projects/#{project.id}/protected_branches", user), post post_endpoint, name: branch_name, merge_access_level: 30
name: branch_name, merge_access_level: 30
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -131,8 +135,7 @@ describe API::ProtectedBranches do ...@@ -131,8 +135,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch and developers can push and merge' do it 'protects a single branch and developers can push and merge' do
post api("/projects/#{project.id}/protected_branches", user), post post_endpoint, name: branch_name, push_access_level: 30, merge_access_level: 30
name: branch_name, push_access_level: 30, merge_access_level: 30
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -141,8 +144,7 @@ describe API::ProtectedBranches do ...@@ -141,8 +144,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch and no one can push' do it 'protects a single branch and no one can push' do
post api("/projects/#{project.id}/protected_branches", user), post post_endpoint, name: branch_name, push_access_level: 0
name: branch_name, push_access_level: 0
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -151,8 +153,7 @@ describe API::ProtectedBranches do ...@@ -151,8 +153,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch and no one can merge' do it 'protects a single branch and no one can merge' do
post api("/projects/#{project.id}/protected_branches", user), post post_endpoint, name: branch_name, merge_access_level: 0
name: branch_name, merge_access_level: 0
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -161,8 +162,7 @@ describe API::ProtectedBranches do ...@@ -161,8 +162,7 @@ describe API::ProtectedBranches do
end end
it 'protects a single branch and no one can push or merge' do it 'protects a single branch and no one can push or merge' do
post api("/projects/#{project.id}/protected_branches", user), post post_endpoint, name: branch_name, push_access_level: 0, merge_access_level: 0
name: branch_name, push_access_level: 0, merge_access_level: 0
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name) expect(json_response['name']).to eq(branch_name)
...@@ -171,7 +171,8 @@ describe API::ProtectedBranches do ...@@ -171,7 +171,8 @@ describe API::ProtectedBranches do
end end
it 'returns a 409 error if the same branch is protected twice' do it 'returns a 409 error if the same branch is protected twice' do
post api("/projects/#{project.id}/protected_branches", user), name: protected_name post post_endpoint, name: protected_name
expect(response).to have_gitlab_http_status(409) expect(response).to have_gitlab_http_status(409)
end end
...@@ -179,10 +180,9 @@ describe API::ProtectedBranches do ...@@ -179,10 +180,9 @@ describe API::ProtectedBranches do
let(:branch_name) { 'feature/*' } let(:branch_name) { 'feature/*' }
it "protects multiple branches with a wildcard in the name" do it "protects multiple branches with a wildcard in the name" do
post api("/projects/#{project.id}/protected_branches", user), name: branch_name post post_endpoint, name: branch_name
expect(response).to have_gitlab_http_status(201) expect_protection_to_be_successful
expect(json_response['name']).to eq(branch_name)
expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER)
expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER)
end end
...@@ -195,7 +195,7 @@ describe API::ProtectedBranches do ...@@ -195,7 +195,7 @@ describe API::ProtectedBranches do
end end
it "returns a 403 error if guest" do it "returns a 403 error if guest" do
post api("/projects/#{project.id}/protected_branches/", user), name: branch_name post post_endpoint, name: branch_name
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
end end
......
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