From 54d26c89f66abb2bfec7403fd6b3ed7700e73766 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 12 Jul 2016 16:31:55 +0200 Subject: API: Expose 'developers_can_push' for branches --- CHANGELOG | 1 + doc/api/branches.md | 14 ++++++--- lib/api/branches.rb | 12 ++++++-- lib/api/entities.rb | 10 ++++-- lib/api/helpers.rb | 7 +++++ spec/requests/api/api_helpers_spec.rb | 23 ++++++++++++++ spec/requests/api/branches_spec.rb | 57 +++++++++++++++++++++++++++++++++-- 7 files changed, 114 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 56b7d2a485f..e30b61c40a4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -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' 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 diff --git a/doc/api/branches.md b/doc/api/branches.md index abc4732c395..e5a1e12ffb9 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -23,6 +23,7 @@ Example response: { "name": "master", "protected": true, + "developers_can_push": false, "commit": { "author_email": "john@example.com", "author_name": "John Smith", @@ -64,6 +65,7 @@ Example response: { "name": "master", "protected": true, + "developers_can_push": false, "commit": { "author_email": "john@example.com", "author_name": "John Smith", @@ -91,13 +93,14 @@ 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 ``` | 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 | Example response: @@ -117,7 +120,8 @@ Example response: ] }, "name": "master", - "protected": true + "protected": true, + "developers_can_push": true } ``` @@ -158,7 +162,8 @@ Example response: ] }, "name": "master", - "protected": false + "protected": false, + "developers_can_push": false } ``` @@ -196,7 +201,8 @@ Example response: ] }, "name": "newbranch", - "protected": false + "protected": false, + "developers_can_push": false } ``` diff --git a/lib/api/branches.rb b/lib/api/branches.rb index d467eb9d474..cd33091d9f4 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -36,6 +36,7 @@ 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 # Example Request: # PUT /projects/:id/repository/branches/:branch/protect put ':id/repository/branches/:branch/protect', @@ -43,9 +44,16 @@ 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]) + + if protected_branch + protected_branch.update(developers_can_push: developers_can_push) unless developers_can_push.nil? + else + user_project.protected_branches.create(name: @branch.name, + developers_can_push: developers_can_push || false) + end present @branch, with: Entities::RepoObject, project: user_project end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3c79a00eb8c..e4ae5adafd6 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -125,9 +125,15 @@ 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 end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 73557cf7db6..d6e4eb2afd7 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -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) diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index 3d5c19aeff3..831889afb6c 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -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 diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index b11ca26ee68..c843e8cf7be 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -32,6 +32,7 @@ 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) end it "should return a 403 error if guest" do @@ -45,14 +46,66 @@ 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(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) + 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) + end + + it 'protects a single branch and developers cannot push' do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), + developers_can_push: 'tru' + + 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) + 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) + end + + it 'updates that a developer can push' do + put api("/projects/#{project.id}/repository/branches/#{protected_branch}/protect", user), + developers_can_push: 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) + 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' + + 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) + end end it "should return a 404 error if branch not found" do -- cgit v1.2.1 From e552b4af26b68a8b4bedc775a128a8ecd59ff689 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 19 Jul 2016 10:36:18 +0200 Subject: API: Expose 'developers_can_merge' for branches --- CHANGELOG | 2 +- doc/api/branches.md | 14 +++++++++---- lib/api/branches.rb | 6 +++++- lib/api/entities.rb | 6 ++++++ spec/requests/api/branches_spec.rb | 40 +++++++++++++++++++++++++++++++++----- 5 files changed, 57 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e30b61c40a4..2892631560b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -57,7 +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' for branches !5208 (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 diff --git a/doc/api/branches.md b/doc/api/branches.md index e5a1e12ffb9..dbe8306c66f 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -24,6 +24,7 @@ Example response: "name": "master", "protected": true, "developers_can_push": false, + "developers_can_merge": false, "commit": { "author_email": "john@example.com", "author_name": "John Smith", @@ -66,6 +67,7 @@ Example response: "name": "master", "protected": true, "developers_can_push": false, + "developers_can_merge": false, "commit": { "author_email": "john@example.com", "author_name": "John Smith", @@ -93,7 +95,7 @@ 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?developers_can_push=true +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 | @@ -101,6 +103,7 @@ curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/ | `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: @@ -121,7 +124,8 @@ Example response: }, "name": "master", "protected": true, - "developers_can_push": true + "developers_can_push": true, + "developers_can_merge": true } ``` @@ -163,7 +167,8 @@ Example response: }, "name": "master", "protected": false, - "developers_can_push": false + "developers_can_push": false, + "developers_can_merge": false } ``` @@ -202,7 +207,8 @@ Example response: }, "name": "newbranch", "protected": false, - "developers_can_push": false + "developers_can_push": false, + "developers_can_merge": false } ``` diff --git a/lib/api/branches.rb b/lib/api/branches.rb index cd33091d9f4..b77eebc729a 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -37,6 +37,7 @@ module API # 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', @@ -47,12 +48,15 @@ module API not_found!('Branch') unless @branch protected_branch = user_project.protected_branches.find_by(name: @branch.name) developers_can_push = to_boolean(params[:developers_can_push]) + developers_can_merge = to_boolean(params[:developers_can_merge]) if protected_branch protected_branch.update(developers_can_push: developers_can_push) unless developers_can_push.nil? + protected_branch.update(developers_can_merge: developers_can_merge) unless developers_can_merge.nil? else user_project.protected_branches.create(name: @branch.name, - developers_can_push: developers_can_push || false) + developers_can_push: developers_can_push || false, + developers_can_merge: developers_can_merge || false) end present @branch, with: Entities::RepoObject, project: user_project diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e4ae5adafd6..d6fed1a1eed 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -136,6 +136,12 @@ module API 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 class RepoTreeObject < Grape::Entity diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index c843e8cf7be..719da27f919 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -33,6 +33,7 @@ describe API::API, api: true do 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 @@ -55,6 +56,7 @@ describe API::API, api: true do 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 @@ -66,17 +68,43 @@ describe API::API, api: true do 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 cannot push' do + it 'protects a single branch and developers can merge' do put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_push: 'tru' + 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 @@ -84,27 +112,29 @@ describe API::API, api: true do before do project.repository.add_branch(user, protected_branch, 'master') - create(:protected_branch, project: project, name: protected_branch, developers_can_push: true) + 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_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_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 -- cgit v1.2.1 From 3e281f95907686ba4a923b8825dc32afb22df038 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 19 Jul 2016 11:33:47 +0200 Subject: Only update once --- lib/api/branches.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/api/branches.rb b/lib/api/branches.rb index b77eebc729a..35efe4f2e4a 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -51,8 +51,9 @@ module API developers_can_merge = to_boolean(params[:developers_can_merge]) if protected_branch - protected_branch.update(developers_can_push: developers_can_push) unless developers_can_push.nil? - protected_branch.update(developers_can_merge: developers_can_merge) unless developers_can_merge.nil? + 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, -- cgit v1.2.1