summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2017-07-03 19:09:14 +0200
committerRémy Coutable <remy@rymai.me>2017-07-06 11:18:26 +0200
commitcdc1179facda972671cd80229a1db43b7f20bd52 (patch)
tree27b9dcb44a7c900b10d40698daefb6c5fba9c0a5
parent19b8d8af2c74e0ff241356d5ccb5890baa6fb7c8 (diff)
downloadgitlab-ce-cdc1179facda972671cd80229a1db43b7f20bd52.tar.gz
Improve feature flag check for the performance bar
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--app/controllers/concerns/with_performance_bar.rb2
-rw-r--r--app/helpers/performance_bar_helper.rb7
-rw-r--r--config/gitlab.yml.example2
-rw-r--r--lib/feature.rb4
-rw-r--r--lib/gitlab/performance_bar.rb4
-rw-r--r--spec/lib/gitlab/performance_bar_spec.rb25
6 files changed, 37 insertions, 7 deletions
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