Merge pull request #178 from edx/cdodge/add-additional-allowances

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

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)