Merge pull request #172 from edx/cdodge/allow-for-review-updates

allow for optional (default False) ability to allow SoftwareSecure to…

edx_proctoring/backends/software_secure.py

@@ -15,6 +15,7 @@ import logging
 from django.conf import settings
 
 from edx_proctoring.backends.backend import ProctoringBackendProvider
+from edx_proctoring import constants
 from edx_proctoring.exceptions import (
     BackendProvideCannotRegisterAttempt,
     StudentExamAttemptDoesNotExistsException,
@@ -188,29 +189,42 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
             )
             raise ProctoredExamSuspiciousLookup(err_msg)
 
-        # do we already have a review for this attempt?!? It should not be updated!
+        # do some limited parsing of the JSON payload
+        review_status = payload['reviewStatus']
+        video_review_link = payload['videoReviewLink']
+
+        # do we already have a review for this attempt?!? We may not allow updates
         review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(attempt_code)
 
         if review:
-            err_msg = (
+            if not constants.ALLOW_REVIEW_UPDATES:
+                err_msg = (
+                    'We already have a review submitted from SoftwareSecure regarding '
+                    'attempt_code {attempt_code}. We do not allow for updates!'.format(
+                        attempt_code=attempt_code
+                    )
+                )
+                raise ProctoredExamReviewAlreadyExists(err_msg)
+
+            # we allow updates
+            warn_msg = (
                 'We already have a review submitted from SoftwareSecure regarding '
-                'attempt_code {attempt_code}. We do not allow for updates!'.format(
+                'attempt_code {attempt_code}. We have been configured to allow for '
+                'updates and will continue...'.format(
                     attempt_code=attempt_code
                 )
             )
-            raise ProctoredExamReviewAlreadyExists(err_msg)
+            log.warn(warn_msg)
+        else:
+            # this is first time we've received this attempt_code, so
+            # make a new record in the review table
+            review = ProctoredExamSoftwareSecureReview()
 
-        # do some limited parsing of the JSON payload
-        review_status = payload['reviewStatus']
-        video_review_link = payload['videoReviewLink']
+        review.attempt_code = attempt_code
+        review.raw_data = json.dumps(payload)
+        review.review_status = review_status
+        review.video_url = video_review_link
 
-        # make a new record in the review table
-        review = ProctoredExamSoftwareSecureReview(
-            attempt_code=attempt_code,
-            raw_data=json.dumps(payload),
-            review_status=review_status,
-            video_url=video_review_link,
-        )
         review.save()
 
         # go through and populate all of the specific comments

edx_proctoring/backends/tests/test_software_secure.py

 # coding=utf-8
+# pylint: disable=too-many-lines, invalid-name
 """
 Tests for the software_secure module
 """
@@ -32,6 +33,7 @@ from edx_proctoring. models import (
     ProctoredExamSoftwareSecureReview,
     ProctoredExamSoftwareSecureComment,
     ProctoredExamStudentAttemptStatus,
+    ProctoredExamSoftwareSecureReviewHistory,
 )
 from edx_proctoring.backends.tests.test_review_payload import TEST_REVIEW_PAYLOAD
 
@@ -471,7 +473,8 @@ class SoftwareSecureTests(TestCase):
 
         self.assertEqual(len(comments), 6)
 
-    def test_review_resubmission(self):
+    @patch('edx_proctoring.constants.ALLOW_REVIEW_UPDATES', False)
+    def test_disallow_review_resubmission(self):
         """
         Tests that an exception is raised if a review report is resubmitted for the same
         attempt
@@ -508,3 +511,70 @@ class SoftwareSecureTests(TestCase):
         # now call again
         with self.assertRaises(ProctoredExamReviewAlreadyExists):
             provider.on_review_callback(json.loads(test_payload))
+
+    @patch('edx_proctoring.constants.ALLOW_REVIEW_UPDATES', True)
+    def test_allow_review_resubmission(self):
+        """
+        Tests that an resubmission is allowed
+        """
+
+        provider = get_backend_provider()
+
+        exam_id = create_exam(
+            course_id='foo/bar/baz',
+            content_id='content',
+            exam_name='Sample Exam',
+            time_limit_mins=10,
+            is_proctored=True
+        )
+
+        # be sure to use the mocked out SoftwareSecure handlers
+        with HTTMock(mock_response_content):
+            attempt_id = create_exam_attempt(
+                exam_id,
+                self.user.id,
+                taking_as_proctored=True
+            )
+
+        attempt = get_exam_attempt_by_id(attempt_id)
+        self.assertIsNotNone(attempt['external_id'])
+
+        test_payload = Template(TEST_REVIEW_PAYLOAD).substitute(
+            attempt_code=attempt['attempt_code'],
+            external_id=attempt['external_id']
+        )
+
+        provider.on_review_callback(json.loads(test_payload))
+
+        # make sure history table is empty
+        records = ProctoredExamSoftwareSecureReviewHistory.objects.filter(attempt_code=attempt['attempt_code'])
+        self.assertEqual(len(records), 0)
+
+        # now call again, this will not throw exception
+        test_payload = test_payload.replace('Clean', 'Suspicious')
+        provider.on_review_callback(json.loads(test_payload))
+
+        # make sure that what we have in the Database matches what we expect
+        review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(attempt['attempt_code'])
+
+        self.assertIsNotNone(review)
+        self.assertEqual(review.review_status, 'Suspicious')
+        self.assertEqual(
+            review.video_url,
+            'http://www.remoteproctor.com/AdminSite/Account/Reviewer/DirectLink-Generic.aspx?ID=foo'
+        )
+        self.assertIsNotNone(review.raw_data)
+
+        # make sure history table is no longer empty
+        records = ProctoredExamSoftwareSecureReviewHistory.objects.filter(attempt_code=attempt['attempt_code'])
+        self.assertEqual(len(records), 1)
+        self.assertEqual(records[0].review_status, 'Clean')
+
+        # now try to delete the record and make sure it was archived
+
+        review.delete()
+
+        records = ProctoredExamSoftwareSecureReviewHistory.objects.filter(attempt_code=attempt['attempt_code'])
+        self.assertEqual(len(records), 2)
+        self.assertEqual(records[0].review_status, 'Clean')
+        self.assertEqual(records[1].review_status, 'Suspicious')

edx_proctoring/constants.py

@@ -24,3 +24,8 @@ CONTACT_EMAIL = (
     settings.PROCTORING_SETTINGS['CONTACT_EMAIL'] if
     'CONTACT_EMAIL' in settings.PROCTORING_SETTINGS else getattr(settings, 'CONTACT_EMAIL', FROM_EMAIL)
 )
+
+ALLOW_REVIEW_UPDATES = (
+    settings.PROCTORING_SETTINGS['ALLOW_REVIEW_UPDATES'] if
+    'ALLOW_REVIEW_UPDATES' in settings.PROCTORING_SETTINGS else getattr(settings, 'ALLOW_REVIEW_UPDATES', True)
+)

edx_proctoring/models.py

@@ -3,7 +3,7 @@ Data models for the proctoring subsystem
 """
 from django.db import models
 from django.db.models import Q
-from django.db.models.signals import post_save, pre_delete
+from django.db.models.signals import pre_save, pre_delete
 from django.dispatch import receiver
 from model_utils.models import TimeStampedModel
 from django.utils.translation import ugettext as _
@@ -543,15 +543,16 @@ class ProctoredExamStudentAllowanceHistory(TimeStampedModel):
 
 
 # Hook up the post_save signal to record creations in the ProctoredExamStudentAllowanceHistory table.
-@receiver(post_save, sender=ProctoredExamStudentAllowance)
-def on_allowance_saved(sender, instance, created, **kwargs):  # pylint: disable=unused-argument
+@receiver(pre_save, sender=ProctoredExamStudentAllowance)
+def on_allowance_saved(sender, instance, **kwargs):  # pylint: disable=unused-argument
     """
     Archiving all changes made to the Student Allowance.
     Will only archive on update, and not on new entries created.
     """
 
-    if not created:
-        _make_archive_copy(instance)
+    if instance.id:
+        original = ProctoredExamStudentAllowance.objects.get(id=instance.id)
+        _make_archive_copy(original)
 
 
 @receiver(pre_delete, sender=ProctoredExamStudentAllowance)
@@ -614,6 +615,66 @@ class ProctoredExamSoftwareSecureReview(TimeStampedModel):
             return None
 
 
+class ProctoredExamSoftwareSecureReviewHistory(TimeStampedModel):
+    """
+    When records get updated, we will archive them here
+    """
+
+    # which student attempt is this feedback for?
+    attempt_code = models.CharField(max_length=255, db_index=True)
+
+    # overall status of the review
+    review_status = models.CharField(max_length=255)
+
+    # The raw payload that was received back from the
+    # reviewing service
+    raw_data = models.TextField()
+
+    # URL for the exam video that had been reviewed
+    video_url = models.TextField()
+
+    class Meta:
+        """ Meta class for this Django model """
+        db_table = 'proctoring_proctoredexamsoftwaresecurereviewhistory'
+        verbose_name = 'proctored exam review history'
+
+
+# Hook up the post_save signal to record creations in the ProctoredExamStudentAllowanceHistory table.
+@receiver(pre_save, sender=ProctoredExamSoftwareSecureReview)
+def on_review_saved(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+    Archiving all changes made to the Student Allowance.
+    Will only archive on update, and not on new entries created.
+    """
+
+    if instance.id:
+        original = ProctoredExamSoftwareSecureReview.objects.get(id=instance.id)
+        _make_review_archive_copy(original)
+
+
+@receiver(pre_delete, sender=ProctoredExamSoftwareSecureReview)
+def on_review_deleted(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+    Archive the allowance when the item is about to be deleted
+    """
+
+    _make_review_archive_copy(instance)
+
+
+def _make_review_archive_copy(instance):
+    """
+    Do the copying into the history table
+    """
+
+    archive_object = ProctoredExamSoftwareSecureReviewHistory(
+        attempt_code=instance.attempt_code,
+        review_status=instance.review_status,
+        raw_data=instance.raw_data,
+        video_url=instance.video_url,
+    )
+    archive_object.save()
+
+
 class ProctoredExamSoftwareSecureComment(TimeStampedModel):
     """
     This is where we store the proctored exam review comments