Skip to content

Commit

Permalink
remove rules as parameter for create and update of exam review policy
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelRoytman committed Dec 10, 2018
1 parent 06131ca commit c271b33
Show file tree
Hide file tree
Showing 13 changed files with 81 additions and 137 deletions.
2 changes: 1 addition & 1 deletion edx_proctoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 5 additions & 15 deletions edx_proctoring/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
"""
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
),
]
8 changes: 0 additions & 8 deletions edx_proctoring/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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()

Expand Down
3 changes: 1 addition & 2 deletions edx_proctoring/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
68 changes: 1 addition & 67 deletions edx_proctoring/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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 "".
Expand All @@ -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'])
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand All @@ -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):
Expand Down
9 changes: 0 additions & 9 deletions edx_proctoring/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
Loading

0 comments on commit c271b33

Please sign in to comment.