Remove feature flag for multiple content items
This reverts commit def240c3c5
.
This feature flag is no longer needed. And, there was actually a bug in
it which prevented adding multiple content items from working right: a
the js ENV key was called PROCESS_MULTIPLE_CONTENT_ITEMS but we were
checking ENV.process_multiple_content_items_modules_index
The module items were actually being added, but the dialog was not being
closed and the page refreshed as it should have been.
Note that an error (warning?) "invalid messageType:
LtiDeepLinkingResponse" appears in the console still. This appears to
not affect any functionality but I will look at fixing it soon.
Also note that sometimes when processing the deep linking response, I
will apparently get logged out of Canvas. I'm not sure what that's all
about, but this commit shouldn't make that any worse.
Closes INTEROP-6446
flag=process_multiple_content_items_modules_index
Test plan:
- Have the LTI 1.3 test tool installed with the Resource Selection or
Link Selection placement
- From the module index page, add a module item and choose "External
Tool" and choose the LTI 1.3 test tool
- Select "Return multiple Content items" and click "Submit" in the tool
- Three new module items should be created, and the "Add Item to..."
dialog should be closed, and the page refreshed to show the new items.
Change-Id: I85391199493eb0f96b7861c7e9d2318553bc5c91
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256472
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Mysti Lilla <mysti@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
This commit is contained in:
parent
67f8d5be1f
commit
ffbb9730d6
|
@ -143,10 +143,7 @@ class ContextModulesController < ApplicationController
|
|||
end
|
||||
add_body_class('padless-content')
|
||||
js_bundle :context_modules
|
||||
js_env(
|
||||
CONTEXT_MODULE_ASSIGNMENT_INFO_URL: context_url(@context, :context_context_modules_assignment_info_url),
|
||||
PROCESS_MULTIPLE_CONTENT_ITEMS: Account.site_admin.feature_enabled?(:process_multiple_content_items_modules_index)
|
||||
)
|
||||
js_env(CONTEXT_MODULE_ASSIGNMENT_INFO_URL: context_url(@context, :context_context_modules_assignment_info_url))
|
||||
css_bundle :content_next, :context_modules2
|
||||
render stream: can_stream_template?
|
||||
end
|
||||
|
|
|
@ -25,13 +25,6 @@ developer_key_support_includes:
|
|||
description: |-
|
||||
Provides a flag on the Devloper Key to support Include Parameters on API requests
|
||||
applies_to: SiteAdmin
|
||||
process_multiple_content_items_modules_index:
|
||||
state: hidden
|
||||
display_name: Process multiple content items from course modules page
|
||||
description: |-
|
||||
If enabled, Canvas will allow tools to return multiple content items
|
||||
in a single request at the course modules page
|
||||
applies_to: SiteAdmin
|
||||
site_admin_keys_only:
|
||||
state: hidden
|
||||
display_name: Only show site admin keys in the site admin UI
|
||||
|
|
|
@ -105,23 +105,13 @@ module Lti::Messages
|
|||
|
||||
private
|
||||
|
||||
def accept_multiple_overrides
|
||||
{
|
||||
'link_selection' => Account.site_admin.feature_enabled?(:process_multiple_content_items_modules_index)
|
||||
}
|
||||
end
|
||||
|
||||
def add_deep_linking_request_claims!
|
||||
@message.deep_linking_settings.deep_link_return_url = return_url
|
||||
@message.deep_linking_settings.accept_types = DEEP_LINKING_DETAILS.dig(placement, :accept_types)
|
||||
@message.deep_linking_settings.accept_presentation_document_targets = DEEP_LINKING_DETAILS.dig(placement, :document_targets)
|
||||
@message.deep_linking_settings.accept_media_types = DEEP_LINKING_DETAILS.dig(placement, :media_types).join(',')
|
||||
@message.deep_linking_settings.auto_create = DEEP_LINKING_DETAILS.dig(placement, :auto_create)
|
||||
@message.deep_linking_settings.accept_multiple = if accept_multiple_overrides.key?(placement)
|
||||
accept_multiple_overrides[placement]
|
||||
else
|
||||
DEEP_LINKING_DETAILS.dig(placement, :accept_multiple)
|
||||
end
|
||||
@message.deep_linking_settings.accept_multiple = DEEP_LINKING_DETAILS.dig(placement, :accept_multiple)
|
||||
end
|
||||
|
||||
def placement
|
||||
|
|
|
@ -59,7 +59,7 @@ SelectContentDialog.deepLinkingListener = event => {
|
|||
event.data &&
|
||||
event.data.messageType === 'LtiDeepLinkingResponse'
|
||||
) {
|
||||
if (event.data.content_items.length > 1 && ENV.process_multiple_content_items_modules_index) {
|
||||
if (event.data.content_items.length > 1) {
|
||||
processMultipleContentItems(event)
|
||||
.then(result => {
|
||||
const $dialog = $('#resource_selection_dialog')
|
||||
|
|
|
@ -102,16 +102,8 @@ describe Lti::Messages::DeepLinkingRequest do
|
|||
expect(subject['auto_create']).to eq false
|
||||
end
|
||||
|
||||
it 'sets "accept_multiple to false"' do
|
||||
expect(subject['accept_multiple']).to eq false
|
||||
end
|
||||
|
||||
context 'when "process_multiple_content_items_modules_index" is enabled' do
|
||||
before { Account.site_admin.enable_feature!(:process_multiple_content_items_modules_index) }
|
||||
|
||||
it 'sets "accept_multiple to true"' do
|
||||
expect(subject['accept_multiple']).to eq true
|
||||
end
|
||||
it 'sets "accept_multiple to true"' do
|
||||
expect(subject['accept_multiple']).to eq true
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue