From 943a269aa7a45b1c46d955c6fa74b7afccd0d040 Mon Sep 17 00:00:00 2001 From: Bianca Severino Date: Wed, 3 Nov 2021 14:36:01 -0400 Subject: [PATCH] feat: remove references to ready_to_resume and resumed statuses --- CHANGELOG.rst | 4 ++ edx_proctoring/__init__.py | 2 +- edx_proctoring/admin.py | 1 - edx_proctoring/api.py | 18 ++---- edx_proctoring/exceptions.py | 2 +- .../js/views/proctored_exam_attempt_view.js | 1 - .../spec/proctored_exam_attempt_spec.js | 13 ++-- ...proctored-exam-attempts-grouped.underscore | 6 +- edx_proctoring/statuses.py | 19 +----- edx_proctoring/tests/test_api.py | 64 ++++++------------- edx_proctoring/tests/test_views.py | 13 ++-- edx_proctoring/tests/utils.py | 12 ++-- package.json | 2 +- 13 files changed, 58 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bcbb0221139..0025d1b48aa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,10 @@ Change Log Unreleased ~~~~~~~~~~ +[4.6.0] - 2021-11-03 +~~~~~~~~~~~~~~~~~~~~ +* Remove references to "ready_to_resume" and "resumed" statuses. + [4.5.0] - 2021-11-01 ~~~~~~~~~~~~~~~~~~~~ * Remove references to VERIFIED_NAME_FLAG Django waffle flag. diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 7724cd05af8..2c3bc535850 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__ = '4.5.0' +__version__ = '4.6.0' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/admin.py b/edx_proctoring/admin.py index 8ff858212ee..73fb95582f1 100644 --- a/edx_proctoring/admin.py +++ b/edx_proctoring/admin.py @@ -397,7 +397,6 @@ class Meta: (ProctoredExamStudentAttemptStatus.verified, _('Verified')), (ProctoredExamStudentAttemptStatus.rejected, _('Rejected')), (ProctoredExamStudentAttemptStatus.error, _('Error')), - (ProctoredExamStudentAttemptStatus.ready_to_resume, _('Ready To Resume')), ] if settings.DEBUG: STATUS_CHOICES.extend([ diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 578862d4d20..121f31aba02 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -1232,7 +1232,7 @@ def mark_exam_attempt_timeout(attempt_id): def mark_exam_attempt_as_ready(attempt_id): """ - Marks the exam attemp as ready to start + Marks the exam attempt as ready to start """ return update_attempt_status(attempt_id, ProctoredExamStudentAttemptStatus.ready_to_start) @@ -1282,28 +1282,18 @@ def is_attempt_ready_to_resume(attempt): """ # if the attempt has been marked as ready to resume, check to see that it has not been resumed yet - # we also want to check if the status is ready to resume for backwards compatibility. Older attempts - # may still have this status, but not have the correct value for the ready_to_resume_field - return ( - (attempt['ready_to_resume'] or attempt['status'] == ProctoredExamStudentAttemptStatus.ready_to_resume) - and not attempt['resumed'] - ) + return attempt['ready_to_resume'] and not attempt['resumed'] def is_attempt_in_resume_process(attempt_obj): """ Determine if an exam attempt is in the resume process. This should check if either of the resume - related fields are set to true, or if the attempt status is a resume status. We must check both - in order for older attempts with the resume statuses to function as expected. + related fields are set to true. Arguments: attempt_obj: attempt object """ - return ( - attempt_obj.ready_to_resume - or attempt_obj.resumed - or ProctoredExamStudentAttemptStatus.is_resume_status(attempt_obj.status) - ) + return attempt_obj.ready_to_resume or attempt_obj.resumed def is_state_transition_legal(from_status, to_status): diff --git a/edx_proctoring/exceptions.py b/edx_proctoring/exceptions.py index 4427f2032ac..26581c65ac5 100644 --- a/edx_proctoring/exceptions.py +++ b/edx_proctoring/exceptions.py @@ -151,7 +151,7 @@ class ProctoredExamIllegalStatusTransition(ProctoredBaseException): class ProctoredExamIllegalResumeUpdate(ProctoredBaseException): """ Raised if an update to the ready_to_resume or resumed fields should not be allowed, - e.g. if we try to update ready_to_resume to True on an examp attempt, but the attempt + e.g. if we try to update ready_to_resume to True on an exam attempt, but the attempt is not resumable. """ diff --git a/edx_proctoring/static/proctoring/js/views/proctored_exam_attempt_view.js b/edx_proctoring/static/proctoring/js/views/proctored_exam_attempt_view.js index 1acbd231f10..18c909bae76 100644 --- a/edx_proctoring/static/proctoring/js/views/proctored_exam_attempt_view.js +++ b/edx_proctoring/static/proctoring/js/views/proctored_exam_attempt_view.js @@ -11,7 +11,6 @@ edx = edx || {}; created: gettext('Created'), download_software_clicked: gettext('Download Software Clicked'), ready_to_start: gettext('Ready to start'), - ready_to_resume: gettext('Ready to resume'), started: gettext('Started'), ready_to_submit: gettext('Ready to submit'), declined: gettext('Declined'), diff --git a/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js b/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js index bdacd691ea3..c7f6e350899 100644 --- a/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js +++ b/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js @@ -290,8 +290,7 @@ describe('ProctoredExamAttemptView', function() { '<% } else { %> N/A <% } %>' + '' + '<% if (' + - '(proctored_exam_attempt.ready_to_resume || proctored_exam_attempt.status == "ready_to_resume") ' + - '&& !proctored_exam_attempt.resumed' + + 'proctored_exam_attempt.ready_to_resume && !proctored_exam_attempt.resumed' + ') { %>' + '' + '' + @@ -361,8 +360,7 @@ describe('ProctoredExamAttemptView', function() { '<%= getExamAttemptStatus(proctored_exam_attempt.status) %>' + '<% } else { %> N/A <% } %> ' + '<% if (' + - '(proctored_exam_attempt.ready_to_resume || proctored_exam_attempt.status == "ready_to_resume") ' + - '&& !proctored_exam_attempt.resumed' + + 'proctored_exam_attempt.ready_to_resume && !proctored_exam_attempt.resumed' + ') { %>' + '' + '' + @@ -590,7 +588,7 @@ describe('ProctoredExamAttemptView', function() { 'Content-Type': 'application/json' }, JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson( - 'ready_to_resume', false, false, false, true) + 'error', false, false, false, true) ) ] ); @@ -651,7 +649,9 @@ describe('ProctoredExamAttemptView', function() { { 'Content-Type': 'application/json' }, - JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson('ready_to_resume')) + JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson( + 'error', false, false, true, false + )) ] ); @@ -685,7 +685,6 @@ describe('ProctoredExamAttemptView', function() { expect(this.proctored_exam_attempt_view.$el.find('tbody').html()).toContain('testuser1'); expect(this.proctored_exam_attempt_view.$el.find('tbody').html()).toContain('Normal Exam'); - expect(this.proctored_exam_attempt_view.$el.find('tbody.accordion-panel').html()).toContain('Ready to resume'); expect(this.proctored_exam_attempt_view.$el.find('tbody.accordion-panel').html()).toContain('fa-check-circle'); expect(this.proctored_exam_attempt_view.$el.find('.actions-dropdown').hasClass('is-visible')).toEqual(false); }); diff --git a/edx_proctoring/static/proctoring/templates/student-proctored-exam-attempts-grouped.underscore b/edx_proctoring/static/proctoring/templates/student-proctored-exam-attempts-grouped.underscore index 39a728822f9..620eb6ee8f4 100644 --- a/edx_proctoring/static/proctoring/templates/student-proctored-exam-attempts-grouped.underscore +++ b/edx_proctoring/static/proctoring/templates/student-proctored-exam-attempts-grouped.underscore @@ -194,8 +194,7 @@ <% } %> <% if ( - (proctored_exam_attempt.ready_to_resume || proctored_exam_attempt.status == "ready_to_resume") - && !proctored_exam_attempt.resumed + proctored_exam_attempt.ready_to_resume && !proctored_exam_attempt.resumed ) { %> @@ -264,8 +263,7 @@ <% } %> <% if ( - (proctored_exam_attempt.ready_to_resume || proctored_exam_attempt.status == "ready_to_resume") - && !proctored_exam_attempt.resumed + proctored_exam_attempt.ready_to_resume && !proctored_exam_attempt.resumed ) { %> diff --git a/edx_proctoring/statuses.py b/edx_proctoring/statuses.py index 07c680290e3..859dcb13f50 100644 --- a/edx_proctoring/statuses.py +++ b/edx_proctoring/statuses.py @@ -65,12 +65,6 @@ class ProctoredExamStudentAttemptStatus: # the course end date has passed expired = 'expired' - # the learner is ready to resume their errored proctored exam - ready_to_resume = 'ready_to_resume' - - # the exam has been resumed and new attempt has been created - resumed = 'resumed' - # the onboarding attempt has been reset onboarding_reset = 'onboarding_reset' @@ -94,8 +88,8 @@ def is_completed_status(cls, status): """ return status in [ cls.declined, cls.timed_out, cls.submitted, cls.second_review_required, - cls.verified, cls.rejected, cls.error, cls.ready_to_resume, cls.resumed, - cls.onboarding_missing, cls.onboarding_pending, cls.onboarding_failed, cls.onboarding_expired + cls.verified, cls.rejected, cls.error, cls.onboarding_missing, cls.onboarding_pending, + cls.onboarding_failed, cls.onboarding_expired ] @classmethod @@ -161,15 +155,6 @@ def is_in_progress_status(cls, status): cls.started, cls.ready_to_submit ] - @classmethod - def is_resume_status(cls, status): - """ - Returns a boolean if the status passed is "resumed" or "ready to resume" - """ - return status in [ - cls.ready_to_resume, cls.resumed - ] - class ReviewStatus: """ diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index 88d5fe37259..97ed8325e39 100644 --- a/edx_proctoring/tests/test_api.py +++ b/edx_proctoring/tests/test_api.py @@ -1032,7 +1032,7 @@ def test_resume_exam_attempt(self, should_resume): """ # create an attempt that has been marked ready to resume initial_attempt = self._create_exam_attempt( - self.proctored_exam_id, ProctoredExamStudentAttemptStatus.ready_to_resume + self.proctored_exam_id, ProctoredExamStudentAttemptStatus.error, ready_to_resume=True ) # populate the remaining time initial_attempt.time_remaining_seconds = 600 @@ -1068,7 +1068,7 @@ def test_resume_from_invalid_attempt(self, status): """ An exam cannot be resumed if the previous attempt is not 'ready to resume' or 'resumed' """ - self._create_exam_attempt(self.proctored_exam_id, status) + self._create_exam_attempt(self.proctored_exam_id, status, ready_to_resume=False, resumed=False) with self.assertRaises(StudentExamAttemptAlreadyExistsException): create_exam_attempt(self.proctored_exam_id, self.user_id) @@ -1144,15 +1144,14 @@ def test_get_user_attempts_by_exam_id(self): """ first_attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id, taking_as_proctored=True) update_attempt_status(first_attempt_id, ProctoredExamStudentAttemptStatus.error) - update_attempt_status(first_attempt_id, ProctoredExamStudentAttemptStatus.ready_to_resume) + mark_exam_attempt_as_ready_to_resume(first_attempt_id) second_attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id, taking_as_proctored=True) update_attempt_status(second_attempt_id, ProctoredExamStudentAttemptStatus.error) - update_attempt_status(second_attempt_id, ProctoredExamStudentAttemptStatus.ready_to_resume) + mark_exam_attempt_as_ready_to_resume(second_attempt_id) third_attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id, taking_as_proctored=True) update_attempt_status(third_attempt_id, ProctoredExamStudentAttemptStatus.error) - update_attempt_status(third_attempt_id, ProctoredExamStudentAttemptStatus.ready_to_resume) attempts = get_user_attempts_by_exam_id(self.user_id, self.proctored_exam_id) self.assertEqual(len(attempts), 3) @@ -1426,14 +1425,15 @@ def test_get_filtered_exam_attempts_resumed(self): # create the first attempt first_exam_attempt = self._create_exam_attempt( exam_id=self.proctored_exam_id, - status=ProctoredExamStudentAttemptStatus.ready_to_resume, - time_remaining_seconds=600 + status=ProctoredExamStudentAttemptStatus.error, + time_remaining_seconds=600, + ready_to_resume=True ) - # create the second attempt, transitioning from error to ready_to_resume + # create the second attempt, setting the status to error and mark ready to resume second_exam_attempt_id = create_exam_attempt(exam_id=self.proctored_exam_id, user_id=self.user_id) update_attempt_status(second_exam_attempt_id, ProctoredExamStudentAttemptStatus.error) update_exam_attempt(second_exam_attempt_id, time_remaining_seconds=500) - update_attempt_status(second_exam_attempt_id, ProctoredExamStudentAttemptStatus.ready_to_resume) + mark_exam_attempt_as_ready_to_resume(second_exam_attempt_id) # create the third attempt, then assert that all attempts return correctly third_exam_attempt_id = create_exam_attempt(exam_id=self.proctored_exam_id, user_id=self.user_id) all_attempts = get_filtered_exam_attempts(self.course_id, self.user.username) @@ -1443,7 +1443,7 @@ def test_get_filtered_exam_attempts_resumed(self): self.assertEqual(all_attempts[2]['id'], first_exam_attempt.id) # the time remaining on the newest attempt should match the previous attempt self.assertEqual(all_attempts[0]['time_remaining_seconds'], all_attempts[1]['time_remaining_seconds']) - # when a new attempt is created, the previous attempt should have resumed sest to true + # when a new attempt is created, the previous attempt should have resumed set to true self.assertFalse(all_attempts[0]['resumed']) self.assertTrue(all_attempts[1]['resumed']) self.assertTrue(all_attempts[2]['resumed']) @@ -2437,7 +2437,7 @@ def test_update_exam_attempt_ready_to_resume(self, resumable_status): """ Assert that an attempted transition of a proctored exam attempt from an error state to a ready_to_resume state completes successfully and does not raise a - ProctoredExamIllegalStatusTransition exception. + ProctoredExamIllegalResumeUpdate exception. """ exam_attempt = self._create_started_exam_attempt() @@ -2457,31 +2457,12 @@ def test_update_exam_attempt_ready_to_resume(self, resumable_status): attempt = get_exam_attempt_by_id(exam_attempt.id) self.assertEqual(attempt['status'], resumable_status) + self.assertTrue(attempt['is_resumable']) - update_attempt_status( - exam_attempt.id, - ProctoredExamStudentAttemptStatus.ready_to_resume - ) + mark_exam_attempt_as_ready_to_resume(exam_attempt.id) attempt = get_exam_attempt_by_id(exam_attempt.id) - self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.ready_to_resume) - - @ddt.data( - ProctoredExamStudentAttemptStatus.ready_to_resume, - ProctoredExamStudentAttemptStatus.resumed - ) - def test_update_exam_attempt_resumed(self, from_status): - """ - Assert that transition status to 'resumed' is successful if the previous status is - 'ready_to_resume' or 'resumed'. - """ - exam_attempt = self._create_exam_attempt(self.proctored_exam_id, status=from_status) - update_attempt_status( - exam_attempt.id, - ProctoredExamStudentAttemptStatus.resumed - ) - attempt = get_exam_attempt_by_id(exam_attempt.id) - self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.resumed) + self.assertTrue(attempt['ready_to_resume']) @ddt.data(True, False) @patch('edx_proctoring.api.exam_attempt_status_signal.send') @@ -2595,16 +2576,12 @@ def test_mark_ready_to_resume(self): with self.assertRaises(ProctoredExamIllegalResumeUpdate): mark_exam_attempt_as_ready_to_resume(exam_attempt.id) - @ddt.data( - (ProctoredExamStudentAttemptStatus.ready_to_resume, False, False), - (ProctoredExamStudentAttemptStatus.resumed, False, False), - (ProctoredExamStudentAttemptStatus.error, True, False), - (ProctoredExamStudentAttemptStatus.error, False, True), - (ProctoredExamStudentAttemptStatus.ready_to_resume, True, False) - ) + @ddt.data((True, False), (False, True)) @ddt.unpack - def test_mark_resumed(self, status, ready_to_resume, expect_error): - exam_attempt = self._create_exam_attempt(self.proctored_exam_id, status=status) + def test_mark_resumed(self, ready_to_resume, expect_error): + exam_attempt = self._create_exam_attempt( + self.proctored_exam_id, status=ProctoredExamStudentAttemptStatus.error + ) exam_attempt.ready_to_resume = ready_to_resume exam_attempt.save() @@ -3683,7 +3660,8 @@ def test_get_exam_attempt_has_total_time_if_status_is_ready_to_resume(self): taking_as_proctored=True, is_sample_attempt=False, external_id=proctored_exam.external_id, - status=ProctoredExamStudentAttemptStatus.ready_to_resume + status=ProctoredExamStudentAttemptStatus.error, + ready_to_resume=True ) data = get_exam_attempt_data(proctored_exam.id, attempt.id) diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index 7b57620500d..765702308fe 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -26,6 +26,7 @@ create_exam_attempt, get_backend_provider, get_exam_attempt_by_id, + mark_exam_attempt_as_ready_to_resume, reset_practice_exam, update_attempt_status ) @@ -3380,12 +3381,12 @@ def test_get_grouped_exam_attempts(self): # create two attempts each for exam 1 attempt_1 = create_exam_attempt(exam_id_1, self.user.id, taking_as_proctored=True) update_attempt_status(attempt_1, ProctoredExamStudentAttemptStatus.error) - update_attempt_status(attempt_1, ProctoredExamStudentAttemptStatus.ready_to_resume) + mark_exam_attempt_as_ready_to_resume(attempt_1) attempt_2 = create_exam_attempt(exam_id_1, self.user.id, taking_as_proctored=True) attempt_3 = create_exam_attempt(exam_id_1, self.second_user.id, taking_as_proctored=True) update_attempt_status(attempt_3, ProctoredExamStudentAttemptStatus.error) - update_attempt_status(attempt_3, ProctoredExamStudentAttemptStatus.ready_to_resume) + mark_exam_attempt_as_ready_to_resume(attempt_3) attempt_4 = create_exam_attempt(exam_id_1, self.second_user.id, taking_as_proctored=True) # create one attempt each for exam 2 @@ -4377,7 +4378,7 @@ def test_action_not_mark_ready_to_resume_attempt_for_other_as_staff(self, action self.assertEqual(response.status_code, 403) self.assertRaises(ProctoredExamPermissionDenied) - # Make sure the exam attempt is in the ready_to_resume state. + # Make sure the exam attempt is in the original state. attempt = get_exam_attempt_by_id(old_attempt_id) self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.created) @@ -6001,7 +6002,8 @@ def _create_attempts(self, last_status): True ) ready_first = ProctoredExamStudentAttempt.objects.get(id=first_attempt_id) - ready_first.status = ProctoredExamStudentAttemptStatus.ready_to_resume + ready_first.status = ProctoredExamStudentAttemptStatus.error + ready_first.resumed = True ready_first.save() second_attempt_id = create_exam_attempt( self.exam_id, @@ -6009,7 +6011,8 @@ def _create_attempts(self, last_status): True ) ready_second = ProctoredExamStudentAttempt.objects.get(id=second_attempt_id) - ready_second.status = ProctoredExamStudentAttemptStatus.ready_to_resume + ready_second.status = ProctoredExamStudentAttemptStatus.error + ready_second.resumed = True ready_second.save() third_attempt_id = create_exam_attempt( self.exam_id, diff --git a/edx_proctoring/tests/utils.py b/edx_proctoring/tests/utils.py index 47f4ad0c1a2..d0a0978a268 100644 --- a/edx_proctoring/tests/utils.py +++ b/edx_proctoring/tests/utils.py @@ -18,7 +18,7 @@ from django.test import TestCase from django.test.client import Client -from edx_proctoring.api import create_exam, create_exam_review_policy +from edx_proctoring.api import create_exam, create_exam_review_policy, is_attempt_in_resume_process from edx_proctoring.models import ProctoredExamStudentAttempt from edx_proctoring.runtime import set_runtime_service from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus @@ -284,8 +284,10 @@ def _create_disabled_exam(self): is_active=False ) - def _create_exam_attempt(self, exam_id, status=ProctoredExamStudentAttemptStatus.created, - is_practice_exam=False, time_remaining_seconds=None): + def _create_exam_attempt( + self, exam_id, status=ProctoredExamStudentAttemptStatus.created, is_practice_exam=False, + time_remaining_seconds=None, ready_to_resume=False, resumed=False + ): """ Creates the ProctoredExamStudentAttempt object. """ @@ -298,6 +300,8 @@ def _create_exam_attempt(self, exam_id, status=ProctoredExamStudentAttemptStatus taking_as_proctored=True, is_sample_attempt=is_practice_exam, time_remaining_seconds=time_remaining_seconds, + ready_to_resume=ready_to_resume, + resumed=resumed ) if status in (ProctoredExamStudentAttemptStatus.started, @@ -307,7 +311,7 @@ def _create_exam_attempt(self, exam_id, status=ProctoredExamStudentAttemptStatus if ProctoredExamStudentAttemptStatus.is_completed_status(status): attempt.completed_at = datetime.now(pytz.UTC) - if status == ProctoredExamStudentAttemptStatus.error: + if status == ProctoredExamStudentAttemptStatus.error and not is_attempt_in_resume_process(attempt): attempt.is_resumable = True attempt.save() diff --git a/package.json b/package.json index 8d0074488b4..98acf91b99c 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@edx/edx-proctoring", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "4.5.0", + "version": "4.6.0", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test"