From 594ee5fd61b6a7214037daf22ee46f21faf2c980 Mon Sep 17 00:00:00 2001 From: Shimul Chowdhury <shimul@opencraft.com> Date: Tue, 29 Dec 2020 19:33:21 +0600 Subject: [PATCH] Add support for link content type Fix broken tests Add serializer for `link` content type --- lti_consumer/lti_1p3/constants.py | 2 +- .../extensions/rest_framework/constants.py | 6 +- .../extensions/rest_framework/serializers.py | 59 ++++++ lti_consumer/lti_1p3/tests/test_consumer.py | 2 +- lti_consumer/plugin/views.py | 9 +- .../html/lti-dl/render_dl_content.html | 5 + .../templates/html/lti-dl/render_link.html | 18 ++ .../plugin/test_views_lti_deep_linking.py | 188 ++++++++++++++++++ 8 files changed, 282 insertions(+), 7 deletions(-) create mode 100644 lti_consumer/templates/html/lti-dl/render_link.html diff --git a/lti_consumer/lti_1p3/constants.py b/lti_consumer/lti_1p3/constants.py index 7515006..9029041 100644 --- a/lti_consumer/lti_1p3/constants.py +++ b/lti_consumer/lti_1p3/constants.py @@ -54,8 +54,8 @@ LTI_1P3_ACCESS_TOKEN_SCOPES = [ LTI_DEEP_LINKING_ACCEPTED_TYPES = [ 'ltiResourceLink', + 'link', # TODO: implement "image" support, - # TODO: implement "link" support, # TODO: implement "file" support, # TODO: implement "html" support, ] diff --git a/lti_consumer/lti_1p3/extensions/rest_framework/constants.py b/lti_consumer/lti_1p3/extensions/rest_framework/constants.py index 214f2db..c05a765 100644 --- a/lti_consumer/lti_1p3/extensions/rest_framework/constants.py +++ b/lti_consumer/lti_1p3/extensions/rest_framework/constants.py @@ -1,9 +1,13 @@ """ LTI 1.3/Advantage DRF Related Constants """ -from .serializers import LtiDlLtiResourceLinkSerializer +from .serializers import ( + LtiDlLtiResourceLinkSerializer, + LtiDlLinkSerializer, +) LTI_DL_CONTENT_TYPE_SERIALIZER_MAP = { "ltiResourceLink": LtiDlLtiResourceLinkSerializer, + "link": LtiDlLinkSerializer, } diff --git a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py index b9f9125..bc340ed 100644 --- a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py +++ b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py @@ -271,3 +271,62 @@ class LtiDlLtiResourceLinkSerializer(serializers.Serializer): lineItem = LtiDlLineItemSerializer(required=False) available = LtiDlTimeDeltaSerializer(required=False) submission = LtiDlTimeDeltaSerializer(required=False) + + +# pylint: disable=abstract-method +class LtiDLIconPropertySerializer(serializers.Serializer): + """ + LTI Deep Linking - `icon` or `thumbnail` property serializer. + """ + url = serializers.URLField(max_length=500) + width = serializers.IntegerField(min_value=1) + height = serializers.IntegerField(min_value=1) + + +# pylint: disable=abstract-method +class LtiDlEmbedPropertySerializer(serializers.Serializer): + """ + LTI Deep Linking - `embed` property serializer. + """ + html = serializers.CharField() + + +# pylint: disable=abstract-method +class LtiDlWindowPropertySerializer(serializers.Serializer): + """ + LTI Deep Linking - `window` property serializer. + """ + targetName = serializers.CharField(max_length=255, required=False) + width = serializers.IntegerField(min_value=1, required=False) + height = serializers.IntegerField(min_value=1, required=False) + windowFeatures = serializers.CharField(required=False) + + +# pylint: disable=abstract-method +class LtiDlIframePropertySerializer(serializers.Serializer): + """ + LTI Deep Linking - `iframe` property serializer. + """ + src = serializers.URLField(max_length=500) + width = serializers.IntegerField(min_value=1) + height = serializers.IntegerField(min_value=1) + + +# pylint: disable=abstract-method +class LtiDlLinkSerializer(serializers.Serializer): + """ + LTI Deep Linking - Link Serializer. + + This serializer implements validation for the Link content type. + + Reference: + http://www.imsglobal.org/spec/lti-dl/v2p0#link + """ + url = serializers.URLField(max_length=500) + title = serializers.CharField(max_length=255, required=False) + text = serializers.CharField(required=False) + icon = LtiDLIconPropertySerializer(required=False) + thumbnail = LtiDLIconPropertySerializer(required=False) + embed = LtiDlEmbedPropertySerializer(required=False) + window = LtiDlWindowPropertySerializer(required=False) + iframe = LtiDlIframePropertySerializer(required=False) diff --git a/lti_consumer/lti_1p3/tests/test_consumer.py b/lti_consumer/lti_1p3/tests/test_consumer.py index 6890916..91c19fe 100644 --- a/lti_consumer/lti_1p3/tests/test_consumer.py +++ b/lti_consumer/lti_1p3/tests/test_consumer.py @@ -664,7 +664,7 @@ class TestLtiAdvantageConsumer(TestCase): "https://purl.imsglobal.org/spec/lti/claim/message_type": "LtiDeepLinkingResponse", "https://purl.imsglobal.org/spec/lti-dl/claim/content_items": [ { - "type": "link", + "type": "wrongContentType", "url": "https://something.example.com/page.html", }, ] diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py index 778c6a2..e7b7973 100644 --- a/lti_consumer/plugin/views.py +++ b/lti_consumer/plugin/views.py @@ -194,10 +194,11 @@ def deep_linking_response_endpoint(request, lti_config_id=None): LtiDlContentItem.objects.filter(lti_configuration=lti_config).delete() for content_item in content_items: + + content_type = content_item.get('type') + # Retrieve serializer (or throw error) - serializer_cls = LTI_DL_CONTENT_TYPE_SERIALIZER_MAP[ - content_item.get('type') - ] + serializer_cls = LTI_DL_CONTENT_TYPE_SERIALIZER_MAP[content_type] # Validate content item data serializer = serializer_cls(data=content_item) @@ -206,7 +207,7 @@ def deep_linking_response_endpoint(request, lti_config_id=None): # Save content item LtiDlContentItem.objects.create( lti_configuration=lti_config, - content_type=LtiDlContentItem.LTI_RESOURCE_LINK, + content_type=content_type, attributes=serializer.validated_data, ) 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 384e89b..d3eab61 100644 --- a/lti_consumer/templates/html/lti-dl/render_dl_content.html +++ b/lti_consumer/templates/html/lti-dl/render_dl_content.html @@ -5,5 +5,10 @@ <title>{{ block.display_name }} | Deep Linking Contents</title> </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 %} + {% endif %} + {% endfor %} </body> </html> diff --git a/lti_consumer/templates/html/lti-dl/render_link.html b/lti_consumer/templates/html/lti-dl/render_link.html new file mode 100644 index 0000000..533eefa --- /dev/null +++ b/lti_consumer/templates/html/lti-dl/render_link.html @@ -0,0 +1,18 @@ +{% if attrs.title %}<h2>{{ attrs.title }}</h2>{% endif %} +{% if attrs.text %}<p>{{ attrs.text }}</p>{% endif %} +<a + {% if attrs.window %} + onclick="window.open('{{ attrs.url }}', '{{ attrs.window.targetName|default_if_none:''}}', '{{ attrs.window.windowFeatures|default_if_none:'' }}')" + href="#" + {% else %} + href="{{ attrs.url }}" + {% endif %} + > + {% if attrs.icon %} + <img src="{{ attrs.icon.url }}" width="{{ attrs.icon.width }}" height="{{ attrs.icon.height }}" /> + {% endif %} + {% if attrs.thumbnail %} + <img src="{{ attrs.thumbnail.url }}" width="{{ attrs.thumbnail.width }}" height="{{ attrs.thumbnail.height }}" /> + {% endif %} + {{ attrs.url }} +</a> 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 f88fbca..e62a587 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 @@ -1,11 +1,13 @@ """ Tests for LTI Advantage Assignments and Grades Service views. """ +import ddt from mock import patch, PropertyMock, Mock from Cryptodome.PublicKey import RSA from jwkest.jwk import RSAKey from rest_framework.test import APITransactionTestCase +from rest_framework.exceptions import ValidationError from lti_consumer.lti_xblock import LtiConsumerXBlock @@ -77,6 +79,7 @@ class LtiDeepLinkingTestCase(APITransactionTestCase): self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock +@ddt.ddt class LtiDeepLinkingResponseEndpointTestCase(LtiDeepLinkingTestCase): """ Test `deep_linking_response_endpoint` for LTI Deep Linking compliance. @@ -219,7 +222,114 @@ class LtiDeepLinkingResponseEndpointTestCase(LtiDeepLinkingTestCase): self.assertEqual(content_items[0].content_type, "ltiResourceLink") self.assertEqual(content_items[0].attributes["url"], "https://example.com/lti") + def _content_type_validation_test_helper(self, content_item, is_valid): + """ + A helper method to test content type data. + + Performs tests based on wether data is valid or not. + """ + response_token = self._build_deep_linking_response( + content_items=[content_item] + ) + + if is_valid: + # Make request to endpoint + response = self.client.post( + self.url, + { + 'JWT': response_token + }, + ) + + self.assertEqual(response.status_code, 200) + + # Check if content item was created + content_items = LtiDlContentItem.objects.all() + self.assertEqual(content_items.count(), 1) + self.assertEqual(content_items[0].content_type, content_item["type"]) + del content_item["type"] + self.assertEqual(content_items[0].attributes, content_item) + else: + # If the datastructure is not valid, we expect to have a Validation Error. + with self.assertRaises(ValidationError): + self.client.post( + self.url, + { + 'JWT': response_token + }, + ) + + @ddt.data( + ({"type": "link"}, False), + ({"type": "link", "url": "https://example.com/link"}, True), + ({"type": "link", "url": "https://example.com/link", "text": "This is a link"}, True), + + # invalid icon + ({"type": "link", "url": "https://example.com/link", "text": "This is a link", "icon": {}}, False), + # valid icon + ({ + "type": "link", + "url": "https://example.com/link", + "text": "This is a link", + "icon": {"url": "https://ex.com/icon", "width": 20, "height": 20} + }, True), + + # invalid thumbnail + ({"type": "link", "url": "https://example.com/link", "text": "This is a link", "thumbnail": {}}, False), + # valid thumbnail + ({ + "type": "link", + "url": "https://example.com/link", + "text": "This is a link", + "thumbnail": {"url": "https://ex.com/icon", "width": 20, "height": 20} + }, True), + + # invalid embed + ({"type": "link", "url": "https://example.com/link", "embed": {}}, False), + # valid embed + ({ + "type": "link", + "url": "https://example.com/link", + "embed": {"html": "<p>Hello</p>"} + }, True), + + # window + ({"type": "link", "url": "https://example.com/link", "window": {}}, True), + ({"type": "link", "url": "https://example.com/link", "window": {"targetName": "targetWindow"}}, True), + ({ + "type": "link", + "url": "https://example.com/link", + "window": { + "targetName": "targetWindow", + "width": 200, + "height": 200, + "windowFeatures": "menubar=yes,location=yes,resizable=yes" + } + }, True), + + # iframe + ({"type": "link", "url": "https://example.com/link", "iframe": {}}, False), + ({"type": "link", "url": "https://example.com/link", "iframe": {"src": "http://ex.com/iframe"}}, False), + ({ + "type": "link", + "url": "https://example.com/link", + "iframe": {"src": "http://ex.com/iframe", "width": 200, "height": 200} + }, True), + ) + def test_link_content_type(self, test_data): + """ + Tests validation for `link` content type. + + Args: + self + test_data (tuple): 1st element is the datastructure to test, + and the second one indicates wether it's valid or not. + """ + content_item, is_valid = test_data + self._content_type_validation_test_helper(content_item, is_valid) + +@ddt.ddt class LtiDeepLinkingContentEndpointTestCase(LtiDeepLinkingTestCase): """ Test ``deep_linking_content_endpoint`` view. @@ -267,3 +377,81 @@ class LtiDeepLinkingContentEndpointTestCase(LtiDeepLinkingTestCase): self.assertEqual(resp.status_code, 200) expected_title = '{} | Deep Linking Contents'.format(self.lti_config.block.display_name) self.assertContains(resp, expected_title) + + @ddt.data( + {'url': 'https://example.com'}, + {'url': 'https://example.com', 'title': 'With Title'}, + {'url': 'https://example.com', 'title': 'With Title', 'text': 'With Text'}, + { + 'url': 'https://example.com', 'title': 'With Title', 'text': 'With Text', + 'icon': {'url': 'https://link.to.icon', 'width': '20px', 'height': '20px'}, + }, + { + 'url': 'https://example.com', 'title': 'With Title', 'text': 'With Text', + 'thumbnail': {'url': 'https://link.to.thumbnail', 'width': '20px', 'height': '20px'}, + }, + { + 'url': 'https://example.com', 'title': 'With Title', 'text': 'With Text', + 'window': {'targetName': 'newWindow', 'windowFeatures': 'width=200px,height=200px'}, + }, + ) + @patch('lti_consumer.plugin.views.has_block_access', return_value=True) + def test_dl_content_type_link(self, test_data, has_block_access_patcher): # pylint: disable=unused-argument + """ + Test if link content type successfully rendered. + """ + attributes = {'type': LtiDlContentItem.LINK} + attributes.update(test_data) + + LtiDlContentItem.objects.create( + lti_configuration=self.lti_config, + content_type=LtiDlContentItem.LINK, + attributes=attributes + ) + + resp = self.client.get(self.url) + self.assertEqual(resp.status_code, 200) + + if test_data.get('title'): + self.assertContains(resp, '<h2>{}</h2>'.format(test_data['title'])) + + if test_data.get('text'): + self.assertContains(resp, '<p>{}</p>'.format(test_data['text'])) + + # if icon exists + if test_data.get('icon'): + self.assertContains( + resp, + '<img src="{}" width="{}" height="{}" />'.format( + test_data['icon']['url'], + test_data['icon']['width'], + test_data['icon']['height'], + ) + ) + + # if thumbnail exists + if test_data.get('thumbnail'): + self.assertContains( + resp, + '<img src="{}" width="{}" height="{}" />'.format( + test_data['thumbnail']['url'], + test_data['thumbnail']['width'], + test_data['thumbnail']['height'], + ) + ) + + # if window property exists + if test_data.get('window'): + self.assertContains( + resp, + 'onclick="window.open(\'{}\', \'{}\', \'{}\')"'.format( + test_data['url'], + test_data['window']['targetName'], + test_data['window']['windowFeatures'], + ) + ) + # if window property exists then only onclick will work. + self.assertContains(resp, 'href="#"') + else: + # otherwise, the link should be on the href of the anchor tag. + self.assertContains(resp, 'href="{}"'.format(test_data['url'])) -- GitLab