diff options
| -rw-r--r-- | app/models/project.rb | 39 | ||||
| -rw-r--r-- | spec/lib/gitlab/checks/push_check_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 19 |
3 files changed, 35 insertions, 25 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index cd558752080..930f6c9911f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1928,9 +1928,19 @@ class Project < ActiveRecord::Base .where('project_authorizations.project_id = merge_requests.target_project_id') .limit(1) .select(1) - source_of_merge_requests.opened - .where(allow_collaboration: true) - .where('EXISTS (?)', developer_access_exists) + merge_requests_allowing_collaboration.where('EXISTS (?)', developer_access_exists) + end + + 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 def branch_allows_collaboration?(user, branch_name) @@ -2018,6 +2028,10 @@ class Project < ActiveRecord::Base private + def merge_requests_allowing_collaboration + source_of_merge_requests.opened.where(allow_collaboration: true) + end + def create_new_pool_repository pool = begin create_pool_repository!(shard: Shard.by_name(repository_storage), source_project: self) @@ -2143,24 +2157,11 @@ class Project < ActiveRecord::Base end 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? - merge_requests = source_of_merge_requests.opened - .where(allow_collaboration: true) - - # 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 + merge_request = merge_requests_allowing_collaboration.find_by(source_branch: branch_name) + merge_request&.can_be_merged_by?(user) end end diff --git a/spec/lib/gitlab/checks/push_check_spec.rb b/spec/lib/gitlab/checks/push_check_spec.rb index 25f0d428cb9..e1bd52d6c0b 100644 --- a/spec/lib/gitlab/checks/push_check_spec.rb +++ b/spec/lib/gitlab/checks/push_check_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Checks::PushCheck do context 'when the user is not allowed to push to the repo' 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_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.') end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4b6592020c1..131de62fe03 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3875,14 +3875,23 @@ describe Project do end end - describe '#branch_allows_collaboration_push?' do - it 'allows access if the user can merge the merge request' do - expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) + describe '#any_branch_allows_collaboration?' do + it 'allows access when there are merge requests open allowing collaboration' do + expect(project.any_branch_allows_collaboration?(user)) .to be_truthy end - it 'allows access when there are merge requests open but no branch name is given' do - expect(project.branch_allows_collaboration?(user, nil)) + it 'does not allow access when there are no merge requests open allowing collaboration' do + 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 end |