Improve feature flag check for the performance bar
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
parent
19b8d8af2c
commit
cdc1179fac
|
|
@ -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?
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue