Commit d5cadf00 by Waheed Ahmed

Really large posts to ORA like peer grading feedback fixed.

ORA-201
parent 0c7e2036
......@@ -23,6 +23,7 @@ log = logging.getLogger(__name__)
EXTERNAL_GRADER_NO_CONTACT_ERROR = "Failed to contact external graders. Please notify course staff."
MAX_ALLOWED_FEEDBACK_LENGTH = 5000
class PeerGradingFields(object):
......@@ -344,6 +345,10 @@ class PeerGradingModule(PeerGradingFields, XModule):
if not success:
return self._err_response(message)
success, message = self._check_feedback_length(data)
if not success:
return self._err_response(message)
data_dict = {k:data.get(k) for k in required}
if 'rubric_scores[]' in required:
data_dict['rubric_scores'] = data.getall('rubric_scores[]')
......@@ -638,6 +643,15 @@ class PeerGradingModule(PeerGradingFields, XModule):
return json.dumps(state)
def _check_feedback_length(self, data):
feedback = data.get("feedback")
if feedback and len(feedback) > MAX_ALLOWED_FEEDBACK_LENGTH:
return False, "Feedback is too long, Max length is {0} characters.".format(
MAX_ALLOWED_FEEDBACK_LENGTH
)
else:
return True, ""
class PeerGradingDescriptor(PeerGradingFields, RawDescriptor):
"""
......
......@@ -11,7 +11,7 @@ from xmodule.modulestore import Location
from xmodule.tests import get_test_system, get_test_descriptor_system
from xmodule.tests.test_util_open_ended import DummyModulestore
from xmodule.open_ended_grading_classes.peer_grading_service import MockPeerGradingService
from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor
from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor, MAX_ALLOWED_FEEDBACK_LENGTH
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
log = logging.getLogger(__name__)
......@@ -158,6 +158,27 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
"""
self.peer_grading.get_instance_state()
def test_save_grade_with_long_feedback(self):
"""
Test if feedback is too long save_grade() should return error message.
"""
feedback_fragment = "This is very long feedback."
self.save_dict["feedback"] = feedback_fragment * (
(MAX_ALLOWED_FEEDBACK_LENGTH / len(feedback_fragment) + 1)
)
response = self.peer_grading.save_grade(self.save_dict)
# Should not succeed.
self.assertEqual(response['success'], False)
self.assertEqual(
response['error'],
"Feedback is too long, Max length is {0} characters.".format(
MAX_ALLOWED_FEEDBACK_LENGTH
)
)
class MockPeerGradingServiceProblemList(MockPeerGradingService):
def get_problem_list(self, course_id, grader_id):
......
......@@ -30,6 +30,7 @@ STAFF_ERROR_MESSAGE = _(
tech_support_email=settings.TECH_SUPPORT_EMAIL
)
)
MAX_ALLOWED_FEEDBACK_LENGTH = 5000
class MockStaffGradingService(object):
......@@ -353,19 +354,21 @@ def save_grade(request, course_id):
#If the instructor has skipped grading the submission, then there will not be any rubric scores.
#Only add in the rubric scores if the instructor has not skipped.
if not skipped:
required|=set(['rubric_scores[]'])
required.add('rubric_scores[]')
actual = set(p.keys())
missing = required - actual
if len(missing) > 0:
return _err_response('Missing required keys {0}'.format(
', '.join(missing)))
grader_id = unique_id_for_user(request.user)
success, message = check_feedback_length(p)
if not success:
return _err_response(message)
grader_id = unique_id_for_user(request.user)
location = p['location']
try:
result_json = staff_grading_service().save_grade(course_id,
grader_id,
......@@ -402,3 +405,13 @@ def save_grade(request, course_id):
# Ok, save_grade seemed to work. Get the next submission to grade.
return HttpResponse(_get_next(course_id, grader_id, location),
mimetype="application/json")
def check_feedback_length(data):
feedback = data.get("feedback")
if feedback and len(feedback) > MAX_ALLOWED_FEEDBACK_LENGTH:
return False, "Feedback is too long, Max length is {0} characters.".format(
MAX_ALLOWED_FEEDBACK_LENGTH
)
else:
return True, ""
......@@ -217,6 +217,40 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
# Check that the error text is correct.
self.assertIn("Cannot find", response['error'])
def test_save_grade_with_long_feedback(self):
"""
Test if feedback is too long save_grade() should return error message.
"""
self.login(self.instructor, self.password)
url = reverse('staff_grading_save_grade', kwargs={'course_id': self.course_id})
data = {
'score': '12',
'feedback': '',
'submission_id': '123',
'location': self.location,
'submission_flagged': "false",
'rubric_scores[]': ['1', '2']
}
feedback_fragment = "This is very long feedback."
data["feedback"] = feedback_fragment * (
(staff_grading_service.MAX_ALLOWED_FEEDBACK_LENGTH / len(feedback_fragment) + 1)
)
response = check_for_post_code(self, 200, url, data)
content = json.loads(response.content)
# Should not succeed.
self.assertEquals(content['success'], False)
self.assertEquals(
content['error'],
"Feedback is too long, Max length is {0} characters.".format(
staff_grading_service.MAX_ALLOWED_FEEDBACK_LENGTH
)
)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
......@@ -371,6 +405,38 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.assertTrue(response['error'].find('Missing required keys:') > -1)
self.assertFalse('actual_score' in response)
def test_save_grade_with_long_feedback(self):
"""
Test if feedback is too long save_grade() should return error message.
"""
data = {
'rubric_scores[]': [0, 0],
'location': self.location,
'submission_id': 1,
'submission_key': 'fake key',
'score': 2,
'feedback': '',
'submission_flagged': 'false',
'answer_unknown': 'false',
'rubric_scores_complete': 'true'
}
feedback_fragment = "This is very long feedback."
data["feedback"] = feedback_fragment * (
(staff_grading_service.MAX_ALLOWED_FEEDBACK_LENGTH / len(feedback_fragment) + 1)
)
response_dict = self.peer_module.save_grade(data)
# Should not succeed.
self.assertEquals(response_dict['success'], False)
self.assertEquals(
response_dict['error'],
"Feedback is too long, Max length is {0} characters.".format(
staff_grading_service.MAX_ALLOWED_FEEDBACK_LENGTH
)
)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class TestPanel(ModuleStoreTestCase):
......
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