Merge pull request #479 from edx/mroytman/EDUCATOR-3799-remove-rules-…

…configuration

remove rules as parameter for create and update of exam review policy

edx_proctoring/__init__.py

@@ -5,6 +5,6 @@
 from __future__ import absolute_import
 
 # Be sure to update the version number in edx_proctoring/package.json
-__version__ = '1.5.0rc1'
+__version__ = '1.5.0rc2'
 
 default_app_config = 'edx_proctoring.apps.EdxProctoringConfig'  # pylint: disable=invalid-name

edx_proctoring/api.py

@@ -114,7 +114,7 @@ def create_exam(course_id, content_id, exam_name, time_limit_mins, due_date=None
     return proctored_exam.id
 
 
-def create_exam_review_policy(exam_id, set_by_user_id, review_policy, rules):
+def create_exam_review_policy(exam_id, set_by_user_id, review_policy):
     """
     Creates a new exam_review_policy entity, if the review_policy
     for exam_id does not already exist. If it exists, then raise exception.
@@ -123,7 +123,6 @@ def create_exam_review_policy(exam_id, set_by_user_id, review_policy, rules):
         exam_id: the ID of the exam with which the review policy is to be associated
         set_by_user_id: the ID of the user setting this review policy
         review_policy: the review policy for the exam
-        rules: the dictionary of rules for the exam
 
     Returns: id (PK)
     """
@@ -135,53 +134,47 @@ def create_exam_review_policy(exam_id, set_by_user_id, review_policy, rules):
         proctored_exam_id=exam_id,
         set_by_user_id=set_by_user_id,
         review_policy=review_policy,
-        rules=rules,
     )
 
     log_msg = (
         u'Created ProctoredExamReviewPolicy ({review_policy}) with parameters: exam_id={exam_id}, '
-        u'set_by_user_id={set_by_user_id}, rules={rules}'.format(
+        u'set_by_user_id={set_by_user_id}'.format(
             exam_id=exam_id,
             review_policy=review_policy,
             set_by_user_id=set_by_user_id,
-            rules=rules,
         )
     )
     log.info(log_msg)
 
     return exam_review_policy.id
 
 
-def update_review_policy(exam_id, set_by_user_id, review_policy, rules):
+def update_review_policy(exam_id, set_by_user_id, review_policy):
     """
     Given a exam id, update/remove the existing record, otherwise raise exception if not found.
 
     Arguments:
         exam_id: the ID of the exam whose review policy is being updated
         set_by_user_id: the ID of the user updating this review policy
         review_policy: the review policy for the exam
-        rules: the dictionary of rules for the exam
 
     Returns: review_policy_id
     """
     log_msg = (
         u'Updating exam review policy with exam_id {exam_id} '
         u'set_by_user_id={set_by_user_id}, review_policy={review_policy} '
-        u'rules={rules}'
         .format(
-            exam_id=exam_id, set_by_user_id=set_by_user_id, review_policy=review_policy,
-            rules=rules
+            exam_id=exam_id, set_by_user_id=set_by_user_id, review_policy=review_policy
         )
     )
     log.info(log_msg)
     exam_review_policy = ProctoredExamReviewPolicy.get_review_policy_for_exam(exam_id)
     if exam_review_policy is None:
         raise ProctoredExamReviewPolicyNotFoundException
 
-    if review_policy or rules:
+    if review_policy:
         exam_review_policy.set_by_user_id = set_by_user_id
         exam_review_policy.review_policy = review_policy
-        exam_review_policy.rules = rules
         exam_review_policy.save()
         msg = 'Updated exam review policy with {exam_id}'.format(exam_id=exam_id)
         log.info(msg)
@@ -225,9 +218,6 @@ def _save_exam_on_backend(sender, instance, **kwargs):  # pylint: disable=unused
         exam = ProctoredExamSerializer(exam_obj).data
         if review_policy:
             exam['rule_summary'] = review_policy.review_policy
-            # When the rules are defined as boolean options,
-            # save them here
-            exam['rules'] = review_policy.rules
         backend = get_backend_provider(exam)
         external_id = backend.on_exam_saved(exam)
         if external_id and external_id != exam_obj.external_id:

edx_proctoring/migrations/0009_proctoredexamreviewpolicy_remove_rules.py

@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-12-10 16:47
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edx_proctoring', '0008_auto_20181116_1551'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='proctoredexamreviewpolicy',
+            name='rules',
+        ),
+        migrations.RemoveField(
+            model_name='proctoredexamreviewpolicyhistory',
+            name='rules',
+        ),
+    ]

edx_proctoring/models.py

@@ -16,7 +16,6 @@
 from django.dispatch import receiver
 from django.utils.translation import ugettext_noop
 
-from jsonfield import JSONField
 from model_utils.models import TimeStampedModel
 
 from edx_proctoring.exceptions import (
@@ -133,9 +132,6 @@ class ProctoredExamReviewPolicy(TimeStampedModel):
     # policy that will be passed to reviewers
     review_policy = models.TextField(default='')
 
-    # JSON rules that will be passed to reviewers
-    rules = JSONField(null=True)
-
     def __str__(self):
         # pragma: no cover
         return u"ProctoredExamReviewPolicy: {set_by_user} ({proctored_exam})".format(
@@ -179,9 +175,6 @@ class ProctoredExamReviewPolicyHistory(TimeStampedModel):
     # policy that will be passed to reviewers
     review_policy = models.TextField()
 
-    # JSON rules that will be passed to reviewers
-    rules = JSONField(null=True)
-
     class Meta:
         """ Meta class for this Django model """
         db_table = 'proctoring_proctoredexamreviewpolicyhistory'
@@ -228,7 +221,6 @@ def _make_review_policy_archive_copy(instance):
         set_by_user_id=instance.set_by_user_id,
         proctored_exam=instance.proctored_exam,
         review_policy=instance.review_policy,
-        rules=instance.rules,
     )
     archive_object.save()
 

edx_proctoring/serializers.py

@@ -116,13 +116,12 @@ class ProctoredExamReviewPolicySerializer(serializers.ModelSerializer):
     """
     proctored_exam = ProctoredExamSerializer()
     set_by_user = UserSerializer()
-    rules = serializers.JSONField()
 
     class Meta:
         """
         Meta Class
         """
         model = ProctoredExamReviewPolicy
         fields = (
-            "id", "created", "modified", "set_by_user", "proctored_exam", "review_policy", "rules"
+            "id", "created", "modified", "set_by_user", "proctored_exam", "review_policy",
         )

edx_proctoring/tests/test_api.py

@@ -206,16 +206,11 @@ def test_create_exam_review_policy(self):
         proctored exam and tests that it stores in the
         db correctly
         """
-        rules = {
-            'allow_grok': True
-        }
-
         proctored_exam = get_exam_by_id(self.proctored_exam_id)
         create_exam_review_policy(
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of paper',
-            rules=rules
         )
 
         # now get the exam review policy for the proctored exam
@@ -224,7 +219,6 @@ def test_create_exam_review_policy(self):
         self.assertEqual(exam_review_policy['proctored_exam']['id'], proctored_exam['id'])
         self.assertEqual(exam_review_policy['set_by_user']['id'], self.user_id)
         self.assertEqual(exam_review_policy['review_policy'], u'allow use of paper')
-        self.assertEqual(exam_review_policy['rules'], rules)
 
         # this tests that the backend received the callback when the review policy changed
         backend = get_backend_provider(proctored_exam)
@@ -240,7 +234,6 @@ def test_get_exam_review_policy(self):
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of paper',
-            rules={'allow_grok': True}
         )
 
         # now get the exam review policy for the proctored exam
@@ -254,68 +247,18 @@ def test_update_exam_review_policy_updates_review_policy(self):
         policy for proctored exam and tests that it stores in the
         db correctly.
         """
-        rules = {
-            'allow_grok': True
-        }
-
         proctored_exam = get_exam_by_id(self.proctored_exam_id)
         create_exam_review_policy(
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of paper',
-            rules=rules
         )
 
         # now update the exam review policy's review policy for the proctored exam
         update_review_policy(
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of calculator',
-            rules=rules
-        )
-
-        # now get the updated exam review policy for the proctored exam
-        exam_review_policy = get_review_policy_by_exam_id(proctored_exam['id'])
-
-        self.assertEqual(exam_review_policy['proctored_exam']['id'], proctored_exam['id'])
-        self.assertEqual(exam_review_policy['set_by_user']['id'], self.user_id)
-        self.assertEqual(exam_review_policy['review_policy'], u'allow use of calculator')
-        self.assertEqual(exam_review_policy['rules'], rules)
-
-    def test_update_exam_review_policy_updates_rules(self):
-        """
-        Test to update existing exam review policy's rules for
-        proctored exam and tests that it stores in the
-        db correctly.
-        """
-        rules = {
-            'allow_grok': True
-        }
-
-        proctored_exam = get_exam_by_id(self.proctored_exam_id)
-        create_exam_review_policy(
-            exam_id=proctored_exam['id'],
-            set_by_user_id=self.user_id,
-            review_policy=u'allow use of paper',
-            rules=rules
-        )
-
-        # this tests that the backend received the callback when the review policy changed
-        backend = get_backend_provider(proctored_exam)
-        self.assertEqual(backend.last_exam['rule_summary'], u'allow use of paper')
-        self.assertEqual(backend.last_exam['rules'], rules)
-
-        updated_rules = {
-            'allow_foo': False
-        }
-
-        # now update the exam review policy's rules for the proctored exam
-        # now update the exam review policy for the proctored exam
-        update_review_policy(
-            exam_id=proctored_exam['id'],
-            set_by_user_id=self.user_id,
-            review_policy=u'allow use of calculator',
-            rules=updated_rules
         )
 
         # now get the updated exam review policy for the proctored exam
@@ -324,11 +267,8 @@ def test_update_exam_review_policy_updates_rules(self):
         self.assertEqual(exam_review_policy['proctored_exam']['id'], proctored_exam['id'])
         self.assertEqual(exam_review_policy['set_by_user']['id'], self.user_id)
         self.assertEqual(exam_review_policy['review_policy'], u'allow use of calculator')
-        self.assertEqual(exam_review_policy['rules'], updated_rules)
-        self.assertEqual(backend.last_exam['rule_summary'], u'allow use of calculator')
-        self.assertEqual(backend.last_exam['rules'], updated_rules)
 
-    def test_update_review_policy_with_empty_review_policy_and_rules_removes_review_policy(self):
+    def test_update_review_policy_with_empty_review_policy_removes_review_policy(self):
         """
         Test that updating an proctored exam's exam review policy with an
         empty review policy removes the exam review policy.
@@ -338,7 +278,6 @@ def test_update_review_policy_with_empty_review_policy_and_rules_removes_review_
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of paper',
-            rules={'allow_grok': True}
         )
         # now update the exam review policy for the proctored exam
         # with review_policy value to "" and rules value to "".
@@ -348,7 +287,6 @@ def test_update_review_policy_with_empty_review_policy_and_rules_removes_review_
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'',
-            rules=u''
         )
         with self.assertRaises(ProctoredExamReviewPolicyNotFoundException):
             get_review_policy_by_exam_id(proctored_exam['id'])
@@ -363,7 +301,6 @@ def test_remove_existing_exam_review_policy(self):
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of paper',
-            rules={'allow_grok': True}
         )
 
         # now remove the exam review policy for the proctored exam
@@ -403,7 +340,6 @@ def test_update_non_existing_exam_review_policy(self):
                 exam_id=self.practice_exam_id,
                 set_by_user_id=10,
                 review_policy=u'allow use of calculator',
-                rules={'allow_grok': True}
             )
 
     def test_create_exam_review_policy_with_same_exam_id(self):
@@ -415,7 +351,6 @@ def test_create_exam_review_policy_with_same_exam_id(self):
             exam_id=proctored_exam['id'],
             set_by_user_id=self.user_id,
             review_policy=u'allow use of paper',
-            rules={'allow_grok': True}
         )
 
         # create the same review policy again will raise exception
@@ -424,7 +359,6 @@ def test_create_exam_review_policy_with_same_exam_id(self):
                 exam_id=proctored_exam['id'],
                 set_by_user_id=self.user_id,
                 review_policy=u'allow use of paper',
-                rules={'allow_grok': True}
             )
 
     def test_get_non_existing_review_policy_raises_exception(self):

edx_proctoring/tests/test_models.py

@@ -305,13 +305,10 @@ def test_exam_review_policy(self):
             time_limit_mins=90
         )
 
-        rules = {'allow_grok': True}
-
         policy = ProctoredExamReviewPolicy.objects.create(
             set_by_user_id=self.user.id,
             proctored_exam=proctored_exam,
             review_policy='Foo Policy',
-            rules=rules
         )
 
         attempt = ProctoredExamStudentAttempt.create_exam_attempt(
@@ -330,10 +327,7 @@ def test_exam_review_policy(self):
         self.assertEqual(len(history), 0)
 
         # now update it
-        updated_rules = {'allow_foo': False}
-
         policy.review_policy = 'Updated Foo Policy'
-        policy.rules = updated_rules
         policy.save()
 
         # look in history
@@ -344,7 +338,6 @@ def test_exam_review_policy(self):
         self.assertEqual(previous.proctored_exam_id, proctored_exam.id)
         self.assertEqual(previous.original_id, policy.id)
         self.assertEqual(previous.review_policy, 'Foo Policy')
-        self.assertEqual(previous.rules, rules)
 
         # now delete updated one
         deleted_id = policy.id
@@ -358,14 +351,12 @@ def test_exam_review_policy(self):
         self.assertEqual(previous.proctored_exam_id, proctored_exam.id)
         self.assertEqual(previous.original_id, deleted_id)
         self.assertEqual(previous.review_policy, 'Foo Policy')
-        self.assertEqual(previous.rules, rules)
 
         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')
-        self.assertEqual(previous.rules, updated_rules)
 
         # assert that we cannot delete history!
         with self.assertRaises(NotImplementedError):

package.json

@@ -1,7 +1,8 @@
 {
   "name": "@edx/edx-proctoring",
   "//": "Be sure to update the version number in edx_proctoring/__init__.py",
-  "version": "1.5.0-rc.1",
+  "//": "Note that the version format is slightly different than that of the Python version when using prereleases.",
+  "version": "1.5.0-rc.2",
   "main": "edx_proctoring/static/index.js",
   "repository": {
     "type": "git",