Lti::LogService accepts a launch_url parameter

Lti::LogService.new needs to accept a launch_url named parameter, that
defaults to nil, and all applicable callers of LogService.new should
pass the actual launch url.

closes INTEROP-8794
flag=none

test plan:
- if you don't have config/vault_content.yml then run
  `cp config/vault_content.yml.example config/vault_content.yml` and
  then uncomment the `pandata_creds` section
- in `config/dynamic_settings.yml` add
  ```
    pandata/events:
    enabled_for_canvas: true
    url: 'http://your.simple.web.server/here'
  ```
  to `development: config: canvas:`
- run a small server that can accept and log POST requests using that
  url
- launch a tool from several placements in Canvas and verify that the
  proper launch_url is included in the POST request body
  - from global navigation
  - from course navigation
  - from assignment

Change-Id: I90f48f9a5e0571da8c87817629a57876179c54e6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/356184
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
This commit is contained in:
Steve McGee 2024-08-28 16:32:19 -06:00 committed by SaltNPepa
parent bc11b3a9a6
commit 600e0ab666
8 changed files with 44 additions and 12 deletions

View File

@ -2257,7 +2257,7 @@ class ApplicationController < ActionController::Base
@lti_launch.link_text = @resource_title
@lti_launch.analytics_id = @tool.tool_id
Lti::LogService.new(tool: @tool, context:, user: @current_user, session_id: session[:session_id], placement: nil, launch_type: :content_item).call
Lti::LogService.new(tool: @tool, context:, user: @current_user, session_id: session[:session_id], placement: nil, launch_type: :content_item, launch_url: @resource_url).call
@append_template = "context_modules/tool_sequence_footer" if render_external_tool_append_template?
render Lti::AppUtil.display_template(external_tool_redirect_display_type)

View File

@ -183,7 +183,7 @@ class ExternalToolsController < ApplicationController
end
launch_type = placement.present? ? :indirect_link : :content_item
Lti::LogService.new(tool:, context: @context, user: @current_user, session_id: session[:session_id], placement:, launch_type:).call
Lti::LogService.new(tool:, context: @context, user: @current_user, session_id: session[:session_id], placement:, launch_type:, launch_url: url).call
display_override = params["borderless"] ? "borderless" : params[:display]
render Lti::AppUtil.display_template(@tool.display_type(placement), display_override:)
@ -352,7 +352,7 @@ class ExternalToolsController < ApplicationController
if tool
placement = launch_settings.dig("metadata", "placement")
launch_type = launch_settings.dig("metadata", "launch_type")&.to_sym
Lti::LogService.new(tool:, context: @context, user: @current_user, session_id: session[:session_id], placement:, launch_type:).call
Lti::LogService.new(tool:, context: @context, user: @current_user, session_id: session[:session_id], placement:, launch_type:, launch_url: launch_settings["launch_url"]).call
log_asset_access(tool, "external_tools", "external_tools", overwrite: false)
end
@ -495,7 +495,7 @@ class ExternalToolsController < ApplicationController
js_env(LTI_LAUNCH_RESOURCE_URL: @lti_launch.resource_url)
set_tutorial_js_env
Lti::LogService.new(tool: @tool, context: @context, user: @current_user, session_id: session[:session_id], placement:, launch_type: :direct_link).call
Lti::LogService.new(tool: @tool, context: @context, user: @current_user, session_id: session[:session_id], placement:, launch_type: :direct_link, launch_url: @tool.url_with_environment_overrides(launch_url || @tool.url)).call
render Lti::AppUtil.display_template(@tool.display_type(placement), display_override: params[:display])
timing_meta.tags = { lti_version: @tool&.lti_version }.compact

View File

@ -1551,7 +1551,7 @@ class UsersController < ApplicationController
@lti_launch.resource_url = @tool.login_or_launch_url(extension_type: placement)
@lti_launch.link_text = @tool.label_for(placement, I18n.locale)
@lti_launch.analytics_id = @tool.tool_id
Lti::LogService.new(tool: @tool, context: @domain_root_account, user: @current_user, session_id: session[:session_id], placement:, launch_type: :direct_link).call
Lti::LogService.new(tool: @tool, context: @domain_root_account, user: @current_user, session_id: session[:session_id], placement:, launch_type: :direct_link, launch_url: @lti_launch.resource_url).call
set_active_tab @tool.asset_string
add_crumb(@current_user.short_name, user_profile_path(@current_user))

View File

@ -26,7 +26,7 @@ module Lti
resource_selection
].freeze
def initialize(tool:, context:, user:, session_id:, launch_type:, placement: nil)
def initialize(tool:, context:, user:, session_id:, launch_type:, launch_url: nil, placement: nil)
raise ArgumentError, "context must be a Course, Account, or Group" unless [Course, Account, Group].include? context.class
raise ArgumentError, "launch_type must be one of #{LAUNCH_TYPES.join(", ")}" unless LAUNCH_TYPES.include?(launch_type.to_sym)
@ -35,6 +35,7 @@ module Lti
@context = context
@user = user
@launch_type = launch_type
@launch_url = launch_url
@placement = placement
@session_id = session_id
end
@ -59,6 +60,7 @@ module Lti
account_id: account_for_context.id.to_s,
root_account_uuid: @context.root_account.uuid,
launch_type: @launch_type,
launch_url: @launch_url,
message_type:,
placement: @placement,
context_id: @context.id.to_s,

View File

@ -1542,7 +1542,8 @@ RSpec.describe ApplicationController do
user:,
session_id: nil,
placement: nil,
launch_type: :content_item
launch_type: :content_item,
launch_url: "http://www.example.com/basic_lti"
)
end

View File

@ -239,6 +239,29 @@ describe ExternalToolsController do
end
end
end
context "logging" do
before do
allow(Lti::LogService).to receive(:new) do
double("Lti::LogService").tap { |s| allow(s).to receive(:call) }
end
user_session(@teacher)
end
it "logs launch with placement and indirect_link launch_type" do
expect(Lti::LogService).to receive(:new).with(
tool:,
context: @course,
user: @teacher,
session_id: nil,
placement: "course_navigation",
launch_type: :direct_link,
launch_url: "http://www.example.com/basic_lti"
)
subject
end
end
end
context "with a bad launch url" do
@ -1256,7 +1279,8 @@ describe ExternalToolsController do
user: @teacher,
session_id: nil,
placement:,
launch_type: :indirect_link
launch_type: :indirect_link,
launch_url: "http://www.example.com/basic_lti?first=john&last=smith"
)
get "retrieve", params: { course_id: @course.id, url: tool.url, placement: }
@ -1271,7 +1295,8 @@ describe ExternalToolsController do
user: @teacher,
session_id: nil,
placement: nil,
launch_type: :content_item
launch_type: :content_item,
launch_url: "http://www.example.com/basic_lti?first=john&last=smith"
)
get "retrieve", params: { course_id: @course.id, url: tool.url }
@ -3244,7 +3269,8 @@ describe ExternalToolsController do
user: @user,
session_id: nil,
placement: "course_navigation",
launch_type: :direct_link
launch_type: :direct_link,
launch_url: "http://www.example.com/basic_lti"
)
end
end

View File

@ -127,7 +127,8 @@ describe UsersController do
user:,
session_id: nil,
placement: :user_navigation,
launch_type: :direct_link
launch_type: :direct_link,
launch_url: tool.url
)
end

View File

@ -19,7 +19,7 @@
describe Lti::LogService do
let(:service) do
Lti::LogService.new(tool:, context:, user:, session_id:, placement:, launch_type:)
Lti::LogService.new(tool:, context:, user:, session_id:, placement:, launch_type:, launch_url:)
end
let_once(:session_id) { SecureRandom.hex }
@ -29,6 +29,7 @@ describe Lti::LogService do
let_once(:context) { course_model(root_account: account) }
let_once(:placement) { :course_navigation }
let_once(:launch_type) { :direct_link }
let_once(:launch_url) { "https://example.com/basic_lti_tool/" }
describe ".new" do
context "when context is not valid type" do
@ -124,6 +125,7 @@ describe Lti::LogService do
account_id: account.id.to_s,
root_account_uuid: account.uuid,
launch_type:,
launch_url:,
message_type: service.message_type,
placement:,
context_id: context.id.to_s,