Hide inherited enabled FFs

fixes FOO-599

test plan:
- Enable a feature flag in siteadmin
- It should not appear in regular accounts
- Enable a course and account flag in a regular account
- On refresh they should still appear
- The course flag should not appear in courses in the account

Change-Id: I6af96794b0f0f9a5534374dd32b3b539a4b7f486
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/244506
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
QA-Review: Charley Kline <ckline@instructure.com>
Product-Review: Jonathan Fenton <jfenton@instructure.com>
This commit is contained in:
Jacob Burroughs 2020-08-07 09:56:36 -04:00
parent 6b10d90197
commit f6a1b11f59
6 changed files with 36 additions and 8 deletions

View File

@ -32,7 +32,7 @@ export default class FeatureFlagCollection extends PaginatedCollection {
}
fetch(options = {}) {
options.data = {per_page: 20, ...options.data}
options.data = {hide_inherited_enabled: true, per_page: 20, ...options.data}
return super.fetch(options)
}
}

View File

@ -162,9 +162,13 @@ class FeatureFlagsController < ApplicationController
flags = features.map { |fd|
@context.lookup_feature_flag(fd.feature,
override_hidden: Account.site_admin.grants_right?(@current_user, session, :read),
skip_cache: skip_cache
skip_cache: skip_cache,
# Hide flags that are forced ON at a higher level
# Undocumented flag for frontend use only
hide_inherited_enabled: params[:hide_inherited_enabled]
)
}.compact
render json: flags.map { |flag| feature_with_flag_json(flag, @context, @current_user, session) }
end
end

View File

@ -45,7 +45,7 @@ export default class FeatureFlags extends React.Component {
retrieveFlagsConfig() {
this.setState({isLoading: true})
const url = `/api/v1/accounts/${window.ENV.ACCOUNT.id}/features`
const url = `/api/v1/accounts/${window.ENV.ACCOUNT.id}/features?hide_inherited_enabled=true`
axios.get(url).then(response => this.loadData(response.data))
}

View File

@ -109,14 +109,14 @@ module FeatureFlags
# find the feature flag setting that applies to this object
# it may be defined on the object or inherited
def lookup_feature_flag(feature, override_hidden: false, skip_cache: false)
def lookup_feature_flag(feature, override_hidden: false, skip_cache: false, hide_inherited_enabled: false)
feature = feature.to_s
feature_def = Feature.definitions[feature]
raise "no such feature - #{feature}" unless feature_def
return nil unless feature_def.applies_to_object(self)
return nil if feature_def.visible_on.is_a?(Proc) && !feature_def.visible_on.call(self)
return feature_def unless feature_def.allowed? || feature_def.hidden?
return return_flag(feature_def, hide_inherited_enabled) unless feature_def.allowed? || feature_def.hidden?
is_root_account = self.is_a?(Account) && self.root_account?
is_site_admin = self.is_a?(Account) && self.site_admin?
@ -125,7 +125,7 @@ module FeatureFlags
retval = feature_def.clone_for_cache unless feature_def.hidden? && !is_site_admin && !override_hidden
@feature_flag_cache ||= {}
return @feature_flag_cache[feature] if @feature_flag_cache.key?(feature)
return return_flag(@feature_flag_cache[feature], hide_inherited_enabled) if @feature_flag_cache.key?(feature)
# find the highest flag that doesn't allow override,
# or the most specific flag otherwise
@ -157,6 +157,20 @@ module FeatureFlags
end
@feature_flag_cache[feature] = retval
return_flag(retval, hide_inherited_enabled)
end
def return_flag(retval, hide_inherited_enabled)
return nil unless retval
unless hide_inherited_enabled && retval.enabled? && (
# Hide feature flag configs if they belong to a different context
(!retval.default? && (retval.context_type != self.class.name || retval.context_id != self.id)) ||
# Hide flags that are forced on in config as well
retval.default?
)
retval
end
end
end

View File

@ -581,7 +581,12 @@ describe "admin settings tab" do
Feature.applicable_features(Account.site_admin).each do |feature|
next if feature.visible_on && !feature.visible_on.call(Account.site_admin)
expect(f(".feature.#{feature.feature}")).to be_displayed
# We don't want flags that are enabled in code to appear in the UI
if feature.enabled?
expect(f(".feature")).to_not contain_jqcss(".#{feature.feature}")
else
expect(f(".feature.#{feature.feature}")).to be_displayed
end
end
end
end

View File

@ -560,7 +560,12 @@ describe "admin settings tab" do
Feature.applicable_features(Account.site_admin).each do |feature|
next if feature.visible_on && !feature.visible_on.call(Account.site_admin)
expect(f(".feature.#{feature.feature}")).to be_displayed
# We don't want flags that are enabled in code to appear in the UI
if feature.enabled?
expect(f(".feature")).to_not contain_jqcss(".#{feature.feature}")
else
expect(f(".feature.#{feature.feature}")).to be_displayed
end
end
end
end