Skip to content

Commit

Permalink
Merge pull request #807 from edx/mroytman/resume-exam-fix
Browse files Browse the repository at this point in the history
Fix Course ID being Incorrectly Determined by StudentProctoredExamAttempt PUT handler, preventing course staff from marking exam attempts as "ready_to_resume".
  • Loading branch information
MichaelRoytman authored Mar 5, 2021
2 parents 8566363 + 0a5b7ab commit 09a111e
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ Change Log
Unreleased
~~~~~~~~~~
[3.7.6] - 2021-03-05
~~~~~~~~~~~~~~~~~~~~
* Fix bug with StudentProctoredExamAttempt put handler where course_id was being incorrectly determined,
preventing course staff from marking learners' attempts as "ready_to_resume".

[3.7.5] - 2021-03-05
~~~~~~~~~~~~~~~~~~~~
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"""

# Be sure to update the version number in edx_proctoring/package.json
__version__ = '3.7.5'
__version__ = '3.7.6'

default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name
77 changes: 75 additions & 2 deletions edx_proctoring/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3073,7 +3073,13 @@ def test_mark_ready_to_resume_attempt_for_self(self):
attempt = get_exam_attempt_by_id(old_attempt_id)
self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.ready_to_resume)

def test_mark_ready_to_resume_attempt_for_other_as_staff(self):
@ddt.data(
(True, True),
(True, False),
(False, True)
)
@ddt.unpack
def test_mark_ready_to_resume_attempt_for_other_as_staff(self, is_staff, is_course_staff):
"""
Assert that a staff user can submit a "mark_ready_to_resume" action on
behalf of another user when supplying the user's id in the request body.
Expand Down Expand Up @@ -3130,6 +3136,12 @@ def test_mark_ready_to_resume_attempt_for_other_as_staff(self):
attempt = get_exam_attempt_by_id(old_attempt_id)
self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.error)

if not is_course_staff:
set_runtime_service('instructor', MockInstructorService(is_user_course_staff=False))
if not is_staff:
self.user.is_staff = False
self.user.save()

# Log in the staff user.
self.client.login_user(self.user)

Expand All @@ -3151,7 +3163,13 @@ def test_mark_ready_to_resume_attempt_for_other_as_staff(self):
attempt = get_exam_attempt_by_id(old_attempt_id)
self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.ready_to_resume)

def test_mark_ready_to_resume_attempt_for_other_as_staff_no_user_id(self):
@ddt.data(
(True, True),
(True, False),
(False, True)
)
@ddt.unpack
def test_mark_ready_to_resume_attempt_for_other_as_staff_no_user_id(self, is_staff, is_course_staff):
"""
Assert that a staff user cannot submit any action on behalf of another user without
specifying the user's id in the request body.Assert that a ProctoredExamPermissionDenied
Expand Down Expand Up @@ -3209,6 +3227,12 @@ def test_mark_ready_to_resume_attempt_for_other_as_staff_no_user_id(self):
attempt = get_exam_attempt_by_id(old_attempt_id)
self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.error)

if not is_course_staff:
set_runtime_service('instructor', MockInstructorService(is_user_course_staff=False))
if not is_staff:
self.user.is_staff = False
self.user.save()

# Log in the staff user.
self.client.login_user(self.user)

Expand Down Expand Up @@ -3310,6 +3334,55 @@ def test_mark_ready_to_resume_attempt_for_other_not_as_staff(self):
attempt = get_exam_attempt_by_id(old_attempt_id)
self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.error)

def test_is_user_course_or_global_staff_called_correct_args(self):
# Log in as student taking the exam.
self.client.login_user(self.student_taking_exam)

# Create an exam.
proctored_exam = ProctoredExam.objects.create(
course_id='a/b/c',
content_id='test_content',
exam_name='Test Exam',
external_id='123aXqe3',
time_limit_mins=90,
)

# POST an exam attempt.
attempt_data = {
'exam_id': proctored_exam.id,
'external_id': proctored_exam.external_id,
'start_clock': False,
}

response = self.client.post(
reverse('edx_proctoring:proctored_exam.attempt.collection'),
attempt_data
)

response_data = json.loads(response.content.decode('utf-8'))
attempt_id = response_data['exam_attempt_id']

# Log in the staff user.
self.client.login_user(self.student_taking_exam)

# Transition the exam attempt into the ready_to_resume state. This is not
# a valid state transition, but that's okay because we're not testing that.
with patch('edx_proctoring.views.is_user_course_or_global_staff') as is_staff_mock:
response = self.client.put(
reverse('edx_proctoring:proctored_exam.attempt', args=[attempt_id]),
json.dumps({
'action': 'mark_ready_to_resume',
'user_id': self.student_taking_exam.id,
}),
content_type='application/json'
)
is_staff_mock.assert_called_once()

# We can't assert that is_user_course_or_global_staff is called with self.user because
# Django lazily loads request.user, so it's a SimpleLazyObject.
(_, course_id) = is_staff_mock.call_args.args
assert course_id == proctored_exam.course_id

@ddt.data(
'stop',
'start',
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ def put(self, request, attempt_id):
)
raise StudentExamAttemptDoesNotExistsException(err_msg)

course_id = attempt['proctored_exam']['id']
course_id = attempt['proctored_exam']['course_id']
action = request.data.get('action')

err_msg = (
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@edx/edx-proctoring",
"//": "Be sure to update the version number in edx_proctoring/__init__.py",
"//": "Note that the version format is slightly different than that of the Python version when using prereleases.",
"version": "3.7.5",
"version": "3.7.6",
"main": "edx_proctoring/static/index.js",
"repository": {
"type": "git",
Expand Down

0 comments on commit 09a111e

Please sign in to comment.