Improve EE compatibility with protected branch access levels.

1. Change a few incorrect `access_level` to `access_levels.first` that were missed in e805a647. 2. `API::Entities` can iterate over all access levels instead of just the first one. This makes no difference to CE, and makes it more compatible with EE.

Improve EE compatibility with protected branch access levels.
4ddbbcd1 · Timothy Andrew · e9f48335 · 4ddbbcd1 · 4ddbbcd1 · 4ddbbcd1
Commit 4ddbbcd1 authored Aug 16, 2016 by Timothy Andrew
Hide whitespace changes
Inline Side-by-side

Showing with 9 additions and 7 deletions

entities.rb lib/api/entities.rb +4 -2

user_access.rb lib/gitlab/user_access.rb +2 -2

git_push_service_spec.rb spec/services/git_push_service_spec.rb +3 -3

No files found.
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -129,12 +129,14 @@ module API

      expose :developers_can_push do |repo_branch, options|
        project = options[:project]
-        project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.push_access_levels.first.access_level == Gitlab::Access::DEVELOPER }
+        access_levels = project.protected_branches.matching(repo_branch.name).map(&:push_access_levels).flatten
+        access_levels.any? { |access_level| access_level.access_level == Gitlab::Access::DEVELOPER }
      end

      expose :developers_can_merge do |repo_branch, options|
        project = options[:project]
-        project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.merge_access_levels.first.access_level == Gitlab::Access::DEVELOPER }
+        access_levels = project.protected_branches.matching(repo_branch.name).map(&:merge_access_levels).flatten
+        access_levels.any? { |access_level| access_level.access_level == Gitlab::Access::DEVELOPER }
      end
    end


--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -32,7 +32,7 @@ module Gitlab
      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_levels).flatten
        access_levels.any? { |access_level| access_level.check_access(user) }
      else
        user.can?(:push_code, project)
@@ -43,7 +43,7 @@ module Gitlab
      return false unless user

      if project.protected_branch?(ref)
-        access_levels = project.protected_branches.matching(ref).map(&:merge_access_level)
+        access_levels = project.protected_branches.matching(ref).map(&:merge_access_levels).flatten
        access_levels.any? { |access_level| access_level.check_access(user) }
      else
        user.can?(:push_code, project)

--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -227,7 +227,7 @@ describe GitPushService, services: true do
        expect(project.default_branch).to eq("master")
        execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
        expect(project.protected_branches).not_to be_empty
-        expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER)
+        expect(project.protected_branches.first.push_access_levels.first.access_level).to eq(Gitlab::Access::MASTER)
        expect(project.protected_branches.first.merge_access_levels.first.access_level).to eq(Gitlab::Access::MASTER)
      end

@@ -249,7 +249,7 @@ describe GitPushService, services: true do
        execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )

        expect(project.protected_branches).not_to be_empty
-        expect(project.protected_branches.last.push_access_level.access_level).to eq(Gitlab::Access::DEVELOPER)
+        expect(project.protected_branches.last.push_access_levels.first.access_level).to eq(Gitlab::Access::DEVELOPER)
        expect(project.protected_branches.last.merge_access_levels.first.access_level).to eq(Gitlab::Access::MASTER)
      end

@@ -260,7 +260,7 @@ describe GitPushService, services: true do
        expect(project.default_branch).to eq("master")
        execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
        expect(project.protected_branches).not_to be_empty
-        expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER)
+        expect(project.protected_branches.first.push_access_levels.first.access_level).to eq(Gitlab::Access::MASTER)
        expect(project.protected_branches.first.merge_access_levels.first.access_level).to eq(Gitlab::Access::DEVELOPER)
      end