diff --git a/lib/feature.rb b/lib/feature.rb index 688fb1ac14a..d7a854ba567 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -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 }, diff --git a/lib/services/rich_content.rb b/lib/services/rich_content.rb index 8394354d93c..6bbec519d42 100644 --- a/lib/services/rich_content.rb +++ b/lib/services/rich_content.rb @@ -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 diff --git a/spec/controllers/eportfolios_controller_spec.rb b/spec/controllers/eportfolios_controller_spec.rb index 5b7df90e898..e31904884d4 100644 --- a/spec/controllers/eportfolios_controller_spec.rb +++ b/spec/controllers/eportfolios_controller_spec.rb @@ -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 diff --git a/spec/factories/account_factory.rb b/spec/factories/account_factory.rb index 10a8d3902f6..d0aae7dc9c1 100644 --- a/spec/factories/account_factory.rb +++ b/spec/factories/account_factory.rb @@ -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 diff --git a/spec/lib/services/rich_content_spec.rb b/spec/lib/services/rich_content_spec.rb index 55fb583ecfe..e04b8dff2ba 100644 --- a/spec/lib/services/rich_content_spec.rb +++ b/spec/lib/services/rich_content_spec.rb @@ -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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 80c15015f53..88bb6e7dc73 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -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,