moved listener to signal.py, refactored models.py, added due and star…

…t date, updated tests.

lti_consumer/apps.py

@@ -25,3 +25,7 @@ class LTIConsumerApp(AppConfig):
             }
         }
     }
+
+    def ready(self):
+        # pylint: disable=unused-import,import-outside-toplevel
+        import lti_consumer.signals

lti_consumer/lti_1p3/ags.py

@@ -18,7 +18,7 @@ class LtiAgs:
     """
     def __init__(
         self,
-        lineitems_url=None,
+        lineitems_url,
         lineitem_url=None,
         allow_creating_lineitems=True,
         results_service_enabled=True,
@@ -63,11 +63,17 @@ def get_lti_ags_launch_claim(self):
         """
         Returns LTI AGS Claim to be injected in the LTI launch message.
         """
+
+        claim_values = {
+            "scope": self.get_available_scopes(),
+            "lineitems": self.lineitems_url,
+        }
+
+        if self.lineitem_url:
+            claim_values["lineitem"] = self.lineitem_url
+
         ags_claim = {
-            "https://purl.imsglobal.org/spec/lti-ags/claim/endpoint": {
-                "scope": self.get_available_scopes(),
-                "lineitems": self.lineitems_url,
-            }
+            "https://purl.imsglobal.org/spec/lti-ags/claim/endpoint": claim_values
         }
 
         return ags_claim

lti_consumer/models.py

@@ -2,8 +2,6 @@
 LTI configuration and linking models.
 """
 from django.db import models
-from django.db.models.signals import post_save
-from django.dispatch import receiver
 from django.core.validators import MinValueValidator
 from django.core.exceptions import ValidationError
 
@@ -17,7 +15,7 @@
     get_lms_base,
     get_lti_ags_lineitems_url,
 )
-from lti_consumer.plugin.compat import submit_grade
+from lti_consumer.plugin.compat import load_block
 
 
 class LtiConfiguration(models.Model):
@@ -83,11 +81,7 @@ def block(self):
         if block is None:
             if self.location is None:
                 raise ValueError("Block location not set, it's not possible to retrieve the block.")
-
-            # Import on runtime only
-            # pylint: disable=import-outside-toplevel,import-error
-            from xmodule.modulestore.django import modulestore
-            block = self._block = modulestore().get_item(self.location)
+            block = self._block = load_block(self.location)
         return block
 
     @block.setter
@@ -139,14 +133,23 @@ def _get_lti_1p3_consumer(self):
             # Check if enabled and setup LTI-AGS
             if self.block.has_score:
 
+                default_values = {
+                    'resource_id': self.block.location,
+                    'score_maximum': self.block.weight,
+                    'label': self.block.display_name
+                }
+
+                if hasattr(self.block, 'start'):
+                    default_values['start_date_time'] = self.block.start
+
+                if hasattr(self.block, 'due'):
+                    default_values['end_date_time'] = self.block.due
+
                 # create LineItem if there is none for current lti configuration
                 lineitem, _ = LtiAgsLineItem.objects.get_or_create(
                     lti_configuration=self,
-                    resource_id=self.block.location,
-                    defaults={
-                        'score_maximum': self.block.weight,
-                        'label': self.block.display_name
-                    }
+                    resource_link_id=self.block.location,
+                    defaults=default_values
                 )
 
                 consumer.enable_ags(
@@ -314,13 +317,3 @@ def __str__(self):
 
     class Meta:
         unique_together = (('line_item', 'user_id'),)
-
-
-@receiver(post_save, sender=LtiAgsScore)
-def update_student_grade(sender, instance, **kwargs):  # pylint: disable=unused-argument
-    """
-    Submit grade to xblock whenever score saved/updated and its
-    grading_progress is set to FullyGraded.
-    """
-    if instance.grading_progress == LtiAgsScore.FULLY_GRADED:
-        submit_grade(instance)

lti_consumer/plugin/compat.py

@@ -41,27 +41,27 @@ def get_user_from_external_user_id(external_user_id):
         raise LtiError('Invalid userID') from exception
 
 
-def submit_grade(score):
+def load_block(key):
+    # pylint: disable=import-outside-toplevel,import-error
+    from xmodule.modulestore.django import modulestore
+    return modulestore().get_item(key)
+
+
+def publish_grade(block, user, score, possible, only_if_higher=False, score_deleted=None, comment=None):
     """
-    Import grades signals and submit grade given a LtiAgsScore instance.
+    Import grades signals and publishes score by triggering SCORE_PUBLISHED signal.
     """
     # pylint: disable=import-error,import-outside-toplevel
     from lms.djangoapps.grades.api import signals as grades_signals
 
-    # get lti config
-    lti_config = score.line_item.lti_configuration
-
-    # find user
-    user = get_user_from_external_user_id(score.user_id)
-
     # publish score
     grades_signals.SCORE_PUBLISHED.send(
         sender=None,
-        block=lti_config.block,
+        block=block,
         user=user,
-        raw_earned=score.score_given,
-        raw_possible=score.score_maximum,
-        only_if_higher=False,
-        # score_deleted=event.get('score_deleted'),
-        grader_response=score.comment
+        raw_earned=score,
+        raw_possible=possible,
+        only_if_higher=only_if_higher,
+        score_deleted=score_deleted,
+        grader_response=comment
     )

lti_consumer/signals.py

@@ -0,0 +1,31 @@
+"""
+LTI Consumer related Signal handlers
+"""
+
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+
+from lti_consumer.models import LtiAgsScore
+from lti_consumer.plugin.compat import (
+    publish_grade,
+    load_block,
+    get_user_from_external_user_id,
+)
+
+
+@receiver(post_save, sender=LtiAgsScore, dispatch_uid='publish_grade_on_score_update')
+def publish_grade_on_score_update(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+    Publish grade to xblock whenever score saved/updated and its grading_progress is set to FullyGraded.
+    """
+    if instance.grading_progress == LtiAgsScore.FULLY_GRADED:
+        block = load_block(instance.line_item.resource_link_id)
+        if not block.is_past_due():
+            user = get_user_from_external_user_id(instance.user_id)
+            publish_grade(
+                block,
+                user,
+                instance.score_given,
+                instance.score_maximum,
+                comment=instance.comment
+            )

lti_consumer/tests/unit/plugin/test_views_lti_ags.py

@@ -2,7 +2,7 @@
 Tests for LTI Advantage Assignments and Grades Service views.
 """
 import json
-from mock import patch, PropertyMock
+from mock import patch, PropertyMock, MagicMock
 
 from Cryptodome.PublicKey import RSA
 import ddt
@@ -50,6 +50,8 @@ def setUp(self):
         # Set dummy location so that UsageKey lookup is valid
         self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test'
 
+        self.xblock.is_past_due = MagicMock(return_value=False)
+
         # Create configuration
         self.lti_config = LtiConfiguration.objects.create(
             location=str(self.xblock.location),
@@ -67,12 +69,26 @@ def setUp(self):
         self.addCleanup(patcher.stop)
         self._lti_block_patch = patcher.start()
 
-        submit_grade_patcher = patch(
-            'lti_consumer.models.submit_grade',
+        publish_grade_patcher = patch(
+            'lti_consumer.signals.publish_grade',
+            return_value=None
+        )
+        self.addCleanup(publish_grade_patcher.stop)
+        self._publish_grade_patcher = publish_grade_patcher.start()
+
+        load_block_patcher = patch(
+            'lti_consumer.signals.load_block',
+            return_value=self.xblock
+        )
+        self.addCleanup(load_block_patcher.stop)
+        self._load_block_patcher = load_block_patcher.start()
+
+        get_user_from_external_user_id_patcher = patch(
+            'lti_consumer.signals.get_user_from_external_user_id',
             return_value=None
         )
-        self.addCleanup(submit_grade_patcher.stop)
-        self._submit_block_patch = submit_grade_patcher.start()
+        self.addCleanup(get_user_from_external_user_id_patcher.stop)
+        self._get_user_from_external_user_id_patcher = get_user_from_external_user_id_patcher.start()
 
     def _set_lti_token(self, scopes=None):
         """
@@ -399,7 +415,7 @@ def test_create_score(self):
         LtiAgsScore.FAILED,
         LtiAgsScore.NOT_READY
     )
-    def test_xblock_grade_submit_on_score_save(self, grading_progress):
+    def test_xblock_grade_publish_on_score_save(self, grading_progress):
         """
         Test on LtiAgsScore save, if gradingProgress is Fully Graded then xblock grade should be submitted.
         """
@@ -421,12 +437,42 @@ def test_xblock_grade_submit_on_score_save(self, grading_progress):
 
         if grading_progress == LtiAgsScore.FULLY_GRADED:
             score = LtiAgsScore.objects.get(line_item=self.line_item, user_id=self.primary_user_id)
-            self._submit_block_patch.assert_called_once()
-            call_args = self._submit_block_patch.call_args.args
-            self.assertEqual(len(call_args), 1)
-            self.assertEqual(call_args[0], score)
+            self._publish_grade_patcher.assert_called_once()
+            self._get_user_from_external_user_id_patcher.assert_called_once()
+            self._load_block_patcher.assert_called_once()
+            call_args = self._publish_grade_patcher.call_args.args
+            call_kwargs = self._publish_grade_patcher.call_args.kwargs
+            self.assertEqual(call_args, (self.xblock, None, score.score_given, score.score_maximum,))
+            self.assertEqual(call_kwargs['comment'], score.comment)
         else:
-            self._submit_block_patch.assert_not_called()
+            self._load_block_patcher.assert_not_called()
+            self._get_user_from_external_user_id_patcher.assert_not_called()
+            self._publish_grade_patcher.assert_not_called()
+
+    def test_xblock_grade_publish_passed_due_date(self):
+        """
+        Test grade publish after due date. Grade shouldn't published
+        """
+        self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/score')
+
+        self.xblock.is_past_due = MagicMock(return_value=True)
+        self.client.post(
+            self.scores_endpoint,
+            data=json.dumps({
+                "timestamp": self.early_timestamp,
+                "scoreGiven": 83,
+                "scoreMaximum": 100,
+                "comment": "This is exceptional work.",
+                "activityProgress": LtiAgsScore.COMPLETED,
+                "gradingProgress": LtiAgsScore.FULLY_GRADED,
+                "userId": self.primary_user_id
+            }),
+            content_type="application/vnd.ims.lis.v1.score+json",
+        )
+
+        self._load_block_patcher.assert_called_once()
+        self._get_user_from_external_user_id_patcher.assert_not_called()
+        self._publish_grade_patcher.assert_not_called()
 
     def test_create_multiple_scores_with_multiple_users(self):
         """

lti_consumer/tests/unit/test_models.py

@@ -5,7 +5,7 @@
 from django.test.testcases import TestCase
 
 from jwkest.jwk import RSAKey
-from mock import patch
+from mock import patch, MagicMock
 
 from lti_consumer.lti_xblock import LtiConsumerXBlock
 from lti_consumer.models import LtiAgsLineItem, LtiConfiguration, LtiAgsScore
@@ -103,7 +103,8 @@ def test_lti_consumer_ags_enabled(self):
                         'https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly',
                         'https://purl.imsglobal.org/spec/lti-ags/scope/score',
                     ],
-                    'lineitems': 'https://example.com/api/lti_consumer/v1/lti/2/lti-ags'
+                    'lineitems': 'https://example.com/api/lti_consumer/v1/lti/2/lti-ags',
+                    'lineitem': 'https://example.com/api/lti_consumer/v1/lti/2/lti-ags/1'
                 }
             }
         )
@@ -142,12 +143,26 @@ class TestLtiAgsScoreModel(TestCase):
     def setUp(self):
         super().setUp()
 
-        submit_grade_patcher = patch(
-            'lti_consumer.models.submit_grade',
+        publish_grade_patcher = patch(
+            'lti_consumer.signals.publish_grade',
             return_value=None
         )
-        self.addCleanup(submit_grade_patcher.stop)
-        self._submit_block_patch = submit_grade_patcher.start()
+        self.addCleanup(publish_grade_patcher.stop)
+        self._publish_grade_patcher = publish_grade_patcher.start()
+
+        load_block_patcher = patch(
+            'lti_consumer.signals.load_block',
+            return_value=make_xblock('lti_consumer', LtiConsumerXBlock, {}, MagicMock(return_value=False))
+        )
+        self.addCleanup(load_block_patcher.stop)
+        self._load_block_patcher = load_block_patcher.start()
+
+        get_user_from_external_user_id_patcher = patch(
+            'lti_consumer.signals.get_user_from_external_user_id',
+            return_value=None
+        )
+        self.addCleanup(get_user_from_external_user_id_patcher.stop)
+        self._get_user_from_external_user_id_patcher = get_user_from_external_user_id_patcher.start()
 
         self.dummy_location = 'block-v1:course+test+2020+type@problem+block@test'
         self.line_item = LtiAgsLineItem.objects.create(
@@ -176,3 +191,11 @@ def test_repr(self):
             str(self.score),
             "LineItem 1: score 10.0 out of 100.0 - FullyGraded"
         )
+
+    def test_score_update_signal(self):
+        """
+        Test score update signal connected
+        """
+        self._load_block_patcher.assert_called_once()
+        self._get_user_from_external_user_id_patcher.assert_called_once()
+        self._publish_grade_patcher.assert_called_once()

lti_consumer/tests/unit/test_utils.py

@@ -11,7 +11,7 @@
 FAKE_USER_ID = 'fake_user_id'
 
 
-def make_xblock(xblock_name, xblock_cls, attributes):
+def make_xblock(xblock_name, xblock_cls, attributes, is_past_due_patch=None):
     """
     Helper to construct XBlock objects
     """
@@ -27,6 +27,11 @@ def make_xblock(xblock_name, xblock_cls, attributes):
     xblock.runtime = Mock(
         hostname='localhost',
     )
+
+    # mock is_past_due method
+    if is_past_due_patch:
+        xblock.is_past_due = is_past_due_patch
+
     xblock.course_id = 'course-v1:edX+DemoX+Demo_Course'
     for key, value in attributes.items():
         setattr(xblock, key, value)

lti_consumer/utils.py

@@ -74,12 +74,14 @@ def get_lti_ags_lineitems_url(lti_config_id, lineitem_id=None):
     :param lineitem_id: LTI Line Item id. Single line item if given an id,
         otherwise returns list url
     """
-    url = "{lms_base}/api/lti_consumer/v1/lti/{lti_config_id}/lti-ags".format(
+    base_ags_url = "{lms_base}/api/lti_consumer/v1/lti/{lti_config_id}/lti-ags".format(
         lms_base=get_lms_base(),
         lti_config_id=str(lti_config_id),
     )
 
     if lineitem_id:
-        url = "{url}/{lineitem_id}".format(url=url, lineitem_id=lineitem_id)
+        url = "{url}/{lineitem_id}".format(url=base_ags_url, lineitem_id=lineitem_id)
+    else:
+        url = base_ags_url
 
     return url