From 0a5b7ab7aca7b84cae6e7c5b99c3e2a900c6c030 Mon Sep 17 00:00:00 2001 From: Michael Roytman Date: Fri, 5 Mar 2021 16:48:44 -0500 Subject: [PATCH] Fix bug with StudentProctoredExamAttempt put handler where course_id was being incorrectly determined, preventing course staff from marking learners' attempts as 'ready_to_resume'. --- CHANGELOG.rst | 4 ++ edx_proctoring/__init__.py | 2 +- edx_proctoring/tests/test_views.py | 77 +++++++++++++++++++++++++++++- edx_proctoring/views.py | 2 +- package.json | 2 +- 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 52b90e4fd17..e10d9d20967 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -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 ~~~~~~~~~~~~~~~~~~~~ diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 8ce693bffa8..be3a5e7c84f 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -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 diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index f992df4c7c5..a969cc20fd2 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -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. @@ -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) @@ -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 @@ -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) @@ -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', diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 224d3b2c4fa..61f643b9c43 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -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 = ( diff --git a/package.json b/package.json index 1f71ae6ca3e..08f94336b88 100644 --- a/package.json +++ b/package.json @@ -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",