From ff97545c478c0470de5c458d322418045e55249a Mon Sep 17 00:00:00 2001 From: Alie Langston Date: Tue, 26 Oct 2021 17:07:27 -0400 Subject: [PATCH] fix: exam attempt should be resumable if it has been marked as ready to resume MST-1124. If an exam attempt has been previously marked as ready to resume, it should remain resumable by a learner regardless of whether or not they receive a review. In order to do this, resume states should no longer be represented as statuses, but instead boolean fields on the attempt model. --- CHANGELOG.rst | 7 + edx_proctoring/api.py | 107 ++++++++++---- edx_proctoring/exceptions.py | 8 + ...tudentattempt_add_readytoresume_resumed.py | 33 +++++ edx_proctoring/models.py | 8 + edx_proctoring/serializers.py | 2 +- .../spec/proctored_exam_attempt_spec.js | 99 ++++++++++++- ...proctored-exam-attempts-grouped.underscore | 22 +++ edx_proctoring/tests/test_api.py | 137 +++++++----------- edx_proctoring/tests/test_views.py | 22 ++- edx_proctoring/views.py | 6 +- 11 files changed, 327 insertions(+), 124 deletions(-) create mode 100644 edx_proctoring/migrations/0022_proctoredexamstudentattempt_add_readytoresume_resumed.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e0eb2659f04..b44819834f8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,13 @@ Change Log Unreleased ~~~~~~~~~~ + +[4.4.0] - 2021-10-29 +~~~~~~~~~~~~~~~~~~~~ +* Exam attempt should remain resumable after they have been marked as ready to resume. In order + for that to be true, the resume states are no longer represented as a status, but instead that + information is contained within the `ready_to_resume` and `resumed` fields. + [4.3.3] - 2021-10-29 ~~~~~~~~~~~~~~~~~~~~ * Remove ProctoredExamSoftwareSecureReview.video_url column from database. diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 841ece7fa1f..04d8b2c0794 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -31,6 +31,7 @@ BackendProviderSentNoAttemptID, ProctoredBaseException, ProctoredExamAlreadyExists, + ProctoredExamIllegalResumeUpdate, ProctoredExamIllegalStatusTransition, ProctoredExamNotActiveException, ProctoredExamNotFoundException, @@ -795,8 +796,7 @@ def get_exam_attempt_data(exam_id, attempt_id, is_learning_mfe=False): critically_low_threshold_pct * float(allowed_time_limit_mins) * 60 ) - if (not allowed_time_limit_mins - or (attempt and attempt['status'] == ProctoredExamStudentAttemptStatus.ready_to_resume)): + if not allowed_time_limit_mins or (attempt and is_attempt_ready_to_resume(attempt)): allowed_time_limit_mins = _calculate_allowed_mins(exam, attempt['user']['id']) total_time = humanized_time(allowed_time_limit_mins) @@ -825,6 +825,7 @@ def get_exam_attempt_data(exam_id, attempt_id, is_learning_mfe=False): 'edx_proctoring:proctored_exam.attempt', args=[attempt['id']] ), + 'attempt_ready_to_resume': is_attempt_ready_to_resume(attempt), } if provider: @@ -1070,7 +1071,7 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False): time_remaining_seconds = None # only practice exams and exams with resume states may have multiple attempts if existing_attempt: - if ProctoredExamStudentAttemptStatus.is_resume_status(existing_attempt.status): + if is_attempt_in_resume_process(existing_attempt): # save remaining time and mark the most recent attempt as resumed, if it isn't already time_remaining_seconds = existing_attempt.time_remaining_seconds mark_exam_attempt_as_resumed(existing_attempt.id) @@ -1244,10 +1245,72 @@ def mark_exam_attempt_as_resumed(attempt_id): """ Marks the current exam attempt as resumed """ - return update_attempt_status(attempt_id, ProctoredExamStudentAttemptStatus.resumed) + exam_attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt_by_id(attempt_id) + ready_to_resume = is_attempt_in_resume_process(exam_attempt_obj) + if ready_to_resume: + exam_attempt_obj.resumed = True + exam_attempt_obj.is_resumable = False + exam_attempt_obj.save() + return exam_attempt_obj.id + + raise ProctoredExamIllegalResumeUpdate( + f'Attempted to mark attempt_id={attempt_id} as resumed, but ' + f'attempt was never marked as ready_to_resume. This is not allowed.' + ) + + +def mark_exam_attempt_as_ready_to_resume(attempt_id): + """ + Marks the current exam attempt as ready to resume + """ + exam_attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt_by_id(attempt_id) + is_resumable = exam_attempt_obj.is_resumable + if is_resumable: + exam_attempt_obj.ready_to_resume = True + exam_attempt_obj.is_resumable = False + exam_attempt_obj.save() + return exam_attempt_obj.id + + raise ProctoredExamIllegalResumeUpdate( + f'Attempted to mark attempt_id={attempt_id} as ready_to_resume, but ' + f'attempt is not resumable. This is not allowed.' + ) + + +def is_attempt_ready_to_resume(attempt): + """ + Determine if an exam attempt is ready to resume + + Arguments: + attempt: serialized attempt obj + """ + + # 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'] + ) + + +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. + + Arguments: + attempt_obj: attempt object + """ + return ( + attempt_obj.ready_to_resume + or attempt_obj.resumed + or ProctoredExamStudentAttemptStatus.is_resume_status(attempt_obj.status) + ) -def is_state_transition_legal(from_status, to_status, attempt_obj): +def is_state_transition_legal(from_status, to_status): """ Determine and return as a boolean whether a proctored exam attempt state transition from from_status to to_status is an allowed state transition. @@ -1255,7 +1318,6 @@ def is_state_transition_legal(from_status, to_status, attempt_obj): Arguments: from_status: original status of a proctored exam attempt to_status: future status of a proctored exam attempt - attempt_obj: the actual student exam attempt """ in_completed_status = ProctoredExamStudentAttemptStatus.is_completed_status(from_status) to_incompleted_status = ProctoredExamStudentAttemptStatus.is_incomplete_status(to_status) @@ -1264,16 +1326,6 @@ def is_state_transition_legal(from_status, to_status, attempt_obj): # if a re-attempt is desired then the current attempt must be deleted if in_completed_status and to_incompleted_status: return False - # only allow a state transition to the ready_to_resume state when the attempt is resumable - if (to_status == ProctoredExamStudentAttemptStatus.ready_to_resume and - not attempt_obj.is_resumable): - return False - # only allowed state transition to the resumed state from ready_to_resume (or resumed). - # this accounts for cases where the previous attempt was marked as resumed, but a new - # attempt failed to be created. - if (to_status == ProctoredExamStudentAttemptStatus.resumed and - not ProctoredExamStudentAttemptStatus.is_resume_status(from_status)): - return False return True @@ -1328,12 +1380,7 @@ def _is_attempt_resumable(attempt_obj, to_status): Based on the attempt object and the status it's transitioning to, return whether the attempt should be resumable, or not """ - status_to_reset_resumability = ( - ProctoredExamStudentAttemptStatus.submitted, - ProctoredExamStudentAttemptStatus.resumed, - ProctoredExamStudentAttemptStatus.ready_to_resume, - ) - if to_status in status_to_reset_resumability: + if to_status == ProctoredExamStudentAttemptStatus.submitted or is_attempt_in_resume_process(attempt_obj): # Make sure we have resumable to be false in conditions where the # attempt is either in the resume process, or it's successfully submitted return False @@ -1401,7 +1448,7 @@ def update_attempt_status(attempt_id, to_status, exam = get_exam_by_id(exam_id) backend = get_backend_provider(exam) - if not is_state_transition_legal(from_status, to_status, exam_attempt_obj): + if not is_state_transition_legal(from_status, to_status): illegal_status_transition_msg = ( f'A status transition from "{from_status}" to "{to_status}" was attempted ' f'on exam_id={exam_id} for user_id={user_id}. This is not ' @@ -2559,7 +2606,7 @@ def _get_proctored_exam_context(exam, attempt, user_id, course_id, is_practice_e # if there is no attempt or an attempt with no time limit attribute, calculate the allowed time # also, if the attempt is in the ready to resume status, calculate the allowed time to correctly # display the time remaining - if not attempt_time or (attempt and attempt['status'] == ProctoredExamStudentAttemptStatus.ready_to_resume): + if not attempt_time or (attempt and is_attempt_ready_to_resume(attempt)): attempt_time = _calculate_allowed_mins(exam, user_id) total_time = humanized_time(attempt_time) @@ -2648,6 +2695,8 @@ def _get_practice_exam_view(exam, context, exam_id, user_id, course_id): student_view_template = 'proctored_exam/inactive_account.html' elif not attempt_status: student_view_template = 'practice_exam/entrance.html' + elif is_attempt_ready_to_resume(attempt): + student_view_template = 'proctored_exam/ready_to_resume.html' elif attempt_status == ProctoredExamStudentAttemptStatus.started: provider = get_backend_provider(exam) if provider.should_block_access_to_exam_material(): @@ -2666,8 +2715,6 @@ def _get_practice_exam_view(exam, context, exam_id, user_id, course_id): student_view_template = 'practice_exam/submitted.html' elif attempt_status == ProctoredExamStudentAttemptStatus.ready_to_submit: student_view_template = 'proctored_exam/ready_to_submit.html' - elif attempt_status == ProctoredExamStudentAttemptStatus.ready_to_resume: - student_view_template = 'proctored_exam/ready_to_resume.html' if student_view_template: template = loader.get_template(student_view_template) @@ -2699,6 +2746,8 @@ def _get_onboarding_exam_view(exam, context, exam_id, user_id, course_id): student_view_template = 'proctored_exam/expired.html' else: student_view_template = 'onboarding_exam/entrance.html' + elif is_attempt_ready_to_resume(attempt): + student_view_template = 'proctored_exam/ready_to_resume.html' elif attempt_status == ProctoredExamStudentAttemptStatus.started: # when we're taking the exam we should not override the view return None @@ -2718,8 +2767,6 @@ def _get_onboarding_exam_view(exam, context, exam_id, user_id, course_id): student_view_template = 'onboarding_exam/verified.html' elif attempt_status == ProctoredExamStudentAttemptStatus.rejected: student_view_template = 'onboarding_exam/rejected.html' - elif attempt_status == ProctoredExamStudentAttemptStatus.ready_to_resume: - student_view_template = 'proctored_exam/ready_to_resume.html' if student_view_template: template = loader.get_template(student_view_template) @@ -2812,6 +2859,8 @@ def _get_proctored_exam_view(exam, context, exam_id, user_id, course_id): # When the exam is past due, we should prevent learners from accessing the exam even if # they already accessed the exam before, but haven't completed. student_view_template = 'proctored_exam/expired.html' + elif is_attempt_ready_to_resume(attempt): + student_view_template = 'proctored_exam/ready_to_resume.html' elif attempt_status == ProctoredExamStudentAttemptStatus.started: provider = get_backend_provider(exam) if provider.should_block_access_to_exam_material(): @@ -2873,8 +2922,6 @@ def _get_proctored_exam_view(exam, context, exam_id, user_id, course_id): is_active=True, is_practice_exam=True).first() context['onboarding_link'] = reverse('jump_to', args=[course_id, onboarding_exam.content_id]) - elif attempt_status == ProctoredExamStudentAttemptStatus.ready_to_resume: - student_view_template = 'proctored_exam/ready_to_resume.html' if student_view_template: template = loader.get_template(student_view_template) diff --git a/edx_proctoring/exceptions.py b/edx_proctoring/exceptions.py index feff0f160ee..4427f2032ac 100644 --- a/edx_proctoring/exceptions.py +++ b/edx_proctoring/exceptions.py @@ -148,6 +148,14 @@ 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 + is not resumable. + """ + + class BackendProviderCannotRetireUser(ProctoredBaseException): """ Raised when a back-end provider cannot retire a user diff --git a/edx_proctoring/migrations/0022_proctoredexamstudentattempt_add_readytoresume_resumed.py b/edx_proctoring/migrations/0022_proctoredexamstudentattempt_add_readytoresume_resumed.py new file mode 100644 index 00000000000..c74daad95a6 --- /dev/null +++ b/edx_proctoring/migrations/0022_proctoredexamstudentattempt_add_readytoresume_resumed.py @@ -0,0 +1,33 @@ +# Generated by Django 3.2.7 on 2021-10-26 18:44 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('edx_proctoring', '0021_auto_20211029_1353'), + ] + + operations = [ + migrations.AddField( + model_name='historicalproctoredexamstudentattempt', + name='ready_to_resume', + field=models.BooleanField(default=False, verbose_name='Ready to Resume'), + ), + migrations.AddField( + model_name='historicalproctoredexamstudentattempt', + name='resumed', + field=models.BooleanField(default=False, verbose_name='Resumed'), + ), + migrations.AddField( + model_name='proctoredexamstudentattempt', + name='ready_to_resume', + field=models.BooleanField(default=False, verbose_name='Ready to Resume'), + ), + migrations.AddField( + model_name='proctoredexamstudentattempt', + name='resumed', + field=models.BooleanField(default=False, verbose_name='Resumed'), + ), + ] diff --git a/edx_proctoring/models.py b/edx_proctoring/models.py index 35be50dda1e..8d72a8f97f4 100644 --- a/edx_proctoring/models.py +++ b/edx_proctoring/models.py @@ -412,6 +412,14 @@ class ProctoredExamStudentAttempt(TimeStampedModel): # has not yet marked submitted is resumable. is_resumable = models.BooleanField(default=False, verbose_name=ugettext_noop("Is Resumable")) + # marks whether or not an attempt has been marked as ready to resume + # by staff. The value of this field does not necessarily mean that an + # attempt is ready to resume by a learner, only that the staff has marked it as such. + ready_to_resume = models.BooleanField(default=False, verbose_name=ugettext_noop("Ready to Resume")) + + # marks whether or not an attempt has been resumed by a learner. + resumed = models.BooleanField(default=False, verbose_name=ugettext_noop("Resumed")) + history = HistoricalRecords(table_name='proctoring_proctoredexamstudentattempt_history') class Meta: diff --git a/edx_proctoring/serializers.py b/edx_proctoring/serializers.py index 40e433ab944..6ffe07aed24 100644 --- a/edx_proctoring/serializers.py +++ b/edx_proctoring/serializers.py @@ -96,7 +96,7 @@ class Meta: "external_id", "status", "proctored_exam", "allowed_time_limit_mins", "attempt_code", "is_sample_attempt", "taking_as_proctored", "review_policy_id", "is_status_acknowledged", - "time_remaining_seconds", "is_resumable" + "time_remaining_seconds", "is_resumable", "ready_to_resume", "resumed" ) 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 3d3ddf93e19..bdacd691ea3 100644 --- a/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js +++ b/edx_proctoring/static/proctoring/spec/proctored_exam_attempt_spec.js @@ -88,7 +88,9 @@ describe('ProctoredExamAttemptView', function() { ); } - function getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson(status, isPracticeExam, isResumable) { + function getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson( + status, isPracticeExam, isResumable, readyToResume, resumed + ) { // eslint-disable-next-line no-param-reassign isPracticeExam = typeof isPracticeExam !== 'undefined' ? isPracticeExam : false; // eslint-disable-next-line no-param-reassign @@ -114,6 +116,8 @@ describe('ProctoredExamAttemptView', function() { started_at: '2015-08-10T09:15:45Z', status: status, is_resumable: isResumable, + ready_to_resume: readyToResume, + resumed: resumed, taking_as_proctored: true, proctored_exam: { content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c', @@ -144,6 +148,8 @@ describe('ProctoredExamAttemptView', function() { started_at: '2015-08-10T09:15:45Z', status: status, is_resumable: isResumable, + ready_to_resume: readyToResume, + resumed: resumed, taking_as_proctored: true, proctored_exam: { content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c', @@ -174,6 +180,8 @@ describe('ProctoredExamAttemptView', function() { started_at: '2015-08-10T09:15:45Z', status: 'resumed', is_resumable: false, + ready_to_resume: true, + resumed: true, taking_as_proctored: true, proctored_exam: { content_id: 'i4x://edX/DemoX/sequential/9f5e9b018a244ea38e5d157e0019e60c', @@ -246,6 +254,7 @@ describe('ProctoredExamAttemptView', function() { 'Started At' + 'Completed At' + 'Status' + + '<%- gettext("Ready to Resume") %> ' + 'Actions' + '' + '<% if (is_proctored_attempts) { %>\n' + @@ -280,8 +289,19 @@ 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' + + ') { %>' + + '' + + '' + + '' + '<% } else { %>' + '' + + '<% } %>' + + '<% } else { %>' + + '' + + '' + '' + '' + '<% } %>' + @@ -340,7 +360,18 @@ describe('ProctoredExamAttemptView', function() { '<% if (proctored_exam_attempt.status) { %>' + '<%= 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' + + ') { %>' + + '' + + '' + + '' + + '<% } else { %>' + + '' + + '<% } %>' + + ' ' + + ' <% }); %>' + ' <% }%> <% }); %> <% } %>' + '' + '<% if (!is_proctored_attempts) { %>' + @@ -517,6 +548,69 @@ describe('ProctoredExamAttemptView', function() { expect(this.proctored_exam_attempt_view.$el.find('tr.allowance-items').html()).toContain('testuser1'); expect(this.proctored_exam_attempt_view.$el.find('tr.allowance-items').html()).toContain('Normal Exam'); }); + + it('should display check when exam attempt is ready to resume', function() { + var rows; + + this.server.respondWith('GET', '/api/edx_proctoring/v1/proctored_exam/attempt/grouped/course_id/test_course_id', + [ + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson( + 'error', false, true, true, false) + ) + ] + ); + this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView(); + + // Process all requests so far + this.server.respond(); + this.server.respond(); + + rows = this.proctored_exam_attempt_view.$el.find('tbody').children(); + expect(rows.length).toEqual(3); + + // check that ready to resume check does not appear in outer level + expect(rows[0].outerHTML).not.toContain('fa-check-circle'); + + // check that status is present in other two rows + expect(rows[1].outerHTML).toContain('fa-check-circle'); + expect(rows[2].outerHTML).not.toContain('fa-check-circle'); + }); + + it('should not display check when exam attempt has status ready to resume but has been resumed', function() { + var rows; + + this.server.respondWith('GET', '/api/edx_proctoring/v1/proctored_exam/attempt/grouped/course_id/test_course_id', + [ + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify(getExpectedGroupedProctoredExamAttemptWithAttemptStatusJson( + 'ready_to_resume', false, false, false, true) + ) + ] + ); + this.proctored_exam_attempt_view = new edx.instructor_dashboard.proctoring.ProctoredExamAttemptView(); + + // Process all requests so far + this.server.respond(); + this.server.respond(); + + rows = this.proctored_exam_attempt_view.$el.find('tbody').children(); + expect(rows.length).toEqual(3); + + // check that ready to resume check does not appear in outer level + expect(rows[0].outerHTML).not.toContain('fa-check-circle'); + + // check that status is present in other two rows + expect(rows[1].outerHTML).not.toContain('fa-check-circle'); + expect(rows[2].outerHTML).not.toContain('fa-check-circle'); + }); + it('should mark exam attempt "ready_to_resume" on resume', function() { this.server.respondWith('GET', '/api/edx_proctoring/v1/proctored_exam/attempt/grouped/course_id/test_course_id', [ @@ -592,6 +686,7 @@ 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 d219100fd63..39a728822f9 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 @@ -154,6 +154,7 @@ <%- gettext("Started At") %> <%- gettext("Completed At") %> <%- gettext("Status") %> + <%- gettext("Ready to Resume") %> <%- gettext("Actions") %> @@ -192,10 +193,21 @@ N/A <% } %> + <% if ( + (proctored_exam_attempt.ready_to_resume || proctored_exam_attempt.status == "ready_to_resume") + && !proctored_exam_attempt.resumed + ) { %> + + + + <% } else { %> + + <% } %> <% } else { %> + <% } %> <% if (proctored_exam_attempt.status){ %> @@ -251,6 +263,16 @@ N/A <% } %> + <% if ( + (proctored_exam_attempt.ready_to_resume || proctored_exam_attempt.status == "ready_to_resume") + && !proctored_exam_attempt.resumed + ) { %> + + + + <% } else { %> + + <% } %> <% }); %> diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index 455f557f13f..0c339f5c9dc 100644 --- a/edx_proctoring/tests/test_api.py +++ b/edx_proctoring/tests/test_api.py @@ -49,8 +49,11 @@ get_last_verified_onboarding_attempts_per_user, get_review_policy_by_exam_id, get_user_attempts_by_exam_id, + is_attempt_ready_to_resume, is_backend_dashboard_available, mark_exam_attempt_as_ready, + mark_exam_attempt_as_ready_to_resume, + mark_exam_attempt_as_resumed, mark_exam_attempt_timeout, remove_allowance_for_user, remove_exam_attempt, @@ -70,6 +73,7 @@ AllowanceValueNotAllowedException, BackendProviderSentNoAttemptID, ProctoredExamAlreadyExists, + ProctoredExamIllegalResumeUpdate, ProctoredExamIllegalStatusTransition, ProctoredExamNotFoundException, ProctoredExamPermissionDenied, @@ -1028,26 +1032,32 @@ def test_attempt_with_allowance(self): self.assertEqual(attempt['allowed_time_limit_mins'], self.default_time_limit + allowed_extra_time) @ddt.data( - ProctoredExamStudentAttemptStatus.ready_to_resume, - ProctoredExamStudentAttemptStatus.resumed, + True, + False ) - def test_resume_exam_attempt(self, status): + def test_resume_exam_attempt(self, should_resume): """ Create a resumed exam attempt with remaining time saved from the previous attempt. """ # create an attempt that has been marked ready to resume - initial_attempt = self._create_exam_attempt(self.proctored_exam_id, status) + initial_attempt = self._create_exam_attempt( + self.proctored_exam_id, ProctoredExamStudentAttemptStatus.ready_to_resume + ) # populate the remaining time initial_attempt.time_remaining_seconds = 600 initial_attempt.save() + if should_resume: + mark_exam_attempt_as_resumed(initial_attempt.id) + # create a new attempt, which should save the remaining time # and update the previous attempt's status to 'resumed' current_attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id) previous_attempt = get_exam_attempt_by_id(initial_attempt.id) current_attempt = get_exam_attempt_by_id(current_attempt_id) self.assertEqual(current_attempt['time_remaining_seconds'], 600) - self.assertEqual(previous_attempt['status'], ProctoredExamStudentAttemptStatus.resumed) + self.assertTrue(previous_attempt['resumed']) + self.assertFalse(is_attempt_ready_to_resume(previous_attempt)) @ddt.data( ProctoredExamStudentAttemptStatus.eligible, @@ -1442,10 +1452,10 @@ 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 transition from ready_to_resume to resumed - self.assertEqual(all_attempts[0]['status'], ProctoredExamStudentAttemptStatus.created) - self.assertEqual(all_attempts[1]['status'], ProctoredExamStudentAttemptStatus.resumed) - self.assertEqual(all_attempts[2]['status'], ProctoredExamStudentAttemptStatus.resumed) + # when a new attempt is created, the previous attempt should have resumed sest to true + self.assertFalse(all_attempts[0]['resumed']) + self.assertTrue(all_attempts[1]['resumed']) + self.assertTrue(all_attempts[2]['resumed']) def test_get_last_exam_completion_date_when_course_is_incomplete(self): """ @@ -2426,34 +2436,6 @@ def test_update_exam_attempt(self): self.assertEqual(attempt['time_remaining_seconds'], 600) - @ddt.data( - ProctoredExamStudentAttemptStatus.eligible, - ProctoredExamStudentAttemptStatus.created, - ProctoredExamStudentAttemptStatus.download_software_clicked, - ProctoredExamStudentAttemptStatus.ready_to_start, - ProctoredExamStudentAttemptStatus.started, - ProctoredExamStudentAttemptStatus.ready_to_submit, - ProctoredExamStudentAttemptStatus.declined, - ProctoredExamStudentAttemptStatus.timed_out, - ProctoredExamStudentAttemptStatus.submitted, - ProctoredExamStudentAttemptStatus.second_review_required, - ProctoredExamStudentAttemptStatus.rejected, - ProctoredExamStudentAttemptStatus.expired, - ProctoredExamStudentAttemptStatus.resumed - ) - def test_update_exam_attempt_ready_to_resume_invalid_transition(self, initial_status): - """ - Assert that an attempted transition of a proctored exam attempt from a non-error state - to the ready_to_resume_state raises a ProctoredExamIllegalStatusTransition exception. - """ - exam_attempt = self._create_exam_attempt(self.proctored_exam_id, status=initial_status) - - with self.assertRaises(ProctoredExamIllegalStatusTransition): - update_attempt_status( - exam_attempt.id, - ProctoredExamStudentAttemptStatus.ready_to_resume - ) - @ddt.data( ProctoredExamStudentAttemptStatus.error, ProctoredExamStudentAttemptStatus.verified, @@ -2493,33 +2475,6 @@ def test_update_exam_attempt_ready_to_resume(self, resumable_status): attempt = get_exam_attempt_by_id(exam_attempt.id) self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.ready_to_resume) - @ddt.data( - ProctoredExamStudentAttemptStatus.eligible, - ProctoredExamStudentAttemptStatus.created, - ProctoredExamStudentAttemptStatus.download_software_clicked, - ProctoredExamStudentAttemptStatus.ready_to_start, - ProctoredExamStudentAttemptStatus.started, - ProctoredExamStudentAttemptStatus.ready_to_submit, - ProctoredExamStudentAttemptStatus.declined, - ProctoredExamStudentAttemptStatus.timed_out, - ProctoredExamStudentAttemptStatus.submitted, - ProctoredExamStudentAttemptStatus.second_review_required, - ProctoredExamStudentAttemptStatus.rejected, - ProctoredExamStudentAttemptStatus.expired - ) - def test_update_exam_attempt_resumed_invalid_transition(self, from_status): - """ - Assert that an attempted transition of a proctored exam attempt to 'resumed' from a state - that is not 'ready_to_resume' or 'resumed' raises an exception. - """ - exam_attempt = self._create_exam_attempt(self.proctored_exam_id, status=from_status) - - with self.assertRaises(ProctoredExamIllegalStatusTransition): - update_attempt_status( - exam_attempt.id, - ProctoredExamStudentAttemptStatus.resumed - ) - @ddt.data( ProctoredExamStudentAttemptStatus.ready_to_resume, ProctoredExamStudentAttemptStatus.resumed @@ -2615,16 +2570,6 @@ def test_create_and_update_exam_attempt_signal_verified_name( ProctoredExamStudentAttemptStatus.ready_to_submit, False ), - ( - ProctoredExamStudentAttemptStatus.error, - ProctoredExamStudentAttemptStatus.ready_to_resume, - False - ), - ( - ProctoredExamStudentAttemptStatus.ready_to_resume, - ProctoredExamStudentAttemptStatus.resumed, - False - ), ( ProctoredExamStudentAttemptStatus.error, ProctoredExamStudentAttemptStatus.verified, @@ -2656,6 +2601,37 @@ def test_exam_attempt_is_resumable(self, from_status, to_status, expected_is_res attempt = get_exam_attempt_by_id(exam_attempt.id) self.assertEqual(attempt['is_resumable'], expected_is_resumable) + def test_mark_ready_to_resume(self): + exam_attempt = self._create_exam_attempt( + self.proctored_exam_id, + status=ProctoredExamStudentAttemptStatus.verified + ) + self.assertFalse(exam_attempt.is_resumable) + + 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.unpack + def test_mark_resumed(self, status, ready_to_resume, expect_error): + exam_attempt = self._create_exam_attempt(self.proctored_exam_id, status=status) + exam_attempt.ready_to_resume = ready_to_resume + exam_attempt.save() + + if expect_error: + with self.assertRaises(ProctoredExamIllegalResumeUpdate): + mark_exam_attempt_as_resumed(exam_attempt.id) + else: + mark_exam_attempt_as_resumed(exam_attempt.id) + attempt = get_exam_attempt_by_id(exam_attempt.id) + self.assertTrue(attempt['resumed']) + def test_requirement_status_order(self): """ Make sure that we get a correct ordered list of all statuses sorted in the correct @@ -3097,11 +3073,10 @@ def test_grade_certificate_release_with_multiple_attempts( first_attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id, taking_as_proctored=True) # move to ready to resume 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) # check that status has been updated - self.assertEqual( - get_exam_attempt_by_id(first_attempt_id)['status'], - ProctoredExamStudentAttemptStatus.ready_to_resume + self.assertTrue( + get_exam_attempt_by_id(first_attempt_id)['ready_to_resume'] ) # create second attempt second_attempt_id = create_exam_attempt(self.proctored_exam_id, self.user_id, taking_as_proctored=True) @@ -3110,9 +3085,8 @@ def test_grade_certificate_release_with_multiple_attempts( get_exam_attempt_by_id(second_attempt_id)['status'], ProctoredExamStudentAttemptStatus.created ) - self.assertEqual( - get_exam_attempt_by_id(first_attempt_id)['status'], - ProctoredExamStudentAttemptStatus.resumed + self.assertTrue( + get_exam_attempt_by_id(first_attempt_id)['resumed'] ) if update_in_order: @@ -3703,6 +3677,7 @@ def test_get_exam_attempt(self, is_proctored): self.assertEqual(data['critically_low_threshold_sec'], 270) # make sure we have the accessible human string self.assertEqual(data['accessibility_time_string'], 'you have 1 hour and 30 minutes remaining') + self.assertFalse(data['attempt_ready_to_resume']) def test_get_exam_attempt_has_total_time_if_status_is_ready_to_resume(self): """ diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index c8989ef5359..7b57620500d 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -3988,7 +3988,8 @@ def test_mark_ready_to_resume_attempt_for_self(self): # Make sure the exam attempt is in the ready_to_resume state. attempt = get_exam_attempt_by_id(old_attempt_id) - self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.ready_to_resume) + self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.error) + self.assertTrue(attempt['ready_to_resume']) self.assertFalse(attempt['is_resumable']) @ddt.data( @@ -4079,7 +4080,8 @@ def test_mark_ready_to_resume_attempt_for_other_as_staff(self, is_staff, is_cour # Make sure the exam attempt is in the ready_to_resume state. attempt = get_exam_attempt_by_id(old_attempt_id) - self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.ready_to_resume) + self.assertEqual(attempt['status'], ProctoredExamStudentAttemptStatus.error) + self.assertTrue(attempt['ready_to_resume']) @ddt.data( (True, True), @@ -4280,6 +4282,15 @@ def test_is_user_course_or_global_staff_called_correct_args(self): response_data = json.loads(response.content.decode('utf-8')) attempt_id = response_data['exam_attempt_id'] + # Transition the exam attempt into the error state. + response = self.client.put( + reverse('edx_proctoring:proctored_exam.attempt', args=[attempt_id]), + json.dumps({ + 'action': 'error', + }), + content_type='application/json' + ) + # Log in the staff user. self.client.login_user(self.student_taking_exam) @@ -4435,10 +4446,9 @@ def test_resume_exam_attempt(self): self.assertEqual(response.status_code, 200) response_data = json.loads(response.content.decode('utf-8')) self.assertEqual(len(response_data['proctored_exam_attempts'][0]['all_attempts']), 2) - # assert that the older attempt has transitioned to the 'resumed' status - self.assertEqual( - response_data['proctored_exam_attempts'][0]['all_attempts'][1]['status'], - ProctoredExamStudentAttemptStatus.resumed + # assert that the older attempt has transitioned to the resumed + self.assertTrue( + response_data['proctored_exam_attempts'][0]['all_attempts'][1]['resumed'], ) # Make sure the resumed attempt is no longer resumable again self.assertFalse( diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 5f2ba0daea8..52d233a58d7 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -52,6 +52,7 @@ get_user_attempts_by_exam_id, is_exam_passed_due, mark_exam_attempt_as_ready, + mark_exam_attempt_as_ready_to_resume, remove_allowance_for_user, remove_exam_attempt, reset_practice_exam, @@ -1253,10 +1254,7 @@ def put(self, request, attempt_id): ProctoredExamStudentAttemptStatus.declined ) elif action == 'mark_ready_to_resume': - exam_attempt_id = update_attempt_status( - attempt_id, - ProctoredExamStudentAttemptStatus.ready_to_resume - ) + exam_attempt_id = mark_exam_attempt_as_ready_to_resume(attempt_id) data = {"exam_attempt_id": exam_attempt_id} return Response(data)