Commit 3d4a853e by Usman Khalid

Merge pull request #5926 from Stanford-Online/kluo/lti-grades-past-due

Make LTI grade submissions respect deadlines
parents fcd7736d af9dce82
...@@ -64,6 +64,9 @@ class LTI20ModuleMixin(object): ...@@ -64,6 +64,9 @@ class LTI20ModuleMixin(object):
if self.system.debug: if self.system.debug:
self._log_correct_authorization_header(request) 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: try:
anon_id = self.parse_lti_2_0_handler_suffix(suffix) anon_id = self.parse_lti_2_0_handler_suffix(suffix)
except LTIError: except LTIError:
......
...@@ -51,6 +51,8 @@ What is supported: ...@@ -51,6 +51,8 @@ What is supported:
GET / PUT / DELETE HTTP methods respectively GET / PUT / DELETE HTTP methods respectively
""" """
import datetime
from django.utils.timezone import UTC
import logging import logging
import oauthlib.oauth1 import oauthlib.oauth1
from oauthlib.oauth1.rfc5849 import signature from oauthlib.oauth1.rfc5849 import signature
...@@ -234,6 +236,13 @@ class LTIFields(object): ...@@ -234,6 +236,13 @@ class LTIFields(object):
scope=Scope.settings 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): class LTIModule(LTIFields, LTI20ModuleMixin, XModule):
""" """
...@@ -423,7 +432,7 @@ 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_username': self.ask_to_send_username,
'ask_to_send_email': self.ask_to_send_email, 'ask_to_send_email': self.ask_to_send_email,
'button_text': self.button_text, 'button_text': self.button_text,
'accept_grades_past_due': self.accept_grades_past_due,
} }
def get_html(self): def get_html(self):
...@@ -702,7 +711,8 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -702,7 +711,8 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
'response': '' 'response': ''
} }
# Returns if: # 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 parse response from TP;
# - can't verify OAuth signing or OAuth signing is incorrect. # - can't verify OAuth signing or OAuth signing is incorrect.
failure_values = { failure_values = {
...@@ -712,6 +722,10 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -712,6 +722,10 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
'response': '' '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: try:
imsx_messageIdentifier, sourcedId, score, action = self.parse_grade_xml_body(request.body) imsx_messageIdentifier, sourcedId, score, action = self.parse_grade_xml_body(request.body)
except Exception as e: except Exception as e:
...@@ -862,6 +876,17 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} ...@@ -862,6 +876,17 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
return key, secret return key, secret
return '', '' 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): class LTIDescriptor(LTIFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor):
""" """
......
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
"""Tests for LTI Xmodule LTIv2.0 functional logic.""" """Tests for LTI Xmodule LTIv2.0 functional logic."""
import datetime
import textwrap import textwrap
from django.utils.timezone import UTC
from mock import Mock from mock import Mock
from xmodule.lti_module import LTIDescriptor from xmodule.lti_module import LTIDescriptor
from xmodule.lti_2_util import LTIError from xmodule.lti_2_util import LTIError
...@@ -21,6 +23,8 @@ class LTI20RESTResultServiceTest(LogicTest): ...@@ -21,6 +23,8 @@ class LTI20RESTResultServiceTest(LogicTest):
self.system.rebind_noauth_module_to_user = Mock() self.system.rebind_noauth_module_to_user = Mock()
self.user_id = self.xmodule.runtime.anonymous_student_id self.user_id = self.xmodule.runtime.anonymous_student_id
self.lti_id = self.xmodule.lti_id self.lti_id = self.xmodule.lti_id
self.xmodule.due = None
self.xmodule.graceperiod = None
def test_sanitize_get_context(self): def test_sanitize_get_context(self):
"""Tests that the get_context function does basic sanitization""" """Tests that the get_context function does basic sanitization"""
...@@ -369,3 +373,14 @@ class LTI20RESTResultServiceTest(LogicTest): ...@@ -369,3 +373,14 @@ class LTI20RESTResultServiceTest(LogicTest):
mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT) 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") response = self.xmodule.lti_2_0_result_rest_handler(mock_request, "user/abcd")
self.assertEqual(response.status_code, 404) 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)
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
"""Test for LTI Xmodule functional logic.""" """Test for LTI Xmodule functional logic."""
import datetime
from django.utils.timezone import UTC
from mock import Mock, patch, PropertyMock from mock import Mock, patch, PropertyMock
import textwrap import textwrap
from lxml import etree from lxml import etree
...@@ -8,7 +10,7 @@ from webob.request import Request ...@@ -8,7 +10,7 @@ from webob.request import Request
from copy import copy from copy import copy
import urllib import urllib
from xmodule.fields import Timedelta
from xmodule.lti_module import LTIDescriptor from xmodule.lti_module import LTIDescriptor
from xmodule.lti_2_util import LTIError from xmodule.lti_2_util import LTIError
...@@ -66,6 +68,9 @@ class LTIModuleTest(LogicTest): ...@@ -66,6 +68,9 @@ class LTIModuleTest(LogicTest):
'messageIdentifier': '528243ba5241b', 'messageIdentifier': '528243ba5241b',
} }
self.xmodule.due = None
self.xmodule.graceperiod = None
def get_request_body(self, params=None): def get_request_body(self, params=None):
"""Fetches the body of a request specified by params""" """Fetches the body of a request specified by params"""
if params is None: if params is None:
...@@ -161,6 +166,26 @@ class LTIModuleTest(LogicTest): ...@@ -161,6 +166,26 @@ class LTIModuleTest(LogicTest):
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) 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): def test_grade_not_in_range(self):
""" """
Grade returned from Tool Provider is outside the range 0.0-1.0. Grade returned from Tool Provider is outside the range 0.0-1.0.
......
...@@ -91,6 +91,7 @@ class TestLTI(BaseTestXmodule): ...@@ -91,6 +91,7 @@ class TestLTI(BaseTestXmodule):
'ask_to_send_email': self.item_descriptor.ask_to_send_email, 'ask_to_send_email': self.item_descriptor.ask_to_send_email,
'description': self.item_descriptor.description, 'description': self.item_descriptor.description,
'button_text': self.item_descriptor.button_text, 'button_text': self.item_descriptor.button_text,
'accept_grades_past_due': self.item_descriptor.accept_grades_past_due,
} }
def mocked_sign(self, *args, **kwargs): def mocked_sign(self, *args, **kwargs):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment