ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

change rcs feature flag text, remove first 2 flags 

the first two rcs flags have been enabled for all hosted accounts
for a long time. we are changing the lable of the 3rd flag and removing the
first two to provide better clarity as we enable the 3rd flag for
accounts.

closes CNVS-37261

test plan:
- go to the feature flags page for an account
  - there should only be one rcs flag with an updated message
- ensure that flag is disabled
- editing wiki pages should still have the new sidebar
- high risk area (i.e. quizzes, assignments) should not have the new
  sidebar
- enable the flag
- all editors should be using rcs with the new sidebar

Change-Id: I689eb9ebaf4d3a5fc111c231d13861cc28ae5d87
Reviewed-on: https://gerrit.instructure.com/116542
Tested-by: Jenkins
Reviewed-by: brian kirkby <bkirkby@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Brent Burgoyne <bburgoyne@instructure.com>
This commit is contained in:
Brent Burgoyne 2017-06-22 09:11:46 -06:00
parent 2cfaf5fb22
commit 179bf7c5fc
6 changed files with 44 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/feature.rb
View File

@ -432,33 +432,13 @@ END
root_opt_in: true,
touch_context: true
},
'rich_content_service' =>
{
display_name: -> { I18n.t('Use remote version of Rich Content Editor') },
description: -> { I18n.t('In cases where it is available, load the RCE from a canvas rich content service') },
applies_to: 'RootAccount',
state: 'allowed',
beta: true,
development: false,
root_opt_in: false
},
'rich_content_service_with_sidebar' =>
{
display_name: -> { I18n.t('Use remote version of Rich Content Editor AND sidebar') },
description: -> { I18n.t('In cases where it is available, load the RCE and the wiki sidebar from a canvas rich content service') },
applies_to: 'RootAccount',
state: 'hidden',
beta: true,
development: false,
root_opt_in: false
},
'rich_content_service_high_risk' =>
{
display_name: -> { I18n.t('Use remote version of Rich Content Editor AND sidebar in high-risk areas like quizzes') },
description: -> { I18n.t('Always load the RCE and Sidebar from a canvas rich content service everywhere') },
display_name: -> { I18n.t('Rich Content Editor Sidebar Enhancements') },
description: -> { I18n.t('Use new rich content editor with enhanced sidebar everywhere') },
applies_to: 'RootAccount',
state: 'hidden',
beta: true,
beta: false,
development: false,
root_opt_in: false
},

6
lib/services/rich_content.rb
View File

@ -67,11 +67,7 @@ module Services
end
def contextually_on(root_account, risk_level)
check_feature_flag(root_account, :rich_content_service) && (
risk_level == :basic ||
(risk_level == :sidebar && check_feature_flag(root_account, :rich_content_service_with_sidebar)) ||
check_feature_flag(root_account, :rich_content_service_high_risk)
)
risk_level == :basic || risk_level == :sidebar || check_feature_flag(root_account, :rich_content_service_high_risk)
end
end
end

4
spec/controllers/eportfolios_controller_spec.rb
View File

@ -77,7 +77,7 @@ describe EportfoliosController do
end
it "exposes the feature state for rich content service to js_env" do
@user.account.root_account.enable_feature!(:rich_content_service)
@user.account.root_account.enable_feature!(:rich_content_service_high_risk)
Canvas::DynamicSettings.stubs(:find).with("rich-content-service", use_env: false).returns({
'app-host' => 'rce.docker',
'cdn-host' => 'rce.docker'
@ -143,7 +143,7 @@ describe EportfoliosController do
end
it "exposes the feature state for rich content service to js_env" do
@user.account.root_account.disable_feature!(:rich_content_service)
@user.account.root_account.disable_feature!(:rich_content_service_high_risk)
get 'user_index'
expect(assigns[:js_env][:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
end

4
spec/factories/account_factory.rb
View File

@ -44,14 +44,10 @@ module Factories
def account_rcs_model(opts={})
@account = factory_with_protected_attributes(Account, valid_account_attributes.merge(opts))
enable_all_rcs(@account)
LoadAccount.default_domain_root_account.enable_feature!(:rich_content_service)
LoadAccount.default_domain_root_account.enable_feature!(:rich_content_service_with_sidebar)
LoadAccount.default_domain_root_account.enable_feature!(:rich_content_service_high_risk)
end
def enable_all_rcs(account)
account.enable_feature!(:rich_content_service)
account.enable_feature!(:rich_content_service_with_sidebar)
account.enable_feature!(:rich_content_service_high_risk)
end

76
spec/lib/services/rich_content_spec.rb
View File

@ -21,6 +21,7 @@ require_dependency "services/rich_content"
module Services
describe RichContent do
before do
allow(Services::RichContent).to receive(:contextually_on).and_call_original
allow(Canvas::DynamicSettings).to receive(:find)
.with('rich-content-service', use_env: false)
.and_return({
@ -113,42 +114,10 @@ module Services
expect(env2[:RICH_CONTENT_CAN_UPLOAD_FILES]).to eq(false)
end
context "with only lowest level flag on" do
let(:root_account){ stub("root_account") }
before(:each) do
root_account.stubs(:feature_enabled?).with(:rich_content_service).returns(true)
root_account.stubs(:feature_enabled?).with(:rich_content_service_with_sidebar).returns(false)
root_account.stubs(:feature_enabled?).with(:rich_content_service_high_risk).returns(false)
end
it "assumes high risk without being specified" do
env = described_class.env_for(root_account)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
end
it "is contextually on for low risk areas" do
env = described_class.env_for(root_account, risk_level: :basic)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
end
it "is contextually off for medium risk areas" do
env = described_class.env_for(root_account, risk_level: :sidebar)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
end
it "is contextually off for high risk areas" do
env = described_class.env_for(root_account, risk_level: :highrisk)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
end
end
context "with all flags on" do
let(:root_account){ stub("root_account") }
before(:each) do
root_account.stubs(:feature_enabled?).with(:rich_content_service).returns(true)
root_account.stubs(:feature_enabled?).with(:rich_content_service_with_sidebar).returns(true)
root_account.stubs(:feature_enabled?).with(:rich_content_service_high_risk).returns(true)
end
@ -168,7 +137,7 @@ module Services
end
end
context "with all flags off" do
context "with flag off" do
let(:root_account){ stub("root_account") }
before(:each) do
@ -180,24 +149,20 @@ module Services
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
end
it "is contextually off even for low risk areas" do
it "is contextually on for low risk areas" do
env = described_class.env_for(root_account, risk_level: :basic)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
end
end
it "can be totally disabled with the lowest flag" do
root_account = stub("root_account")
root_account.stubs(:feature_enabled?).with(:rich_content_service).returns(false)
root_account.stubs(:feature_enabled?).with(:rich_content_service_with_sidebar).returns(true)
root_account.stubs(:feature_enabled?).with(:rich_content_service_high_risk).returns(true)
env = described_class.env_for(root_account)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
it "is contextually on for lower risk areas with sidebar" do
env = described_class.env_for(root_account, risk_level: :sidebar)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
end
end
it "treats nil feature values as false" do
root_account = stub("root_account")
root_account.stubs(:feature_enabled?).with(:rich_content_service).returns(nil)
root_account.stubs(:feature_enabled?).with(:rich_content_service_high_risk).returns(nil)
env = described_class.env_for(root_account)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to eq(false)
end
@ -205,12 +170,31 @@ module Services
context "integrating with a real account and feature flags" do
it "sets all levels to true when all flags set" do
account = account_model
account.enable_feature!(:rich_content_service)
account.enable_feature!(:rich_content_service_with_sidebar)
account.enable_feature!(:rich_content_service_high_risk)
env = described_class.env_for(account)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
end
it "on for basic if flag is disabled" do
account = account_model
account.disable_feature!(:rich_content_service_high_risk)
env = described_class.env_for(account, risk_level: :basic)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
end
it "on for sidebar if flag is disabled" do
account = account_model
account.disable_feature!(:rich_content_service_high_risk)
env = described_class.env_for(account, risk_level: :sidebar)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
end
it "off for high risk if flag is disabled" do
account = account_model
account.disable_feature!(:rich_content_service_high_risk)
env = described_class.env_for(account)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_falsey
end
end
end
end

8
spec/spec_helper.rb
View File

@ -473,6 +473,14 @@ RSpec.configure do |config|
Canvas.redis_used = false
end
# This should be removed once all tests using the legacy rce have been removed
# Only use RCS in tests if high risk flag is enabled
config.before :each do
allow(Services::RichContent).to receive(:contextually_on) do |root_account|
root_account.feature_enabled?(:rich_content_service_high_risk)
end
end
#****************************************************************
# There used to be a lot of factory methods here!
# In an effort to move us toward a nicer test factory solution,