[BD-24] Implement LTI AGS Score Publish Service and Results (#108)

* BD-24 Implement LTI AGS Score Publish Service and Results Service

* Address PR comments and add more validation

* Address PR comments

* Add tests; Fix error with scoreMaximum; Fix quality issues; Adjust user_id results url slightly

* Add permissions tests and address other PR comments

* Fix quality test

* Address PR comments

.gitignore

@@ -19,3 +19,6 @@ var/
 
 # virtualenvironment
 venv/
+
+# pyenv
+.python-version

lti_consumer/lti_1p3/constants.py

@@ -47,6 +47,8 @@
     # LTI-AGS Scopes
     'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly',
     'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
+    'https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly',
+    'https://purl.imsglobal.org/spec/lti-ags/scope/score',
 ]
 
 

lti_consumer/lti_1p3/extensions/rest_framework/parsers.py

@@ -14,3 +14,12 @@ class LineItemParser(parsers.JSONParser):
     It's the same as JSON parser, but uses a custom media_type.
     """
     media_type = 'application/vnd.ims.lis.v2.lineitem+json'
+
+
+class LineItemScoreParser(parsers.JSONParser):
+    """
+    Line Item Parser.
+
+    It's the same as JSON parser, but uses a custom media_type.
+    """
+    media_type = 'application/vnd.ims.lis.v1.score+json'

lti_consumer/lti_1p3/extensions/rest_framework/permissions.py

@@ -23,25 +23,32 @@ def has_permission(self, request, view):
         """
         Check if LTI AGS permissions are set in auth token.
         """
-        has_perm = False
-
         # Retrieves token from request, which was already checked by
         # the Authentication class, so we assume it's a sane value.
         auth_token = request.headers['Authorization'].split()[1]
 
+        scopes = []
         if view.action in ['list', 'retrieve']:
             # We don't need to wrap this around a try-catch because
             # the token was already tested by the Authentication class.
-            has_perm = request.lti_consumer.check_token(
-                auth_token,
-                [
-                    'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly',
-                    'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
-                ],
-            )
+            scopes = [
+                'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly',
+                'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
+            ]
         elif view.action in ['create', 'update', 'partial_update', 'delete']:
-            has_perm = request.lti_consumer.check_token(
-                auth_token,
-                ['https://purl.imsglobal.org/spec/lti-ags/scope/lineitem']
-            )
-        return has_perm
+            scopes = [
+                'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
+            ]
+        elif view.action in ['results']:
+            scopes = [
+                'https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly'
+            ]
+        elif view.action in ['scores']:
+            scopes = [
+                'https://purl.imsglobal.org/spec/lti-ags/scope/score',
+            ]
+
+        if scopes:
+            return request.lti_consumer.check_token(auth_token, scopes)
+
+        return False

lti_consumer/lti_1p3/extensions/rest_framework/renderers.py

@@ -26,3 +26,25 @@ class LineItemRenderer(renderers.JSONRenderer):
     """
     media_type = 'application/vnd.ims.lis.v2.lineitem+json'
     format = 'json'
+
+
+class LineItemScoreRenderer(renderers.JSONRenderer):
+    """
+    Score Renderer.
+
+    It's a JSON renderer, but uses a custom media_type.
+    Reference: https://www.imsglobal.org/spec/lti-ags/v2p0#media-types-and-schemas
+    """
+    media_type = 'application/vnd.ims.lis.v1.score+json'
+    format = 'json'
+
+
+class LineItemResultsRenderer(renderers.JSONRenderer):
+    """
+    Results Renderer.
+
+    It's a JSON renderer, but uses a custom media_type.
+    Reference: https://www.imsglobal.org/spec/lti-ags/v2p0#media-types-and-schemas
+    """
+    media_type = 'application/vnd.ims.lis.v2.resultcontainer+json'
+    format = 'json'

lti_consumer/lti_1p3/extensions/rest_framework/serializers.py

@@ -1,12 +1,13 @@
 """
 Serializers for LTI-related endpoints
 """
-from rest_framework import serializers
+from django.utils import timezone
+from rest_framework import serializers, ISO_8601
 from rest_framework.reverse import reverse
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import UsageKey
 
-from lti_consumer.models import LtiAgsLineItem
+from lti_consumer.models import LtiAgsLineItem, LtiAgsScore
 
 
 class UsageKeyField(serializers.Field):
@@ -89,3 +90,140 @@ class Meta:
             'startDateTime',
             'endDateTime',
         )
+
+
+class LtiAgsScoreSerializer(serializers.ModelSerializer):
+    """
+    LTI AGS LineItemScore Serializer.
+
+    This maps out the internally stored LtiAgsScore to
+    the LTI-AGS API Specification, as shown in the example
+    response below:
+
+    {
+      "timestamp": "2017-04-16T18:54:36.736+00:00",
+      "scoreGiven" : 83,
+      "scoreMaximum" : 100,
+      "comment" : "This is exceptional work.",
+      "activityProgress" : "Completed",
+      "gradingProgress": "FullyGraded",
+      "userId" : "5323497"
+    }
+
+    Reference:
+    https://www.imsglobal.org/spec/lti-ags/v2p0#example-application-vnd-ims-lis-v1-score-json-representation
+    """
+
+    # NOTE: `serializers.DateTimeField` always outputs the value in the local timezone of the server running the code
+    # This is because Django is time aware (see settings.USE_TZ) and because Django is unable to determine the timezone
+    # of the person making the API request, thus falling back on the local timezone. As such, since all outputs will
+    # necessarily be in a singular timezone, that timezone should be `utc`
+    timestamp = serializers.DateTimeField(input_formats=[ISO_8601], format=ISO_8601, default_timezone=timezone.utc)
+    scoreGiven = serializers.FloatField(source='score_given', required=False, allow_null=True, default=None)
+    scoreMaximum = serializers.FloatField(source='score_maximum', required=False, allow_null=True, default=None)
+    comment = serializers.CharField(required=False, allow_null=True)
+    activityProgress = serializers.CharField(source='activity_progress')
+    gradingProgress = serializers.CharField(source='grading_progress')
+    userId = serializers.CharField(source='user_id')
+
+    def validate_timestamp(self, value):
+        """
+        Ensure that if an existing record is being updated, that the timestamp is in the after the existing one
+        """
+        if self.instance:
+            if self.instance.timestamp > value:
+                raise serializers.ValidationError('Score timestamp can only be updated to a later point in time')
+
+            if self.instance.timestamp == value:
+                raise serializers.ValidationError('Score already exists for the provided timestamp')
+
+        return value
+
+    def validate_scoreMaximum(self, value):
+        """
+        Ensure that scoreMaximum is set when scoreGiven is provided and not None
+        """
+        if not value and self.initial_data.get('scoreGiven', None) is not None:
+            raise serializers.ValidationError('scoreMaximum is a required field when providing a scoreGiven value.')
+        return value
+
+    class Meta:
+        model = LtiAgsScore
+        fields = (
+            'timestamp',
+            'scoreGiven',
+            'scoreMaximum',
+            'comment',
+            'activityProgress',
+            'gradingProgress',
+            'userId',
+        )
+
+
+class LtiAgsResultSerializer(serializers.ModelSerializer):
+    """
+    LTI AGS LineItemResult Serializer.
+
+    This maps out the internally stored LtiAgsScpre to
+    the LTI-AGS API Specification, as shown in the example
+    response below:
+
+    {
+      "id": "https://lms.example.com/context/2923/lineitems/1/results/5323497",
+      "scoreOf": "https://lms.example.com/context/2923/lineitems/1",
+      "userId": "5323497",
+      "resultScore": 0.83,
+      "resultMaximum": 1,
+      "comment": "This is exceptional work."
+    }
+
+    Reference:
+    https://www.imsglobal.org/spec/lti-ags/v2p0#example-application-vnd-ims-lis-v1-score-json-representation
+    """
+
+    id = serializers.SerializerMethodField()
+    scoreOf = serializers.SerializerMethodField()
+    userId = serializers.CharField(source='user_id')
+    resultScore = serializers.FloatField(source='score_given')
+    resultMaximum = serializers.SerializerMethodField()
+    comment = serializers.CharField()
+
+    def get_id(self, obj):
+        request = self.context.get('request')
+        return reverse(
+            'lti_consumer:lti-ags-view-results',
+            kwargs={
+                'lti_config_id': obj.line_item.lti_configuration.id,
+                'pk': obj.line_item.pk,
+                'user_id': obj.user_id,
+            },
+            request=request,
+        )
+
+    def get_scoreOf(self, obj):
+        request = self.context.get('request')
+        return reverse(
+            'lti_consumer:lti-ags-view-detail',
+            kwargs={
+                'lti_config_id': obj.line_item.lti_configuration.id,
+                'pk': obj.line_item.pk
+            },
+            request=request,
+        )
+
+    def get_resultMaximum(self, obj):
+        if obj.score_maximum <= 0:
+            return 1
+
+        return obj.score_maximum
+
+    class Meta:
+        model = LtiAgsScore
+        fields = (
+            'id',
+            'scoreOf',
+            'userId',
+            'resultScore',
+            'resultMaximum',
+            'comment',
+        )

lti_consumer/lti_1p3/tests/extensions/rest_framework/test_permissions.py

@@ -72,14 +72,20 @@ def _make_token(self, scopes):
         )
 
     @ddt.data(
-        ["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly"],
-        ["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem"],
-        [
-            "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly",
-            "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem",
-        ]
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly"], True),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem"], True),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/score"], False),
+        (
+            [
+                "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly",
+                "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem",
+            ],
+            True
+        ),
     )
-    def test_read_only_lineitem_list(self, token_scopes):
+    @ddt.unpack
+    def test_read_only_lineitem_list(self, token_scopes, is_allowed):
         """
         Test if LineItem is readable when any of the allowed scopes is
         included in the token.
@@ -95,14 +101,16 @@ def test_read_only_lineitem_list(self, token_scopes):
 
         # Test list view
         mock_view.action = 'list'
-        self.assertTrue(
+        self.assertEqual(
             perm_class.has_permission(self.mock_request, mock_view),
+            is_allowed,
         )
 
         # Test retrieve view
         mock_view.action = 'retrieve'
-        self.assertTrue(
+        self.assertEqual(
             perm_class.has_permission(self.mock_request, mock_view),
+            is_allowed,
         )
 
     def test_lineitem_no_permissions(self):
@@ -134,13 +142,15 @@ def test_lineitem_no_permissions(self):
     @ddt.data(
         (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly"], False),
         (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem"], True),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/score"], False),
         (
             [
                 "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly",
                 "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem",
             ],
             True
-        )
+        ),
     )
     @ddt.unpack
     def test_lineitem_write_permissions(self, token_scopes, is_allowed):
@@ -182,3 +192,57 @@ def test_unregistered_action_not_allowed(self):
         self.assertFalse(
             perm_class.has_permission(self.mock_request, mock_view),
         )
+
+    @ddt.data(
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly"], True),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/score"], False),
+    )
+    @ddt.unpack
+    def test_results_action_permissions(self, token_scopes, is_allowed):
+        """
+        Test if write operations on LineItem are allowed with the correct token.
+        """
+        perm_class = LtiAgsPermissions()
+        mock_view = MagicMock()
+
+        # Make token and include it in the mock request
+        token = self._make_token(token_scopes)
+        self.mock_request.headers = {
+            "Authorization": "Bearer {}".format(token)
+        }
+
+        # Test results view
+        mock_view.action = 'results'
+        self.assertEqual(
+            perm_class.has_permission(self.mock_request, mock_view),
+            is_allowed,
+        )
+
+    @ddt.data(
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/lineitem"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly"], False),
+        (["https://purl.imsglobal.org/spec/lti-ags/scope/score"], True),
+    )
+    @ddt.unpack
+    def test_scores_action_permissions(self, token_scopes, is_allowed):
+        """
+        Test if write operations on LineItem are allowed with the correct token.
+        """
+        perm_class = LtiAgsPermissions()
+        mock_view = MagicMock()
+
+        # Make token and include it in the mock request
+        token = self._make_token(token_scopes)
+        self.mock_request.headers = {
+            "Authorization": "Bearer {}".format(token)
+        }
+
+        # Test scores view
+        mock_view.action = 'scores'
+        self.assertEqual(
+            perm_class.has_permission(self.mock_request, mock_view),
+            is_allowed,
+        )