diff options
-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 |