From 9bb15dabd794cd17ac2dc03928cd0d16b58abb9f Mon Sep 17 00:00:00 2001 From: Zachary Hancock Date: Tue, 9 Mar 2021 15:13:56 -0500 Subject: [PATCH] onboard verified state precedence (#809) * onboard verification in another course will take precedence over non-verified states --- CHANGELOG.rst | 5 ++ edx_proctoring/__init__.py | 2 +- edx_proctoring/tests/test_views.py | 80 +++++++++++++++++++++++++++--- edx_proctoring/views.py | 35 ++++++++----- package.json | 2 +- 5 files changed, 101 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b9194f7b74b..7cab8aa1226 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,11 @@ Change Log Unreleased ~~~~~~~~~~ +[3.7.9] - 2021-03-09 +~~~~~~~~~~~~~~~~~~~~ +* Update onboarding status logic such that 'approved in another course' will take precedence over + a non verified state in the requested course. + [3.7.8] - 2021-03-08 ~~~~~~~~~~~~~~~~~~~~ * Add enrollment mode column to onboarding status panel on instructor dashboard diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 7f544b793f3..856a58d3340 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.8' +__version__ = '3.7.9' 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 a33a742db22..491ccec6002 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -604,7 +604,7 @@ def test_get_verified_attempt(self): def test_verified_in_another_course(self): """ - Test that, if there are no onboarding attempts in the current course, but there is at least + Test that, if there are no verified onboarding attempts in the current course, but there is at least one verified attempt in another course, the status will return `other_course_approved` and it will also return an `expiration_date` """ @@ -621,7 +621,11 @@ def test_verified_in_another_course(self): is_practice_exam=True, backend=proctoring_backend ) + # Create a submitted attempt in the current course + attempt_id = create_exam_attempt(self.onboarding_exam_id, self.user_id, True) + update_attempt_status(attempt_id, ProctoredExamStudentAttemptStatus.submitted) # Create an attempt in the other course that has been verified + attempt_id = create_exam_attempt(self.onboarding_exam_id, self.user_id, True) self._create_exam_attempt( other_course_onboarding_exam.id, ProctoredExamStudentAttemptStatus.verified, True ) @@ -638,6 +642,44 @@ def test_verified_in_another_course(self): )) self.assertIsNotNone(response_data['expiration_date']) + def test_verified_in_multiple_courses(self): + """ + Test that, if there is both a verified onboarding attempt in the current course, and there is + a verified attempt in another course, the status will return `verified` + """ + proctoring_backend = 'test' + other_course_id = 'x/y/z' + other_course_onboarding_exam = ProctoredExam.objects.create( + course_id=other_course_id, + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=True, + is_practice_exam=True, + backend=proctoring_backend + ) + # Create a verified attempt in the current course + attempt_id = create_exam_attempt(self.onboarding_exam_id, self.user_id, True) + update_attempt_status(attempt_id, ProctoredExamStudentAttemptStatus.verified) + # Create an attempt in the other course that has been verified + attempt_id = create_exam_attempt(self.onboarding_exam_id, self.user_id, True) + self._create_exam_attempt( + other_course_onboarding_exam.id, ProctoredExamStudentAttemptStatus.verified, True + ) + response = self.client.get( + reverse('edx_proctoring:user_onboarding.status') + + '?course_id={}'.format(self.course_id) + ) + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + self.assertEqual(response_data['onboarding_status'], 'verified') + self.assertEqual(response_data['onboarding_link'], reverse( + 'jump_to', + args=[self.course_id, self.onboarding_exam.content_id] + )) + def test_only_onboarding_exam(self): """ Test that only onboarding exam attempts are evaluated when requesting onboarding status @@ -1159,15 +1201,37 @@ def test_other_course_verified(self): # get serialized onboarding_attempt to get modified time serialized_onboarding_attempt_1 = get_exam_attempt_by_id(onboarding_attempt_1) - # Setup Learner 2's attempt in current course - onboarding_attempt_2 = create_exam_attempt( + # Setup Learner 2's attempt in current course and verified attempt in another course + onboarding_attempt_2_course = create_exam_attempt( self.onboarding_exam_id, self.learner_2.id, True, ) - update_attempt_status(onboarding_attempt_2, ProctoredExamStudentAttemptStatus.download_software_clicked) + onboarding_attempt_2_other_course = create_exam_attempt( + other_onboarding_exam_id, + self.learner_2.id, + True, + ) + update_attempt_status(onboarding_attempt_2_course, ProctoredExamStudentAttemptStatus.download_software_clicked) + update_attempt_status(onboarding_attempt_2_other_course, ProctoredExamStudentAttemptStatus.verified) + + # Setup Learner with verified attempt in both current course and another course + onboarding_attempt_course = create_exam_attempt( + self.onboarding_exam_id, + self.user.id, + True, + ) + onboarding_attempt_other_course = create_exam_attempt( + other_onboarding_exam_id, + self.user.id, + True, + ) + update_attempt_status(onboarding_attempt_course, ProctoredExamStudentAttemptStatus.verified) + update_attempt_status(onboarding_attempt_other_course, ProctoredExamStudentAttemptStatus.verified) + serialized_onboarding_attempt_course = get_exam_attempt_by_id(onboarding_attempt_course) + # get serialized onboarding_attempt to get modified time - serialized_onboarding_attempt_2 = get_exam_attempt_by_id(onboarding_attempt_2) + serialized_onboarding_attempt_2 = get_exam_attempt_by_id(onboarding_attempt_2_other_course) response = self.client.get(reverse( 'edx_proctoring:user_onboarding.status.course', @@ -1182,8 +1246,8 @@ def test_other_course_verified(self): { 'username': self.user.username, 'enrollment_mode': self.enrollment_modes[0], - 'status': InstructorDashboardOnboardingAttemptStatus.not_started, - 'modified': None, + 'status': InstructorDashboardOnboardingAttemptStatus.verified, + 'modified': serialized_onboarding_attempt_course.get('modified') }, { 'username': self.learner_1.username, @@ -1194,7 +1258,7 @@ def test_other_course_verified(self): { 'username': self.learner_2.username, 'enrollment_mode': self.enrollment_modes[2], - 'status': InstructorDashboardOnboardingAttemptStatus.setup_started, + 'status': InstructorDashboardOnboardingAttemptStatus.other_course_approved, 'modified': serialized_onboarding_attempt_2.get('modified'), }, ], diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index c6c810e8205..864ab86c6fd 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -371,23 +371,26 @@ def get(self, request): attempts = ProctoredExamStudentAttempt.objects.get_proctored_practice_attempts_by_course_id(course_id, [user]) - if len(attempts) == 0: - # If there are no attempts in the current course, check for a verified attempt in another course + # Default to the most recent attempt in the course if there are no verified attempts + relevant_attempt = attempts[0] if attempts else None + for attempt in attempts: + if attempt.status == ProctoredExamStudentAttemptStatus.verified: + relevant_attempt = attempt + break + + # If there is no verified attempt in the current course, check for a verified attempt in another course + verified_attempt = None + if not relevant_attempt or relevant_attempt.status != ProctoredExamStudentAttemptStatus.verified: attempt_dict = get_last_verified_onboarding_attempts_per_user( [user], onboarding_exam.backend, ) verified_attempt = attempt_dict.get(user.id) - if verified_attempt: - data['onboarding_status'] = InstructorDashboardOnboardingAttemptStatus.other_course_approved - data['expiration_date'] = verified_attempt.modified + timedelta(days=730) - else: - # Default to the most recent attempt in the course if there are no verified attempts - relevant_attempt = attempts[0] - for attempt in attempts: - if attempt.status == ProctoredExamStudentAttemptStatus.verified: - relevant_attempt = attempt - break + + if verified_attempt: + data['onboarding_status'] = InstructorDashboardOnboardingAttemptStatus.other_course_approved + data['expiration_date'] = verified_attempt.modified + timedelta(days=730) + elif relevant_attempt: data['onboarding_status'] = relevant_attempt.status return Response(data) @@ -484,7 +487,13 @@ def get(self, request, course_id): 'enrollment_mode': enrollment_modes_by_user_id.get(user.id), } - if not user_attempt and other_verified_attempt: + if ( + other_verified_attempt and + ( + not user_attempt or + (user_attempt and user_attempt.get('status') != ProctoredExamStudentAttemptStatus.verified) + ) + ): data['status'] = InstructorDashboardOnboardingAttemptStatus.other_course_approved data['modified'] = other_verified_attempt.modified else: diff --git a/package.json b/package.json index e8f05782e97..916b39e81a7 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.8", + "version": "3.7.9", "main": "edx_proctoring/static/index.js", "repository": { "type": "git",