diff --git a/app/controllers/concerns/with_performance_bar.rb b/app/controllers/concerns/with_performance_bar.rb index ed253042701..d08f6e17f88 100644 --- a/app/controllers/concerns/with_performance_bar.rb +++ b/app/controllers/concerns/with_performance_bar.rb @@ -3,6 +3,8 @@ module WithPerformanceBar included do include Peek::Rblineprof::CustomControllerHelpers + alias_method :performance_bar_enabled?, :peek_enabled? + helper_method :performance_bar_enabled? end def peek_enabled? diff --git a/app/helpers/nav_helper.rb b/app/helpers/nav_helper.rb index e589ed4e56d..4bb04867e0f 100644 --- a/app/helpers/nav_helper.rb +++ b/app/helpers/nav_helper.rb @@ -23,7 +23,7 @@ module NavHelper def nav_header_class class_name = '' class_name << " with-horizontal-nav" if defined?(nav) && nav - class_name << " with-peek" if peek_enabled? + class_name << " with-peek" if performance_bar_enabled? class_name end diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index abb6dc2e9f3..3d3e29733ff 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -30,7 +30,7 @@ = stylesheet_link_tag "application", media: "all" = stylesheet_link_tag "print", media: "print" = stylesheet_link_tag "test", media: "all" if Rails.env.test? - = stylesheet_link_tag 'peek' if peek_enabled? + = stylesheet_link_tag 'peek' if performance_bar_enabled? - if show_new_nav? = stylesheet_link_tag "new_nav", media: "all" @@ -44,7 +44,7 @@ = webpack_bundle_tag "main" = webpack_bundle_tag "raven" if current_application_settings.clientside_sentry_enabled = webpack_bundle_tag "test" if Rails.env.test? - = webpack_bundle_tag 'peek' if peek_enabled? + = webpack_bundle_tag 'peek' if performance_bar_enabled? - if content_for?(:page_specific_javascripts) = yield :page_specific_javascripts diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index 8ec9613a4b7..bfab8c77a4b 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -3,4 +3,6 @@ require 'flipper/middleware/memoizer' unless Rails.env.test? Rails.application.config.middleware.use Flipper::Middleware::Memoizer, lambda { Feature.flipper } + + Feature.register_feature_groups end diff --git a/lib/feature.rb b/lib/feature.rb index 853a00c75a4..5470c2802d5 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -54,15 +54,13 @@ class Feature adapter = Flipper::Adapters::ActiveRecord.new( feature_class: FlipperFeature, gate_class: FlipperGate) - Flipper.new(adapter).tap do - register_feature_groups - end + Flipper.new(adapter) end end def register_feature_groups Flipper.register(:performance_team) do |actor| - Gitlab::PerformanceBar.allowed_actor?(actor) + actor.thing&.is_a?(User) && Gitlab::PerformanceBar.allowed_user?(actor.thing) end end end diff --git a/lib/gitlab/performance_bar.rb b/lib/gitlab/performance_bar.rb index 53812633888..af73cae3c9f 100644 --- a/lib/gitlab/performance_bar.rb +++ b/lib/gitlab/performance_bar.rb @@ -4,15 +4,15 @@ module Gitlab Feature.enabled?(:gitlab_performance_bar, current_user) end - def self.allowed_actor?(actor) - return false unless actor.thing&.is_a?(User) && allowed_group + def self.allowed_user?(user) + return false unless allowed_group if RequestStore.active? RequestStore.fetch('performance_bar:user_member_of_allowed_group') do - user_member_of_allowed_group?(actor.thing) + user_member_of_allowed_group?(user) end else - user_member_of_allowed_group?(actor.thing) + user_member_of_allowed_group?(user) end end @@ -29,10 +29,7 @@ module Gitlab end def self.user_member_of_allowed_group?(user) - GroupMembersFinder.new(allowed_group) - .execute - .where(user_id: user.id) - .any? + GroupMembersFinder.new(allowed_group).execute.exists?(user_id: user.id) end end end diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb index 56cd8813f0d..8667f458c98 100644 --- a/spec/lib/gitlab/performance_bar_spec.rb +++ b/spec/lib/gitlab/performance_bar_spec.rb @@ -25,43 +25,35 @@ describe Gitlab::PerformanceBar do end end - describe '.allowed_actor?' do - it 'returns false when given actor is not a User' do - actor = double('actor', thing: double) + describe '.allowed_user?' do + let(:user) { create(:user) } - expect(described_class.allowed_actor?(actor)).to be_falsy + before do + stub_performance_bar_setting(allowed_group: 'my-group') end - context 'when given actor is a User' do - let(:actor) { double('actor', thing: create(:user)) } - - before do - stub_performance_bar_setting(allowed_group: 'my-group') + context 'when allowed group does not exist' do + it 'returns false' do + expect(described_class.allowed_user?(user)).to be_falsy end + end - context 'when allowed group does not exist' do + context 'when allowed group exists' do + let!(:my_group) { create(:group, path: 'my-group') } + + context 'when user is not a member of the allowed group' do it 'returns false' do - expect(described_class.allowed_actor?(actor)).to be_falsy + expect(described_class.allowed_user?(user)).to be_falsy end end - context 'when allowed group exists' do - let!(:my_group) { create(:group, path: 'my-group') } - - context 'when user is not a member of the allowed group' do - it 'returns false' do - expect(described_class.allowed_actor?(actor)).to be_falsy - end + context 'when user is a member of the allowed group' do + before do + my_group.add_developer(user) end - context 'when user is a member of the allowed group' do - before do - my_group.add_developer(actor.thing) - end - - it 'returns true' do - expect(described_class.allowed_actor?(actor)).to be_truthy - end + it 'returns true' do + expect(described_class.allowed_user?(user)).to be_truthy end end end