Extract any-branch-allows-collaboration logic into dedicated method
This commit is contained in:
parent
55723c223f
commit
bc7a1affe3
3 changed files with 35 additions and 25 deletions
|
@ -1928,9 +1928,19 @@ class Project < ActiveRecord::Base
|
||||||
.where('project_authorizations.project_id = merge_requests.target_project_id')
|
.where('project_authorizations.project_id = merge_requests.target_project_id')
|
||||||
.limit(1)
|
.limit(1)
|
||||||
.select(1)
|
.select(1)
|
||||||
source_of_merge_requests.opened
|
merge_requests_allowing_collaboration.where('EXISTS (?)', developer_access_exists)
|
||||||
.where(allow_collaboration: true)
|
end
|
||||||
.where('EXISTS (?)', developer_access_exists)
|
|
||||||
|
def any_branch_allows_collaboration?(user)
|
||||||
|
return false unless user
|
||||||
|
return false if empty_repo?
|
||||||
|
|
||||||
|
# Issue for N+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/49322
|
||||||
|
Gitlab::GitalyClient.allow_n_plus_1_calls do
|
||||||
|
merge_requests_allowing_collaboration.any? do |merge_request|
|
||||||
|
merge_request.can_be_merged_by?(user)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def branch_allows_collaboration?(user, branch_name)
|
def branch_allows_collaboration?(user, branch_name)
|
||||||
|
@ -2018,6 +2028,10 @@ class Project < ActiveRecord::Base
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def merge_requests_allowing_collaboration
|
||||||
|
source_of_merge_requests.opened.where(allow_collaboration: true)
|
||||||
|
end
|
||||||
|
|
||||||
def create_new_pool_repository
|
def create_new_pool_repository
|
||||||
pool = begin
|
pool = begin
|
||||||
create_pool_repository!(shard: Shard.by_name(repository_storage), source_project: self)
|
create_pool_repository!(shard: Shard.by_name(repository_storage), source_project: self)
|
||||||
|
@ -2143,24 +2157,11 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def fetch_branch_allows_collaboration?(user, branch_name)
|
def fetch_branch_allows_collaboration?(user, branch_name)
|
||||||
check_access = -> do
|
Gitlab::SafeRequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
|
||||||
next false if empty_repo?
|
next false if empty_repo?
|
||||||
|
|
||||||
merge_requests = source_of_merge_requests.opened
|
merge_request = merge_requests_allowing_collaboration.find_by(source_branch: branch_name)
|
||||||
.where(allow_collaboration: true)
|
merge_request&.can_be_merged_by?(user)
|
||||||
|
|
||||||
# Issue for N+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/49322
|
|
||||||
Gitlab::GitalyClient.allow_n_plus_1_calls do
|
|
||||||
if branch_name
|
|
||||||
merge_requests.find_by(source_branch: branch_name)&.can_be_merged_by?(user)
|
|
||||||
else
|
|
||||||
merge_requests.any? { |merge_request| merge_request.can_be_merged_by?(user) }
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
Gitlab::SafeRequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
|
|
||||||
check_access.call
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -13,7 +13,7 @@ describe Gitlab::Checks::PushCheck do
|
||||||
context 'when the user is not allowed to push to the repo' do
|
context 'when the user is not allowed to push to the repo' do
|
||||||
it 'raises an error' do
|
it 'raises an error' do
|
||||||
expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false)
|
expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false)
|
||||||
expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false)
|
expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false)
|
||||||
|
|
||||||
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.')
|
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.')
|
||||||
end
|
end
|
||||||
|
|
|
@ -3875,14 +3875,23 @@ describe Project do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#branch_allows_collaboration_push?' do
|
describe '#any_branch_allows_collaboration?' do
|
||||||
it 'allows access if the user can merge the merge request' do
|
it 'allows access when there are merge requests open allowing collaboration' do
|
||||||
expect(project.branch_allows_collaboration?(user, 'awesome-feature-1'))
|
expect(project.any_branch_allows_collaboration?(user))
|
||||||
.to be_truthy
|
.to be_truthy
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'allows access when there are merge requests open but no branch name is given' do
|
it 'does not allow access when there are no merge requests open allowing collaboration' do
|
||||||
expect(project.branch_allows_collaboration?(user, nil))
|
merge_request.close!
|
||||||
|
|
||||||
|
expect(project.any_branch_allows_collaboration?(user))
|
||||||
|
.to be_falsey
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#branch_allows_collaboration?' do
|
||||||
|
it 'allows access if the user can merge the merge request' do
|
||||||
|
expect(project.branch_allows_collaboration?(user, 'awesome-feature-1'))
|
||||||
.to be_truthy
|
.to be_truthy
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue