Merge pull request #186 from edx/cdodge/release-rebase2

Cdodge/release rebase2

.coveragerc

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

edx_proctoring/admin.py

+"""
+Django Admin pages
+"""
+# pylint: disable=no-self-argument, no-member
+
+from django.contrib import admin
+from django.utils.translation import ugettext_lazy as _
+from django import forms
+from edx_proctoring.models import (
+    ProctoredExamReviewPolicy,
+    ProctoredExamSoftwareSecureReview,
+    ProctoredExamSoftwareSecureReviewHistory,
+)
+from edx_proctoring.utils import locate_attempt_by_attempt_code
+from edx_proctoring.backends import get_backend_provider
+
+
+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()
+
+
+class ProctoredExamSoftwareSecureReviewForm(forms.ModelForm):
+    """Admin Form to display for reading/updating a Review"""
+    class Meta(object):  # pylint: disable=missing-docstring
+        model = ProctoredExamSoftwareSecureReview
+
+    REVIEW_STATUS_CHOICES = [
+        ('Clean', 'Clean'),
+        ('Rules Violation', 'Rules Violation'),
+        ('Suspicious', 'Suspicious'),
+        ('Not Reviewed', 'Not Reviewed'),
+    ]
+
+    review_status = forms.ChoiceField(choices=REVIEW_STATUS_CHOICES)
+    video_url = forms.URLField()
+    raw_data = forms.CharField(widget=forms.Textarea, label='Reviewer Notes')
+
+
+def video_url_for_review(obj):
+    """Return hyperlink to review video url"""
+    return (
+        '<a href="{video_url}" target="_blank">{video_url}</a>'.format(video_url=obj.video_url)
+    )
+video_url_for_review.allow_tags = True
+
+
+class ReviewListFilter(admin.SimpleListFilter):
+    """
+    Quick filter to allow admins to see which reviews have not been reviewed internally
+    """
+
+    title = _('internally reviewed')
+
+    parameter_name = 'reviewed_by'
+
+    def lookups(self, request, model_admin):
+        """
+        List of values to allow admin to select
+        """
+        return (
+            ('all_unreviewed', _('All Unreviewed')),
+            ('all_unreviewed_failures', _('All Unreviewed Failures')),
+        )
+
+    def queryset(self, request, queryset):
+        """
+        Return the filtered queryset
+        """
+
+        if self.value() == 'all_unreviewed':
+            return queryset.filter(reviewed_by__isnull=True)
+        elif self.value() == 'all_unreviewed_failures':
+            return queryset.filter(reviewed_by__isnull=True, review_status='Suspicious')
+        else:
+            return queryset
+
+
+class ProctoredExamSoftwareSecureReviewAdmin(admin.ModelAdmin):
+    """
+    The admin panel for SoftwareSecure Review records
+    """
+
+    readonly_fields = [video_url_for_review, 'attempt_code', 'exam', 'student', 'reviewed_by', 'modified']
+    list_filter = ['review_status', 'exam__course_id', 'exam__exam_name', ReviewListFilter]
+    list_select_related = True
+    search_fields = ['student__username', 'attempt_code']
+    form = ProctoredExamSoftwareSecureReviewForm
+
+    def _get_exam_from_attempt_code(self, code):
+        """Get exam from attempt code. Note that the attempt code could be an archived one"""
+        attempt = locate_attempt_by_attempt_code(code)
+        return attempt.proctored_exam if attempt else None
+
+    def course_id_for_review(self, obj):
+        """Return course_id associated with review"""
+        if obj.exam:
+            return obj.exam.course_id
+        else:
+            exam = self._get_exam_from_attempt_code(obj.attempt_code)
+            return exam.exam_name if exam else '(none)'
+
+    def exam_name_for_review(self, obj):
+        """Return course_id associated with review"""
+        if obj.exam:
+            return obj.exam.exam_name
+        else:
+            exam = self._get_exam_from_attempt_code(obj.attempt_code)
+            return exam.exam_name if exam else '(none)'
+
+    def student_username_for_review(self, obj):
+        """Return username of student who took the test"""
+        if obj.student:
+            return obj.student.username
+        else:
+            attempt = locate_attempt_by_attempt_code(obj.attempt_code)
+            return attempt.user.username if attempt else '(None)'
+
+    list_display = [
+        'course_id_for_review',
+        'exam_name_for_review',
+        'student_username_for_review',
+        'attempt_code',
+        'modified',
+        'reviewed_by',
+        'review_status'
+    ]
+
+    def has_add_permission(self, request):
+        """Don't allow adds"""
+        return False
+
+    def has_delete_permission(self, request, obj=None):
+        """Don't allow deletes"""
+        return False
+
+    def save_model(self, request, review, form, change):
+        """
+        Override callback so that we can inject the user_id that made the change
+        """
+        review.reviewed_by = request.user
+        review.save()
+        # call the review saved and since it's coming from
+        # the Django admin will we accept failures
+        get_backend_provider().on_review_saved(review, allow_status_update_on_fail=True)
+
+    def get_form(self, request, obj=None, **kwargs):
+        form = super(ProctoredExamSoftwareSecureReviewAdmin, self).get_form(request, obj, **kwargs)
+        del form.base_fields['video_url']
+        return form
+
+
+class ProctoredExamSoftwareSecureReviewHistoryAdmin(ProctoredExamSoftwareSecureReviewAdmin):
+    """
+    The admin panel for SoftwareSecure Review records
+    """
+
+    readonly_fields = [
+        video_url_for_review,
+        'review_status',
+        'raw_data',
+        'attempt_code',
+        'exam',
+        'student',
+        'reviewed_by',
+        'modified',
+    ]
+
+    def save_model(self, request, review, form, change):
+        """
+        History can't be updated
+        """
+        return
+
+
+admin.site.register(ProctoredExamReviewPolicy, ProctoredExamReviewPolicyAdmin)
+admin.site.register(ProctoredExamSoftwareSecureReview, ProctoredExamSoftwareSecureReviewAdmin)
+admin.site.register(ProctoredExamSoftwareSecureReviewHistory, ProctoredExamSoftwareSecureReviewHistoryAdmin)

edx_proctoring/api.py

@@ -31,6 +31,7 @@ from edx_proctoring.models import (
     ProctoredExamStudentAllowance,
     ProctoredExamStudentAttempt,
     ProctoredExamStudentAttemptStatus,
+    ProctoredExamReviewPolicy,
 )
 from edx_proctoring.serializers import (
     ProctoredExamSerializer,
@@ -346,19 +347,16 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
     allowed_time_limit_mins = exam['time_limit_mins']
 
     # add in the allowed additional time
-    allowance = ProctoredExamStudentAllowance.get_allowance_for_user(
-        exam_id,
-        user_id,
-        "Additional time (minutes)"
-    )
-
-    if allowance:
-        allowance_extra_mins = int(allowance.value)
+    allowance_extra_mins = ProctoredExamStudentAllowance.get_additional_time_granted(exam_id, user_id)
+    if allowance_extra_mins:
         allowed_time_limit_mins += allowance_extra_mins
 
     attempt_code = unicode(uuid.uuid4()).upper()
 
     external_id = None
+    review_policy = ProctoredExamReviewPolicy.get_review_policy_for_exam(exam_id)
+    review_policy_exception = ProctoredExamStudentAllowance.get_review_policy_exception(exam_id, user_id)
+
     if taking_as_proctored:
         scheme = 'https' if getattr(settings, 'HTTPS', 'on') == 'on' else 'http'
         callback_url = '{scheme}://{hostname}{path}'.format(
@@ -378,16 +376,33 @@ 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
+            })
+
+        # see if there is a review policy exception for this *user*
+        # exceptions are granted on a individual basis as an
+        # allowance
+        if review_policy_exception:
+            context.update({
+                'review_policy_exception': review_policy_exception
+            })
+
         # 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 +413,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 = (
@@ -920,17 +936,17 @@ def _check_eligibility_of_prerequisites(credit_state):
 STATUS_SUMMARY_MAP = {
     '_default': {
         'short_description': _('Taking As Proctored Exam'),
-        'suggested_icon': 'fa-lock',
+        'suggested_icon': 'fa-pencil-square-o',
         'in_completed_state': False
     },
     ProctoredExamStudentAttemptStatus.eligible: {
         'short_description': _('Proctored Option Available'),
-        'suggested_icon': 'fa-lock',
+        'suggested_icon': 'fa-pencil-square-o',
         'in_completed_state': False
     },
     ProctoredExamStudentAttemptStatus.declined: {
         'short_description': _('Taking As Open Exam'),
-        'suggested_icon': 'fa-unlock',
+        'suggested_icon': 'fa-pencil-square-o',
         'in_completed_state': False
     },
     ProctoredExamStudentAttemptStatus.submitted: {
@@ -959,7 +975,7 @@ STATUS_SUMMARY_MAP = {
 PRACTICE_STATUS_SUMMARY_MAP = {
     '_default': {
         'short_description': _('Ungraded Practice Exam'),
-        'suggested_icon': 'fa-lock',
+        'suggested_icon': '',
         'in_completed_state': False
     },
     ProctoredExamStudentAttemptStatus.submitted: {

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/backend.py

@@ -50,3 +50,11 @@ class ProctoringBackendProvider(object):
         Called when the reviewing 3rd party service posts back the results
         """
         raise NotImplementedError()
+
+    @abc.abstractmethod
+    def on_review_saved(self, review):
+        """
+        called when a review has been save - either through API or via Django Admin panel
+        in order to trigger any workflow.
+        """
+        raise NotImplementedError()

edx_proctoring/backends/null.py

@@ -41,3 +41,9 @@ class NullBackendProvider(ProctoringBackendProvider):
         """
         Called when the reviewing 3rd party service posts back the results
         """
+
+    def on_review_saved(self, review):
+        """
+        called when a review has been save - either through API or via Django Admin panel
+        in order to trigger any workflow
+        """

edx_proctoring/backends/software_secure.py

@@ -23,12 +23,10 @@ from edx_proctoring.exceptions import (
     ProctoredExamReviewAlreadyExists,
     ProctoredExamBadReviewStatus,
 )
-
+from edx_proctoring.utils import locate_attempt_by_attempt_code
 from edx_proctoring. models import (
     ProctoredExamSoftwareSecureReview,
     ProctoredExamSoftwareSecureComment,
-    ProctoredExamStudentAttempt,
-    ProctoredExamStudentAttemptHistory,
     ProctoredExamStudentAttemptStatus,
 )
 
@@ -154,20 +152,13 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         # what we recorded as the external_id. We need to look in both
         # the attempt table as well as the archive table
 
-        attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt_by_code(attempt_code)
-
-        is_archived_attempt = False
+        (attempt_obj, is_archived_attempt) = locate_attempt_by_attempt_code(attempt_code)
         if not attempt_obj:
-            # try archive table
-            attempt_obj = ProctoredExamStudentAttemptHistory.get_exam_attempt_by_code(attempt_code)
-            is_archived_attempt = True
-
-            if not attempt_obj:
-                # still can't find, error out
-                err_msg = (
-                    'Could not locate attempt_code: {attempt_code}'.format(attempt_code=attempt_code)
-                )
-                raise StudentExamAttemptDoesNotExistsException(err_msg)
+            # still can't find, error out
+            err_msg = (
+                'Could not locate attempt_code: {attempt_code}'.format(attempt_code=attempt_code)
+            )
+            raise StudentExamAttemptDoesNotExistsException(err_msg)
 
         # then make sure we have the right external_id
         # note that SoftwareSecure might send a case insensitive
@@ -224,6 +215,11 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         review.raw_data = json.dumps(payload)
         review.review_status = review_status
         review.video_url = video_review_link
+        review.student = attempt_obj.user
+        review.exam = attempt_obj.proctored_exam
+        # set reviewed_by to None because it was reviewed by our 3rd party
+        # service provider, not a user in our database
+        review.reviewed_by = None
 
         review.save()
 
@@ -234,21 +230,54 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         for comment in payload.get('desktopComments', []):
             self._save_review_comment(review, comment)
 
-        # we could have gottent a review for an archived attempt
+        # we could have gotten a review for an archived attempt
         # this should *not* cause an update in our credit
         # eligibility table
         if not is_archived_attempt:
             # update our attempt status, note we have to import api.py here because
             # api.py imports software_secure.py, so we'll get an import circular reference
 
-            from edx_proctoring.api import update_attempt_status
+            allow_status_update_on_fail = not constants.REQUIRE_FAILURE_SECOND_REVIEWS
+
+            self.on_review_saved(review, allow_status_update_on_fail=allow_status_update_on_fail)
+
+    def on_review_saved(self, review, allow_status_update_on_fail=False):  # pylint: disable=arguments-differ
+        """
+        called when a review has been save - either through API (on_review_callback) or via Django Admin panel
+        in order to trigger any workflow associated with proctoring review results
+        """
+
+        (attempt_obj, is_archived_attempt) = locate_attempt_by_attempt_code(review.attempt_code)
+
+        if not attempt_obj:
+            # This should not happen, but it is logged in the help
+            # method
+            return
 
-            # only 'Clean' and 'Rules Violation' could as passing
-            status = (
-                ProctoredExamStudentAttemptStatus.verified
-                if review_status in self.passing_review_status
-                else ProctoredExamStudentAttemptStatus.rejected
+        if is_archived_attempt:
+            # we don't trigger workflow on reviews on archived attempts
+            err_msg = (
+                'Got on_review_save() callback for an archived attempt with '
+                'attempt_code {attempt_code}. Will not trigger workflow...'.format(
+                    attempt_code=review.attempt_code
+                )
             )
+            log.warn(err_msg)
+            return
+
+        # only 'Clean' and 'Rules Violation' count as passing
+        status = (
+            ProctoredExamStudentAttemptStatus.verified
+            if review.review_status in self.passing_review_status
+            else ProctoredExamStudentAttemptStatus.rejected
+        )
+
+        # are we allowed to update the status if we have a failure status
+        # i.e. do we need a review to come in from Django Admin panel?
+        if status == ProctoredExamStudentAttemptStatus.verified or allow_status_update_on_fail:
+            # updating attempt status will trigger workflow
+            # (i.e. updating credit eligibility table)
+            from edx_proctoring.api import update_attempt_status
 
             update_attempt_status(
                 attempt_obj.proctored_exam_id,
@@ -285,6 +314,19 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         encrypted_text = cipher.encrypt(pad(pwd))
         return base64.b64encode(encrypted_text)
 
+    def _split_fullname(self, full_name):
+        """
+        Utility to break Full Name to first and last name
+        """
+        first_name = ''
+        last_name = ''
+        name_elements = full_name.split(' ')
+        first_name = name_elements[0]
+        if len(name_elements) > 1:
+            last_name = ' '.join(name_elements[1:])
+
+        return (first_name, last_name)
+
     def _get_payload(self, exam, context):
         """
         Constructs the data payload that Software Secure expects
@@ -295,14 +337,20 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         is_sample_attempt = context['is_sample_attempt']
         callback_url = context['callback_url']
         full_name = context['full_name']
-        first_name = ''
-        last_name = ''
+        review_policy = context.get('review_policy', constants.DEFAULT_SOFTWARE_SECURE_REVIEW_POLICY)
+        review_policy_exception = context.get('review_policy_exception')
+
+        # compile the notes to the reviewer
+        # this is a combination of the Exam Policy which is for all students
+        # combined with any exceptions granted to the particular student
+        reviewer_notes = review_policy
+        if review_policy_exception:
+            reviewer_notes = '{notes}; {exception}'.format(
+                notes=reviewer_notes,
+                exception=review_policy_exception
+            )
 
-        if full_name:
-            name_elements = full_name.split(' ')
-            first_name = name_elements[0]
-            if len(name_elements) > 1:
-                last_name = ' '.join(name_elements[1:])
+        (first_name, last_name) = self._split_fullname(full_name)
 
         now = datetime.datetime.utcnow()
         start_time_str = now.strftime("%a, %d %b %Y %H:%M:%S GMT")
@@ -314,10 +362,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": reviewer_notes,
             "examPassword": self._encrypt_password(self.crypto_key, attempt_code),
             "examSponsor": self.exam_sponsor,
             "examName": exam['exam_name'],

edx_proctoring/backends/tests/test_backend.py

@@ -44,6 +44,12 @@ class TestBackendProvider(ProctoringBackendProvider):
         Called when the reviewing 3rd party service posts back the results
         """
 
+    def on_review_saved(self, review):
+        """
+        called when a review has been save - either through API or via Django Admin panel
+        in order to trigger any workflow
+        """
+
 
 class PassthroughBackendProvider(ProctoringBackendProvider):
     """
@@ -92,6 +98,13 @@ class PassthroughBackendProvider(ProctoringBackendProvider):
         """
         return super(PassthroughBackendProvider, self).on_review_callback(payload)
 
+    def on_review_saved(self, review):
+        """
+        called when a review has been save - either through API or via Django Admin panel
+        in order to trigger any workflow
+        """
+        return super(PassthroughBackendProvider, self).on_review_saved(review)
+
 
 class TestBackends(TestCase):
     """
@@ -120,6 +133,9 @@ class TestBackends(TestCase):
         with self.assertRaises(NotImplementedError):
             provider.on_review_callback(None)
 
+        with self.assertRaises(NotImplementedError):
+            provider.on_review_saved(None)
+
     def test_null_provider(self):
         """
         Assert that the Null provider does nothing
@@ -132,3 +148,4 @@ class TestBackends(TestCase):
         self.assertIsNone(provider.stop_exam_attempt(None, None))
         self.assertIsNone(provider.get_software_download_url())
         self.assertIsNone(provider.on_review_callback(None))
+        self.assertIsNone(provider.on_review_saved(None))

edx_proctoring/constants.py

@@ -29,3 +29,15 @@ 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')
+)
+
+REQUIRE_FAILURE_SECOND_REVIEWS = (
+    settings.PROCTORING_SETTINGS['REQUIRE_FAILURE_SECOND_REVIEWS'] if
+    'REQUIRE_FAILURE_SECOND_REVIEWS' in settings.PROCTORING_SETTINGS
+    else getattr(settings, 'REQUIRE_FAILURE_SECOND_REVIEWS', True)
+)

edx_proctoring/serializers.py

 """Defines serializers used by the Proctoring API."""
 from rest_framework import serializers
+from rest_framework.fields import DateTimeField
 from django.contrib.auth.models import User
 from edx_proctoring.models import ProctoredExam, ProctoredExamStudentAttempt, ProctoredExamStudentAllowance
 
 
-class StrictBooleanField(serializers.BooleanField):
-    """
-    Boolean field serializer to cater for a bug in DRF BooleanField serializer
-    where required=True is ignored.
-    """
-    def from_native(self, value):
-        if value in ('true', 't', 'True', '1'):
-            return True
-        if value in ('false', 'f', 'False', '0'):
-            return False
-        return None
-
-
 class ProctoredExamSerializer(serializers.ModelSerializer):
     """
     Serializer for the ProctoredExam Model.
@@ -28,9 +16,9 @@ class ProctoredExamSerializer(serializers.ModelSerializer):
     exam_name = serializers.CharField(required=True)
     time_limit_mins = serializers.IntegerField(required=True)
 
-    is_active = StrictBooleanField(required=True)
-    is_practice_exam = StrictBooleanField(required=True)
-    is_proctored = StrictBooleanField(required=True)
+    is_active = serializers.BooleanField(required=True)
+    is_practice_exam = serializers.BooleanField(required=True)
+    is_proctored = serializers.BooleanField(required=True)
 
     class Meta:
         """
@@ -70,6 +58,13 @@ class ProctoredExamStudentAttemptSerializer(serializers.ModelSerializer):
     proctored_exam = ProctoredExamSerializer()
     user = UserSerializer()
 
+    # Django Rest Framework v3 defaults to `settings.DATE_FORMAT` when serializing
+    # datetime fields.  We need to specify `format=None` to maintain the old behavior
+    # of returning raw `datetime` objects instead of unicode.
+    started_at = DateTimeField(format=None)
+    completed_at = DateTimeField(format=None)
+    last_poll_timestamp = DateTimeField(format=None)
+
     class Meta:
         """
         Meta Class
@@ -80,7 +75,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/static/proctoring/js/views/proctored_exam_add_allowance_view.js

@@ -14,6 +14,7 @@ var edx = edx || {};
             this.proctored_exams = options.proctored_exams;
             this.proctored_exam_allowance_view = options.proctored_exam_allowance_view;
             this.course_id = options.course_id;
+            this.allowance_types = options.allowance_types;
             this.model = new edx.instructor_dashboard.proctoring.ProctoredExamAllowanceModel();
             _.bindAll(this, "render");
             this.loadTemplateData();
@@ -156,11 +157,9 @@ var edx = edx || {};
         },
 
         render: function () {
-            var allowance_types = ['Additional time (minutes)'];
-
             $(this.el).html(this.template({
                 proctored_exams: this.proctored_exams,
-                allowance_types: allowance_types
+                allowance_types: this.allowance_types
             }));
 
             this.$form = {

edx_proctoring/static/proctoring/js/views/proctored_exam_allowance_view.js

@@ -8,6 +8,12 @@ var edx = edx || {};
 
     edx.instructor_dashboard.proctoring.ProctoredExamAllowanceView = Backbone.View.extend({
         initialize: function () {
+
+            this.allowance_types = [
+                ['additional_time_granted', gettext('Additional Time (minutes)')],
+                ['review_policy_exception', gettext('Review Policy Exception')]
+            ];
+
             this.collection = new edx.instructor_dashboard.proctoring.ProctoredExamAllowanceCollection();
             this.proctoredExamCollection = new edx.instructor_dashboard.proctoring.ProctoredExamCollection();
             /* unfortunately we have to make some assumptions about what is being set up in HTML */
@@ -121,6 +127,20 @@ var edx = edx || {};
         },
         render: function () {
             if (this.template !== null) {
+                var self = this;
+                this.collection.each(function(item){
+                    var key = item.get('key');
+                    var i
+                    for (i=0; i<self.allowance_types.length; i++) {
+                        if (key === self.allowance_types[i][0]) {
+                            item.set('key_display_name', self.allowance_types[i][1]);
+                            break;
+                        }
+                    }
+                    if (!item.has('key_display_name')) {
+                        item.set('key_display_name', key);
+                    }
+                });
                 var html = this.template({proctored_exam_allowances: this.collection.toJSON()});
                 this.$el.html(html);
             }
@@ -132,7 +152,8 @@ var edx = edx || {};
                     var add_allowance_view = new edx.instructor_dashboard.proctoring.AddAllowanceView({
                         course_id: self.course_id,
                         proctored_exams: self.proctoredExamCollection.toJSON(),
-                        proctored_exam_allowance_view: self
+                        proctored_exam_allowance_view: self,
+                        allowance_types: self.allowance_types
                     });
                 }
             });

edx_proctoring/static/proctoring/spec/proctored_exam_add_allowance_spec.js

@@ -9,7 +9,7 @@ describe('ProctoredExamAddAllowanceView', function () {
             created: "2015-08-10T09:15:45Z",
             id: 1,
             modified: "2015-08-10T09:15:45Z",
-            key: "Additional time (minutes)",
+            key: "additional_time_granted",
             value: "1",
             proctored_exam: {
                 content_id: "i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c",
@@ -59,8 +59,8 @@ describe('ProctoredExamAddAllowanceView', function () {
             '<label>Allowance Type</label>' +
             '</td><td><select id="allowance_type">' +
             '<% _.each(allowance_types, function(allowance_type){ %>' +
-            '<option value="<%= allowance_type %>">' +
-            '<%- interpolate(gettext(" %(allowance_type)s "), { allowance_type: allowance_type }, true) %>' +
+            '<option value="<%= allowance_type[0] %>">' +
+            '<%= allowance_type[1] %>' +
             '</option>' +
             '<% }); %>' +
             '</select></td></tr><tr><td>' +
@@ -101,7 +101,7 @@ describe('ProctoredExamAddAllowanceView', function () {
             '<td>N/A</td><td>N/A</td>' +
             '<% } %>' +
             '<td>' +
-            '<%- interpolate(gettext(" %(allowance_name)s "), { allowance_name: proctored_exam_allowance.key }, true) %>' +
+            '<%- interpolate(gettext(" %(allowance_name)s "), { allowance_name: proctored_exam_allowance.key_display_name }, true) %>' +
             '</td>' +
             '<td>' +
             '<%= proctored_exam_allowance.value %>' +
@@ -185,7 +185,7 @@ describe('ProctoredExamAddAllowanceView', function () {
 
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('testuser1');
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('testuser1@test.com');
-        expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('Additional time (minutes)');
+        expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('Additional Time (minutes)');
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('Test Exam');
 
         // add the proctored exam allowance
@@ -213,7 +213,7 @@ describe('ProctoredExamAddAllowanceView', function () {
         //select the form values
 
         $('#proctored_exam').val('Test Exam');
-        $('#allowance_type').val('Additional time (minutes)');
+        $('#allowance_type').val('additional_time_granted');
         $('#allowance_value').val('1');
         $("#user_info").val('testuser1');
 
@@ -227,7 +227,7 @@ describe('ProctoredExamAddAllowanceView', function () {
 
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).toContain('testuser1');
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).toContain('testuser1@test.com');
-        expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).toContain('Additional time (minutes)');
+        expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).toContain('Additional Time (minutes)');
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).toContain('Test Exam');
     });
     it("should send error when adding proctored exam allowance", function () {
@@ -254,7 +254,7 @@ describe('ProctoredExamAddAllowanceView', function () {
 
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('testuser1');
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('testuser1@test.com');
-        expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('Additional time (minutes)');
+        expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('Additional Time (minutes)');
         expect(this.proctored_exam_allowance.$el.find('tr.allowance-items').html()).not.toContain('Test Exam');
 
         // add the proctored exam allowance
@@ -282,7 +282,7 @@ describe('ProctoredExamAddAllowanceView', function () {
         //select the form values
         // invalid user_info returns error
         $('#proctored_exam').val('Test Exam');
-        $('#allowance_type').val('Additional time (minutes)');
+        $('#allowance_type').val('additional_time_granted');
         $('#allowance_value').val('2');
         $("#user_info").val('testuser112321');
 
@@ -299,7 +299,7 @@ describe('ProctoredExamAddAllowanceView', function () {
         //select the form values
         // empty value returns error
         $('#proctored_exam').val('Test Exam');
-        $('#allowance_type').val('Additional time (minutes)');
+        $('#allowance_type').val('Additional Time (minutes)');
         $('#allowance_value').val('');
         $("#user_info").val('testuser1');
 

edx_proctoring/static/proctoring/spec/proctored_exam_allowance_spec.js

@@ -59,7 +59,7 @@ describe('ProctoredExamAllowanceView', function () {
         '<td>N/A</td><td>N/A</td>' +
         '<% } %>' +
         '<td>' +
-        '<%- interpolate(gettext(" %(allowance_name)s "), { allowance_name: proctored_exam_allowance.key }, true) %>' +
+        '<%- interpolate(gettext(" %(allowance_name)s "), { allowance_name: proctored_exam_allowance.key_display_name }, true) %>' +
         '</td>' +
         '<td>' +
         '<%= proctored_exam_allowance.value %>' +

edx_proctoring/static/proctoring/templates/add-new-allowance.underscore

@@ -23,8 +23,8 @@
             <td>
                 <select id="allowance_type">
                     <% _.each(allowance_types, function(allowance_type){ %>
-                    <option value="<%= allowance_type %>">
-                    <%- interpolate(gettext(' %(allowance_type)s '), { allowance_type: allowance_type }, true) %>
+                    <option value="<%= allowance_type[0] %>">
+                        <%= allowance_type[1] %>
                     </option>
                     <% }); %>
                 </select>

edx_proctoring/static/proctoring/templates/course_allowances.underscore

@@ -23,23 +23,24 @@
             <% _.each(proctored_exam_allowances, function(proctored_exam_allowance){ %>
                 <tr class="allowance-items">
                     <td>
-                    <%- interpolate(gettext(' %(exam_display_name)s '), { exam_display_name: proctored_exam_allowance.proctored_exam.exam_name }, true) %>
+                      <%- proctored_exam_allowance.proctored_exam.exam_name %>
                     </td>
                     <% if (proctored_exam_allowance.user){ %>
                     <td>
-                     <%- interpolate(gettext(' %(username)s '), { username: proctored_exam_allowance.user.username }, true) %>
+                      <%= proctored_exam_allowance.user.username %>
                      </td>
                     <td>
-                    <%- interpolate(gettext(' %(email)s '), { email: proctored_exam_allowance.user.email }, true) %>
+                      <%= proctored_exam_allowance.user.email %>
                     </td>
                     <% }else{ %>
                         <td>N/A</td>
                         <td>N/A</td>
                     <% } %>
                     <td>
-                    <%- interpolate(gettext(' %(allowance_name)s '), { allowance_name: proctored_exam_allowance.key }, true) %>
+                      <%= proctored_exam_allowance.key_display_name %>
                     </td>
-                   <td><%= proctored_exam_allowance.value %></td>
+                   <td>
+                      <%- proctored_exam_allowance.value %></td>
                    <td>
                    <a data-exam-id="<%= proctored_exam_allowance.proctored_exam.id %>"
                     data-key-name="<%= proctored_exam_allowance.key %>"

edx_proctoring/tests/test_api.py

@@ -54,6 +54,7 @@ from edx_proctoring.models import (
     ProctoredExamStudentAllowance,
     ProctoredExamStudentAttempt,
     ProctoredExamStudentAttemptStatus,
+    ProctoredExamReviewPolicy,
 )
 
 from .utils import (
@@ -384,6 +385,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.
@@ -392,7 +411,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
         add_allowance_for_user(
             self.proctored_exam_id,
             self.user.username,
-            "Additional time (minutes)",
+            ProctoredExamStudentAllowance.ADDITIONAL_TIME_GRANTED,
             str(allowed_extra_time)
         )
         attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id)
@@ -1523,7 +1542,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.eligible, {
                 'status': ProctoredExamStudentAttemptStatus.eligible,
                 'short_description': 'Proctored Option Available',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': 'fa-pencil-square-o',
                 'in_completed_state': False
             }
         ),
@@ -1531,7 +1550,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.declined, {
                 'status': ProctoredExamStudentAttemptStatus.declined,
                 'short_description': 'Taking As Open Exam',
-                'suggested_icon': 'fa-unlock',
+                'suggested_icon': 'fa-pencil-square-o',
                 'in_completed_state': False
             }
         ),
@@ -1571,7 +1590,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.created, {
                 'status': ProctoredExamStudentAttemptStatus.created,
                 'short_description': 'Taking As Proctored Exam',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': 'fa-pencil-square-o',
                 'in_completed_state': False
             }
         ),
@@ -1579,7 +1598,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.ready_to_start, {
                 'status': ProctoredExamStudentAttemptStatus.ready_to_start,
                 'short_description': 'Taking As Proctored Exam',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': 'fa-pencil-square-o',
                 'in_completed_state': False
             }
         ),
@@ -1587,7 +1606,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.started, {
                 'status': ProctoredExamStudentAttemptStatus.started,
                 'short_description': 'Taking As Proctored Exam',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': 'fa-pencil-square-o',
                 'in_completed_state': False
             }
         ),
@@ -1595,7 +1614,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.ready_to_submit, {
                 'status': ProctoredExamStudentAttemptStatus.ready_to_submit,
                 'short_description': 'Taking As Proctored Exam',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': 'fa-pencil-square-o',
                 'in_completed_state': False
             }
         )
@@ -1626,7 +1645,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.eligible, {
                 'status': ProctoredExamStudentAttemptStatus.eligible,
                 'short_description': 'Ungraded Practice Exam',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': '',
                 'in_completed_state': False
             }
         ),
@@ -1697,7 +1716,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
             ProctoredExamStudentAttemptStatus.eligible, {
                 'status': ProctoredExamStudentAttemptStatus.eligible,
                 'short_description': 'Ungraded Practice Exam',
-                'suggested_icon': 'fa-lock',
+                'suggested_icon': '',
                 'in_completed_state': False
             }
         ),
@@ -1750,7 +1769,7 @@ class ProctoredExamApiTests(LoggedInTestCase):
         expected = {
             'status': ProctoredExamStudentAttemptStatus.eligible,
             'short_description': 'Ungraded Practice Exam',
-            'suggested_icon': 'fa-lock',
+            'suggested_icon': '',
             'in_completed_state': False
         }
 

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)

edx_proctoring/tests/test_serializer.py

@@ -30,7 +30,7 @@ class TestProctoredExamSerializer(unittest.TestCase):
         self.assertFalse(serializer.is_valid())
         self.assertDictEqual(
             {
-                'is_proctored': [u'This field is required.'],
-                'is_practice_exam': [u'This field is required.'],
+                'is_proctored': [u'"bla" is not a valid boolean.'],
+                'is_practice_exam': [u'"bla" is not a valid boolean.'],
             }, serializer.errors
         )

edx_proctoring/utils.py

@@ -3,6 +3,7 @@ Helpers for the HTTP APIs
 """
 
 import pytz
+import logging
 from datetime import datetime, timedelta
 
 from django.utils.translation import ugettext as _
@@ -10,6 +11,13 @@ from rest_framework.views import APIView
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.permissions import IsAuthenticated
 
+from edx_proctoring.models import (
+    ProctoredExamStudentAttempt,
+    ProctoredExamStudentAttemptHistory,
+)
+
+log = logging.getLogger(__name__)
+
 
 class AuthenticatedAPIView(APIView):
     """
@@ -82,3 +90,27 @@ def humanized_time(time_in_minutes):
 
     human_time = template.format(num_of_hours=hours, num_of_minutes=minutes)
     return human_time
+
+
+def locate_attempt_by_attempt_code(attempt_code):
+    """
+    Helper method to look up an attempt by attempt_code. This can be either in
+    the ProctoredExamStudentAttempt *OR* ProctoredExamStudentAttemptHistory tables
+    we will return a tuple of (attempt, is_archived_attempt)
+    """
+    attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt_by_code(attempt_code)
+
+    is_archived_attempt = False
+    if not attempt_obj:
+        # try archive table
+        attempt_obj = ProctoredExamStudentAttemptHistory.get_exam_attempt_by_code(attempt_code)
+        is_archived_attempt = True
+
+        if not attempt_obj:
+            # still can't find, error out
+            err_msg = (
+                'Could not locate attempt_code: {attempt_code}'.format(attempt_code=attempt_code)
+            )
+            log.error(err_msg)
+
+    return (attempt_obj, is_archived_attempt)

local_requirements.txt

@@ -2,7 +2,7 @@
 django>=1.4.12,<=1.4.22
 django-model-utils==2.3.1
 South>=0.7.6
-djangorestframework>=2.3.5,<=2.3.14
+djangorestframework>=3.1,<3.2
 django-ipware==1.1.0
 pytz>=2012h
 pycrypto>=2.6