Updated models, apis, and tests.

More documentation, repr, corner case testing, and proper validation

apps/submissions/api.py

@@ -14,7 +14,17 @@ from submissions.models import Submission, StudentItem
 logger = logging.getLogger(__name__)
 
 
-class SubmissionInternalError(Exception):
+class SubmissionError(Exception):
+    """An error that occurs during submission actions.
+
+    This error is raised when the submission API cannot perform a requested
+    action.
+
+    """
+    pass
+
+
+class SubmissionInternalError(SubmissionError):
     """An error internal to the Submission API has occurred.
 
     This error is raised when an error occurs that is not caused by incorrect
@@ -25,7 +35,7 @@ class SubmissionInternalError(Exception):
     pass
 
 
-class SubmissionNotFoundError(Exception):
+class SubmissionNotFoundError(SubmissionError):
     """This error is raised when no submission is found for the request.
 
     If a state is specified in a call to the API that results in no matching
@@ -35,21 +45,16 @@ class SubmissionNotFoundError(Exception):
     pass
 
 
-class SubmissionRequestError(Exception):
+class SubmissionRequestError(SubmissionError):
     """This error is raised when there was a request-specific error
 
     This error is reserved for problems specific to the use of the API.
 
     """
+
     def __init__(self, field_errors):
+        Exception.__init__(self, repr(field_errors))
         self.field_errors = copy.deepcopy(field_errors)
-        super(SubmissionRequestError, self).__init__()
-
-    def __unicode__(self):
-        return repr(self)
-
-    def __repr__(self):
-        return "SubmissionRequestError({!r})".format(self.field_errors)
 
 
 def create_submission(student_item_dict, answer, submitted_at=None,
@@ -63,12 +68,13 @@ def create_submission(student_item_dict, answer, submitted_at=None,
             submission is associated with. This is used to determine which
             course, student, and location this submission belongs to.
         answer (str): The answer given by the student to be evaluated.
-        submitted_at (date): The date in which this submission was submitted.
+        submitted_at (datetime): The date in which this submission was submitted.
             If not specified, defaults to the current date.
         attempt_number (int): A student may be able to submit multiple attempts
             per question. This allows the designated attempt to be overridden.
-            If the attempt is not specified, it will be incremented by the
-            number of submissions associated with this student_item.
+            If the attempt is not specified, it will take the most recent
+            submission, as specified by the submitted_at time, and use its
+            attempt_number plus one.
 
     Returns:
         dict: A representation of the created Submission.
@@ -84,27 +90,37 @@ def create_submission(student_item_dict, answer, submitted_at=None,
     if attempt_number is None:
         try:
             submissions = Submission.objects.filter(
-                student_item=student_item_model)[:0]
+                student_item=student_item_model)[:1]
         except DatabaseError:
-            error_message = u"An error occurred while filtering "
-            u"submissions for student item: {}".format(student_item_dict)
+            error_message = u"An error occurred while filtering submissions for student item: {}".format(
+                student_item_dict)
             logger.exception(error_message)
             raise SubmissionInternalError(error_message)
         attempt_number = submissions[0].attempt_number + 1 if submissions else 1
 
+    try:
+        answer = force_unicode(answer)
+    except UnicodeDecodeError:
+        raise SubmissionRequestError(u"Submission answer could not be properly decoded to unicode.")
+
     model_kwargs = {
         "student_item": student_item_model,
-        "answer": force_unicode(answer),
+        "answer": answer,
         "attempt_number": attempt_number,
     }
     if submitted_at:
         model_kwargs["submitted_at"] = submitted_at
 
     try:
+        validation_data = model_kwargs.copy()
+        validation_data["student_item"] = student_item_model.pk
+        submission_serializer = SubmissionSerializer(data=validation_data)
+        submission_serializer.is_valid()
+        if submission_serializer.errors:
+            raise SubmissionRequestError(submission_serializer.errors)
         submission = Submission.objects.create(**model_kwargs)
     except DatabaseError:
-        error_message = u"An error occurred while creating "
-        u"submission {} for student item: {}".format(
+        error_message = u"An error occurred while creating submission {} for student item: {}".format(
             model_kwargs,
             student_item_dict
         )
@@ -141,19 +157,19 @@ def get_submissions(student_item_dict, limit=None):
     """
     student_item_model = _get_or_create_student_item(student_item_dict)
     try:
-        submission_models = Submission.objects.filter(
-            student_item=student_item_model)
+        submission_models = Submission.objects.filter(student_item=student_item_model)
     except DatabaseError:
-        error_message = u"Error getting submission request for student item {}".format(
-            student_item_dict)
+        error_message = (
+            u"Error getting submission request for student item {}"
+            .format(student_item_dict)
+        )
         logger.exception(error_message)
         raise SubmissionNotFoundError(error_message)
 
     if limit:
         submission_models = submission_models[:limit]
 
-    return [SubmissionSerializer(submission).data for
-            submission in submission_models]
+    return [SubmissionSerializer(submission).data for submission in submission_models]
 
 
 def get_score(student_item):
@@ -187,6 +203,16 @@ def _get_or_create_student_item(student_item_dict):
         SubmissionRequestError: Thrown if the given student item parameters fail
             validation.
 
+    Examples:
+        >>> student_item_dict = dict(
+        >>>    student_id="Tim",
+        >>>    item_id="item_1",
+        >>>    course_id="course_1",
+        >>>    item_type="type_one"
+        >>> )
+        >>> _get_or_create_student_item(student_item_dict)
+        {'item_id': 'item_1', 'item_type': 'type_one', 'course_id': 'course_1', 'student_id': 'Tim'}
+
     """
     try:
         try:

apps/submissions/models.py

@@ -15,8 +15,6 @@ Things to consider probably aren't worth the extra effort/complexity in the MVP:
 * Version ID (this doesn't work until split-mongostore comes into being)
 
 """
-from collections import namedtuple
-
 from django.db import models
 from django.utils.timezone import now
 
@@ -41,6 +39,14 @@ class StudentItem(models.Model):
     # What kind of problem is this? The XBlock tag if it's an XBlock
     item_type = models.CharField(max_length=100)
 
+    def __repr__(self):
+        return repr(dict(
+            student_id=self.student_id,
+            course_id=self.course_id,
+            item_id=self.item_id,
+            item_type=self.item_type,
+        ))
+
     class Meta:
         unique_together = (
             # For integrity reasons, and looking up all of a student's items
@@ -70,6 +76,15 @@ class Submission(models.Model):
     # The actual answer, assumed to be a JSON string
     answer = models.TextField(blank=True)
 
+    def __repr__(self):
+        return repr(dict(
+            student_item=self.student_item,
+            attempt_number=self.attempt_number,
+            submitted_at=self.submitted_at,
+            created_at=self.created_at,
+            answer=self.answer,
+        ))
+
     class Meta:
         ordering = ["-submitted_at"]
 
@@ -84,3 +99,12 @@ class Score(models.Model):
     points_possible = models.PositiveIntegerField(default=0)
     created_at = models.DateTimeField(editable=False, default=now, db_index=True)
 
+    def __repr__(self):
+        return repr(dict(
+            student_item=self.student_item,
+            submission=self.submission,
+            created_at=self.created_at,
+            points_earned=self.points_earned,
+            points_possible=self.points_possible,
+        ))
+

apps/submissions/tests/test_api.py

 import datetime
 
+from ddt import ddt, file_data
 from django.db import DatabaseError
 from django.test import TestCase
 from nose.tools import raises
@@ -15,16 +16,11 @@ STUDENT_ITEM = dict(
     item_type="Peer_Submission",
 )
 
-BAD_STUDENT_ITEM = dict(
-    student_id="Bad Tim",
-    course_id=451,
-    item_id=True,
-)
-
 ANSWER_ONE = u"this is my answer!"
 ANSWER_TWO = u"this is my other answer!"
 
 
+@ddt
 class TestApi(TestCase):
     def test_create_submission(self):
         submission = create_submission(STUDENT_ITEM, ANSWER_ONE)
@@ -36,7 +32,13 @@ class TestApi(TestCase):
         submissions = get_submissions(STUDENT_ITEM)
 
         self._assert_submission(submissions[1], ANSWER_ONE, 1, 1)
-        self._assert_submission(submissions[0], ANSWER_TWO, 1, 1)
+        self._assert_submission(submissions[0], ANSWER_TWO, 1, 2)
+
+    @file_data('test_valid_student_items.json')
+    def test_various_student_items(self, valid_student_item):
+        create_submission(valid_student_item, ANSWER_ONE)
+        submission = get_submissions(valid_student_item)[0]
+        self._assert_submission(submission, ANSWER_ONE, 1, 1)
 
     def test_get_latest_submission(self):
         past_date = datetime.date(2007, 11, 23)
@@ -57,8 +59,13 @@ class TestApi(TestCase):
         self._assert_submission(submissions[0], ANSWER_ONE, 1, 2)
 
     @raises(SubmissionRequestError)
-    def test_error_checking(self):
-        create_submission(BAD_STUDENT_ITEM, -100)
+    @file_data('test_bad_student_items.json')
+    def test_error_checking(self, bad_student_item):
+        create_submission(bad_student_item, -100)
+
+    @raises(SubmissionRequestError)
+    def test_error_checking_submissions(self):
+        create_submission(STUDENT_ITEM, ANSWER_ONE, None, -1)
 
     @patch.object(Submission.objects, 'filter')
     @raises(SubmissionInternalError)
@@ -68,10 +75,15 @@ class TestApi(TestCase):
 
     @patch.object(StudentItem.objects, 'create')
     @raises(SubmissionInternalError)
-    def test_error_on_create_student_item(self, mock_create):
+    def test_create_student_item_validation(self, mock_create):
         mock_create.side_effect = DatabaseError("Bad things happened")
         create_submission(STUDENT_ITEM, ANSWER_ONE)
 
+    def test_unicode_enforcement(self):
+        create_submission(STUDENT_ITEM, "Testing unicode answers.")
+        submissions = get_submissions(STUDENT_ITEM, 1)
+        self.assertEqual(u"Testing unicode answers.", submissions[0]["answer"])
+
     def _assert_submission(self, submission, expected_answer, expected_item,
                            expected_attempt):
         self.assertIsNotNone(submission)

apps/submissions/tests/test_bad_student_items.json

+{
+    "no_item_type": {
+        "student_id": "Bad Tim",
+        "course_id": "451",
+        "item_id": "2"
+    },
+    "no_student_id": {
+        "course_id": "Course_One",
+        "item_id": "5",
+        "item_type": "Peer"
+    },
+    "just_student_and_course": {
+        "student_id": "Tim",
+        "course_id": "Course_One"
+    },
+    "just_student_id": {
+        "student_id": "Tim"
+    },
+    "just_item_id_and_type": {
+        "item_id": "5",
+        "item_type": "Peer"
+    },
+    "just_course_id": {
+        "course_id": "Course_One"
+    },
+    "just_item_id": {
+        "item_id": "5"
+    },
+    "bad_item_id_empty": {
+        "student_id": "Tim",
+        "course_id": "Course_One",
+        "item_id": "",
+        "item_type": "Peer"
+    }
+}
\ No newline at end of file

apps/submissions/tests/test_valid_student_items.json

+{
+    "unicode_characters": {
+        "student_id": "学生",
+        "course_id": "漢字",
+        "item_id": "这是中国",
+        "item_type": "窥视"
+    },
+    "basic_student_item": {
+        "student_id": "Tom",
+        "course_id": "Demo_Course",
+        "item_id": "1",
+        "item_type": "Peer"
+    }
+}
\ No newline at end of file