allow for edX staff to be able to add review policies for exams which will be…

allow for edX staff to be able to add review policies for exams which will be transmitted to SoftwareSecure

.coveragerc

@@ -5,3 +5,4 @@ omit =
     **/__init__.py
     **/migrations/*
     **/tests/*
+    edx_proctoring/admin.py

edx_proctoring/admin.py

+"""
+Django Admin pages
+"""
+
+from django.contrib import admin
+from edx_proctoring.models import (
+    ProctoredExamReviewPolicy,
+    ProctoredExam,
+)
+
+
+class ProctoredExamReviewPolicyInline(admin.TabularInline):
+    """
+    Custom inline definition to show related fields
+    """
+    model = ProctoredExam
+    fields = ('course_id', 'exam_name',)
+    readonly_fields = ('course_id', 'exam_name',)
+
+
+class ProctoredExamReviewPolicyAdmin(admin.ModelAdmin):
+    """
+    The admin panel for Review Policies
+    """
+    readonly_fields = ['set_by_user']
+
+    def course_id(obj):
+        """
+        return course_id of related model
+        """
+        return obj.proctored_exam.course_id
+
+    def exam_name(obj):
+        """
+        return exam name of related model
+        """
+        return obj.proctored_exam.exam_name
+
+    list_display = [
+        course_id,
+        exam_name,
+    ]
+    list_select_related = True
+    search_fields = ['proctored_exam__course_id', 'proctored_exam__exam_name']
+
+    def save_model(self, request, obj, form, change):
+        """
+        Override callback so that we can inject the user_id that made the change
+        """
+        obj.set_by_user = request.user
+        obj.save()
+
+admin.site.register(ProctoredExamReviewPolicy, ProctoredExamReviewPolicyAdmin)

edx_proctoring/api.py

@@ -31,6 +31,7 @@ from edx_proctoring.models import (
     ProctoredExamStudentAllowance,
     ProctoredExamStudentAttempt,
     ProctoredExamStudentAttemptStatus,
+    ProctoredExamReviewPolicy,
 )
 from edx_proctoring.serializers import (
     ProctoredExamSerializer,
@@ -359,6 +360,7 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
     attempt_code = unicode(uuid.uuid4()).upper()
 
     external_id = None
+    review_policy = ProctoredExamReviewPolicy.get_review_policy_for_exam(exam_id)
     if taking_as_proctored:
         scheme = 'https' if getattr(settings, 'HTTPS', 'on') == 'on' else 'http'
         callback_url = '{scheme}://{hostname}{path}'.format(
@@ -378,16 +380,25 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
             credit_state = credit_service.get_credit_state(user_id, exam['course_id'])
             full_name = credit_state['profile_fullname']
 
+        context = {
+            'time_limit_mins': allowed_time_limit_mins,
+            'attempt_code': attempt_code,
+            'is_sample_attempt': exam['is_practice_exam'],
+            'callback_url': callback_url,
+            'full_name': full_name,
+        }
+
+        # see if there is an exam review policy for this exam
+        # if so, then pass it into the provider
+        if review_policy:
+            context.update({
+                'review_policy': review_policy.review_policy
+            })
+
         # now call into the backend provider to register exam attempt
         external_id = get_backend_provider().register_exam_attempt(
             exam,
-            context={
-                'time_limit_mins': allowed_time_limit_mins,
-                'attempt_code': attempt_code,
-                'is_sample_attempt': exam['is_practice_exam'],
-                'callback_url': callback_url,
-                'full_name': full_name,
-            }
+            context=context,
         )
 
     attempt = ProctoredExamStudentAttempt.create_exam_attempt(
@@ -398,7 +409,8 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
         attempt_code,
         taking_as_proctored,
         exam['is_practice_exam'],
-        external_id
+        external_id,
+        review_policy_id=review_policy.id if review_policy else None,
     )
 
     log_msg = (

edx_proctoring/backends/__init__.py

@@ -11,7 +11,7 @@ from django.core.exceptions import ImproperlyConfigured
 _BACKEND_PROVIDER = None
 
 
-def get_backend_provider():
+def get_backend_provider(emphemeral=False):
     """
     Returns an instance of the configured backend provider that is configured
     via the settings file
@@ -19,7 +19,8 @@ def get_backend_provider():
 
     global _BACKEND_PROVIDER  # pylint: disable=global-statement
 
-    if not _BACKEND_PROVIDER:
+    provider = _BACKEND_PROVIDER
+    if not _BACKEND_PROVIDER or emphemeral:
         config = getattr(settings, 'PROCTORING_BACKEND_PROVIDER')
         if not config:
             raise ImproperlyConfigured("Settings not configured with PROCTORING_BACKEND_PROVIDER!")
@@ -34,6 +35,9 @@ def get_backend_provider():
         module_path, _, name = config['class'].rpartition('.')
         class_ = getattr(import_module(module_path), name)
 
-        _BACKEND_PROVIDER = class_(**config['options'])
+        provider = class_(**config['options'])
 
-    return _BACKEND_PROVIDER
+        if not emphemeral:
+            _BACKEND_PROVIDER = provider
+
+    return provider

edx_proctoring/backends/software_secure.py

@@ -295,6 +295,7 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         is_sample_attempt = context['is_sample_attempt']
         callback_url = context['callback_url']
         full_name = context['full_name']
+        review_policy = context.get('review_policy', constants.DEFAULT_SOFTWARE_SECURE_REVIEW_POLICY)
         first_name = ''
         last_name = ''
 
@@ -314,10 +315,7 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
             "reviewedExam": not is_sample_attempt,
             # NOTE: we will have to allow these notes to be authorable in Studio
             # and then we will pull this from the exam database model
-            "reviewerNotes": (
-                'Closed Book; Allow users to take notes on paper during the exam; '
-                'Allow users to use a hand-held calculator during the exam'
-            ),
+            "reviewerNotes": review_policy,
             "examPassword": self._encrypt_password(self.crypto_key, attempt_code),
             "examSponsor": self.exam_sponsor,
             "examName": exam['exam_name'],

edx_proctoring/backends/tests/test_software_secure.py

@@ -16,6 +16,7 @@ from edx_proctoring.runtime import set_runtime_service, get_runtime_service
 
 from edx_proctoring.backends import get_backend_provider
 from edx_proctoring.exceptions import BackendProvideCannotRegisterAttempt
+from edx_proctoring import constants
 
 from edx_proctoring.api import (
     get_exam_attempt_by_id,
@@ -34,6 +35,7 @@ from edx_proctoring. models import (
     ProctoredExamSoftwareSecureComment,
     ProctoredExamStudentAttemptStatus,
     ProctoredExamSoftwareSecureReviewHistory,
+    ProctoredExamReviewPolicy,
 )
 from edx_proctoring.backends.tests.test_review_payload import TEST_REVIEW_PAYLOAD
 
@@ -138,6 +140,100 @@ class SoftwareSecureTests(TestCase):
             self.assertEqual(attempt['external_id'], 'foobar')
             self.assertIsNone(attempt['started_at'])
 
+    def test_attempt_with_review_policy(self):
+        """
+        Create an unstarted proctoring attempt with a review policy associated with it.
+        """
+
+        exam_id = create_exam(
+            course_id='foo/bar/baz',
+            content_id='content',
+            exam_name='Sample Exam',
+            time_limit_mins=10,
+            is_proctored=True
+        )
+
+        policy = ProctoredExamReviewPolicy.objects.create(
+            set_by_user_id=self.user.id,
+            proctored_exam_id=exam_id,
+            review_policy='Foo Policy'
+        )
+
+        def assert_get_payload_mock(exam, context):
+            """
+            Add a mock shim so we can assert that the _get_payload has been called with the right
+            review policy
+            """
+            self.assertIn('review_policy', context)
+            self.assertEqual(policy.review_policy, context['review_policy'])
+
+            # call into real implementation
+            result = get_backend_provider(emphemeral=True)._get_payload(exam, context)  # pylint: disable=protected-access
+
+            # assert that this is in the 'reviewerNotes' field that is passed to SoftwareSecure
+            self.assertEqual(result['reviewerNotes'], context['review_policy'])
+            return result
+
+        with HTTMock(mock_response_content):
+            # patch the _get_payload method on the backend provider
+            # so that we can assert that we are called with the review policy
+            # as well as asserting that _get_payload includes that review policy
+            # that was passed in
+            with patch.object(get_backend_provider(), '_get_payload', assert_get_payload_mock):  # pylint: disable=protected-access
+                attempt_id = create_exam_attempt(
+                    exam_id,
+                    self.user.id,
+                    taking_as_proctored=True
+                )
+                self.assertGreater(attempt_id, 0)
+
+                # make sure we recorded the policy id at the time this was created
+                attempt = get_exam_attempt_by_id(attempt_id)
+                self.assertEqual(attempt['review_policy_id'], policy.id)
+
+    def test_attempt_with_no_review_policy(self):
+        """
+        Create an unstarted proctoring attempt with no review policy associated with it.
+        """
+
+        exam_id = create_exam(
+            course_id='foo/bar/baz',
+            content_id='content',
+            exam_name='Sample Exam',
+            time_limit_mins=10,
+            is_proctored=True
+        )
+
+        def assert_get_payload_mock_no_policy(exam, context):
+            """
+            Add a mock shim so we can assert that the _get_payload has been called with the right
+            review policy
+            """
+            self.assertNotIn('review_policy', context)
+
+            # call into real implementation
+            result = get_backend_provider(emphemeral=True)._get_payload(exam, context)  # pylint: disable=protected-access
+
+            # assert that we use the default that is defined in system configuration
+            self.assertEqual(result['reviewerNotes'], constants.DEFAULT_SOFTWARE_SECURE_REVIEW_POLICY)
+            return result
+
+        with HTTMock(mock_response_content):
+            # patch the _get_payload method on the backend provider
+            # so that we can assert that we are called with the review policy
+            # undefined and that we use the system default
+            with patch.object(get_backend_provider(), '_get_payload', assert_get_payload_mock_no_policy):  # pylint: disable=protected-access
+                attempt_id = create_exam_attempt(
+                    exam_id,
+                    self.user.id,
+                    taking_as_proctored=True
+                )
+                self.assertGreater(attempt_id, 0)
+
+                # make sure we recorded that there is no review policy
+                attempt = get_exam_attempt_by_id(attempt_id)
+                self.assertIsNone(attempt['review_policy_id'])
+
     def test_single_name_attempt(self):
         """
         Tests to make sure we can parse a fullname which does not have any spaces in it

edx_proctoring/constants.py

@@ -29,3 +29,9 @@ ALLOW_REVIEW_UPDATES = (
     settings.PROCTORING_SETTINGS['ALLOW_REVIEW_UPDATES'] if
     'ALLOW_REVIEW_UPDATES' in settings.PROCTORING_SETTINGS else getattr(settings, 'ALLOW_REVIEW_UPDATES', True)
 )
+
+DEFAULT_SOFTWARE_SECURE_REVIEW_POLICY = (
+    settings.PROCTORING_SETTINGS['DEFAULT_REVIEW_POLICY'] if
+    'DEFAULT_REVIEW_POLICY' in settings.PROCTORING_SETTINGS
+    else getattr(settings, 'DEFAULT_REVIEW_POLICY', 'Closed Book')
+)

edx_proctoring/models.py

@@ -47,6 +47,17 @@ class ProctoredExam(TimeStampedModel):
         unique_together = (('course_id', 'content_id'),)
         db_table = 'proctoring_proctoredexam'
 
+    def __unicode__(self):
+        """
+        How to serialize myself as a string
+        """
+
+        return "{course_id}: {exam_name} ({active})".format(
+            course_id=self.course_id,
+            exam_name=self.exam_name,
+            active='active' if self.is_active else 'inactive',
+        )
+
     @classmethod
     def get_exam_by_content_id(cls, course_id, content_id):
         """
@@ -208,6 +219,105 @@ class ProctoredExamStudentAttemptStatus(object):
         return cls.is_completed_status(status) or cls.is_incomplete_status(status)
 
 
+class ProctoredExamReviewPolicy(TimeStampedModel):
+    """
+    This is how an instructor can set review policies for a proctored exam
+    """
+
+    # who set this ProctoredExamReviewPolicy
+    set_by_user = models.ForeignKey(User)
+
+    # for which exam?
+    proctored_exam = models.ForeignKey(ProctoredExam, db_index=True)
+
+    # policy that will be passed to reviewers
+    review_policy = models.TextField()
+
+    class Meta:
+        """ Meta class for this Django model """
+        db_table = 'proctoring_proctoredexamreviewpolicy'
+        verbose_name = 'proctored exam review policy'
+
+    @classmethod
+    def get_review_policy_for_exam(cls, exam_id):
+        """
+        Returns the current exam review policy for the specified
+        exam_id or None if none exists
+        """
+
+        try:
+            return cls.objects.get(proctored_exam_id=exam_id)
+        except cls.DoesNotExist:  # pylint: disable=no-member
+            return None
+
+
+class ProctoredExamReviewPolicyHistory(TimeStampedModel):
+    """
+    Archive table to record all policies that were deleted or updated
+    """
+
+    # what was the original PK for the Review Policy
+    original_id = models.IntegerField(db_index=True)
+
+    # who set this ProctoredExamReviewPolicy
+    set_by_user = models.ForeignKey(User)
+
+    # for which exam?
+    proctored_exam = models.ForeignKey(ProctoredExam, db_index=True)
+
+    # policy that will be passed to reviewers
+    review_policy = models.TextField()
+
+    class Meta:
+        """ Meta class for this Django model """
+        db_table = 'proctoring_proctoredexamreviewpolicyhistory'
+        verbose_name = 'proctored exam review policy history'
+
+    def delete(self, *args, **kwargs):
+        """
+        Don't allow deletions!
+        """
+        raise NotImplementedError()
+
+
+# Hook up the post_save signal to record creations in the ProctoredExamReviewPolicyHistory table.
+@receiver(pre_save, sender=ProctoredExamReviewPolicy)
+def on_review_policy_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:
+        # only for update cases
+        original = ProctoredExamReviewPolicy.objects.get(id=instance.id)
+        _make_review_policy_archive_copy(original)
+
+
+# Hook up the pre_delete signal to record creations in the ProctoredExamReviewPolicyHistory table.
+@receiver(pre_delete, sender=ProctoredExamReviewPolicy)
+def on_review_policy_deleted(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+    Archive the allowance when the item is about to be deleted
+    """
+
+    _make_review_policy_archive_copy(instance)
+
+
+def _make_review_policy_archive_copy(instance):
+    """
+    Do the copying into the history table
+    """
+
+    archive_object = ProctoredExamReviewPolicyHistory(
+        original_id=instance.id,
+        set_by_user_id=instance.set_by_user_id,
+        proctored_exam=instance.proctored_exam,
+        review_policy=instance.review_policy,
+    )
+    archive_object.save()
+
+
 class ProctoredExamStudentAttemptManager(models.Manager):
     """
     Custom manager
@@ -316,6 +426,11 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
 
     student_name = models.CharField(max_length=255)
 
+    # what review policy was this exam submitted under
+    # Note that this is not a foreign key because
+    # this ID might point to a record that is in the History table
+    review_policy_id = models.IntegerField(null=True)
+
     class Meta:
         """ Meta class for this Django model """
         db_table = 'proctoring_proctoredexamstudentattempt'
@@ -324,7 +439,8 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
 
     @classmethod
     def create_exam_attempt(cls, exam_id, user_id, student_name, allowed_time_limit_mins,
-                            attempt_code, taking_as_proctored, is_sample_attempt, external_id):
+                            attempt_code, taking_as_proctored, is_sample_attempt, external_id,
+                            review_policy_id=None):
         """
         Create a new exam attempt entry for a given exam_id and
         user_id.
@@ -340,6 +456,7 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
             is_sample_attempt=is_sample_attempt,
             external_id=external_id,
             status=ProctoredExamStudentAttemptStatus.created,
+            review_policy_id=review_policy_id
         )
 
     def delete_exam_attempt(self):
@@ -389,6 +506,11 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel):
 
     student_name = models.CharField(max_length=255)
 
+    # what review policy was this exam submitted under
+    # Note that this is not a foreign key because
+    # this ID might point to a record that is in the History table
+    review_policy_id = models.IntegerField(null=True)
+
     @classmethod
     def get_exam_attempt_by_code(cls, attempt_code):
         """
@@ -426,7 +548,8 @@ def on_attempt_deleted(sender, instance, **kwargs):  # pylint: disable=unused-ar
         status=instance.status,
         taking_as_proctored=instance.taking_as_proctored,
         is_sample_attempt=instance.is_sample_attempt,
-        student_name=instance.student_name
+        student_name=instance.student_name,
+        review_policy_id=instance.review_policy_id,
     )
     archive_object.save()
 
@@ -648,6 +771,7 @@ def on_review_saved(sender, instance, **kwargs):  # pylint: disable=unused-argum
     """
 
     if instance.id:
+        # only for update cases
         original = ProctoredExamSoftwareSecureReview.objects.get(id=instance.id)
         _make_review_archive_copy(original)
 

edx_proctoring/serializers.py

@@ -80,7 +80,7 @@ class ProctoredExamStudentAttemptSerializer(serializers.ModelSerializer):
             "id", "created", "modified", "user", "started_at", "completed_at",
             "external_id", "status", "proctored_exam", "allowed_time_limit_mins",
             "attempt_code", "is_sample_attempt", "taking_as_proctored", "last_poll_timestamp",
-            "last_poll_ipaddr"
+            "last_poll_ipaddr", "review_policy_id"
         )
 
 

edx_proctoring/tests/test_api.py

@@ -54,6 +54,7 @@ from edx_proctoring.models import (
     ProctoredExamStudentAllowance,
     ProctoredExamStudentAttempt,
     ProctoredExamStudentAttemptStatus,
+    ProctoredExamReviewPolicy,
 )
 
 from .utils import (
@@ -373,6 +374,24 @@ class ProctoredExamApiTests(LoggedInTestCase):
         attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id)
         self.assertGreater(attempt_id, 0)
 
+    def test_attempt_with_review_policy(self):
+        """
+        Create an unstarted exam attempt with a review policy associated with it.
+        """
+
+        policy = ProctoredExamReviewPolicy.objects.create(
+            set_by_user_id=self.user_id,
+            proctored_exam_id=self.proctored_exam_id,
+            review_policy='Foo Policy'
+        )
+
+        attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id)
+        self.assertGreater(attempt_id, 0)
+
+        # make sure we recorded the policy id at the time this was created
+        attempt = get_exam_attempt_by_id(attempt_id)
+        self.assertEqual(attempt['review_policy_id'], policy.id)
+
     def test_attempt_with_allowance(self):
         """
         Create an unstarted exam attempt with additional time.

edx_proctoring/tests/test_models.py

 """
 All tests for the models.py
 """
-from edx_proctoring.models import ProctoredExam, ProctoredExamStudentAllowance, ProctoredExamStudentAllowanceHistory, \
-    ProctoredExamStudentAttempt, ProctoredExamStudentAttemptHistory
+from edx_proctoring.models import (
+    ProctoredExam,
+    ProctoredExamStudentAllowance,
+    ProctoredExamStudentAllowanceHistory,
+    ProctoredExamStudentAttempt,
+    ProctoredExamStudentAttemptHistory,
+    ProctoredExamReviewPolicy,
+    ProctoredExamReviewPolicyHistory,
+)
 
 from .utils import (
     LoggedInTestCase
@@ -166,3 +173,82 @@ class ProctoredExamStudentAttemptTests(LoggedInTestCase):
         with self.assertNumQueries(1):
             exam_attempts = ProctoredExamStudentAttempt.objects.get_all_exam_attempts('a/b/c')
             self.assertEqual(len(exam_attempts), 90)
+
+    def test_exam_review_policy(self):
+        """
+        Assert correct behavior of the Exam Policy model including archiving of updates and deletes
+        """
+
+        # Create an exam.
+        proctored_exam = ProctoredExam.objects.create(
+            course_id='a/b/c',
+            content_id='test_content',
+            exam_name='Test Exam',
+            external_id='123aXqe3',
+            time_limit_mins=90
+        )
+
+        policy = ProctoredExamReviewPolicy.objects.create(
+            set_by_user_id=self.user.id,
+            proctored_exam=proctored_exam,
+            review_policy='Foo Policy'
+        )
+
+        attempt = ProctoredExamStudentAttempt.create_exam_attempt(
+            proctored_exam.id,
+            self.user.id,
+            'test_name{0}'.format(self.user.id),
+            self.user.id + 1,
+            'test_attempt_code{0}'.format(self.user.id),
+            True,
+            False,
+            'test_external_id{0}'.format(self.user.id)
+        )
+        attempt.review_policy_id = policy.id
+        attempt.save()
+
+        history = ProctoredExamReviewPolicyHistory.objects.all()
+        self.assertEqual(len(history), 0)
+
+        # now update it
+        policy.review_policy = 'Updated Foo Policy'
+        policy.save()
+
+        # look in history
+        history = ProctoredExamReviewPolicyHistory.objects.all()
+        self.assertEqual(len(history), 1)
+        previous = history[0]
+        self.assertEqual(previous.set_by_user_id, self.user.id)
+        self.assertEqual(previous.proctored_exam_id, proctored_exam.id)
+        self.assertEqual(previous.original_id, policy.id)
+        self.assertEqual(previous.review_policy, 'Foo Policy')
+
+        # now delete updated one
+        deleted_id = policy.id
+        policy.delete()
+
+        # look in history
+        history = ProctoredExamReviewPolicyHistory.objects.all()
+        self.assertEqual(len(history), 2)
+        previous = history[0]
+        self.assertEqual(previous.set_by_user_id, self.user.id)
+        self.assertEqual(previous.proctored_exam_id, proctored_exam.id)
+        self.assertEqual(previous.original_id, deleted_id)
+        self.assertEqual(previous.review_policy, 'Foo Policy')
+
+        previous = history[1]
+        self.assertEqual(previous.set_by_user_id, self.user.id)
+        self.assertEqual(previous.proctored_exam_id, proctored_exam.id)
+        self.assertEqual(previous.original_id, deleted_id)
+        self.assertEqual(previous.review_policy, 'Updated Foo Policy')
+
+        # assert that we cannot delete history!
+        with self.assertRaises(NotImplementedError):
+            previous.delete()
+
+        # now delete attempt, to make sure we preserve the policy_id in the archive table
+        attempt.delete()
+
+        attempts = ProctoredExamStudentAttemptHistory.objects.all()
+        self.assertEqual(len(attempts), 1)
+        self.assertEqual(attempts[0].review_policy_id, deleted_id)

setup.py

@@ -34,7 +34,7 @@ def load_requirements(*requirements_paths):
 
 setup(
     name='edx-proctoring',
-    version='0.9.6e',
+    version='0.9.11',
     description='Proctoring subsystem for Open edX',
     long_description=open('README.md').read(),
     author='edX',