add missing LTI params to ContentItemSelectionResponse message

Test Plan:
 - Do a ContentItemSelectionResponse
 - Should receive the param 'roles'

fixes PLAT-621

Change-Id: I1dc8ed1af5c237d1333eea8bad18f0dd7a74c464
Reviewed-on: https://gerrit.instructure.com/40617
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Product-Review: Bracken Mosbacker <bracken@instructure.com>
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
This commit is contained in:
Brad Humphrey 2014-09-04 16:27:26 -06:00
parent d11ac9f82e
commit 90d795e33b
7 changed files with 219 additions and 146 deletions

View File

@ -436,12 +436,6 @@ class ExternalToolsController < ApplicationController
protected :basic_lti_launch_request
def content_item_selection_response(tool, placement)
lti_launch = Lti::Launch.new
lti_launch.resource_url = tool.extension_setting(placement, :url)
lti_launch.link_text = tool.label_for(placement.to_sym)
lti_launch.analytics_id = tool.tool_id
#contstruct query params for the export endpoint
query_params = {"export_type" => "common_cartridge"}
media_types = []
@ -500,44 +494,22 @@ class ExternalToolsController < ApplicationController
]
}
params = {
#message params
params = default_lti_params.merge({
#required params
lti_message_type: 'ContentItemSelectionResponse',
lti_version: 'LTI-1p0',
content_items: content_json.to_json,
#common params
context_id: Lti::Asset.opaque_identifier_for(@context),
resource_link_id: Lti::Asset.opaque_identifier_for(@context),
tool_consumer_instance_guid: @domain_root_account.lti_guid,
content_items: content_json.to_json,
launch_presentation_return_url: @return_url,
tool_consumer_instance_name: @domain_root_account.name,
context_title: @context.name,
launch_presentation_return_url: @return_url,
}
}).merge(tool.substituted_custom_fields(placement, common_variable_substitutions))
params.merge!({ user_id: Lti::Asset.opaque_identifier_for(@current_user) }) if @current_user
#adds custom fields to the launch params
custom_fields = {}
tool.set_custom_fields(custom_fields, placement)
#replaces custom fields with their values
substitutions = common_variable_substitutions()
custom_fields.each do |k,v|
if substitutions.has_key?(v)
if substitutions[v].respond_to?(:call)
custom_fields[k] = substitutions[v].call
else
custom_fields[k] = substitutions[v]
end
end
end
#add custom params to launch params
params.merge!(custom_fields)
#sign the launch params
lti_launch = Lti::Launch.new
lti_launch.resource_url = tool.extension_setting(placement, :url)
lti_launch.params = LtiOutbound::ToolLaunch.generate_params(params, lti_launch.resource_url, tool.consumer_key, tool.shared_secret)
lti_launch.link_text = tool.label_for(placement.to_sym)
lti_launch.analytics_id = tool.tool_id
lti_launch
end

View File

@ -557,6 +557,21 @@ class ContextExternalTool < ActiveRecord::Base
end
end
def substituted_custom_fields(placement, substitutions)
custom_fields = {}
set_custom_fields(custom_fields, placement)
custom_fields.each do |k,v|
if substitutions.has_key?(v)
if substitutions[v].respond_to?(:call)
custom_fields[k] = substitutions[v].call
else
custom_fields[k] = substitutions[v]
end
end
end
end
def resource_selection_settings
settings[:resource_selection]
end

View File

@ -76,5 +76,22 @@ module Lti
substitutions
end
def default_lti_params
lti_helper = Lti::SubstitutionsHelper.new(@context, @domain_root_account, @current_user)
params = {
context_id: Lti::Asset.opaque_identifier_for(@context),
tool_consumer_instance_guid: @domain_root_account.lti_guid,
roles: lti_helper.current_lis_roles,
launch_presentation_locale: I18n.locale || I18n.default_locale.to_s,
launch_presentation_document_target: 'iframe',
# launch_presentation_width:,
# launch_presentation_height:,
# launch_presentation_return_url: return_url,
}
params.merge(user_id: Lti::Asset.opaque_identifier_for(@current_user)) if @current_user
params
end
end
end

View File

@ -73,10 +73,13 @@ describe ExternalToolsController do
lti_launch.params['lti_version'].should == 'LTI-1p0'
lti_launch.params['context_id'].should == 'ABC123'
lti_launch.params['resource_link_id'].should == 'ABC123'
lti_launch.params['context_title'].should == 'a course'
lti_launch.params['roles'].should == 'Instructor'
lti_launch.params['tool_consumer_instance_guid'].should == 'root-account-guid'
lti_launch.params['tool_consumer_instance_name'].should == 'root account'
lti_launch.params['context_title'].should == 'a course'
lti_launch.params['launch_presentation_return_url'].should start_with 'http'
lti_launch.params['launch_presentation_locale'].should == 'en'
lti_launch.params['launch_presentation_document_target'].should == 'iframe'
end
it "sends content item json for a course" do

View File

@ -31,148 +31,179 @@ module Lti
let(:account) { Account.new(root_account: root_account) }
let(:course) { Course.new(account: account) }
let(:user) { User.new }
let(:substitution_helper) { mock(account: account) }
let(:substitution_helper) { mock }
before(:each) {
subject.domain_root_account = root_account
subject.context = account
}
it 'has substitution for $Canvas.api.domain' do
subject.stubs(:request).returns(mock(host: '/my/url'))
subject.common_variable_substitutions['$Canvas.api.domain'].call.should == 'localhost'
end
it 'has substitution for $Canvas.xapi.url' do
subject.stubs(:lti_xapi_url).returns('/xapi')
subject.common_variable_substitutions['$Canvas.xapi.url'].call.should == '/xapi'
end
it 'has substitution for $Canvas.account.id' do
account.stubs(:id).returns(12345)
subject.common_variable_substitutions['$Canvas.account.id'].should == 12345
end
it 'has substitution for $Canvas.account.name' do
account.name = 'Some Account'
subject.common_variable_substitutions['$Canvas.account.name'].should == 'Some Account'
end
it 'has substitution for $Canvas.account.sisSourceId' do
account.sis_source_id = 'ab23'
subject.common_variable_substitutions['$Canvas.account.sisSourceId'].should == 'ab23'
end
it 'has substitution for $Canvas.rootAccount.id' do
root_account.stubs(:id).returns(54321)
subject.common_variable_substitutions['$Canvas.rootAccount.id'].should == 54321
end
it 'has substitution for $Canvas.rootAccount.sisSourceId' do
root_account.sis_source_id = 'cd45'
subject.common_variable_substitutions['$Canvas.rootAccount.sisSourceId'].should == 'cd45'
end
it 'has substitution for $Canvas.root_account.id' do
root_account.stubs(:id).returns(54321)
subject.common_variable_substitutions['$Canvas.root_account.id'].should == 54321
end
it 'has substitution for $Canvas.root_account.sisSourceId' do
root_account.sis_source_id = 'cd45'
subject.common_variable_substitutions['$Canvas.root_account.sisSourceId'].should == 'cd45'
end
context 'context is a course' do
before(:each) {
subject.domain_root_account = root_account
subject.context = course
}
it 'has substitution for $Canvas.course.id' do
course.stubs(:id).returns(123)
subject.common_variable_substitutions['$Canvas.course.id'].should == 123
describe "#common_variable_substitutions" do
before(:each) do
substitution_helper.stubs(:account).returns(account)
end
it 'has substitution for $Canvas.course.sisSourceId' do
course.sis_source_id = 'course1'
subject.common_variable_substitutions['$Canvas.course.sisSourceId'].should == 'course1'
it 'has substitution for $Canvas.api.domain' do
subject.stubs(:request).returns(mock(host: '/my/url'))
subject.common_variable_substitutions['$Canvas.api.domain'].call.should == 'localhost'
end
it 'has substitution for $Canvas.enrollment.enrollmentState' do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:enrollment_state).returns('active')
subject.common_variable_substitutions['$Canvas.enrollment.enrollmentState'].call.should == 'active'
it 'has substitution for $Canvas.xapi.url' do
subject.stubs(:lti_xapi_url).returns('/xapi')
subject.common_variable_substitutions['$Canvas.xapi.url'].call.should == '/xapi'
end
it 'has substitution for $Canvas.membership.roles' do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:current_canvas_roles).returns('teacher,student')
subject.common_variable_substitutions['$Canvas.membership.roles'].call.should == 'teacher,student'
it 'has substitution for $Canvas.account.id' do
account.stubs(:id).returns(12345)
subject.common_variable_substitutions['$Canvas.account.id'].should == 12345
end
it 'has substitution for $Canvas.membership.concludedRoles' do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:concluded_lis_roles).returns('learner')
subject.common_variable_substitutions['$Canvas.membership.concludedRoles'].call.should == 'learner'
end
end
context 'user is logged in' do
before :each do
subject.current_user = user
it 'has substitution for $Canvas.account.name' do
account.name = 'Some Account'
subject.common_variable_substitutions['$Canvas.account.name'].should == 'Some Account'
end
it 'has substitution for $Person.name.full' do
user.name = 'Uncle Jake'
subject.common_variable_substitutions['$Person.name.full'].should == 'Uncle Jake'
it 'has substitution for $Canvas.account.sisSourceId' do
account.sis_source_id = 'ab23'
subject.common_variable_substitutions['$Canvas.account.sisSourceId'].should == 'ab23'
end
it 'has substitution for $Person.name.family' do
user.name = 'Uncle Jake'
subject.common_variable_substitutions['$Person.name.family'].should == 'Jake'
it 'has substitution for $Canvas.rootAccount.id' do
root_account.stubs(:id).returns(54321)
subject.common_variable_substitutions['$Canvas.rootAccount.id'].should == 54321
end
it 'has substitution for $Person.name.given' do
user.name = 'Uncle Jake'
subject.common_variable_substitutions['$Person.name.given'].should == 'Uncle'
it 'has substitution for $Canvas.rootAccount.sisSourceId' do
root_account.sis_source_id = 'cd45'
subject.common_variable_substitutions['$Canvas.rootAccount.sisSourceId'].should == 'cd45'
end
it 'has substitution for $Person.email.primary' do
user.email = 'someone@somewhere'
subject.common_variable_substitutions['$Person.email.primary'].should == 'someone@somewhere'
it 'has substitution for $Canvas.root_account.id' do
root_account.stubs(:id).returns(54321)
subject.common_variable_substitutions['$Canvas.root_account.id'].should == 54321
end
it 'has substitution for $Person.address.timezone' do
subject.common_variable_substitutions['$Person.address.timezone'].should == 'Etc/UTC'
it 'has substitution for $Canvas.root_account.sisSourceId' do
root_account.sis_source_id = 'cd45'
subject.common_variable_substitutions['$Canvas.root_account.sisSourceId'].should == 'cd45'
end
it 'has substitution for $User.image' do
user.stubs(:avatar_url).returns('/my/pic')
subject.common_variable_substitutions['$User.image'].call.should == '/my/pic'
context 'context is a course' do
before(:each) {
subject.domain_root_account = root_account
subject.context = course
}
it 'has substitution for $Canvas.course.id' do
course.stubs(:id).returns(123)
subject.common_variable_substitutions['$Canvas.course.id'].should == 123
end
it 'has substitution for $Canvas.course.sisSourceId' do
course.sis_source_id = 'course1'
subject.common_variable_substitutions['$Canvas.course.sisSourceId'].should == 'course1'
end
it 'has substitution for $Canvas.enrollment.enrollmentState' do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:enrollment_state).returns('active')
subject.common_variable_substitutions['$Canvas.enrollment.enrollmentState'].call.should == 'active'
end
it 'has substitution for $Canvas.membership.roles' do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:current_canvas_roles).returns('teacher,student')
subject.common_variable_substitutions['$Canvas.membership.roles'].call.should == 'teacher,student'
end
it 'has substitution for $Canvas.membership.concludedRoles' do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:concluded_lis_roles).returns('learner')
subject.common_variable_substitutions['$Canvas.membership.concludedRoles'].call.should == 'learner'
end
end
it 'has substitution for $Canvas.user.id' do
user.stubs(:id).returns(456)
subject.common_variable_substitutions['$Canvas.user.id'].should == 456
end
context 'pseudonym' do
let(:pseudonym) { Pseudonym.new }
context 'user is logged in' do
before :each do
user.stubs(:find_pseudonym_for_account).returns(pseudonym)
subject.current_user = user
end
it 'has substitution for $Canvas.user.sisSourceId' do
pseudonym.sis_user_id = '1a2b3c'
subject.common_variable_substitutions['$Canvas.user.sisSourceId'].should == '1a2b3c'
it 'has substitution for $Person.name.full' do
user.name = 'Uncle Jake'
subject.common_variable_substitutions['$Person.name.full'].should == 'Uncle Jake'
end
it 'has substitution for $Canvas.user.loginId' do
pseudonym.unique_id = 'username'
subject.common_variable_substitutions['$Canvas.user.loginId'].should == 'username'
it 'has substitution for $Person.name.family' do
user.name = 'Uncle Jake'
subject.common_variable_substitutions['$Person.name.family'].should == 'Jake'
end
it 'has substitution for $Person.name.given' do
user.name = 'Uncle Jake'
subject.common_variable_substitutions['$Person.name.given'].should == 'Uncle'
end
it 'has substitution for $Person.email.primary' do
user.email = 'someone@somewhere'
subject.common_variable_substitutions['$Person.email.primary'].should == 'someone@somewhere'
end
it 'has substitution for $Person.address.timezone' do
subject.common_variable_substitutions['$Person.address.timezone'].should == 'Etc/UTC'
end
it 'has substitution for $User.image' do
user.stubs(:avatar_url).returns('/my/pic')
subject.common_variable_substitutions['$User.image'].call.should == '/my/pic'
end
it 'has substitution for $Canvas.user.id' do
user.stubs(:id).returns(456)
subject.common_variable_substitutions['$Canvas.user.id'].should == 456
end
context 'pseudonym' do
let(:pseudonym) { Pseudonym.new }
before :each do
user.stubs(:find_pseudonym_for_account).returns(pseudonym)
end
it 'has substitution for $Canvas.user.sisSourceId' do
pseudonym.sis_user_id = '1a2b3c'
subject.common_variable_substitutions['$Canvas.user.sisSourceId'].should == '1a2b3c'
end
it 'has substitution for $Canvas.user.loginId' do
pseudonym.unique_id = 'username'
subject.common_variable_substitutions['$Canvas.user.loginId'].should == 'username'
end
end
end
end
describe "#default_lti_params" do
it "generates context_id" do
subject.default_lti_params[:context_id].should == Lti::Asset.opaque_identifier_for(account)
end
it "generates tool_consumer_instance_guid" do
root_account.lti_guid = 'guid'
subject.default_lti_params[:tool_consumer_instance_guid].should == 'guid'
end
it "generates roles" do
Lti::SubstitutionsHelper.stubs(:new).returns(substitution_helper)
substitution_helper.stubs(:current_lis_roles).returns('Learner')
subject.default_lti_params[:roles].should == 'Learner'
end
it "generates launch_presentation_locale" do
subject.default_lti_params[:launch_presentation_locale].should == :en
end
it "generates launch_presentation_document_target" do
subject.default_lti_params[:launch_presentation_document_target].should == 'iframe'
end
end
end

View File

@ -123,6 +123,11 @@ module Lti
subject.course_enrollments.should include teacher_enrollment
subject.course_enrollments.should_not include inactive_enrollment
end
it 'returns an empty array if there is no user' do
helper = SubstitutionsHelper.new(account, root_account, nil)
helper.course_enrollments.should == []
end
end
describe '#account_enrollments' do
@ -140,6 +145,11 @@ module Lti
subject.account_enrollments.should == [enrollment]
end
it 'returns an empty array if there is no user' do
helper = SubstitutionsHelper.new(account, root_account, nil)
helper.account_enrollments.should == []
end
end
describe '#current_lis_roles' do

View File

@ -347,6 +347,31 @@ describe ContextExternalTool do
end
end
end
describe "#substituted_custom_fields" do
it "substitutes custom fields for a placement" do
subject.user_navigation = {custom_fields: {'custom_a' => '$Custom.substitution'}}
custom_params = subject.substituted_custom_fields(:user_navigation, {'$Custom.substitution' => 'substituted_value'})
custom_params.should == {'custom_a' => 'substituted_value'}
end
it "executes procs" do
subject.course_navigation = {custom_fields: {'custom_b' => '$Custom.substitution'}}
custom_params = subject.substituted_custom_fields(:course_navigation, {'$Custom.substitution' => -> {'substituted_value'}})
custom_params.should == {'custom_b' => 'substituted_value'}
end
it "ignores constants" do
subject.account_navigation = {custom_fields: {'custom_c' => 'constant'}}
custom_params = subject.substituted_custom_fields(:account_navigation, {'$Custom.substitution' => 'substituted_value'})
custom_params.should == {'custom_c' => 'constant'}
end
end
describe "all_tools_for" do
it "should retrieve all tools in alphabetical order" do