diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-08-16 13:34:56 +0530 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-08-16 13:42:34 +0530 |
commit | dd3b738d5b3eb70217d7ac7f9fe441498d2e8e7e (patch) | |
tree | 830a6a3693ef760b295f66c6f1ae4660d40df066 | |
parent | 37651d2f4ce12c16945a5b67360c67768cddb465 (diff) | |
download | gitlab-ce-dd3b738d5b3eb70217d7ac7f9fe441498d2e8e7e.tar.gz |
Fix failing tests relating to backporting ee!581.
1. `GitPushService` was still using `{merge,push}_access_level_attributes` instead
of `{merge,push}_access_levels_attributes`.
2. The branches API creates access levels regardless of the state of the
`developers_can_{push,merge}` parameters. This is in line with the UI,
where Master access is the default for a new protected branch.
3. Use `after(:build)` to create access levels in the
`protected_branches` factory, so that `factories_spec` passes. It
only builds records, so we need to create access levels on `build` as
well.
-rw-r--r-- | app/services/git_push_service.rb | 8 | ||||
-rw-r--r-- | lib/api/branches.rb | 29 | ||||
-rw-r--r-- | spec/factories/protected_branches.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 6 |
4 files changed, 25 insertions, 20 deletions
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 6f521462cf3..d5fb2018d24 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -91,12 +91,12 @@ class GitPushService < BaseService params = { name: @project.default_branch, - push_access_level_attributes: { + push_access_levels_attributes: [{ access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }, - merge_access_level_attributes: { + }], + merge_access_levels_attributes: [{ access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - } + }] } ProtectedBranches::CreateService.new(@project, current_user, params).execute diff --git a/lib/api/branches.rb b/lib/api/branches.rb index a77afe634f6..b615703df93 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -61,22 +61,27 @@ module API name: @branch.name } - unless developers_can_merge.nil? - protected_branch_params.merge!({ - merge_access_level_attributes: { - access_level: developers_can_merge ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - } - }) + # If `developers_can_merge` is switched off, _all_ `DEVELOPER` + # merge_access_levels need to be deleted. + if developers_can_merge == false + protected_branch.merge_access_levels.where(access_level: Gitlab::Access::DEVELOPER).destroy_all end - unless developers_can_push.nil? - protected_branch_params.merge!({ - push_access_level_attributes: { - access_level: developers_can_push ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - } - }) + # If `developers_can_push` is switched off, _all_ `DEVELOPER` + # push_access_levels need to be deleted. + if developers_can_push == false + protected_branch.push_access_levels.where(access_level: Gitlab::Access::DEVELOPER).destroy_all end + protected_branch_params.merge!( + merge_access_levels_attributes: [{ + access_level: developers_can_merge ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }], + push_access_levels_attributes: [{ + access_level: developers_can_push ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }] + ) + if protected_branch service = ProtectedBranches::UpdateService.new(user_project, current_user, protected_branch_params) service.execute(protected_branch) diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb index 42853cac112..b2695e0482a 100644 --- a/spec/factories/protected_branches.rb +++ b/spec/factories/protected_branches.rb @@ -3,7 +3,7 @@ FactoryGirl.define do name project - before(:create) do |protected_branch| + after(:build) do |protected_branch| protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER) protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER) end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9c3b4712cab..0a32a486703 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1089,13 +1089,13 @@ describe Project, models: true do let(:project) { create(:project) } it 'returns true when the branch matches a protected branch via direct match' do - project.protected_branches.create!(name: 'foo') + create(:protected_branch, project: project, name: "foo") expect(project.protected_branch?('foo')).to eq(true) end it 'returns true when the branch matches a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + create(:protected_branch, project: project, name: "production/*") expect(project.protected_branch?('production/some-branch')).to eq(true) end @@ -1105,7 +1105,7 @@ describe Project, models: true do end it 'returns false when the branch does not match a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + create(:protected_branch, project: project, name: "production/*") expect(project.protected_branch?('staging/some-branch')).to eq(false) end |