From edec2a68282a2a1fc2b2036e1989d60688fa6b19 Mon Sep 17 00:00:00 2001
From: Giovanni Cimolin da Silva <giovannicimolin@gmail.com>
Date: Thu, 18 Mar 2021 18:25:41 -0300
Subject: [PATCH] feat: improve content presentation and add support for
displaying multiple content items (#148)
* Improve content presentation and add multiple content display support
This commit:
* Improves LTI content presentation handling, and correctly displays LTI DL content in the LMS
* Adds support for presenting multiple DL content items on the same block
* Addressing review comments
* Nits
* Improve test and fix issue
[BD-24][TNL-8072]
---
README.rst | 20 +++
lti_consumer/api.py | 54 +++++--
lti_consumer/lti_1p3/deep_linking.py | 4 +-
.../lti_1p3/tests/test_deep_linking.py | 2 +-
lti_consumer/lti_xblock.py | 5 +-
lti_consumer/plugin/views.py | 1 +
.../html/lti-dl/render_dl_content.html | 19 ++-
.../html/lti-dl/render_lti_resource_link.html | 9 ++
.../templatetags/get_dl_lti_launch_url.py | 26 ++++
.../plugin/test_views_lti_deep_linking.py | 25 ++++
lti_consumer/tests/unit/test_api.py | 139 ++++++++++++------
lti_consumer/tests/unit/test_lti_xblock.py | 2 +-
lti_consumer/utils.py | 12 ++
13 files changed, 249 insertions(+), 69 deletions(-)
create mode 100644 lti_consumer/templates/html/lti-dl/render_lti_resource_link.html
create mode 100644 lti_consumer/templatetags/get_dl_lti_launch_url.py
diff --git a/README.rst b/README.rst
index abc5243..9181a43 100644
--- a/README.rst
+++ b/README.rst
@@ -222,6 +222,26 @@ To configure parameter processors add the following snippet to your Ansible vari
- 'customer_package.lti_processors:team_and_cohort'
- 'example_package.lti_processors:extra_lti_params'
+LTI Advantage Features
+======================
+
+This XBlock supports LTI 1.3 and the following LTI Avantage services:
+
+* Deep Linking (LTI-DL)
+* Assignments and Grades services (LTI-AGS)
+
+To enable LTI-AGS, your block needs two properties set: `has_score` and `weight`. Once that is configured, the
+service will be automatically enabled and the tool will be able to send scores back to the platform through
+a `LineItem`.
+
+To enable LTI-DL and its capabilities, you need to set the following feature flag:
+
+.. code:: yaml
+
+ FEATURES:
+ LTI_DEEP_LINKING_ENABLED: true
+
+
Development
===========
diff --git a/lti_consumer/api.py b/lti_consumer/api.py
index 5490f35..4a83181 100644
--- a/lti_consumer/api.py
+++ b/lti_consumer/api.py
@@ -7,6 +7,7 @@ return plaintext to allow easy testing/mocking.
from .exceptions import LtiError
from .models import LtiConfiguration, LtiDlContentItem
from .utils import (
+ get_lti_deeplinking_content_url,
get_lms_lti_keyset_link,
get_lms_lti_launch_link,
get_lms_lti_access_token_link,
@@ -112,7 +113,7 @@ def get_lti_1p3_launch_info(config_id=None, block=None):
}
-def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=False, hint=""):
+def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=False, dl_content_id=None, hint=""):
"""
Computes and retrieves the LTI URL that starts the OIDC flow.
"""
@@ -122,18 +123,14 @@ def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=Fa
# Change LTI hint depending on LTI launch type
lti_hint = ""
+ # Case 1: Performs Deep Linking configuration flow. Triggered by staff users to
+ # configure tool options and select content to be presented.
if deep_link_launch:
lti_hint = "deep_linking_launch"
- else:
- # Check if there's any LTI DL content item
- # This should only yield one result since we
- # don't support multiple content items yet.
- content_items = lti_config.ltidlcontentitem_set.filter(
- content_type=LtiDlContentItem.LTI_RESOURCE_LINK,
- ).only('id')
-
- if content_items.count():
- lti_hint = f"deep_linking_content_launch:{content_items.get().id}"
+ # Case 2: Perform a LTI Launch for `ltiResourceLink` content types, since they
+ # need to use the launch mechanism from the callback view.
+ elif dl_content_id:
+ lti_hint = f"deep_linking_content_launch:{dl_content_id}"
# Prepare and return OIDC flow start url
return lti_consumer.prepare_preflight_url(
@@ -143,6 +140,41 @@ def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=Fa
)
+def get_lti_1p3_content_url(config_id=None, block=None, hint=""):
+ """
+ Computes and returns which URL the LTI consumer should launch to.
+
+ This can return:
+ 1. A LTI Launch link if:
+ a. No deep linking is set
+ b. Deep Linking is configured, but a single ltiResourceLink was selected.
+ 2. The Deep Linking content presentation URL if there's more than one
+ Lti DL content in the database.
+ """
+ # Retrieve LTI consumer
+ lti_config = _get_lti_config(config_id, block)
+
+ # List content items
+ content_items = lti_config.ltidlcontentitem_set.all()
+
+ # If there's no content items, return normal LTI launch URL
+ if not content_items.count():
+ return get_lti_1p3_launch_start_url(config_id, block, hint=hint)
+
+ # If there's a single `ltiResourceLink` content, return the launch
+ # url for that specif deep link
+ if content_items.count() == 1 and content_items.get().content_type == LtiDlContentItem.LTI_RESOURCE_LINK:
+ return get_lti_1p3_launch_start_url(
+ config_id,
+ block,
+ dl_content_id=content_items.get().id,
+ hint=hint,
+ )
+
+ # If there's more than one content item, return content presentation URL
+ return get_lti_deeplinking_content_url(lti_config.id)
+
+
def get_deep_linking_data(deep_linking_id, config_id=None, block=None):
"""
Retrieves deep linking attributes.
diff --git a/lti_consumer/lti_1p3/deep_linking.py b/lti_consumer/lti_1p3/deep_linking.py
index 9b3cb0d..d483e5b 100644
--- a/lti_consumer/lti_1p3/deep_linking.py
+++ b/lti_consumer/lti_1p3/deep_linking.py
@@ -52,8 +52,8 @@ class LtiDeepLinking:
"window",
"embed"
],
- # Only accept a single item return from Deep Linking operation.
- "accept_multiple": False,
+ # Accept multiple items on from Deep Linking responses.
+ "accept_multiple": True,
# Automatically saves Content Items without asking to user
"auto_create": True,
# Other parameters
diff --git a/lti_consumer/lti_1p3/tests/test_deep_linking.py b/lti_consumer/lti_1p3/tests/test_deep_linking.py
index 5bfee8a..cf21c18 100644
--- a/lti_consumer/lti_1p3/tests/test_deep_linking.py
+++ b/lti_consumer/lti_1p3/tests/test_deep_linking.py
@@ -65,7 +65,7 @@ class TestLtiDeepLinking(TestCase):
'window',
'embed'
],
- 'accept_multiple': False,
+ 'accept_multiple': True,
'auto_create': True,
'title': '',
'text': '',
diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py
index 58a9432..1d5985a 100644
--- a/lti_consumer/lti_xblock.py
+++ b/lti_consumer/lti_xblock.py
@@ -1420,12 +1420,11 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
else:
# Runtime import since this will only run in the
# Open edX LMS/Studio environments.
- from lti_consumer.api import get_lti_1p3_launch_start_url # pylint: disable=import-outside-toplevel
+ from lti_consumer.api import get_lti_1p3_content_url # pylint: disable=import-outside-toplevel
# Retrieve and set LTI 1.3 Launch start URL
- lti_block_launch_handler = get_lti_1p3_launch_start_url(
+ lti_block_launch_handler = get_lti_1p3_content_url(
block=self,
- deep_link_launch=False,
hint=str(self.location) # pylint: disable=no-member
)
diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py
index 53101b0..9b63416 100644
--- a/lti_consumer/plugin/views.py
+++ b/lti_consumer/plugin/views.py
@@ -259,6 +259,7 @@ def deep_linking_response_endpoint(request, lti_config_id=None):
@require_http_methods(['GET'])
+@xframe_options_sameorigin
def deep_linking_content_endpoint(request, lti_config_id=None):
"""
Deep Linking endpoint for rendering Deep Linking Content Items.
diff --git a/lti_consumer/templates/html/lti-dl/render_dl_content.html b/lti_consumer/templates/html/lti-dl/render_dl_content.html
index 7e00c3b..0afa083 100644
--- a/lti_consumer/templates/html/lti-dl/render_dl_content.html
+++ b/lti_consumer/templates/html/lti-dl/render_dl_content.html
@@ -6,13 +6,18 @@
</head>
<body>
{% for item in content_items %}
- {% if item.content_type == 'link' %}
- {% include "html/lti-dl/render_link.html" with item=item attrs=item.attributes %}
- {% elif item.content_type == 'html' %}
- {% include "html/lti-dl/render_html.html" with item=item attrs=item.attributes %}
- {% elif item.content_type == 'image' %}
- {% include "html/lti-dl/render_image.html" with item=item attrs=item.attributes %}
- {% endif %}
+ <div>
+ {% if item.content_type == 'ltiResourceLink' %}
+ {% include "html/lti-dl/render_lti_resource_link.html" with item=item attrs=item.attributes %}
+ {% elif item.content_type == 'link' %}
+ {% include "html/lti-dl/render_link.html" with item=item attrs=item.attributes %}
+ {% elif item.content_type == 'html' %}
+ {% include "html/lti-dl/render_html.html" with item=item attrs=item.attributes %}
+ {% elif item.content_type == 'image' %}
+ {% include "html/lti-dl/render_image.html" with item=item attrs=item.attributes %}
+ {% endif %}
+ </div>
+ <br />
{% endfor %}
</body>
</html>
diff --git a/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html b/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html
new file mode 100644
index 0000000..8e5b10a
--- /dev/null
+++ b/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html
@@ -0,0 +1,9 @@
+{% load get_dl_lti_launch_url %}
+
+{% if attrs.title %}<h2>{{ attrs.title }}</h2>{% endif %}
+{% if attrs.text %}<p>{{ attrs.text }}</p>{% endif %}
+
+<a href="{{ item | get_dl_lti_launch_url }}" target="_blank">
+ Click here to launch the LTI content in a new tab
+ <i class="icon fa fa-external-link"></i>
+</a>
diff --git a/lti_consumer/templatetags/get_dl_lti_launch_url.py b/lti_consumer/templatetags/get_dl_lti_launch_url.py
new file mode 100644
index 0000000..1dd97c9
--- /dev/null
+++ b/lti_consumer/templatetags/get_dl_lti_launch_url.py
@@ -0,0 +1,26 @@
+"""
+Template tags and helper functions for sanitizing html.
+"""
+from django import template
+from lti_consumer.api import get_lti_1p3_launch_start_url
+
+
+register = template.Library()
+
+
+@register.filter()
+def get_dl_lti_launch_url(content_item):
+ """
+ Template tag to retrive the LTI launch URL for `ltiResourceLink` content.
+
+ This uses the LTI Consumer API, but hardcodes the `hint` parameter to the
+ block id (used when LTI launches are tied to XBlocks).
+
+ TODO: Refactor `hint` to use generic ID once LTI launches out of XBlocks are
+ supported.
+ """
+ return get_lti_1p3_launch_start_url(
+ config_id=content_item.lti_configuration.id,
+ dl_content_id=content_item.id,
+ hint=str(content_item.lti_configuration.location),
+ )
diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py
index 364b258..894ef3f 100644
--- a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py
+++ b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py
@@ -604,3 +604,28 @@ class LtiDeepLinkingContentEndpointTestCase(LtiDeepLinkingTestCase):
if test_data.get('height'):
self.assertContains(resp, 'height="{}"'.format(test_data['height']))
+
+ @patch('lti_consumer.plugin.views.has_block_access', return_value=True)
+ def test_dl_content_multiple_lti_resource_links(self, has_block_access): # pylint: disable=unused-argument
+ """
+ Test if multiple `ltiResourceLink` content types are successfully rendered.
+ """
+ content_items = []
+ for _ in range(3):
+ content_items.append(
+ LtiDlContentItem.objects.create(
+ lti_configuration=self.lti_config,
+ content_type=LtiDlContentItem.LTI_RESOURCE_LINK,
+ attributes={}
+ )
+ )
+
+ resp = self.client.get(self.url)
+ self.assertEqual(resp.status_code, 200)
+
+ # Check that there's three LTI Resource links presented
+ for item in content_items:
+ self.assertContains(
+ resp,
+ f"lti_message_hint=deep_linking_content_launch%3A{item.id}"
+ )
diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py
index 4a7b0e4..64af91f 100644
--- a/lti_consumer/tests/unit/test_api.py
+++ b/lti_consumer/tests/unit/test_api.py
@@ -8,6 +8,7 @@ from django.test.testcases import TestCase
from lti_consumer.api import (
_get_or_create_local_lti_config,
+ get_lti_1p3_content_url,
get_deep_linking_data,
get_lti_1p3_launch_info,
get_lti_1p3_launch_start_url,
@@ -18,6 +19,45 @@ from lti_consumer.models import LtiConfiguration, LtiDlContentItem
from lti_consumer.tests.unit.test_utils import make_xblock
+class Lti1P3TestCase(TestCase):
+ """
+ Reusable test case for testing LTI 1.3 configurations.
+ """
+ def setUp(self):
+ """
+ Set up an empty block configuration.
+ """
+ self.xblock = None
+ self.lti_config = None
+
+ return super().setUp()
+
+ def _setup_lti_block(self):
+ """
+ Set's up an LTI block that is used in some tests.
+ """
+ # Generate RSA and save exports
+ rsa_key = RSA.generate(1024)
+ public_key = rsa_key.publickey().export_key()
+
+ xblock_attributes = {
+ 'lti_version': 'lti_1p3',
+ 'lti_1p3_launch_url': 'http://tool.example/launch',
+ 'lti_1p3_oidc_url': 'http://tool.example/oidc',
+ # We need to set the values below because they are not automatically
+ # generated until the user selects `lti_version == 'lti_1p3'` on the
+ # Studio configuration view.
+ 'lti_1p3_tool_public_key': public_key,
+ }
+ self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, xblock_attributes)
+ # Set dummy location so that UsageKey lookup is valid
+ self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test'
+ # Create lti configuration
+ self.lti_config = LtiConfiguration.objects.create(
+ location=self.xblock.location
+ )
+
+
class TestGetOrCreateLocalLtiConfiguration(TestCase):
"""
Unit tests for _get_or_create_local_lti_config API method.
@@ -164,69 +204,44 @@ class TestGetLti1p3LaunchInfo(TestCase):
block = Mock()
block.location = 'block-v1:course+test+2020+type@problem+block@test'
block.lti_version = LtiConfiguration.LTI_1P3
- LtiConfiguration.objects.create(location=block.location)
+
+ # Create LTI Config and Deep linking object
+ lti_config = LtiConfiguration.objects.create(location=block.location)
+ LtiDlContentItem.objects.create(
+ lti_configuration=lti_config,
+ content_type=LtiDlContentItem.IMAGE,
+ attributes={"test": "this is a test attribute"}
+ )
# Call API
launch_info = get_lti_1p3_launch_info(block=block)
# Retrieve created config and check full launch info data
lti_config = LtiConfiguration.objects.get()
- self.assertCountEqual(
+ self.assertEqual(
launch_info,
{
'client_id': lti_config.lti_1p3_client_id,
'keyset_url': 'https://example.com/api/lti_consumer/v1/public_keysets/{}'.format(
- lti_config.lti_1p3_client_id
+ lti_config.location
),
'deployment_id': '1',
'oidc_callback': 'https://example.com/api/lti_consumer/v1/launch/',
'token_url': 'https://example.com/api/lti_consumer/v1/token/{}'.format(
- lti_config.lti_1p3_client_id
+ lti_config.location
),
- 'deep_linking_launch_url': 'https://example.com',
- 'deep_linking_content_items': []
+ 'deep_linking_launch_url': 'http://example.com',
+ 'deep_linking_content_items': [{
+ "test": "this is a test attribute",
+ }]
}
)
-class TestGetLti1p3LaunchUrl(TestCase):
+class TestGetLti1p3LaunchUrl(Lti1P3TestCase):
"""
Unit tests for get_lti_1p3_launch_start_url API method.
"""
- def setUp(self):
- """
- Set up an empty block configuration.
- """
- self.xblock = None
- self.lti_config = None
-
- return super().setUp()
-
- def _setup_lti_block(self):
- """
- Set's up an LTI block that is used in some tests.
- """
- # Generate RSA and save exports
- rsa_key = RSA.generate(1024)
- public_key = rsa_key.publickey().export_key()
-
- xblock_attributes = {
- 'lti_version': 'lti_1p3',
- 'lti_1p3_launch_url': 'http://tool.example/launch',
- 'lti_1p3_oidc_url': 'http://tool.example/oidc',
- # We need to set the values below because they are not automatically
- # generated until the user selects `lti_version == 'lti_1p3'` on the
- # Studio configuration view.
- 'lti_1p3_tool_public_key': public_key,
- }
- self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, xblock_attributes)
- # Set dummy location so that UsageKey lookup is valid
- self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test'
- # Create lti configuration
- self.lti_config = LtiConfiguration.objects.create(
- location=self.xblock.location
- )
-
def test_no_parameters(self):
"""
Check if the API creates a model if no object matching properties is found.
@@ -249,7 +264,21 @@ class TestGetLti1p3LaunchUrl(TestCase):
launch_url = get_lti_1p3_launch_start_url(block=self.xblock, deep_link_launch=True)
self.assertIn('lti_message_hint=deep_linking_launch', launch_url)
- def test_lti_content_presentation(self):
+
+class TestGetLti1p3ContentUrl(Lti1P3TestCase):
+ """
+ Unit tests for get_lti_1p3_launch_start_url API method.
+ """
+ @patch("lti_consumer.api.get_lti_1p3_launch_start_url")
+ def test_lti_content_presentation(self, mock_get_launch_url):
+ """
+ Check if the correct LTI content presentation is returned on a normal LTI Launch.
+ """
+ mock_get_launch_url.return_value = 'test_url'
+ self._setup_lti_block()
+ self.assertEqual(get_lti_1p3_content_url(block=self.xblock), 'test_url')
+
+ def test_lti_content_presentation_single_link(self):
"""
Check if the correct LTI content presentation is returned if a `ltiResourceLink`
content type is present.
@@ -264,12 +293,34 @@ class TestGetLti1p3LaunchUrl(TestCase):
)
# Call API to retrieve content item URL
- launch_url = get_lti_1p3_launch_start_url(block=self.xblock)
+ launch_url = get_lti_1p3_content_url(block=self.xblock)
self.assertIn(
# Checking for `deep_linking_content_launch:<content_item_id>`
# URL Encoded `:` is `%3A`
f'lti_message_hint=deep_linking_content_launch%3A{lti_content.id}',
- launch_url
+ launch_url,
+ )
+
+ def test_lti_content_presentation_multiple_links(self):
+ """
+ Check if the correct LTI content presentation is returned if multiple LTI DL
+ content items are set up.
+ """
+ self._setup_lti_block()
+
+ # Create LTI DL content items
+ for _ in range(3):
+ LtiDlContentItem.objects.create(
+ lti_configuration=self.lti_config,
+ content_type=LtiDlContentItem.IMAGE,
+ attributes={},
+ )
+
+ # Call API to retrieve content item URL
+ self.assertIn(
+ # Checking for the content presentation URL
+ f"/api/lti_consumer/v1/lti/{self.lti_config.id}/lti-dl/content",
+ get_lti_1p3_content_url(block=self.xblock),
)
diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py
index 94841d2..0072c7a 100644
--- a/lti_consumer/tests/unit/test_lti_xblock.py
+++ b/lti_consumer/tests/unit/test_lti_xblock.py
@@ -1077,7 +1077,7 @@ class TestGetContext(TestLtiConsumerXBlock):
"""
@ddt.data('lti_1p1', 'lti_1p3')
- @patch('lti_consumer.api.get_lti_1p3_launch_start_url')
+ @patch('lti_consumer.api.get_lti_1p3_content_url')
def test_context_keys(self, lti_version, lti_api_patch):
"""
Test `_get_context_for_template` returns dict with correct keys
diff --git a/lti_consumer/utils.py b/lti_consumer/utils.py
index 5ddb016..33f62ee 100644
--- a/lti_consumer/utils.py
+++ b/lti_consumer/utils.py
@@ -102,3 +102,15 @@ def get_lti_deeplinking_response_url(lti_config_id):
lms_base=get_lms_base(),
lti_config_id=str(lti_config_id),
)
+
+
+def get_lti_deeplinking_content_url(lti_config_id):
+ """
+ Return the LTI Deep Linking content presentation endpoint
+
+ :param lti_config_id: LTI configuration id
+ """
+ return "{lms_base}/api/lti_consumer/v1/lti/{lti_config_id}/lti-dl/content".format(
+ lms_base=get_lms_base(),
+ lti_config_id=str(lti_config_id),
+ )
--
GitLab