Cleanup access level shortcut.

3 files changed, 18 insertions, 50 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index a72e3388af8..c462ccfe81c 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -329,34 +329,29 @@ class Project < ActiveRecord::Base
     # If we don't get a block passed, use identity to avoid if/else repetitions
     block = ->(part) { part } unless block_given?
 
-    if user
-      levels = Gitlab::VisibilityLevel.levels_for_user(user)
+    return block.call(public_to_user) unless user
 
-      if Gitlab::VisibilityLevel.all_levels?(levels)
-        # If the user is allowed to see all projects,
-        # we can shortcut and just return.
-        return block.call(all)
-      end
+    # If the user is allowed to see all projects,
+    # we can shortcut and just return.
+    return block.call(all) if user.full_private_access?
 
-      authorized = user
-        .project_authorizations
-        .select(1)
-        .where('project_authorizations.project_id = projects.id')
-      authorized_projects = block.call(where('EXISTS (?)', authorized))
+    authorized = user
+      .project_authorizations
+      .select(1)
+      .where('project_authorizations.project_id = projects.id')
+    authorized_projects = block.call(where('EXISTS (?)', authorized))
 
-      visible_projects = block.call(where('visibility_level IN (?)', levels))
+    levels = Gitlab::VisibilityLevel.levels_for_user(user)
+    visible_projects = block.call(where('visibility_level IN (?)', levels))
 
-      # We use a UNION here instead of OR clauses since this results in better
-      # performance.
-      union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')])
+    # We use a UNION here instead of OR clauses since this results in better
+    # performance.
+    union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')])
 
-      if use_conditions_only
-        where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
-      else
-        from("(#{union.to_sql}) AS #{table_name}")
-      end
+    if use_conditions_only
+      where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
     else
-      block.call(public_to_user)
+      from("(#{union.to_sql}) AS #{table_name}")
     end
   end
 
diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb
index d0ec5d57aab..2612208a927 100644
--- a/lib/gitlab/visibility_level.rb
+++ b/lib/gitlab/visibility_level.rb
@@ -20,7 +20,6 @@ module Gitlab
     PRIVATE  = 0 unless const_defined?(:PRIVATE)
     INTERNAL = 10 unless const_defined?(:INTERNAL)
     PUBLIC   = 20 unless const_defined?(:PUBLIC)
-    ALL_LEVELS = [PRIVATE, INTERNAL, PUBLIC].freeze unless const_defined?(:ALL_LEVELS)
 
     class << self
       delegate :values, to: :options
@@ -29,7 +28,7 @@ module Gitlab
         return [PUBLIC] unless user
 
         if user.full_private_access?
-          ALL_LEVELS
+          [PRIVATE, INTERNAL, PUBLIC]
         elsif user.external?
           [PUBLIC]
         else
@@ -37,10 +36,6 @@ module Gitlab
         end
       end
 
-      def all_levels?(levels = [])
-        levels&.sort == ALL_LEVELS
-      end
-
       def string_values
         string_options.keys
       end
diff --git a/spec/lib/gitlab/visibility_level_spec.rb b/spec/lib/gitlab/visibility_level_spec.rb
index 71eff6e81ef..2c1146ceff5 100644
--- a/spec/lib/gitlab/visibility_level_spec.rb
+++ b/spec/lib/gitlab/visibility_level_spec.rb
@@ -50,28 +50,6 @@ describe Gitlab::VisibilityLevel do
     end
   end
 
-  describe '.all_levels?' do
-    let(:levels) do
-      [
-        Gitlab::VisibilityLevel::PUBLIC,
-        Gitlab::VisibilityLevel::INTERNAL,
-        Gitlab::VisibilityLevel::PRIVATE
-      ].shuffle
-    end
-
-    it 'returns true only when given all levels defined at once' do
-      expect(described_class.all_levels?(levels)).to be_truthy
-    end
-
-    it 'returns true for ALL_LEVELS' do
-      expect(described_class.all_levels?(Gitlab::VisibilityLevel::ALL_LEVELS)).to be_truthy
-    end
-
-    it 'returns false if any one level is missing' do
-      expect(described_class.all_levels?(levels[0..-2])).to be_falsey
-    end
-  end
-
   describe '.allowed_levels' do
     it 'only includes the levels that arent restricted' do
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])