summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Edwards-Jones <jedwardsjones@gitlab.com>2017-11-22 17:08:47 +0000
committerJames Edwards-Jones <jedwardsjones@gitlab.com>2017-12-06 16:39:21 +0000
commitb1353873425594df1e44bd5c6a0c052b667f10d4 (patch)
tree8d993a8baf36f7b55df559115f3b9146d598b29b
parent29be9c1acc9523a513ce32d8a56298db1a038873 (diff)
downloadgitlab-ce-jej/per-user-protected-branches-api-ce.tar.gz
CE backport of ProtectedBranches API changesjej/per-user-protected-branches-api-ce
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
-rw-r--r--app/services/access_level_params.rb32
-rw-r--r--app/services/protected_branches/api_service.rb25
-rw-r--r--doc/api/protected_branches.md2
-rw-r--r--lib/api/protected_branches.rb17
-rw-r--r--spec/requests/api/protected_branches_spec.rb36
5 files changed, 83 insertions, 29 deletions
diff --git a/app/services/access_level_params.rb b/app/services/access_level_params.rb
new file mode 100644
index 00000000000..0c2354a3509
--- /dev/null
+++ b/app/services/access_level_params.rb
@@ -0,0 +1,32 @@
+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
diff --git a/app/services/protected_branches/api_service.rb b/app/services/protected_branches/api_service.rb
new file mode 100644
index 00000000000..7ef2bfdc2e6
--- /dev/null
+++ b/app/services/protected_branches/api_service.rb
@@ -0,0 +1,25 @@
+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
diff --git a/doc/api/protected_branches.md b/doc/api/protected_branches.md
index 81fe854060a..950ead52560 100644
--- a/doc/api/protected_branches.md
+++ b/doc/api/protected_branches.md
@@ -136,7 +136,7 @@ DELETE /projects/:id/protected_branches/:name
```
```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 |
diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb
index b5021e8a712..972cd3f631c 100644
--- a/lib/api/protected_branches.rb
+++ b/lib/api/protected_branches.rb
@@ -39,10 +39,10 @@ module API
end
params do
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,
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,
desc: 'Access levels allowed to merge (defaults: `40`, master access level)'
end
@@ -52,15 +52,12 @@ module API
conflict!("Protected branch '#{params[:name]}' already exists")
end
- protected_branch_params = {
- name: params[:name],
- push_access_levels_attributes: [{ access_level: params[:push_access_level] }],
- merge_access_levels_attributes: [{ access_level: params[:merge_access_level] }]
- }
+ # Replace with `declared(params)` after updating to grape v1.0.2
+ # See https://github.com/ruby-grape/grape/pull/1710
+ 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]
-
- protected_branch = ::ProtectedBranches::CreateService.new(*service_args).execute
+ api_service = ::ProtectedBranches::ApiService.new(user_project, current_user, declared_params)
+ protected_branch = api_service.create
if protected_branch.persisted?
present protected_branch, with: Entities::ProtectedBranch, project: user_project
diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb
index 07d7f96bd70..10e6a3c07c8 100644
--- a/spec/requests/api/protected_branches_spec.rb
+++ b/spec/requests/api/protected_branches_spec.rb
@@ -95,6 +95,12 @@ describe API::ProtectedBranches do
describe 'POST /projects/:id/protected_branches' do
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
before do
@@ -102,7 +108,7 @@ describe API::ProtectedBranches do
end
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(json_response['name']).to eq(branch_name)
@@ -111,8 +117,7 @@ describe API::ProtectedBranches do
end
it 'protects a single branch and developers can push' do
- post api("/projects/#{project.id}/protected_branches", user),
- name: branch_name, push_access_level: 30
+ post post_endpoint, name: branch_name, push_access_level: 30
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
@@ -121,8 +126,7 @@ describe API::ProtectedBranches do
end
it 'protects a single branch and developers can merge' do
- post api("/projects/#{project.id}/protected_branches", user),
- name: branch_name, merge_access_level: 30
+ post post_endpoint, name: branch_name, merge_access_level: 30
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
@@ -131,8 +135,7 @@ describe API::ProtectedBranches do
end
it 'protects a single branch and developers can push and merge' do
- post api("/projects/#{project.id}/protected_branches", user),
- name: branch_name, push_access_level: 30, merge_access_level: 30
+ post post_endpoint, name: branch_name, push_access_level: 30, merge_access_level: 30
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
@@ -141,8 +144,7 @@ describe API::ProtectedBranches do
end
it 'protects a single branch and no one can push' do
- post api("/projects/#{project.id}/protected_branches", user),
- name: branch_name, push_access_level: 0
+ post post_endpoint, name: branch_name, push_access_level: 0
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
@@ -151,8 +153,7 @@ describe API::ProtectedBranches do
end
it 'protects a single branch and no one can merge' do
- post api("/projects/#{project.id}/protected_branches", user),
- name: branch_name, merge_access_level: 0
+ post post_endpoint, name: branch_name, merge_access_level: 0
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
@@ -161,8 +162,7 @@ describe API::ProtectedBranches do
end
it 'protects a single branch and no one can push or merge' do
- post api("/projects/#{project.id}/protected_branches", user),
- name: branch_name, push_access_level: 0, merge_access_level: 0
+ post post_endpoint, name: branch_name, push_access_level: 0, merge_access_level: 0
expect(response).to have_gitlab_http_status(201)
expect(json_response['name']).to eq(branch_name)
@@ -171,7 +171,8 @@ describe API::ProtectedBranches do
end
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)
end
@@ -179,10 +180,9 @@ describe API::ProtectedBranches do
let(:branch_name) { 'feature/*' }
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(json_response['name']).to eq(branch_name)
+ expect_protection_to_be_successful
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)
end
@@ -195,7 +195,7 @@ describe API::ProtectedBranches do
end
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)
end