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.
This commit is contained in:
parent
37651d2f4c
commit
dd3b738d5b
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue