Skip to content

Commit

Permalink
Merge pull request #995 from edx/bseverino/remove-resume-status
Browse files Browse the repository at this point in the history
[MST-1138] Remove references to ready_to_resume and resumed statuses
  • Loading branch information
bseverino authored Nov 4, 2021
2 parents 2ace350 + 943a269 commit d70851d
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 99 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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__ = '4.5.0'
__version__ = '4.6.0'

default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name
1 change: 0 additions & 1 deletion edx_proctoring/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
18 changes: 4 additions & 14 deletions edx_proctoring/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,7 @@ describe('ProctoredExamAttemptView', function() {
'<% } else { %> N/A <% } %>' +
'</td>' +
'<% 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' +
') { %>' +
'<td>' +
'<span class="fa fa-check-circle" aria-hidden="true"></span>' +
Expand Down Expand Up @@ -361,8 +360,7 @@ describe('ProctoredExamAttemptView', function() {
'<%= getExamAttemptStatus(proctored_exam_attempt.status) %>' +
'<% } else { %> N/A <% } %> </td>' +
'<% 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' +
') { %>' +
'<td>' +
'<span class="fa fa-check-circle" aria-hidden="true"></span>' +
Expand Down Expand Up @@ -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)
)
]
);
Expand Down Expand Up @@ -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
))
]
);

Expand Down Expand Up @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,7 @@
<% } %>
</td>
<% 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
) { %>
<td>
<span class="fa fa-check-circle" aria-hidden="true"></span>
Expand Down Expand Up @@ -264,8 +263,7 @@
<% } %>
</td>
<% 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
) { %>
<td>
<span class="fa fa-check-circle" aria-hidden="true"></span>
Expand Down
19 changes: 2 additions & 17 deletions edx_proctoring/statuses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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
Expand Down Expand Up @@ -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:
"""
Expand Down
64 changes: 21 additions & 43 deletions edx_proctoring/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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'])
Expand Down Expand Up @@ -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()

Expand All @@ -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')
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down
13 changes: 8 additions & 5 deletions edx_proctoring/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -6001,15 +6002,17 @@ 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,
self.user.id,
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,
Expand Down
Loading

0 comments on commit d70851d

Please sign in to comment.