From 57ac4cbac137041f8e960dc5639981bc989842e8 Mon Sep 17 00:00:00 2001 From: Evan Battaglia Date: Tue, 7 May 2024 19:00:31 +0000 Subject: [PATCH] Remove completely turned on FF flag=allow_lti_tools_editor_button_placement_without_icon closes INTEROP-8484 Test plan: - Create LTI tool with editor_button placement, with no icon_url at the top level or in the editor button placement. - in the RCE, open the tools and make sure the generated icon shows up for the tool. [fsc-max-nodes=18] [fsc-timeout=40] Change-Id: Ib5b62e2e6a93b20fb26ed3eb08dc377b871ec0fd Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/347025 Tested-by: Service Cloud Jenkins Reviewed-by: Tucker Mcknight QA-Review: Tucker Mcknight Product-Review: Evan Battaglia --- app/models/context_external_tool.rb | 4 ---- config/feature_flags/interop_release_flags.yml | 18 ------------------ spec/models/context_external_tool_spec.rb | 14 -------------- 3 files changed, 36 deletions(-) diff --git a/app/models/context_external_tool.rb b/app/models/context_external_tool.rb index e270fcca749..5836155e48f 100644 --- a/app/models/context_external_tool.rb +++ b/app/models/context_external_tool.rb @@ -836,10 +836,6 @@ class ContextExternalTool < ActiveRecord::Base settings.delete(type) unless extension_setting(type, :url) end - unless root_account.feature_enabled?(:allow_lti_tools_editor_button_placement_without_icon) - settings.delete(:editor_button) unless editor_button(:icon_url) || editor_button(:canvas_icon_class) - end - sync_placements!(Lti::ResourcePlacement::PLACEMENTS.select { |type| settings[type] }.map(&:to_s)) true end diff --git a/config/feature_flags/interop_release_flags.yml b/config/feature_flags/interop_release_flags.yml index 5838946de1d..6db52e8c55e 100644 --- a/config/feature_flags/interop_release_flags.yml +++ b/config/feature_flags/interop_release_flags.yml @@ -20,24 +20,6 @@ ags_scores_multiple_files: environments: development: state: on -allow_lti_tools_editor_button_placement_without_icon: - state: hidden - applies_to: RootAccount - display_name: Allow LTI tools to have an editor button icon without placement (use default tool icon) - description: |- - When disabled, new or updated ContextExternalTool installations (including - LTI 1.3 tools updated automatically due to updates to their developer keys) - will have their editor_button placement removed if there is no icon_url or - canvas_icon_class provided in the settings (legacy behavior). When enabled, - the placement is not removed, so tools without icons will show a default - icon (with a color / letter based on tool name and tool/developer key ID). - environments: - development: - state: allowed_on - test: - state: allowed_on - ci: - state: allowed_on api_auth_error_updates: state: hidden display_name: Updates to JSON API auth error HTTP code and JSON status field diff --git a/spec/models/context_external_tool_spec.rb b/spec/models/context_external_tool_spec.rb index 6a17bbc2c1c..b761d967dba 100644 --- a/spec/models/context_external_tool_spec.rb +++ b/spec/models/context_external_tool_spec.rb @@ -2572,20 +2572,6 @@ describe ContextExternalTool do expect(tool.editor_button).not_to be_nil end - context "when allow_lti_tools_editor_button_placement_without_icon FF is disabled" do - let(:ff) { :allow_lti_tools_editor_button_placement_without_icon } - - before { @root_account.disable_feature! ff } - after { @root_account.enable_feature! ff } - - it "deletes the editor_button if icon_url is not present" do - tool = new_external_tool - tool.settings = { editor_button: { url: "http://www.example.com" } } - tool.save - expect(tool.editor_button).to be_nil - end - end - it "sets user_navigation if navigation configured" do tool = new_external_tool tool.settings = { user_navigation: { url: "http://www.example.com" } }