Merge pull request #225 from edx/cdodge/fix-unicode-output

Remove illegal characters from exam names before registering exam with SoftwareSecure

edx_proctoring/backends/software_secure.py

@@ -33,6 +33,9 @@ from edx_proctoring. models import (
 log = logging.getLogger(__name__)
 
 
+SOFTWARE_SECURE_INVALID_CHARS = '[]<>#:|?/\'"*\\'
+
+
 class SoftwareSecureBackendProvider(ProctoringBackendProvider):
     """
     Implementation of the ProctoringBackendProvider for Software Secure's
@@ -355,6 +358,10 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         now = datetime.datetime.utcnow()
         start_time_str = now.strftime("%a, %d %b %Y %H:%M:%S GMT")
         end_time_str = (now + datetime.timedelta(minutes=time_limit_mins)).strftime("%a, %d %b %Y %H:%M:%S GMT")
+        # remove all illegal characters from the exam name
+        exam_name = exam['exam_name']
+        for character in SOFTWARE_SECURE_INVALID_CHARS:
+            exam_name = exam_name.replace(character, '_')
         return {
             "examCode": attempt_code,
             "organization": self.organization,
@@ -365,7 +372,7 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
             "reviewerNotes": reviewer_notes,
             "examPassword": self._encrypt_password(self.crypto_key, attempt_code),
             "examSponsor": self.exam_sponsor,
-            "examName": exam['exam_name'],
+            "examName": exam_name,
             "ssiProduct": 'rp-now',
             # need to pass in a URL to the LMS?
             "examUrl": callback_url,

edx_proctoring/backends/tests/test_software_secure.py

@@ -41,9 +41,12 @@ from edx_proctoring.models import (
     ProctoredExamStudentAttemptHistory,
     ProctoredExamStudentAllowance
 )
-from edx_proctoring.backends.tests.test_review_payload import TEST_REVIEW_PAYLOAD
+from edx_proctoring.backends.tests.test_review_payload import (
+    TEST_REVIEW_PAYLOAD
+)
 
 from edx_proctoring.tests.test_services import MockCreditService
+from edx_proctoring.backends.software_secure import SOFTWARE_SECURE_INVALID_CHARS
 
 
 @all_requests
@@ -216,14 +219,6 @@ class SoftwareSecureTests(TestCase):
         Create an unstarted proctoring attempt with no review policy associated with it.
         """
 
-        exam_id = create_exam(
-            course_id='foo/bar/baz',
-            content_id='content',
-            exam_name='Sample Exam',
-            time_limit_mins=10,
-            is_proctored=True
-        )
-
         def assert_get_payload_mock_no_policy(exam, context):
             """
             Add a mock shim so we can assert that the _get_payload has been called with the right
@@ -236,23 +231,40 @@ class SoftwareSecureTests(TestCase):
 
             # assert that we use the default that is defined in system configuration
             self.assertEqual(result['reviewerNotes'], constants.DEFAULT_SOFTWARE_SECURE_REVIEW_POLICY)
+
+            # the check that if a colon was passed in for the exam name, then the colon was changed to
+            # a dash. This is because SoftwareSecure cannot handle a colon in the exam name
+            for illegal_char in SOFTWARE_SECURE_INVALID_CHARS:
+                if illegal_char in exam['exam_name']:
+                    self.assertNotIn(illegal_char, result['examName'])
+                    self.assertIn('_', result['examName'])
+
             return result
 
-        with HTTMock(mock_response_content):
-            # patch the _get_payload method on the backend provider
-            # so that we can assert that we are called with the review policy
-            # undefined and that we use the system default
-            with patch.object(get_backend_provider(), '_get_payload', assert_get_payload_mock_no_policy):  # pylint: disable=protected-access
-                attempt_id = create_exam_attempt(
-                    exam_id,
-                    self.user.id,
-                    taking_as_proctored=True
-                )
-                self.assertGreater(attempt_id, 0)
+        for illegal_char in SOFTWARE_SECURE_INVALID_CHARS:
+            exam_id = create_exam(
+                course_id='foo/bar/baz',
+                content_id='content with {}'.format(illegal_char),
+                exam_name='Sample Exam with {} character'.format(illegal_char),
+                time_limit_mins=10,
+                is_proctored=True
+            )
 
-                # make sure we recorded that there is no review policy
-                attempt = get_exam_attempt_by_id(attempt_id)
-                self.assertIsNone(attempt['review_policy_id'])
+            with HTTMock(mock_response_content):
+                # patch the _get_payload method on the backend provider
+                # so that we can assert that we are called with the review policy
+                # undefined and that we use the system default
+                with patch.object(get_backend_provider(), '_get_payload', assert_get_payload_mock_no_policy):  # pylint: disable=protected-access
+                    attempt_id = create_exam_attempt(
+                        exam_id,
+                        self.user.id,
+                        taking_as_proctored=True
+                    )
+                    self.assertGreater(attempt_id, 0)
+
+                    # make sure we recorded that there is no review policy
+                    attempt = get_exam_attempt_by_id(attempt_id)
+                    self.assertIsNone(attempt['review_policy_id'])
 
     def test_single_name_attempt(self):
         """

edx_proctoring/models.py

@@ -58,7 +58,7 @@ class ProctoredExam(TimeStampedModel):
         How to serialize myself as a string
         """
 
-        return "{course_id}: {exam_name} ({active})".format(
+        return u"{course_id}: {exam_name} ({active})".format(
             course_id=self.course_id,
             exam_name=self.exam_name,
             active='active' if self.is_active else 'inactive',

edx_proctoring/tests/test_models.py

+# coding=utf-8
+# pylint: disable=invalid-name
 """
 All tests for the models.py
 """
-# pylint: disable=invalid-name
 from edx_proctoring.models import (
     ProctoredExam,
     ProctoredExamStudentAllowance,
@@ -29,6 +30,20 @@ class ProctoredExamModelTests(LoggedInTestCase):
         """
         super(ProctoredExamModelTests, self).setUp()
 
+    def test_unicode(self):
+        """
+        Make sure we support Unicode characters
+        """
+        proctored_exam = ProctoredExam.objects.create(
+            course_id='test_course',
+            content_id='test_content',
+            exam_name=u'अआईउऊऋऌ अआईउऊऋऌ',
+            external_id='123aXqe3',
+            time_limit_mins=90
+        )
+        output = unicode(proctored_exam)
+        self.assertEquals(output, u"test_course: अआईउऊऋऌ अआईउऊऋऌ (inactive)")
+
     def test_save_proctored_exam_student_allowance_history(self):  # pylint: disable=invalid-name
         """
         Test to Save and update the proctored Exam Student Allowance object.

setup.py

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