Add out additional allowance that will be passed to Software Secure reviewers,…

Add out additional allowance that will be passed to Software Secure reviewers, also refactor existing allowance to not use a display string as a key name

edx_proctoring/api.py

@@ -347,20 +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(
@@ -395,6 +391,14 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
                 '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,

edx_proctoring/backends/software_secure.py

@@ -335,6 +335,18 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         callback_url = context['callback_url']
         full_name = context['full_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
+            )
+
         (first_name, last_name) = self._split_fullname(full_name)
 
         now = datetime.datetime.utcnow()
@@ -347,7 +359,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": review_policy,
+            "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_software_secure.py

@@ -23,6 +23,8 @@ from edx_proctoring.api import (
     create_exam,
     create_exam_attempt,
     remove_exam_attempt,
+    add_allowance_for_user
+
 )
 from edx_proctoring.exceptions import (
     StudentExamAttemptDoesNotExistsException,
@@ -37,6 +39,7 @@ from edx_proctoring.models import (
     ProctoredExamSoftwareSecureReviewHistory,
     ProctoredExamReviewPolicy,
     ProctoredExamStudentAttemptHistory,
+    ProctoredExamStudentAllowance
 )
 from edx_proctoring.backends.tests.test_review_payload import TEST_REVIEW_PAYLOAD
 
@@ -141,7 +144,8 @@ class SoftwareSecureTests(TestCase):
             self.assertEqual(attempt['external_id'], 'foobar')
             self.assertIsNone(attempt['started_at'])
 
-    def test_attempt_with_review_policy(self):
+    @ddt.data(None, 'additional person allowed in room')
+    def test_attempt_with_review_policy(self, review_policy_exception):
         """
         Create an unstarted proctoring attempt with a review policy associated with it.
         """
@@ -154,6 +158,14 @@ class SoftwareSecureTests(TestCase):
             is_proctored=True
         )
 
+        if review_policy_exception:
+            add_allowance_for_user(
+                exam_id,
+                self.user.id,
+                ProctoredExamStudentAllowance.REVIEW_POLICY_EXCEPTION,
+                review_policy_exception
+            )
+
         policy = ProctoredExamReviewPolicy.objects.create(
             set_by_user_id=self.user.id,
             proctored_exam_id=exam_id,
@@ -169,10 +181,17 @@ class SoftwareSecureTests(TestCase):
             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
+            result = get_backend_provider(emphemeral=True)._get_payload(exam, context)
 
             # assert that this is in the 'reviewerNotes' field that is passed to SoftwareSecure
-            self.assertEqual(result['reviewerNotes'], context['review_policy'])
+            expected = context['review_policy']
+            if review_policy_exception:
+                expected = '{base}; {exception}'.format(
+                    base=expected,
+                    exception=review_policy_exception
+                )
+
+            self.assertEqual(result['reviewerNotes'], expected)
             return result
 
         with HTTMock(mock_response_content):
@@ -180,7 +199,7 @@ class SoftwareSecureTests(TestCase):
             # 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
+            with patch.object(get_backend_provider(), '_get_payload', assert_get_payload_mock):
                 attempt_id = create_exam_attempt(
                     exam_id,
                     self.user.id,

edx_proctoring/models.py

@@ -579,6 +579,15 @@ class ProctoredExamStudentAllowance(TimeStampedModel):
     Information about allowing a student additional time on exam.
     """
 
+    # DONT EDIT THE KEYS - THE FIRST VALUE OF THE TUPLE - AS ARE THEY ARE STORED IN THE DATABASE
+    # THE SECOND ELEMENT OF THE TUPLE IS A DISPLAY STRING AND CAN BE EDITED
+    ADDITIONAL_TIME_GRANTED = ('additional_time_granted', _('Additional Time (minutes)'))
+    REVIEW_POLICY_EXCEPTION = ('review_policy_exception', _('Review Policy Exception'))
+
+    all_allowances = [
+        ADDITIONAL_TIME_GRANTED + REVIEW_POLICY_EXCEPTION
+    ]
+
     objects = ProctoredExamStudentAllowanceManager()
 
     user = models.ForeignKey(User)
@@ -625,22 +634,55 @@ class ProctoredExamStudentAllowance(TimeStampedModel):
         """
         Add or (Update) an allowance for a user within a given exam
         """
-        users = User.objects.filter(username=user_info)
-        if not users.exists():
-            users = User.objects.filter(email=user_info)
+        user_id = None
+
+        # see if key is a tuple, if it is, then the first element is the key
+        if isinstance(key, tuple) and len(key) > 0:
+            key = key[0]
+
+        # were we passed a PK?
+        if isinstance(user_info, (int, long)):
+            user_id = user_info
+        else:
+            # we got a string, so try to resolve it
+            users = User.objects.filter(username=user_info)
+            if not users.exists():
+                users = User.objects.filter(email=user_info)
 
-        if not users.exists():
-            err_msg = (
-                'Cannot find user against {user_info}'
-            ).format(user_info=user_info)
-            raise UserNotFoundException(err_msg)
+            if not users.exists():
+                err_msg = (
+                    'Cannot find user against {user_info}'
+                ).format(user_info=user_info)
+                raise UserNotFoundException(err_msg)
+
+            user_id = users[0].id
 
         try:
-            student_allowance = cls.objects.get(proctored_exam_id=exam_id, user_id=users[0].id, key=key)
+            student_allowance = cls.objects.get(proctored_exam_id=exam_id, user_id=user_id, key=key)
             student_allowance.value = value
             student_allowance.save()
         except cls.DoesNotExist:  # pylint: disable=no-member
-            cls.objects.create(proctored_exam_id=exam_id, user_id=users[0].id, key=key, value=value)
+            cls.objects.create(proctored_exam_id=exam_id, user_id=user_id, key=key, value=value)
+
+    @classmethod
+    def get_additional_time_granted(cls, exam_id, user_id):
+        """
+        Helper method to get the additional time granted
+        """
+        allowance = cls.get_allowance_for_user(exam_id, user_id, cls.ADDITIONAL_TIME_GRANTED[0])
+        if allowance:
+            return int(allowance.value)
+
+        return None
+
+    @classmethod
+    def get_review_policy_exception(cls, exam_id, user_id):
+        """
+        Helper method to get the policy exception that reviewers should
+        follow
+        """
+        allowance = cls.get_allowance_for_user(exam_id, user_id, cls.REVIEW_POLICY_EXCEPTION[0])
+        return allowance.value if allowance else None
 
 
 class ProctoredExamStudentAllowanceHistory(TimeStampedModel):

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

@@ -400,7 +400,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)