From ce6fd9fa8d0364225d444413b677fe5ed8e0390c Mon Sep 17 00:00:00 2001 From: Steve McGee Date: Tue, 9 Jul 2024 16:33:01 -0600 Subject: [PATCH] prefer assignment title over resource link title why: we used to pass the assignment title as the resource link claims title, and customers are expecting this behavior, but we changed this at some point to prefer the the resource link title. fixes: INTEROP-8629 flag=none test plan: note: it would be easiest if this was QA'd after 8700 has been merged (including the commit to the LTI 1.3 Test Tool - or cherry-pick from https://gerrit.instructure.com/c/lti-1.3-test-tool/+/351606) and using the LTI 1.3 Test Tool you added {"https://canvas.instructure.com/lti/preserveExistingAssignmentName": true} to the new 'Extra arbitrary content item data' field in the tool configuration to ensure that the assignment title is different and not overwritten by the tool title or the lineItem.label - create an assignment with a 1.3 tool that has a title different from the assignment title and save - if using the LTI 1.3 Test Tool you should see in the decoded jwt on the assignment view that the resource link claims title is the assignment title. Change-Id: Iac0a74a5c47fdddbaa9a5ab1071c0b846984f0c4 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/351965 Tested-by: Service Cloud Jenkins Reviewed-by: Tucker Mcknight QA-Review: Tucker Mcknight Product-Review: Alexis Nast --- lib/lti/messages/resource_link_request.rb | 2 +- .../messages/resource_link_request_spec.rb | 39 ++++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/lti/messages/resource_link_request.rb b/lib/lti/messages/resource_link_request.rb index 139a8994f95..072f1e9a43b 100644 --- a/lib/lti/messages/resource_link_request.rb +++ b/lib/lti/messages/resource_link_request.rb @@ -61,7 +61,7 @@ module Lti::Messages def add_resource_link_request_claims! @message.resource_link.id = launch_resource_link_id @message.resource_link.description = @assignment&.description - @message.resource_link.title = resource_link&.title.presence || @assignment&.title.presence || tag_from_resource_link&.title.presence || @context.name + @message.resource_link.title = @assignment&.title.presence || resource_link&.title.presence || tag_from_resource_link&.title.presence || @context.name end def add_lti1p1_claims! diff --git a/spec/lib/lti/messages/resource_link_request_spec.rb b/spec/lib/lti/messages/resource_link_request_spec.rb index 3cf4af90291..5892ae11046 100644 --- a/spec/lib/lti/messages/resource_link_request_spec.rb +++ b/spec/lib/lti/messages/resource_link_request_spec.rb @@ -398,15 +398,44 @@ describe Lti::Messages::ResourceLinkRequest do it_behaves_like "assignment resource link id check" - context "the resource link has a blank title but the assignment has a title" do - before do - expected_assignment_line_item.resource_link.update!(title: "") - assignment.update!(title: "foo") + context "uses the appropriate title for the resource link claim title" do + let(:content_tag) do + ContentTag.create!( + context: course, + title: "fake title", + url: "https://example.com", + associated_asset: expected_assignment_line_item.resource_link + ) end - it "uses the assignment title for the resource link title" do + before do + content_tag + assignment.update!(title: "foo") + expected_assignment_line_item.resource_link.update!(title: "bar") + expected_assignment_line_item.resource_link.content_tag.update!(title: "baz") + end + + it "prefers the assignment title over other options" do expect(jws[:post_payload].dig("https://purl.imsglobal.org/spec/lti/claim/resource_link", "title")).to eq assignment.title end + + it "uses the resource link title if assignment title is blank" do + assignment.title = "" + expect(jws[:post_payload].dig("https://purl.imsglobal.org/spec/lti/claim/resource_link", "title")).to eq expected_assignment_line_item.resource_link.title + end + + it "uses the resource link tag for the title if the assignment title and the resource link title are blank" do + assignment.title = "" + expected_assignment_line_item.resource_link.update!(title: "") + expect(jws[:post_payload].dig("https://purl.imsglobal.org/spec/lti/claim/resource_link", "title")).to eq expected_assignment_line_item.resource_link.content_tag.title + end + + it "uses the context name if all other options are blank" do + assignment.title = "" + expected_assignment_line_item.resource_link.update!(title: "") + expected_assignment_line_item.resource_link.content_tag.update!(title: "") + expect(jws[:post_payload].dig("https://purl.imsglobal.org/spec/lti/claim/resource_link", "title")).to eq course.name + end end end