diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-10-24 15:55:02 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-10-24 15:55:02 +0300 |
commit | 6e693771e81c7e1f8e80f6364e4b8d9a1e000dc1 (patch) | |
tree | 84ec93a246e8b902df81599dd26c894948089011 /spec | |
parent | f2644adf8b2352ddadc14687ee0dc172a8416d1c (diff) | |
parent | db0182e261936c3e800b546d307a3d3834ff9927 (diff) |
Merge branch '21513-fix-branch-protection-api' into 'master'
Fix branch protection API.
## What does this MR do?
- Fixes the branch protection API.
- Closes #21513
- EE Merge Request: gitlab-org/gitlab-ee!718
## Tasks
- [ ] #21513 !6215 Protected branches API bug
- [x] Investigate
- [x] Test + Fix
- [x] Changelog
- [x] MR
- [x] Wait for build to pass
- [x] Review
- [x] Check for EE conflicts
- [x] Create EE MR
- [x] Refactor + Fix
- [x] Rebase EE MR against EE master
- [x] Wait for builds to pass
- [x] Assign to dbalexandre/douwe
- [x] Implement latest review comments
- [x] Wait for Douwe's review
- [x] Implement changes
- [x] Port changes to EE MR
- [x] Assign both back to Douwe
- [ ] Wait for merge
- [ ] Merge gitlab-org/gitlab-ee!718
See merge request !6215
Diffstat (limited to 'spec')
-rw-r--r-- | spec/requests/api/branches_spec.rb | 190 |
1 files changed, 123 insertions, 67 deletions
diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 3fd989dd7a6..905f762d578 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -48,92 +48,154 @@ describe API::API, api: true do end 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) + context "when a protected branch doesn't already exist" 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) - expect(json_response['developers_can_merge']).to eq(false) - end + 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 + 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 + 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 + 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 + 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 + 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 + 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' + 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) + 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 end - context 'on a protected branch' do - let(:protected_branch) { 'foo' } - + context 'for an existing protected branch' do before do - project.repository.add_branch(user, protected_branch, 'master') - create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: protected_branch) + project.repository.add_branch(user, protected_branch.name, 'master') 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 + context "when developers can push and merge" do + let(:protected_branch) { create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: 'protected_branch') } + + it 'updates that a developer cannot push or merge' do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/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.name) + 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 "doesn't result in 0 access levels when 'developers_can_push' is switched off" do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_push: false + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(protected_branch.name) + expect(protected_branch.reload.push_access_levels.first).to be_present + expect(protected_branch.reload.push_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + end + + it "doesn't result in 0 access levels when 'developers_can_merge' is switched off" do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_merge: false + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(protected_branch.name) + expect(protected_branch.reload.merge_access_levels.first).to be_present + expect(protected_branch.reload.merge_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + end + end + + context "when developers cannot push or merge" do + let(:protected_branch) { create(:protected_branch, project: project, name: 'protected_branch') } + + it 'updates that a developer can push and merge' do + put api("/projects/#{project.id}/repository/branches/#{protected_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(protected_branch.name) + 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 + end + + context "multiple API calls" do + it "returns success when `protect` is called twice" do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch) + expect(json_response['name']).to eq(branch_name) 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' + it "returns success when `protect` is called twice with `developers_can_push` turned on" do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_push: true + 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(protected_branch) + expect(json_response['name']).to eq(branch_name) 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 "returns success when `protect` is called twice with `developers_can_merge` turned on" do + put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_merge: true + 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['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) expect(json_response['developers_can_merge']).to eq(true) end end @@ -147,12 +209,6 @@ describe API::API, api: true do put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user2) expect(response).to have_http_status(403) end - - it "returns success when protect branch again" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) - expect(response).to have_http_status(200) - end end describe "PUT /projects/:id/repository/branches/:branch/unprotect" do |