From cdc1179facda972671cd80229a1db43b7f20bd52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Mon, 3 Jul 2017 19:09:14 +0200
Subject: Improve feature flag check for the performance bar
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 app/controllers/concerns/with_performance_bar.rb |  2 --
 app/helpers/performance_bar_helper.rb            |  7 +++++++
 config/gitlab.yml.example                        |  2 +-
 lib/feature.rb                                   |  4 +++-
 lib/gitlab/performance_bar.rb                    |  4 ++--
 spec/lib/gitlab/performance_bar_spec.rb          | 25 +++++++++++++++++++++++-
 6 files changed, 37 insertions(+), 7 deletions(-)
 create mode 100644 app/helpers/performance_bar_helper.rb

diff --git a/app/controllers/concerns/with_performance_bar.rb b/app/controllers/concerns/with_performance_bar.rb
index d08f6e17f88..ed253042701 100644
--- a/app/controllers/concerns/with_performance_bar.rb
+++ b/app/controllers/concerns/with_performance_bar.rb
@@ -3,8 +3,6 @@ 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/performance_bar_helper.rb b/app/helpers/performance_bar_helper.rb
new file mode 100644
index 00000000000..d24efe37f5f
--- /dev/null
+++ b/app/helpers/performance_bar_helper.rb
@@ -0,0 +1,7 @@
+module PerformanceBarHelper
+  # This is a hack since using `alias_method :performance_bar_enabled?, :peek_enabled?`
+  # in WithPerformanceBar breaks tests (but works in the browser).
+  def performance_bar_enabled?
+    peek_enabled?
+  end
+end
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index e3d1141987e..d6284f26814 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -462,7 +462,7 @@ production: &base
   # Performance bar settings
   performance_bar:
     # This setting controls what group can see the performance bar.
-    # allowed_group: performance-group
+    # allowed_group: my-org/performance-group
 
   #
   # 4. Advanced settings
diff --git a/lib/feature.rb b/lib/feature.rb
index 5470c2802d5..c4e1741df52 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -60,7 +60,9 @@ class Feature
 
     def register_feature_groups
       Flipper.register(:performance_team) do |actor|
-        actor.thing&.is_a?(User) && Gitlab::PerformanceBar.allowed_user?(actor.thing)
+        user = actor.thing
+
+        user&.is_a?(User) && Gitlab::PerformanceBar.allowed_user?(user)
       end
     end
   end
diff --git a/lib/gitlab/performance_bar.rb b/lib/gitlab/performance_bar.rb
index af73cae3c9f..60c8ba5063e 100644
--- a/lib/gitlab/performance_bar.rb
+++ b/lib/gitlab/performance_bar.rb
@@ -21,10 +21,10 @@ module Gitlab
 
       if RequestStore.active?
         RequestStore.fetch('performance_bar:allowed_group') do
-          Group.by_path(Gitlab.config.performance_bar.allowed_group)
+          Group.find_by_full_path(Gitlab.config.performance_bar.allowed_group)
         end
       else
-        Group.by_path(Gitlab.config.performance_bar.allowed_group)
+        Group.find_by_full_path(Gitlab.config.performance_bar.allowed_group)
       end
     end
 
diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb
index 8667f458c98..cab267cde30 100644
--- a/spec/lib/gitlab/performance_bar_spec.rb
+++ b/spec/lib/gitlab/performance_bar_spec.rb
@@ -74,10 +74,33 @@ describe Gitlab::PerformanceBar 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
+        it 'returns the group' do
           expect(described_class.allowed_group).to eq(my_group)
         end
       end
     end
+
+    context 'when allowed group is nested', :nested_groups do
+      let!(:nested_my_group) { create(:group, parent: create(:group, path: 'my-org'), path: 'my-group') }
+
+      before do
+        create(:group, path: 'my-group')
+        stub_performance_bar_setting(allowed_group: 'my-org/my-group')
+      end
+
+      it 'returns the nested group' do
+        expect(described_class.allowed_group).to eq(nested_my_group)
+      end
+    end
+
+    context 'when a nested group has the same path', :nested_groups do
+      before do
+        create(:group, :nested, path: 'my-group')
+      end
+
+      it 'returns false' do
+        expect(described_class.allowed_group).to be_falsy
+      end
+    end
   end
 end
-- 
cgit v1.2.1