ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Prefer a Resource link's own url in export 

why

Previous to g/293521 the lti_resource_links table did not have a url
column. That commit added it, and modified resource links added via RCE
to rely on the url in the resource link. The resource link exporter
wasn't updated to copy this url, instead, using the associated tool's
url. When courses were copied, they use the export/import logic, and so
resource links were being copied without the correct url.

test plan:

Create a course with a single assignment, which is RCE text content.
Inside that text, add a Deep link url chosen from the lti13testtool,
when launching, you'll see that the
`https://purl.imsglobal.org/spec/lti/claim/target_link_uri` claim has a
url with a `deep_link_location` parameter.

Copy the course, and find the same assignmnent you created, and click
the link. The launch should contain the same `deep_link_location`
parameter.

fixes INTEROP-7562

flag=none

Change-Id: I62deea4f9fdc232d9e6069e7f02cd399a3bcd923
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/296776
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Migration-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
This commit is contained in:
Paul Gray 2022-07-21 15:00:58 -04:00
parent f37c2d14a2
commit 9b0e44cbcb
4 changed files with 106 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
db/migrate/20220721000001_fix_resource_links_from_copied_courses.rb Normal file
View File

@ -0,0 +1,27 @@
# frozen_string_literal: true
#
# Copyright (C) 2018 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class FixResourceLinksFromCopiedCourses < ActiveRecord::Migration[6.0]
tag :postdeploy
def up
DataFixup::FixResourceLinksFromCopiedCourses.delay_if_production(priority: Delayed::LOW_PRIORITY).run
end
end

9
lib/cc/lti_resource_links.rb
View File

@ -99,12 +99,15 @@ module CC
cartridge_basiclti_link.blti :title, tool.name
cartridge_basiclti_link.blti :description, tool.description
# Prefer the url on the resource_link
url = resource_link.url || tool.url
# URL element (choose secure or not based on protocol)
case tool.url
case url
when %r{^http://}
cartridge_basiclti_link.blti :launch_url, tool.url
cartridge_basiclti_link.blti :launch_url, url
when %r{^https://}
cartridge_basiclti_link.blti :secure_launch_url, tool.url
cartridge_basiclti_link.blti :secure_launch_url, url
end
# Custom parameters from the resource link

57
lib/data_fixup/fix_resource_links_from_copied_courses.rb Normal file
View File

@ -0,0 +1,57 @@
# frozen_string_literal: true
#
# Copyright (C) 2012 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
module DataFixup::FixResourceLinksFromCopiedCourses
def self.resource_links_to_update
# this is the day before the date of the original commit that added urls to resource links
ActiveRecord::Base.connection.execute(
"WITH original_resource_links AS (
SELECT lti_resource_links.id, lti_resource_links.root_account_id, lti_resource_links.lookup_uuid, lti_resource_links.url,
ROW_NUMBER() OVER(PARTITION BY lti_resource_links.lookup_uuid
ORDER BY lti_resource_links.created_at) AS rk
FROM #{Lti::ResourceLink.quoted_table_name}
)
SELECT copied_resource_links.id as copy_id, original_resource_links.id AS orig_id, original_resource_links.url
FROM #{Lti::ResourceLink.quoted_table_name} AS copied_resource_links
INNER JOIN original_resource_links ON copied_resource_links.lookup_uuid = original_resource_links.lookup_uuid
AND copied_resource_links.root_account_id = original_resource_links.root_account_id
AND copied_resource_links.id <> original_resource_links.id
AND original_resource_links.url IS NOT NULL
AND original_resource_links.url <> copied_resource_links.url
AND original_resource_links.rk = 1
WHERE copied_resource_links.created_at > '2022-06-06'
AND copied_resource_links.context_type = 'Course'"
)
end
def self.run
resource_links_to_update.group_by { |r| r["orig_id"] }.each do |orig_id, resource_links|
copied_ids = ActiveRecord::Base.sanitize_sql(resource_links.map { |rl| rl["copy_id"] }.join(","))
next if resource_links.empty?
ActiveRecord::Base.connection.execute(
"update #{Lti::ResourceLink.quoted_table_name}
set url = orig_resource_link.url
from (select url from #{Lti::ResourceLink.quoted_table_name} where id = #{ActiveRecord::Base.sanitize_sql(orig_id)}) orig_resource_link
where id in (#{copied_ids})
"
)
end
end
end

23
spec/lib/cc/lti_resource_links_spec.rb
View File

@ -28,7 +28,7 @@ describe CC::LtiResourceLinks do
context: tool.context,
context_external_tool: tool,
custom: custom,
url: "http://www.example.com/launch"
url: resource_link_url
)
end
@ -37,12 +37,13 @@ describe CC::LtiResourceLinks do
opts: {
use_1_3: true,
description: "test tool",
url: url
url: tool_url
}
)
end
let(:url) { "https://www.test-tool.com/launch" }
let(:tool_url) { "https://www.test-tool.com/launch" }
let(:resource_link_url) { "https://www.test-tool.com/launch?foo=bar" }
let(:custom) { { foo: "bar", fiz: "buzz" } }
let(:document) { Builder::XmlMarkup.new(target: xml, indent: 2) }
let(:xml) { +"" }
@ -76,7 +77,7 @@ describe CC::LtiResourceLinks do
end
it "sets the secure launch url" do
expect(subject.at_xpath("//blti:secure_launch_url").text).to eq tool.url
expect(subject.at_xpath("//blti:secure_launch_url").text).to eq resource_link_url
end
it "does not set the launch url" do
@ -91,16 +92,24 @@ describe CC::LtiResourceLinks do
).to eq(custom.stringify_keys)
end
context "when the tool URL uses HTTP" do
let(:url) { "http://www.test-tool.com/launch" }
context "when the resource link URL uses HTTP" do
let(:resource_link_url) { "http://www.test-tool.com/launch?foo=bar" }
it "does set the launch url" do
expect(subject.at_xpath("//blti:launch_url").text).to eq tool.url
expect(subject.at_xpath("//blti:launch_url").text).to eq resource_link.url
end
it "does not set the secure launch url" do
expect(subject.at_xpath("//blti:secure_launch_url")).to be_blank
end
end
context "when the resource link URL is nil" do
let(:resource_link_url) { nil }
it "defaults to the tool url" do
expect(subject.at_xpath("//blti:secure_launch_url").text).to eq tool_url
end
end
end
end