From af9dce82a3bf36dbde8209061e2cbf22b4d40ff1 Mon Sep 17 00:00:00 2001 From: Kevin Luo Date: Thu, 6 Nov 2014 11:54:17 -0800 Subject: [PATCH] Make LTI grade submissions respect deadlines --- common/lib/xmodule/xmodule/lti_2_util.py | 3 ++ common/lib/xmodule/xmodule/lti_module.py | 29 +++++++++++++++++-- .../xmodule/xmodule/tests/test_lti20_unit.py | 15 ++++++++++ .../xmodule/xmodule/tests/test_lti_unit.py | 27 ++++++++++++++++- .../courseware/tests/test_lti_integration.py | 1 + 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_2_util.py b/common/lib/xmodule/xmodule/lti_2_util.py index dbfda77787..2067ee09d8 100644 --- a/common/lib/xmodule/xmodule/lti_2_util.py +++ b/common/lib/xmodule/xmodule/lti_2_util.py @@ -64,6 +64,9 @@ class LTI20ModuleMixin(object): if self.system.debug: self._log_correct_authorization_header(request) + if not self.accept_grades_past_due and self.is_past_due(): + return Response(status=404) # have to do 404 due to spec, but 400 is better, with error msg in body + try: anon_id = self.parse_lti_2_0_handler_suffix(suffix) except LTIError: diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 1e02acefba..6881d94dd9 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -51,6 +51,8 @@ What is supported: GET / PUT / DELETE HTTP methods respectively """ +import datetime +from django.utils.timezone import UTC import logging import oauthlib.oauth1 from oauthlib.oauth1.rfc5849 import signature @@ -234,6 +236,13 @@ class LTIFields(object): scope=Scope.settings ) + accept_grades_past_due = Boolean( + display_name=_("Accept grades past deadline"), + help=_("Select True to allow third party systems to post grades past the deadline."), + default=True, + scope=Scope.settings + ) + class LTIModule(LTIFields, LTI20ModuleMixin, XModule): """ @@ -423,7 +432,7 @@ class LTIModule(LTIFields, LTI20ModuleMixin, XModule): 'ask_to_send_username': self.ask_to_send_username, 'ask_to_send_email': self.ask_to_send_email, 'button_text': self.button_text, - + 'accept_grades_past_due': self.accept_grades_past_due, } def get_html(self): @@ -702,7 +711,8 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} 'response': '' } # Returns if: - # - score is out of range; + # - past due grades are not accepted and grade is past due + # - score is out of range # - can't parse response from TP; # - can't verify OAuth signing or OAuth signing is incorrect. failure_values = { @@ -712,6 +722,10 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} 'response': '' } + if not self.accept_grades_past_due and self.is_past_due(): + failure_values['imsx_description'] = "Grade is past due" + return Response(response_xml_template.format(**failure_values), content_type="application/xml") + try: imsx_messageIdentifier, sourcedId, score, action = self.parse_grade_xml_body(request.body) except Exception as e: @@ -862,6 +876,17 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} return key, secret return '', '' + def is_past_due(self): + """ + Is it now past this problem's due date, including grace period? + """ + due_date = self.due # pylint: disable=no-member + if self.graceperiod is not None and due_date: # pylint: disable=no-member + close_date = due_date + self.graceperiod # pylint: disable=no-member + else: + close_date = due_date + return close_date is not None and datetime.datetime.now(UTC()) > close_date + class LTIDescriptor(LTIFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor): """ diff --git a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py index 7c4d9b9fd7..49f16b8464 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti20_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti20_unit.py @@ -1,7 +1,9 @@ # -*- coding: utf-8 -*- """Tests for LTI Xmodule LTIv2.0 functional logic.""" +import datetime import textwrap +from django.utils.timezone import UTC from mock import Mock from xmodule.lti_module import LTIDescriptor from xmodule.lti_2_util import LTIError @@ -21,6 +23,8 @@ class LTI20RESTResultServiceTest(LogicTest): self.system.rebind_noauth_module_to_user = Mock() self.user_id = self.xmodule.runtime.anonymous_student_id self.lti_id = self.xmodule.lti_id + self.xmodule.due = None + self.xmodule.graceperiod = None def test_sanitize_get_context(self): """Tests that the get_context function does basic sanitization""" @@ -369,3 +373,14 @@ class LTI20RESTResultServiceTest(LogicTest): mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") self.assertEqual(response.status_code, 404) + + def test_lti20_request_handler_grade_past_due(self): + """ + Test that we get a 404 when accept_grades_past_due is False and it is past due + """ + self.setup_system_xmodule_mocks_for_lti20_request_test() + self.xmodule.due = datetime.datetime.now(UTC()) + self.xmodule.accept_grades_past_due = False + mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) + response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd") + self.assertEqual(response.status_code, 404) diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index fe3be39fa2..052ae31821 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -1,6 +1,8 @@ # -*- coding: utf-8 -*- """Test for LTI Xmodule functional logic.""" +import datetime +from django.utils.timezone import UTC from mock import Mock, patch, PropertyMock import textwrap from lxml import etree @@ -8,7 +10,7 @@ from webob.request import Request from copy import copy import urllib - +from xmodule.fields import Timedelta from xmodule.lti_module import LTIDescriptor from xmodule.lti_2_util import LTIError @@ -66,6 +68,9 @@ class LTIModuleTest(LogicTest): 'messageIdentifier': '528243ba5241b', } + self.xmodule.due = None + self.xmodule.graceperiod = None + def get_request_body(self, params=None): """Fetches the body of a request specified by params""" if params is None: @@ -161,6 +166,26 @@ class LTIModuleTest(LogicTest): self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) + def test_grade_past_due(self): + """ + Should fail if we do not accept past due grades, and it is past due. + """ + self.xmodule.accept_grades_past_due = False + self.xmodule.due = datetime.datetime.now(UTC()) + self.xmodule.graceperiod = Timedelta().from_json("0 seconds") + request = Request(self.environ) + request.body = self.get_request_body() + response = self.xmodule.grade_handler(request, '') + real_response = self.get_response_values(response) + expected_response = { + 'action': None, + 'code_major': 'failure', + 'description': 'Grade is past due', + 'messageIdentifier': 'unknown', + } + self.assertEqual(response.status_code, 200) + self.assertEqual(expected_response, real_response) + def test_grade_not_in_range(self): """ Grade returned from Tool Provider is outside the range 0.0-1.0. diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index 8c72a183ce..d59a3e2bb2 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -91,6 +91,7 @@ class TestLTI(BaseTestXmodule): 'ask_to_send_email': self.item_descriptor.ask_to_send_email, 'description': self.item_descriptor.description, 'button_text': self.item_descriptor.button_text, + 'accept_grades_past_due': self.item_descriptor.accept_grades_past_due, } def mocked_sign(self, *args, **kwargs):