changes default_branch_protection to allow devs_can_merge protection option aswell

This commit is contained in:
tiagonbotelho 2016-08-01 16:48:15 +01:00
parent edc5f4018e
commit 482d7802cc
4 changed files with 42 additions and 61 deletions

View file

@ -876,14 +876,8 @@ class Project < ActiveRecord::Base
ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present? ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present?
end end
def developers_can_push_to_protected_branch?(branch_name) def user_can_push_to_empty_repo?(user)
return true if empty_repo? && !default_branch_protected? !default_branch_protected? || team.max_member_access(user.id) > Gitlab::Access::DEVELOPER
protected_branches.matching(branch_name).any?(&:developers_can_push)
end
def developers_can_merge_to_protected_branch?(branch_name)
protected_branches.matching(branch_name).any?(&:developers_can_merge)
end end
def forked? def forked?
@ -1278,7 +1272,8 @@ class Project < ActiveRecord::Base
private private
def default_branch_protected? def default_branch_protected?
current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL ||
current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE
end end
def authorized_for_user_by_group?(user, min_access_level) def authorized_for_user_by_group?(user, min_access_level)

View file

@ -30,6 +30,8 @@ module Gitlab
return false unless user return false unless user
if project.protected_branch?(ref) if project.protected_branch?(ref)
return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user)
access_levels = project.protected_branches.matching(ref).map(&:push_access_level) access_levels = project.protected_branches.matching(ref).map(&:push_access_level)
access_levels.any? { |access_level| access_level.check_access(user) } access_levels.any? { |access_level| access_level.check_access(user) }
else else

View file

@ -9,16 +9,19 @@ describe Gitlab::UserAccess, lib: true do
describe 'push to none protected branch' do describe 'push to none protected branch' do
it 'returns true if user is a master' do it 'returns true if user is a master' do
project.team << [user, :master] project.team << [user, :master]
expect(access.can_push_to_branch?('random_branch')).to be_truthy expect(access.can_push_to_branch?('random_branch')).to be_truthy
end end
it 'returns true if user is a developer' do it 'returns true if user is a developer' do
project.team << [user, :developer] project.team << [user, :developer]
expect(access.can_push_to_branch?('random_branch')).to be_truthy expect(access.can_push_to_branch?('random_branch')).to be_truthy
end end
it 'returns false if user is a reporter' do it 'returns false if user is a reporter' do
project.team << [user, :reporter] project.team << [user, :reporter]
expect(access.can_push_to_branch?('random_branch')).to be_falsey expect(access.can_push_to_branch?('random_branch')).to be_falsey
end end
end end
@ -67,16 +70,19 @@ describe Gitlab::UserAccess, lib: true do
it 'returns true if user is a master' do it 'returns true if user is a master' do
project.team << [user, :master] project.team << [user, :master]
expect(access.can_push_to_branch?(branch.name)).to be_truthy expect(access.can_push_to_branch?(branch.name)).to be_truthy
end end
it 'returns false if user is a developer' do it 'returns false if user is a developer' do
project.team << [user, :developer] project.team << [user, :developer]
expect(access.can_push_to_branch?(branch.name)).to be_falsey expect(access.can_push_to_branch?(branch.name)).to be_falsey
end end
it 'returns false if user is a reporter' do it 'returns false if user is a reporter' do
project.team << [user, :reporter] project.team << [user, :reporter]
expect(access.can_push_to_branch?(branch.name)).to be_falsey expect(access.can_push_to_branch?(branch.name)).to be_falsey
end end
end end
@ -88,16 +94,19 @@ describe Gitlab::UserAccess, lib: true do
it 'returns true if user is a master' do it 'returns true if user is a master' do
project.team << [user, :master] project.team << [user, :master]
expect(access.can_push_to_branch?(@branch.name)).to be_truthy expect(access.can_push_to_branch?(@branch.name)).to be_truthy
end end
it 'returns true if user is a developer' do it 'returns true if user is a developer' do
project.team << [user, :developer] project.team << [user, :developer]
expect(access.can_push_to_branch?(@branch.name)).to be_truthy expect(access.can_push_to_branch?(@branch.name)).to be_truthy
end end
it 'returns false if user is a reporter' do it 'returns false if user is a reporter' do
project.team << [user, :reporter] project.team << [user, :reporter]
expect(access.can_push_to_branch?(@branch.name)).to be_falsey expect(access.can_push_to_branch?(@branch.name)).to be_falsey
end end
end end
@ -109,19 +118,21 @@ describe Gitlab::UserAccess, lib: true do
it 'returns true if user is a master' do it 'returns true if user is a master' do
project.team << [user, :master] project.team << [user, :master]
expect(access.can_merge_to_branch?(@branch.name)).to be_truthy expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
end end
it 'returns true if user is a developer' do it 'returns true if user is a developer' do
project.team << [user, :developer] project.team << [user, :developer]
expect(access.can_merge_to_branch?(@branch.name)).to be_truthy expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
end end
it 'returns false if user is a reporter' do it 'returns false if user is a reporter' do
project.team << [user, :reporter] project.team << [user, :reporter]
expect(access.can_merge_to_branch?(@branch.name)).to be_falsey expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
end end
end end
end end
end end

View file

@ -1126,69 +1126,42 @@ describe Project, models: true do
end end
end end
describe "#developers_can_push_to_protected_branch?" do describe '#user_can_push_to_empty_repo?' do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
let(:user) { create(:user) }
context "when the branch matches a protected branch via direct match" do it 'returns false when default_branch_protection is in full protection and user is developer' do
it "returns true if 'Developers can Push' is turned on" do project.team << [user, :developer]
create(:protected_branch, name: "production", project: project, developers_can_push: true) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL)
expect(project.developers_can_push_to_protected_branch?('production')).to be true expect(project.user_can_push_to_empty_repo?(user)).to be_falsey
end
it "returns false if 'Developers can Push' is turned off" do
create(:protected_branch, name: "production", project: project, developers_can_push: false)
expect(project.developers_can_push_to_protected_branch?('production')).to be false
end
end end
context "when project is new" do it 'returns false when default_branch_protection only lets devs merge and user is dev' do
it "returns true if project is unprotected" do project.team << [user, :developer]
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(project.developers_can_push_to_protected_branch?('master')).to be true expect(project.user_can_push_to_empty_repo?(user)).to be_falsey
end
it "returns true if project allows developers to push to protected branch" do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
expect(project.developers_can_push_to_protected_branch?('master')).to be true
end
it "returns false if project does not let developer push to protected branch but let them merge branches" do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(project.developers_can_push_to_protected_branch?('master')).to be false
end
it "returns false if project is on full protection mode" do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL)
expect(project.developers_can_push_to_protected_branch?('master')).to be false
end
end end
context "when the branch matches a protected branch via wilcard match" do it 'returns true when default_branch_protection lets devs push and user is developer' do
it "returns true if 'Developers can Push' is turned on" do project.team << [user, :developer]
create(:protected_branch, name: "production/*", project: project, developers_can_push: true) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true expect(project.user_can_push_to_empty_repo?(user)).to be_truthy
end
it "returns false if 'Developers can Push' is turned off" do
create(:protected_branch, name: "production/*", project: project, developers_can_push: false)
expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false
end
end end
context "when the branch does not match a protected branch" do it 'returns true when default_branch_protection is unprotected and user is developer' do
it "returns false" do project.team << [user, :developer]
create(:protected_branch, name: "production/*", project: project, developers_can_push: true) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false expect(project.user_can_push_to_empty_repo?(user)).to be_truthy
end end
it 'returns true when user is master' do
project.team << [user, :master]
expect(project.user_can_push_to_empty_repo?(user)).to be_truthy
end end
end end