Merge branch 'api-dev-can-push' into 'master'
API: Expose 'developers_can_push' for branches ## What does this MR do? Adds support for the `developers_can_push` flag for the branches API. It also supports creating protected branches with that flag. ## Are there points in the code the reviewer needs to double check? The API call requires an optional boolean parameter `developers_can_push`. If it is not either `true` or `false`, it should not update the value. I'm not sure if this is the right way to do it. Right now it always returns `200`. Maybe the `PUT` method should return different status codes depending if it really created or updated a branch. ## What are the relevant issue numbers? Closes #12735 ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [x] API support added - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !5208
This commit is contained in:
commit
d6bd412be4
|
@ -57,6 +57,7 @@ v 8.10.0 (unreleased)
|
|||
- API: Expose `due_date` for issues (Robert Schilling)
|
||||
- API: Todos !3188 (Robert Schilling)
|
||||
- API: Expose shared groups for projects and shared projects for groups !5050 (Robert Schilling)
|
||||
- API: Expose `developers_can_push` and `developers_can_merge` for branches !5208 (Robert Schilling)
|
||||
- Add "Enabled Git access protocols" to Application Settings
|
||||
- Diffs will create button/diff form on demand no on server side
|
||||
- Reduce size of HTML used by diff comment forms
|
||||
|
|
|
@ -23,6 +23,8 @@ Example response:
|
|||
{
|
||||
"name": "master",
|
||||
"protected": true,
|
||||
"developers_can_push": false,
|
||||
"developers_can_merge": false,
|
||||
"commit": {
|
||||
"author_email": "john@example.com",
|
||||
"author_name": "John Smith",
|
||||
|
@ -64,6 +66,8 @@ Example response:
|
|||
{
|
||||
"name": "master",
|
||||
"protected": true,
|
||||
"developers_can_push": false,
|
||||
"developers_can_merge": false,
|
||||
"commit": {
|
||||
"author_email": "john@example.com",
|
||||
"author_name": "John Smith",
|
||||
|
@ -91,13 +95,15 @@ PUT /projects/:id/repository/branches/:branch/protect
|
|||
```
|
||||
|
||||
```bash
|
||||
curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/repository/branches/master/protect
|
||||
curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/repository/branches/master/protect?developers_can_push=true&developers_can_merge=true
|
||||
```
|
||||
|
||||
| Attribute | Type | Required | Description |
|
||||
| --------- | ---- | -------- | ----------- |
|
||||
| `id` | integer | yes | The ID of a project |
|
||||
| `branch` | string | yes | The name of the branch |
|
||||
| `developers_can_push` | boolean | no | Flag if developers can push to the branch |
|
||||
| `developers_can_merge` | boolean | no | Flag if developers can merge to the branch |
|
||||
|
||||
Example response:
|
||||
|
||||
|
@ -117,7 +123,9 @@ Example response:
|
|||
]
|
||||
},
|
||||
"name": "master",
|
||||
"protected": true
|
||||
"protected": true,
|
||||
"developers_can_push": true,
|
||||
"developers_can_merge": true
|
||||
}
|
||||
```
|
||||
|
||||
|
@ -158,7 +166,9 @@ Example response:
|
|||
]
|
||||
},
|
||||
"name": "master",
|
||||
"protected": false
|
||||
"protected": false,
|
||||
"developers_can_push": false,
|
||||
"developers_can_merge": false
|
||||
}
|
||||
```
|
||||
|
||||
|
@ -196,7 +206,9 @@ Example response:
|
|||
]
|
||||
},
|
||||
"name": "newbranch",
|
||||
"protected": false
|
||||
"protected": false,
|
||||
"developers_can_push": false,
|
||||
"developers_can_merge": false
|
||||
}
|
||||
```
|
||||
|
||||
|
|
|
@ -36,6 +36,8 @@ module API
|
|||
# Parameters:
|
||||
# id (required) - The ID of a project
|
||||
# branch (required) - The name of the branch
|
||||
# developers_can_push (optional) - Flag if developers can push to that branch
|
||||
# developers_can_merge (optional) - Flag if developers can merge to that branch
|
||||
# Example Request:
|
||||
# PUT /projects/:id/repository/branches/:branch/protect
|
||||
put ':id/repository/branches/:branch/protect',
|
||||
|
@ -43,9 +45,20 @@ module API
|
|||
authorize_admin_project
|
||||
|
||||
@branch = user_project.repository.find_branch(params[:branch])
|
||||
not_found!("Branch") unless @branch
|
||||
not_found!('Branch') unless @branch
|
||||
protected_branch = user_project.protected_branches.find_by(name: @branch.name)
|
||||
user_project.protected_branches.create(name: @branch.name) unless protected_branch
|
||||
developers_can_push = to_boolean(params[:developers_can_push])
|
||||
developers_can_merge = to_boolean(params[:developers_can_merge])
|
||||
|
||||
if protected_branch
|
||||
protected_branch.developers_can_push = developers_can_push unless developers_can_push.nil?
|
||||
protected_branch.developers_can_merge = developers_can_merge unless developers_can_merge.nil?
|
||||
protected_branch.save
|
||||
else
|
||||
user_project.protected_branches.create(name: @branch.name,
|
||||
developers_can_push: developers_can_push || false,
|
||||
developers_can_merge: developers_can_merge || false)
|
||||
end
|
||||
|
||||
present @branch, with: Entities::RepoObject, project: user_project
|
||||
end
|
||||
|
|
|
@ -125,9 +125,21 @@ module API
|
|||
end
|
||||
end
|
||||
|
||||
expose :protected do |repo, options|
|
||||
expose :protected do |repo_obj, options|
|
||||
if options[:project]
|
||||
options[:project].protected_branch? repo.name
|
||||
options[:project].protected_branch? repo_obj.name
|
||||
end
|
||||
end
|
||||
|
||||
expose :developers_can_push do |repo_obj, options|
|
||||
if options[:project]
|
||||
options[:project].developers_can_push_to_protected_branch? repo_obj.name
|
||||
end
|
||||
end
|
||||
|
||||
expose :developers_can_merge do |repo_obj, options|
|
||||
if options[:project]
|
||||
options[:project].developers_can_merge_to_protected_branch? repo_obj.name
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -9,6 +9,13 @@ module API
|
|||
[ true, 1, '1', 't', 'T', 'true', 'TRUE', 'on', 'ON' ].include?(value)
|
||||
end
|
||||
|
||||
def to_boolean(value)
|
||||
return true if value =~ /^(true|t|yes|y|1|on)$/i
|
||||
return false if value =~ /^(false|f|no|n|0|off)$/i
|
||||
|
||||
nil
|
||||
end
|
||||
|
||||
def find_user_by_private_token
|
||||
token_string = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s
|
||||
User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string)
|
||||
|
|
|
@ -211,4 +211,27 @@ describe API::Helpers, api: true do
|
|||
expect(sudo_identifier).to eq(' 123')
|
||||
end
|
||||
end
|
||||
|
||||
describe '.to_boolean' do
|
||||
it 'converts a valid string to a boolean' do
|
||||
expect(to_boolean('true')).to be_truthy
|
||||
expect(to_boolean('YeS')).to be_truthy
|
||||
expect(to_boolean('t')).to be_truthy
|
||||
expect(to_boolean('1')).to be_truthy
|
||||
expect(to_boolean('ON')).to be_truthy
|
||||
expect(to_boolean('FaLse')).to be_falsy
|
||||
expect(to_boolean('F')).to be_falsy
|
||||
expect(to_boolean('NO')).to be_falsy
|
||||
expect(to_boolean('n')).to be_falsy
|
||||
expect(to_boolean('0')).to be_falsy
|
||||
expect(to_boolean('oFF')).to be_falsy
|
||||
end
|
||||
|
||||
it 'converts an invalid string to nil' do
|
||||
expect(to_boolean('fals')).to be_nil
|
||||
expect(to_boolean('yeah')).to be_nil
|
||||
expect(to_boolean('')).to be_nil
|
||||
expect(to_boolean(nil)).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -32,6 +32,8 @@ describe API::API, api: true do
|
|||
expect(json_response['name']).to eq(branch_name)
|
||||
expect(json_response['commit']['id']).to eq(branch_sha)
|
||||
expect(json_response['protected']).to eq(false)
|
||||
expect(json_response['developers_can_push']).to eq(false)
|
||||
expect(json_response['developers_can_merge']).to eq(false)
|
||||
end
|
||||
|
||||
it "should return a 403 error if guest" do
|
||||
|
@ -45,14 +47,95 @@ describe API::API, api: true do
|
|||
end
|
||||
end
|
||||
|
||||
describe "PUT /projects/:id/repository/branches/:branch/protect" do
|
||||
it "should protect a single branch" do
|
||||
describe 'PUT /projects/:id/repository/branches/:branch/protect' do
|
||||
it 'protects a single branch' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user)
|
||||
expect(response).to have_http_status(200)
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(branch_name)
|
||||
expect(json_response['commit']['id']).to eq(branch_sha)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(false)
|
||||
expect(json_response['developers_can_merge']).to eq(false)
|
||||
end
|
||||
|
||||
it 'protects a single branch and developers can push' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user),
|
||||
developers_can_push: true
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(branch_name)
|
||||
expect(json_response['commit']['id']).to eq(branch_sha)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(true)
|
||||
expect(json_response['developers_can_merge']).to eq(false)
|
||||
end
|
||||
|
||||
it 'protects a single branch and developers can merge' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user),
|
||||
developers_can_merge: true
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(branch_name)
|
||||
expect(json_response['commit']['id']).to eq(branch_sha)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(false)
|
||||
expect(json_response['developers_can_merge']).to eq(true)
|
||||
end
|
||||
|
||||
it 'protects a single branch and developers can push and merge' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user),
|
||||
developers_can_push: true, developers_can_merge: true
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(branch_name)
|
||||
expect(json_response['commit']['id']).to eq(branch_sha)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(true)
|
||||
expect(json_response['developers_can_merge']).to eq(true)
|
||||
end
|
||||
|
||||
it 'protects a single branch and developers cannot push and merge' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user),
|
||||
developers_can_push: 'tru', developers_can_merge: 'tr'
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(branch_name)
|
||||
expect(json_response['commit']['id']).to eq(branch_sha)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(false)
|
||||
expect(json_response['developers_can_merge']).to eq(false)
|
||||
end
|
||||
|
||||
context 'on a protected branch' do
|
||||
let(:protected_branch) { 'foo' }
|
||||
|
||||
before do
|
||||
project.repository.add_branch(user, protected_branch, 'master')
|
||||
create(:protected_branch, project: project, name: protected_branch, developers_can_push: true, developers_can_merge: true)
|
||||
end
|
||||
|
||||
it 'updates that a developer can push' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{protected_branch}/protect", user),
|
||||
developers_can_push: false, developers_can_merge: false
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(protected_branch)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(false)
|
||||
expect(json_response['developers_can_merge']).to eq(false)
|
||||
end
|
||||
|
||||
it 'does not update that a developer can push' do
|
||||
put api("/projects/#{project.id}/repository/branches/#{protected_branch}/protect", user),
|
||||
developers_can_push: 'foobar', developers_can_merge: 'foo'
|
||||
|
||||
expect(response).to have_http_status(200)
|
||||
expect(json_response['name']).to eq(protected_branch)
|
||||
expect(json_response['protected']).to eq(true)
|
||||
expect(json_response['developers_can_push']).to eq(true)
|
||||
expect(json_response['developers_can_merge']).to eq(true)
|
||||
end
|
||||
end
|
||||
|
||||
it "should return a 404 error if branch not found" do
|
||||
|
|
Loading…
Reference in New Issue