From 2e158b721e5689f2527bc2eb77a1e4cbd7f9e11d Mon Sep 17 00:00:00 2001 From: Michael Roytman Date: Thu, 27 May 2021 08:02:19 -0400 Subject: [PATCH] feat: Use Provider Onboarding Profile API for Course Onboarding API This pull request makes a change to the method of determining learners' onboarding statuses in a course via the StudentOnboardingStatusByCourseView. Instead of using internal logic to determine onboarding statuses, the provider's onboarding API will be used instead. MST-761: https://openedx.atlassian.net/browse/MST-761 --- CHANGELOG.rst | 5 + edx_proctoring/__init__.py | 2 +- edx_proctoring/constants.py | 2 + .../views/proctored_exam_onboarding_view.js | 31 +- .../spec/proctored_exam_onboarding_spec.js | 97 +++++- .../student-onboarding-status.underscore | 17 +- edx_proctoring/statuses.py | 45 ++- edx_proctoring/tests/test_views.py | 316 +++++++++++++++++- edx_proctoring/views.py | 187 ++++++++++- package.json | 2 +- 10 files changed, 662 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 73b0c2adb8b..f14fbf39a8d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,11 @@ Change Log Unreleased ~~~~~~~~~~ +[3.17.0] - 2021-06-23 +~~~~~~~~~~~~~~~~~~~~~ +* Replace internal logic for determing learners' onboarding statuses for the course onboarding API + with provider onboarding API. + [3.16.0] - 2021-06-22 ~~~~~~~~~~~~~~~~~~~~~ * Created a GET api endpoint which groups course allowances by users. diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 392c2c7e1fe..9fcb15208a7 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.16.0' +__version__ = '3.17.0' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/constants.py b/edx_proctoring/constants.py index e754e4d76df..3396b2d9c8e 100644 --- a/edx_proctoring/constants.py +++ b/edx_proctoring/constants.py @@ -70,6 +70,8 @@ ONBOARDING_PROFILE_API = 'edx_proctoring.onboarding_profile_api' +ONBOARDING_PROFILE_INSTRUCTOR_DASHBOARD_API = 'edx_proctoring.onboarding_profile_instructor_dashboard_api' + CONTENT_VIEWABLE_PAST_DUE_DATE = getattr(settings, 'PROCTORED_EXAM_VIEWABLE_PAST_DUE', False) TIME_MULTIPLIER = 'time_multiplier' diff --git a/edx_proctoring/static/proctoring/js/views/proctored_exam_onboarding_view.js b/edx_proctoring/static/proctoring/js/views/proctored_exam_onboarding_view.js index 46e62d24815..2d043f0debc 100644 --- a/edx_proctoring/static/proctoring/js/views/proctored_exam_onboarding_view.js +++ b/edx_proctoring/static/proctoring/js/views/proctored_exam_onboarding_view.js @@ -3,7 +3,7 @@ edx = edx || {}; (function(Backbone, $, _, gettext) { 'use strict'; - var viewHelper, onboardingStatuses, statusAndModeReadableFormat; + var viewHelper, onboardingStatuses, onboardingProfileAPIStatuses, statusAndModeReadableFormat; edx.instructor_dashboard = edx.instructor_dashboard || {}; edx.instructor_dashboard.proctoring = edx.instructor_dashboard.proctoring || {}; onboardingStatuses = [ @@ -16,6 +16,14 @@ edx = edx || {}; 'rejected', 'error' ]; + onboardingProfileAPIStatuses = [ + 'not_started', + 'other_course_approved', + 'submitted', + 'verified', + 'rejected', + 'expired' + ]; statusAndModeReadableFormat = { // Onboarding statuses not_started: gettext('Not Started'), @@ -27,6 +35,7 @@ edx = edx || {}; verified: gettext('Verified'), rejected: gettext('Rejected'), error: gettext('Error'), + expired: gettext('Expired'), // TODO: remove as part of MST-745 onboarding_reset_past_due: gettext('Onboarding Reset Failed Due to Past Due Exam'), // Enrollment modes (Note: 'verified' is both a status and enrollment mode) @@ -186,11 +195,21 @@ edx = edx || {}; $searchIcon = $(document.getElementById('onboarding-search-indicator')); $searchIcon.removeClass('hidden'); }, - error: function() { + error: function(unused, response) { + var data, $searchIcon, $spinner, $errorResponse, $onboardingPanel; + // in the case that there is no onboarding data, we // still want the view to render - var $searchIcon, $spinner; self.render(); + + data = $.parseJSON(response.responseText); + if (data.detail) { + $errorResponse = $('#error-response'); + $errorResponse.html(data.detail); + $onboardingPanel = $('.onboarding-status-content'); + $onboardingPanel.hide(); + } + $spinner = $(document.getElementById('onboarding-loading-indicator')); $spinner.addClass('hidden'); $searchIcon = $(document.getElementById('onboarding-search-indicator')); @@ -202,7 +221,8 @@ edx = edx || {}; this.hydrate(); }, render: function() { - var data, dataJson, html, startPage, endPage; + var data, dataJson, html, startPage, endPage, statuses; + if (this.template !== null) { data = { previousPage: null, @@ -234,12 +254,13 @@ edx = edx || {}; endPage = dataJson.num_pages; } + statuses = dataJson.use_onboarding_profile_api ? onboardingProfileAPIStatuses : onboardingStatuses; data = { previousPage: dataJson.previous, nextPage: dataJson.next, currentPage: this.currentPage, onboardingItems: dataJson.results, - onboardingStatuses: onboardingStatuses, + onboardingStatuses: statuses, inSearchMode: this.inSearchMode, searchText: this.searchText, filters: this.filters, diff --git a/edx_proctoring/static/proctoring/spec/proctored_exam_onboarding_spec.js b/edx_proctoring/static/proctoring/spec/proctored_exam_onboarding_spec.js index 32ae5e486d7..d91f83337e5 100644 --- a/edx_proctoring/static/proctoring/spec/proctored_exam_onboarding_spec.js +++ b/edx_proctoring/static/proctoring/spec/proctored_exam_onboarding_spec.js @@ -7,6 +7,7 @@ describe('ProctoredExamOnboardingView', function() { previous: null, next: null, num_pages: 1, + use_onboarding_profile_api: false, results: [ { username: 'testuser1', @@ -40,11 +41,13 @@ describe('ProctoredExamOnboardingView', function() { previous: null, next: null, num_pages: 1, - results: [] + results: [], + use_onboarding_profile_api: false }]; beforeEach(function() { html = '
' + + '

' + '<% var isOnboardingItems = onboardingItems.length !== 0 %>' + '
' + '
' + @@ -248,16 +251,39 @@ describe('ProctoredExamOnboardingView', function() { this.server.respond(); this.server.respond(); + this.server.respondWith('GET', + '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id?page=1&statuses=submitted,verified', + [ + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify(expectedOnboardingDataJson) + ] + ); + spyOnEvent('.filter-form', 'submit'); $('.status-checkboxes > li > input#submitted').click(); $('.status-checkboxes > li > input#verified').click(); $('.filter-form').submit(); + this.server.respond(); + expect(this.proctored_exam_onboarding_view.filters).toEqual(['submitted', 'verified']); expect(this.proctored_exam_onboarding_view.collection.url).toEqual( '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id?page=1&statuses=submitted,verified' ); + this.server.respondWith('GET', '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id?page=1', + [ + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify(expectedOnboardingDataJson) + ] + ); + spyOnEvent('.clear-filters', 'click'); $('.clear-filters').click(); @@ -277,6 +303,7 @@ describe('ProctoredExamOnboardingView', function() { JSON.stringify(expectedOnboardingDataJson) ] ); + this.proctored_exam_onboarding_view = new edx.instructor_dashboard.proctoring.ProctoredExamOnboardingView(); this.server.respond(); @@ -323,7 +350,7 @@ describe('ProctoredExamOnboardingView', function() { // search for the proctored exam attempt this.server.respondWith( 'GET', - '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id?text_search=' + searchText, + '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id?page=1&text_search=' + searchText, [ 200, { @@ -333,6 +360,7 @@ describe('ProctoredExamOnboardingView', function() { ] ); + // trigger the search attempt event. spyOnEvent('.search-onboarding > span.search', 'click'); $('.search-onboarding > span.search').trigger('click'); @@ -352,4 +380,69 @@ describe('ProctoredExamOnboardingView', function() { expect(this.proctored_exam_onboarding_view.$el.find('#onboarding-loading-indicator').hasClass('hidden')) .toEqual(true); }); + + it('Renders correct filters for onboarding API', function() { + var onboardingData; + + setFixtures( + '
' + + '
' + ); + + onboardingData = JSON.parse(JSON.stringify(expectedOnboardingDataJson)); + onboardingData[0].use_onboarding_profile_api = true; + + this.server.respondWith('GET', '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id', + [ + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify(onboardingData) + ] + ); + + this.proctored_exam_onboarding_view = new edx.instructor_dashboard.proctoring.ProctoredExamOnboardingView(); + + this.server.respond(); + this.server.respond(); + + expect(this.proctored_exam_onboarding_view.$el.find('.status-checkboxes').html()) + .toContain('Not Started'); + expect(this.proctored_exam_onboarding_view.$el.find('.status-checkboxes').html()) + .toContain('Submitted'); + expect(this.proctored_exam_onboarding_view.$el.find('.status-checkboxes').html()) + .toContain('Verified'); + expect(this.proctored_exam_onboarding_view.$el.find('.status-checkboxes').html()) + .toContain('Approved in Another Course'); + expect(this.proctored_exam_onboarding_view.$el.find('.status-checkboxes').html()) + .toContain('Rejected'); + expect(this.proctored_exam_onboarding_view.$el.find('.status-checkboxes').html()) + .not.toContain('Setup Started'); + }); + + it('renders correctly with 503 response', function() { + var errorMessage; + + this.server.respondWith('GET', '/api/edx_proctoring/v1/user_onboarding/status/course_id/test_course_id', + [ + 503, + { + 'Content-Type': 'application/json' + }, + JSON.stringify({detail: 'Error message.'}) + ] + ); + + this.proctored_exam_onboarding_view = new edx.instructor_dashboard.proctoring.ProctoredExamOnboardingView(); + + this.server.respond(); + this.server.respond(); + + errorMessage = this.proctored_exam_onboarding_view.$el.find('.error-response'); + expect(errorMessage.html()).toContain('Error message.'); + expect(errorMessage).toBeVisible(); + expect(this.proctored_exam_onboarding_view.$el.find('.onboarding-status-content')).not.toBeVisible(); + }); }); diff --git a/edx_proctoring/static/proctoring/templates/student-onboarding-status.underscore b/edx_proctoring/static/proctoring/templates/student-onboarding-status.underscore index 4ec9675fee4..a5268898793 100644 --- a/edx_proctoring/static/proctoring/templates/student-onboarding-status.underscore +++ b/edx_proctoring/static/proctoring/templates/student-onboarding-status.underscore @@ -27,11 +27,12 @@ margin-left: 10px; } - .no-onboarding-data { + .no-onboarding-data, .onboarding-error { margin: 20px; font-weight: bold; } +

<% var isOnboardingItems = onboardingItems.length !== 0 %>
@@ -45,8 +46,8 @@
@@ -78,13 +79,13 @@
  • - +
    • <% } else { %>
  • - +
    • <% }%> @@ -98,7 +99,7 @@ checked="true"<% } %>> @@ -126,11 +127,11 @@ <%- interpolate(gettext(" %(username)s "), { username: item.username }, true) %> - <%- interpolate(gettext(" %(enrollmentMode)s "), + <%- interpolate(gettext(" %(enrollmentMode)s "), { enrollmentMode: getReadableString(item.enrollment_mode) }, true) %> - <%- interpolate(gettext(" %(onboardingStatus)s "), + <%- interpolate(gettext(" %(onboardingStatus)s "), { onboardingStatus: getReadableString(item.status) }, true) %> <%= getDateFormat(item.modified) %> diff --git a/edx_proctoring/statuses.py b/edx_proctoring/statuses.py index 8b27b385e6d..843d74500e6 100644 --- a/edx_proctoring/statuses.py +++ b/edx_proctoring/statuses.py @@ -251,6 +251,7 @@ class InstructorDashboardOnboardingAttemptStatus: rejected = 'rejected' verified = 'verified' error = 'error' + expired = 'expired' # The following status is not a true attempt status, but is used when the # user's onboarding profile is approved in a different course. @@ -273,7 +274,8 @@ class InstructorDashboardOnboardingAttemptStatus: ProctoredExamStudentAttemptStatus.submitted: submitted, ProctoredExamStudentAttemptStatus.rejected: rejected, ProctoredExamStudentAttemptStatus.verified: verified, - ProctoredExamStudentAttemptStatus.error: error + ProctoredExamStudentAttemptStatus.error: error, + ProctoredExamStudentAttemptStatus.expired: expired } @classmethod @@ -304,11 +306,20 @@ class VerificientOnboardingProfileStatus: profile_status_mapping = { no_profile: None, - approved: ProctoredExamStudentAttemptStatus.verified, + approved: InstructorDashboardOnboardingAttemptStatus.verified, other_course_approved: InstructorDashboardOnboardingAttemptStatus.other_course_approved, - rejected: ProctoredExamStudentAttemptStatus.rejected, - expired: ProctoredExamStudentAttemptStatus.expired, - pending: ProctoredExamStudentAttemptStatus.submitted + rejected: InstructorDashboardOnboardingAttemptStatus.rejected, + expired: InstructorDashboardOnboardingAttemptStatus.expired, + pending: InstructorDashboardOnboardingAttemptStatus.submitted + } + + filter_status_mapping = { + InstructorDashboardOnboardingAttemptStatus.not_started: no_profile, + InstructorDashboardOnboardingAttemptStatus.submitted: pending, + InstructorDashboardOnboardingAttemptStatus.other_course_approved: other_course_approved, + InstructorDashboardOnboardingAttemptStatus.verified: approved, + InstructorDashboardOnboardingAttemptStatus: rejected, + InstructorDashboardOnboardingAttemptStatus.expired: expired } @classmethod @@ -317,6 +328,28 @@ def get_edx_status_from_profile_status(cls, api_status): Get the internal attempt status given a status from the onboarding api Parameters: - * status (str): + * status (str): status from Verficient's onboarding API endpoint """ return cls.profile_status_mapping.get(api_status) + + @classmethod + def get_profile_status_from_filter(cls, filter_status): + """ + Get the verificient profile status given an edx status for filtering + + Parameters: + * status (str): edX onboarding status + """ + return cls.filter_status_mapping.get(filter_status) + + @classmethod + def get_instructor_status_from_profile_status(cls, api_status): + """ + Get the instructor onboarding status given a status from the onboarding api + + Parameters: + * status (str): status from Verficient's onboarding API endpoint + """ + if api_status == cls.no_profile: + return InstructorDashboardOnboardingAttemptStatus.not_started + return cls.get_edx_status_from_profile_status(api_status) diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index 3d1a507c6b7..cfde79bea60 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -62,6 +62,7 @@ from .test_services import ( MockCertificateService, MockCreditService, + MockEnrollment, MockEnrollmentsService, MockGradesService, MockInstructorService, @@ -1089,7 +1090,7 @@ def test_onboarding_with_api_endpoint(self, api_status, attempt_status, mocked_o ) mocked_onboarding_api.assert_called_with( - course_id=self.onboarding_exam.course_id, + self.onboarding_exam.course_id, user_id=obscured_user_id(self.user_id, self.onboarding_exam.backend) ) @@ -1119,7 +1120,7 @@ def test_onboarding_with_differing_data(self, mocked_onboarding_api, mocked_swit ) mocked_onboarding_api.assert_called_with( - course_id=self.onboarding_exam.course_id, + self.onboarding_exam.course_id, user_id=obscured_user_id(self.user_id, self.onboarding_exam.backend) ) self.assertEqual(response.status_code, 200) @@ -1316,6 +1317,7 @@ def test_multiple_onboarding_exams(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response.status_code, 200) @@ -1373,17 +1375,17 @@ def test_pagination_maintains_query_params(self): previous_url = response_data['previous'] next_url = response_data['next'] text_search_string = 'text_search=test.com' - statuses_filter_string = 'statuses={}'.format(InstructorDashboardOnboardingAttemptStatus.setup_started) + status_filters_string = 'statuses={}'.format(InstructorDashboardOnboardingAttemptStatus.setup_started) self.assertIn(base_url, previous_url) self.assertIn('page=1', previous_url) self.assertIn(text_search_string, previous_url) - self.assertIn(statuses_filter_string, previous_url) + self.assertIn(status_filters_string, previous_url) self.assertIn(base_url, next_url) self.assertIn('page=3', next_url) self.assertIn(text_search_string, next_url) - self.assertIn(statuses_filter_string, next_url) + self.assertIn(status_filters_string, next_url) def test_one_status_filter(self): first_attempt_id = create_exam_attempt(self.onboarding_exam.id, self.user.id, True) @@ -1430,6 +1432,7 @@ def test_one_status_filter(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response.status_code, 200) @@ -1481,6 +1484,7 @@ def test_multiple_status_filters(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response.status_code, 200) @@ -1543,6 +1547,7 @@ def test_returns_correct_onboarding_status(self, attempt_status, expected_onboar 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response.status_code, 200) @@ -1621,6 +1626,7 @@ def test_multiple_exam_attempts(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response.status_code, 200) @@ -1642,6 +1648,7 @@ def test_no_enrollments(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response.status_code, 200) @@ -1737,6 +1744,7 @@ def test_other_course_verified(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response_data, expected_data) @@ -1800,6 +1808,7 @@ def test_onboarding_reset_failed_past_due(self): 'previous': None, 'next': None, 'num_pages': 1, + 'use_onboarding_profile_api': False, } self.assertEqual(response_data, expected_data) @@ -1835,6 +1844,303 @@ def test_staff_only_allowed(self): ) self.assertEqual(response.status_code, 200) + @patch('logging.Logger.error') + @patch('edx_proctoring.views.waffle.switch_is_active') + @patch.object(TestBackendProvider, 'get_onboarding_profile_info') + @ddt.data( + (VerificientOnboardingProfileStatus.no_profile, InstructorDashboardOnboardingAttemptStatus.not_started), + (VerificientOnboardingProfileStatus.other_course_approved, + InstructorDashboardOnboardingAttemptStatus.other_course_approved), + (VerificientOnboardingProfileStatus.approved, ProctoredExamStudentAttemptStatus.verified), + (VerificientOnboardingProfileStatus.rejected, ProctoredExamStudentAttemptStatus.rejected), + (VerificientOnboardingProfileStatus.pending, ProctoredExamStudentAttemptStatus.submitted), + (VerificientOnboardingProfileStatus.expired, ProctoredExamStudentAttemptStatus.expired) + ) + @ddt.unpack + def test_instructor_onboarding_with_api_endpoint(self, api_status, attempt_status, mocked_onboarding_api, + mocked_switch_is_active, mock_logger): + mocked_switch_is_active.return_value = True + + mocked_onboarding_api.return_value = { + 'results': [ + { + 'user_id': obscured_user_id(self.user.id, self.onboarding_exam.backend), + 'status': api_status, + 'expiration_date': '2051-05-21' + }, + ] + } + + response = self.client.get( + reverse( + 'edx_proctoring:user_onboarding.status.course', + kwargs={'course_id': self.onboarding_exam.course_id}, + ) + ) + + mocked_onboarding_api.assert_called() + + expected_data = { + 'results': [ + { + 'username': self.user.username, + 'enrollment_mode': self.enrollment_modes[0], + 'status': attempt_status, + 'modified': None, + }, + { + 'username': self.learner_1.username, + 'enrollment_mode': self.enrollment_modes[1], + 'status': InstructorDashboardOnboardingAttemptStatus.not_started, + 'modified': None, + }, + { + 'username': self.learner_2.username, + 'enrollment_mode': self.enrollment_modes[2], + 'status': InstructorDashboardOnboardingAttemptStatus.not_started, + 'modified': None, + } + ], + 'count': 3, + 'previous': None, + 'next': None, + 'num_pages': 1, + 'use_onboarding_profile_api': True, + } + + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + self.assertEqual(response_data, expected_data) + mock_logger.assert_not_called() + + @patch('logging.Logger.error') + @patch('edx_proctoring.views.waffle.switch_is_active') + @patch.object(TestBackendProvider, 'get_onboarding_profile_info') + def test_instructor_onboarding_with_403_api_response(self, mocked_onboarding_api, + mocked_switch_is_active, mock_logger): + """ + Test that internal logic is used if proctoring backend api endpoint returns non 200 response. + """ + mocked_switch_is_active.return_value = True + + mocked_onboarding_api.side_effect = BackendProviderOnboardingProfilesException('some error', 403) + + response = self.client.get( + reverse( + 'edx_proctoring:user_onboarding.status.course', + kwargs={'course_id': self.onboarding_exam.course_id}, + ) + ) + + mocked_onboarding_api.assert_called() + + self.assertEqual(response.status_code, 503) + response_data = json.loads(response.content.decode('utf-8')) + self.assertEqual( + response_data, + {'detail': 'The onboarding service is temporarily unavailable. Please try again later.'} + ) + mock_logger.assert_called() + + @patch('logging.Logger.error') + @patch('edx_proctoring.views.waffle.switch_is_active') + @patch.object(TestBackendProvider, 'get_onboarding_profile_info') + def test_instructor_onboarding_filter_by_user(self, mocked_onboarding_api, mocked_switch_is_active, mock_logger): + mocked_switch_is_active.return_value = True + + mocked_onboarding_api.return_value = { + 'results': [ + { + 'user_id': obscured_user_id(self.user.id, self.onboarding_exam.backend), + 'status': VerificientOnboardingProfileStatus.approved, + 'expiration_date': '2051-05-21' + }, + ] + } + + mock_enrollments = [MockEnrollment(self.user, self.enrollment_modes[0])] + + # create onboarding attempt to check that modified time is accurate + onboarding_attempt_id = create_exam_attempt( + self.onboarding_exam.id, + self.user.id, + True, + ) + update_attempt_status(onboarding_attempt_id, ProctoredExamStudentAttemptStatus.verified) + serialized_onboarding_attempt = get_exam_attempt_by_id(onboarding_attempt_id) + + with patch( + 'edx_proctoring.tests.test_services.MockEnrollmentsService.get_enrollments_can_take_proctored_exams', + return_value=mock_enrollments + ): + response = self.client.get( + reverse( + 'edx_proctoring:user_onboarding.status.course', + kwargs={'course_id': self.onboarding_exam.course_id}, + ), + {'text_search': self.user.username} + ) + + mocked_onboarding_api.assert_called() + + expected_data = { + 'results': [ + { + 'username': self.user.username, + 'enrollment_mode': self.enrollment_modes[0], + 'status': ProctoredExamStudentAttemptStatus.verified, + 'modified': serialized_onboarding_attempt['modified'], + }, + ], + 'count': 1, + 'previous': None, + 'next': None, + 'num_pages': 1, + 'use_onboarding_profile_api': True, + } + + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + self.assertEqual(response_data, expected_data) + mock_logger.assert_not_called() + + @patch('edx_proctoring.views.waffle.switch_is_active') + @patch.object(TestBackendProvider, 'get_onboarding_profile_info') + def test_instructor_onboarding_filter_by_status(self, mocked_onboarding_api, mocked_switch_is_active): + mocked_switch_is_active.return_value = True + + def side_effect_func(_, **kwargs): + if kwargs['status'] == VerificientOnboardingProfileStatus.approved: + api_response = { + 'results': [ + { + 'user_id': obscured_user_id(self.user.id, self.onboarding_exam.backend), + 'status': VerificientOnboardingProfileStatus.approved, + 'expiration_date': '2051-05-21' + }, + ] + } + else: + api_response = { + 'results': [ + { + 'user_id': obscured_user_id(self.learner_1.id, self.onboarding_exam.backend), + 'status': VerificientOnboardingProfileStatus.pending, + 'expiration_date': '2051-05-21' + }, + ] + } + return api_response + + mocked_onboarding_api.side_effect = side_effect_func + + response = self.client.get( + reverse( + 'edx_proctoring:user_onboarding.status.course', + kwargs={'course_id': self.onboarding_exam.course_id}, + ), + {'statuses': 'verified,submitted'} + ) + + expected_data = { + 'results': [ + { + 'username': self.user.username, + 'enrollment_mode': self.enrollment_modes[0], + 'status': ProctoredExamStudentAttemptStatus.verified, + 'modified': None, + }, + { + 'username': self.learner_1.username, + 'enrollment_mode': self.enrollment_modes[1], + 'status': ProctoredExamStudentAttemptStatus.submitted, + 'modified': None, + }, + ], + 'count': 2, + 'previous': None, + 'next': None, + 'num_pages': 1, + 'use_onboarding_profile_api': True, + } + + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + self.assertEqual(response_data, expected_data) + + @patch('edx_proctoring.views.waffle.switch_is_active') + @patch.object(TestBackendProvider, 'get_onboarding_profile_info') + def test_instructor_onboarding_filter_by_status_no_profile(self, mocked_onboarding_api, mocked_switch_is_active): + mocked_switch_is_active.return_value = True + + def side_effect_func(_, **kwargs): + if kwargs.get('page') == 1: + api_response = { + 'results': [ + { + 'user_id': obscured_user_id(self.user.id, self.onboarding_exam.backend), + 'status': VerificientOnboardingProfileStatus.no_profile, + 'expiration_date': '2051-05-21' + }, + ], + 'next_page_number': 2, + } + else: + api_response = { + 'results': [ + { + 'user_id': obscured_user_id(self.learner_1.id, self.onboarding_exam.backend), + 'status': VerificientOnboardingProfileStatus.no_profile, + 'expiration_date': '2051-05-21' + }, + ], + 'next_page_number': None, + } + return api_response + + mocked_onboarding_api.side_effect = side_effect_func + + response = self.client.get( + reverse( + 'edx_proctoring:user_onboarding.status.course', + kwargs={'course_id': self.onboarding_exam.course_id}, + ), + {'statuses': 'not_started'} + ) + + expected_data = { + 'results': [ + { + 'username': self.user.username, + 'enrollment_mode': self.enrollment_modes[0], + 'status': InstructorDashboardOnboardingAttemptStatus.not_started, + 'modified': None, + }, + { + 'username': self.learner_1.username, + 'enrollment_mode': self.enrollment_modes[1], + 'status': InstructorDashboardOnboardingAttemptStatus.not_started, + 'modified': None, + }, + { + 'username': self.learner_2.username, + 'enrollment_mode': self.enrollment_modes[2], + 'status': InstructorDashboardOnboardingAttemptStatus.not_started, + 'modified': None, + }, + ], + 'count': 3, + 'previous': None, + 'next': None, + 'num_pages': 1, + 'use_onboarding_profile_api': True, + } + + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + print(response_data) + self.assertEqual(response_data, expected_data) + @ddt.ddt class TestStudentProctoredExamAttempt(LoggedInTestCase): diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 39cef599ada..c279bfb5093 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -10,6 +10,7 @@ import pytz import waffle # pylint: disable=invalid-django-waffle-import from crum import get_current_request +from edx_django_utils.monitoring import set_custom_attribute from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator from rest_framework import status @@ -61,7 +62,11 @@ update_exam, update_exam_attempt ) -from edx_proctoring.constants import ONBOARDING_PROFILE_API, PING_FAILURE_PASSTHROUGH_TEMPLATE +from edx_proctoring.constants import ( + ONBOARDING_PROFILE_API, + ONBOARDING_PROFILE_INSTRUCTOR_DASHBOARD_API, + PING_FAILURE_PASSTHROUGH_TEMPLATE +) from edx_proctoring.exceptions import ( AllowanceValueNotAllowedException, BackendProviderOnboardingProfilesException, @@ -597,7 +602,7 @@ def get(self, request): # pylint: disable=too-many-statements if waffle.switch_is_active(ONBOARDING_PROFILE_API): try: obs_user_id = obscured_user_id(user.id, onboarding_exam.backend) - onboarding_profile_data = backend.get_onboarding_profile_info(course_id=course_id, user_id=obs_user_id) + onboarding_profile_data = backend.get_onboarding_profile_info(course_id, user_id=obs_user_id) except BackendProviderOnboardingProfilesException as exc: # if backend raises exception, log message and return data from onboarding exam attempt log_message = ( @@ -756,16 +761,130 @@ def get(self, request, course_id): data_page = request.GET.get('page', 1) text_search = request.GET.get('text_search') - statuses_filter = request.GET.get('statuses') + status_filters = request.GET.get('statuses') enrollments = get_enrollments_can_take_proctored_exams(course_id, text_search) + # add custom attribute to better track performance of this endpoint against the + # number of proctoring eligible enrollments in the given course + set_custom_attribute('num_proctoring_eligible_enrollments', len(enrollments)) + users = [] enrollment_modes_by_user_id = {} for enrollment in enrollments: users.append(enrollment.user) enrollment_modes_by_user_id[enrollment.user.id] = enrollment.mode + onboarding_data = [] + use_onboarding_profile_api = waffle.switch_is_active(ONBOARDING_PROFILE_INSTRUCTOR_DASHBOARD_API) + if use_onboarding_profile_api: + backend = get_backend_provider(name=onboarding_exam.backend) + + onboarding_profile_info, api_response_error = self._get_all_paginated_responses( + backend, + course_id, + status_filters + ) + + if api_response_error: + # if backend raises exception, log message and return data from onboarding exam attempt + log_message = ( + 'Failed to use backend onboarding status API endpoint for course_id={course_id}' + 'with query parameters text_search: {text_search_filter}, status: {status_filters}' + 'because backend failed to respond. Onboarding status' + 'will be determined by the users\'s onboarding attempts. ' + 'Status: {status}, Response: {response}.'.format( + course_id=course_id, + text_search_filter=text_search, + status_filters=status_filters, + response=str(api_response_error), + status=api_response_error.http_status, + ) + ) + LOG.warning(log_message) + + return Response( + status=status.HTTP_503_SERVICE_UNAVAILABLE, + data={'detail': _('The onboarding service is temporarily unavailable. Please try again later.')} + ) + + obscured_user_ids_to_users = {obscured_user_id(user.id, onboarding_exam.backend): user for user in users} + + for onboarding_profile in onboarding_profile_info: + obscured_id = onboarding_profile['user_id'] + user = obscured_user_ids_to_users.get(obscured_id) + onboarding_status = onboarding_profile['status'] + data = { + 'username': user.username, + 'enrollment_mode': enrollment_modes_by_user_id.get(user.id), + 'status': VerificientOnboardingProfileStatus.get_instructor_status_from_profile_status( + onboarding_status + ), + 'modified': None, + } + onboarding_data.append(data) + del obscured_user_ids_to_users[obscured_id] + + if status_filters is None or 'not_started' in status_filters: + for (obscured_id, user) in obscured_user_ids_to_users.items(): + # remaining learners that are not represented in the API response + # have not started onboarding + data = { + 'username': user.username, + 'enrollment_mode': enrollment_modes_by_user_id.get(user.id), + 'status': InstructorDashboardOnboardingAttemptStatus.not_started, + 'modified': None, + } + onboarding_data.append(data) + else: + onboarding_data = self._get_onboarding_info_no_onboarding_api( + course_id, + onboarding_exam, + users, + enrollment_modes_by_user_id, + ) + onboarding_data = self._filter_onboarding_data_by_status_filters(onboarding_data, status_filters) + + query_params = self._get_query_params(text_search, status_filters) + paginated_data = self._paginate_data(onboarding_data, data_page, onboarding_exam.course_id, query_params) + + # add modified time to each user in the paginated data, as Verificient's API does not currently return this data + results = paginated_data['results'] + response_users = [get_user_model().objects.get(username=(user['username'])) for user in results] + response_onboarding_data = self._get_onboarding_info_no_onboarding_api( + course_id, + onboarding_exam, + response_users, + enrollment_modes_by_user_id + ) + + # Get modified date for each learner. Once the onboarding provider has included + # a modified date in their payload, this can be removed. + modified_dict = {} + for attempt in response_onboarding_data: + username = attempt['username'] + modified_dict[username] = attempt['modified'] + + for user in results: + user['modified'] = modified_dict[user['username']] + + paginated_data['results'] = results + paginated_data['use_onboarding_profile_api'] = use_onboarding_profile_api + + return Response(paginated_data) + + def _get_onboarding_info_no_onboarding_api(self, course_id, onboarding_exam, users, enrollment_modes_by_user_id): + """ + Get the onboarding data for a list of users without using the proctoring provider's onboarding API. + This is used as fallback behavior either when the ONBOARDING_PROFILE_INSTRUCTOR_DASHBOARD_API waffle + flag is not enabled or when the aforementioned flag is enabled but the API call fails. + + Parameters: + * course_id: the course ID of the course + * onboarding_exam: the ProctoredExam object representing the onboarding exam in the course + * users: a list of users for whom we should return onboarding profile data + * enrollment_modes_by_user_id: a mapping between users and their enrollment mode in the course + """ # get onboarding attempts for users for the course onboarding_attempts = ProctoredExamStudentAttempt.objects.get_proctored_practice_attempts_by_course_id( course_id, @@ -780,7 +899,6 @@ def get(self, request, course_id): # select a verified, or most recent, exam attempt per user onboarding_attempts_per_user = self._get_relevant_attempt_per_user(onboarding_attempts) - onboarding_data = [] for user in users: user_attempt = onboarding_attempts_per_user.get(user.id, {}) @@ -823,36 +941,45 @@ def get(self, request, course_id): onboarding_data.append(data) + return onboarding_data + + def _filter_onboarding_data_by_status_filters(self, onboarding_data, status_filters): + """ + Filter the list of dictionaries onboarding_data by status_filters. + + Parameters: + * onboarding_data: a list of dictionaries representing learners's onboarding profile information + * status_filters: a comma separated list of statuses to filter onboarding_data by + """ # filter the data by status filter # we filter late than the text_search because users without exam attempts # will have an onboarding status of "not_started", and we want to be # able to filter on that status - if statuses_filter: - statuses = set(statuses_filter.split(',')) - onboarding_data = [ + filtered_onboarding_data = onboarding_data + if status_filters: + statuses = set(status_filters.split(',')) + filtered_onboarding_data = [ data for data in onboarding_data if data['status'] in statuses ] - query_params = self._get_query_params(text_search, statuses_filter) - paginated_data = self._paginate_data(onboarding_data, data_page, onboarding_exam.course_id, query_params) - return Response(paginated_data) + return filtered_onboarding_data - def _get_query_params(self, text_search, statuses_filter): + def _get_query_params(self, text_search, status_filters): """ Return the query parameters as a dictionary, only including key value pairs for supplied keys. Parameters: * text_search: the text search query parameter - * statuses_filter: the statuses filter query parameter + * status_filters: the statuses filter query parameter """ query_params = {} if text_search: query_params['text_search'] = text_search - if statuses_filter: - query_params['statuses'] = statuses_filter + if status_filters: + query_params['statuses'] = status_filters return query_params def _get_relevant_attempt_per_user(self, attempts): @@ -915,6 +1042,38 @@ def _get_url(self, course_id, **kwargs): query_string = urlencode(kwargs) return url + '?' + query_string + def _get_all_paginated_responses(self, backend, course_id, status_filters): + """ + Accumulate a list of all onboarding profile responses from the proctoring provider + """ + http_error = None + results = [] + onboarding_profile_kwargs = {} + + if not status_filters: + status_filters = [None] + else: + status_filters = status_filters.split(',') + + for status_filter in status_filters: + if status_filter is not None: + onboarding_profile_kwargs['status'] = VerificientOnboardingProfileStatus\ + .get_profile_status_from_filter(status_filter) + onboarding_profile_kwargs['page'] = 1 + while onboarding_profile_kwargs['page'] is not None: + try: + request = backend.get_onboarding_profile_info( + course_id, **onboarding_profile_kwargs + ) + results += request.get('results') + onboarding_profile_kwargs['page'] = request.get('next_page_number') + except BackendProviderOnboardingProfilesException as exc: + http_error = exc + # return on the earliest error + return results, http_error + + return results, http_error + class StudentProctoredExamAttempt(ProctoredAPIView): """ diff --git a/package.json b/package.json index 341733d3ddd..925c358bd36 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.16.0", + "version": "3.17.0", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test"