diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b2a471b6842..e50560a619f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,14 @@ Change Log Unreleased ~~~~~~~~~~ +[3.18.0] - 2021-07-15 +~~~~~~~~~~~~~~~~~~~~~ +* Remove old proctored exam attempt url. +* Fix onboarding link generation in proctored exam attempt view when exam attempt is in + onboarding errors status, don't return the link to exams that are not accessible to user. +* Update onboarding link url in student onboarding status view to link + to the learning mfe page instead of LMS. + [3.17.3] - 2021-07-14 ~~~~~~~~~~~~~~~~~~~~~ * Add missing get_proctoring_config method to base backend provider class. diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 778fabe18a3..93aebd9dc21 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.17.3' +__version__ = '3.18.0' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 65067884603..49621f03728 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -58,15 +58,17 @@ ) from edx_proctoring.statuses import InstructorDashboardOnboardingAttemptStatus, ProctoredExamStudentAttemptStatus from edx_proctoring.utils import ( + categorize_inaccessible_exams_by_date, emit_event, get_exam_due_date, get_exam_type, + get_exam_url, get_time_remaining_for_attempt, + get_user_course_outline_details, has_due_date_passed, humanized_time, is_reattempting_exam, obscured_user_id, - resolve_exam_url_for_learning_mfe, verify_and_add_wait_deadline ) @@ -81,6 +83,38 @@ USER_MODEL = get_user_model() +def get_onboarding_exam_link(course_id, user, is_learning_mfe): + """ + Get link to the onboarding exam available to the user for given course. + + Parameters: + * course_id: id of a course where search for the onboarding exam is performed + * user: user for whom the link is built + * is_learning_mfe: Boolean, indicates if the url should be built for the learning mfe application. + + Returns: url pointing to the available exam, if there is no onboarding exam available + to the user returns empty string. + """ + onboarding_exams = list(ProctoredExam.get_practice_proctored_exams_for_course(course_id).order_by('-created')) + if not onboarding_exams or not get_backend_provider(name=onboarding_exams[0].backend).supports_onboarding: + onboarding_link = '' + else: + details = get_user_course_outline_details(user, course_id) + categorized_exams = categorize_inaccessible_exams_by_date(onboarding_exams, details) + non_date_inaccessible_exams = categorized_exams[0] + + # remove onboarding exams not accessible to learners + for onboarding_exam in non_date_inaccessible_exams: + onboarding_exams.remove(onboarding_exam) + + if onboarding_exams: + onboarding_exam = onboarding_exams[0] + onboarding_link = get_exam_url(course_id, onboarding_exam.content_id, is_learning_mfe) + else: + onboarding_link = '' + return onboarding_link + + def get_total_allowed_time_for_exam(exam, user_id): """ Returns total allowed time in humanized form for exam including allowance time. @@ -750,10 +784,7 @@ def get_exam_attempt_data(exam_id, attempt_id, is_learning_mfe=False): # resolve the LMS url, note we can't assume we're running in # a same process as the LMS - if is_learning_mfe: - exam_url_path = resolve_exam_url_for_learning_mfe(exam['course_id'], exam['content_id']) - else: - exam_url_path = reverse('jump_to', args=[exam['course_id'], exam['content_id']]) + exam_url_path = get_exam_url(exam['course_id'], exam['content_id'], is_learning_mfe) attempt_data = { 'in_timed_exam': True, diff --git a/edx_proctoring/tests/test_mfe_views.py b/edx_proctoring/tests/test_mfe_views.py index 4a2b74e0d2b..a8c369edfa1 100644 --- a/edx_proctoring/tests/test_mfe_views.py +++ b/edx_proctoring/tests/test_mfe_views.py @@ -7,17 +7,21 @@ import ddt from mock import patch +from opaque_keys.edx.locator import BlockUsageLocator from django.conf import settings from django.test.utils import override_settings from django.urls import reverse +from django.utils import timezone from edx_proctoring.api import get_exam_by_id, get_review_policy_by_exam_id from edx_proctoring.exceptions import BackendProviderNotConfigured, ProctoredExamNotFoundException from edx_proctoring.models import ProctoredExam, ProctoredExamStudentAllowance +from edx_proctoring.runtime import set_runtime_service from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus from edx_proctoring.utils import humanized_time +from .test_services import MockLearningSequencesService, MockScheduleItemData from .utils import ProctoredExamTestCase @@ -48,6 +52,18 @@ def setUp(self): self.expected_exam_url = '{}/course/{}/{}'.format( settings.LEARNING_MICROFRONTEND_URL, self.course_id, self.content_id ) + yesterday = timezone.now() - timezone.timedelta(days=1) + self.course_scheduled_sections = { + BlockUsageLocator.from_string(self.content_id_onboarding): MockScheduleItemData(yesterday), + BlockUsageLocator.from_string( + 'block-v1:test+course+1+type@sequential+block@assignment' + ): MockScheduleItemData(yesterday), + } + + set_runtime_service('learning_sequences', MockLearningSequencesService( + list(self.course_scheduled_sections.keys()), + self.course_scheduled_sections, + )) def assertHasExamData(self, response_data, has_attempt, has_verification_url=False, has_download_url=False, content_id=None): @@ -286,9 +302,7 @@ def test_exam_data_contains_link_to_onboarding_exam_if_attempt_in_onboarding_err """ self._create_exam_attempt(self.proctored_exam_id, status=status) - onboarding_exam = ProctoredExam.objects.filter( - course_id=self.course_id, is_active=True, is_practice_exam=True - ).first() + onboarding_exam = ProctoredExam.objects.get(id=self.onboarding_exam_id) if is_learning_mfe: expected_exam_url = '{}/course/{}/{}'.format( @@ -332,7 +346,35 @@ def test_exam_data_does_not_fail_if_onboarding_errors_and_no_onboarding_exam(sel response = self.client.get(url) response_data = json.loads(response.content.decode('utf-8')) exam = response_data['exam'] - assert 'onboarding_link' not in exam + assert 'onboarding_link' in exam + self.assertEqual(exam['onboarding_link'], '') + + def test_onboarding_errors_and_onboarding_exam_not_available(self): + """ + Tests the GET exam attempts data not contain link to onboarding exam if + when user tries to take proctored exam and has not yet completed required + onboarding exam and onboarding exams are not available for the user. + """ + self._create_exam_attempt(self.proctored_exam_id, status=ProctoredExamStudentAttemptStatus.onboarding_missing) + set_runtime_service('learning_sequences', MockLearningSequencesService( + [], # sections user can see (none) + self.course_scheduled_sections, # all scheduled sections + )) + url = reverse( + 'edx_proctoring:proctored_exam.exam_attempts', + kwargs={ + 'course_id': self.course_id, + } + ) + '?' + urlencode({ + 'content_id': self.content_id, + 'is_learning_mfe': True, + }) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + exam = response_data['exam'] + assert 'onboarding_link' in exam + self.assertEqual(exam['onboarding_link'], '') class ProctoredSettingsViewTests(ProctoredExamTestCase): diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index cfde79bea60..47f6bc0beba 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -55,7 +55,7 @@ ) from edx_proctoring.tests import mock_perm from edx_proctoring.urls import urlpatterns -from edx_proctoring.utils import obscured_user_id +from edx_proctoring.utils import obscured_user_id, resolve_exam_url_for_learning_mfe from edx_proctoring.views import require_course_or_global_staff, require_staff from mock_apps.models import Profile @@ -551,6 +551,22 @@ def test_no_onboarding_exam(self): message = 'There is no onboarding exam related to this course id.' self.assertEqual(response_data['detail'], message) + @override_settings(LEARNING_MICROFRONTEND_URL='https://learningmfe') + def test_onboarding_mfe_link(self): + """ + Test that the request returns correct link to onboarding exam for learning mfe application. + """ + response = self.client.get( + reverse('edx_proctoring:user_onboarding.status') + + '?course_id={}&is_learning_mfe=True'.format(self.course_id) + ) + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content.decode('utf-8')) + self.assertEqual( + response_data['onboarding_link'], + resolve_exam_url_for_learning_mfe(self.course_id, self.onboarding_exam.content_id) + ) + def test_no_exam_attempts(self): """ Test that the onboarding status is None if there are no exam attempts diff --git a/edx_proctoring/urls.py b/edx_proctoring/urls.py index fb3bf3bfcac..24e7e7ba09a 100644 --- a/edx_proctoring/urls.py +++ b/edx_proctoring/urls.py @@ -131,13 +131,6 @@ views.ProctoredExamAttemptView.as_view(), name='proctored_exam.exam_attempts' ), - # TODO: remove url after updating frontend-lib-special-exams - url( - r'edx_proctoring/v1/proctored_exam/attempt/course_id/{}/content_id/{}$'.format( - settings.COURSE_ID_PATTERN, CONTENT_ID_PATTERN), - views.ProctoredExamAttemptView.as_view(), - name='proctored_exam.exam_attempts_old' - ), url( r'edx_proctoring/v1/proctored_exam/settings/exam_id/(?P\d+)/$', views.ProctoredSettingsView.as_view(), diff --git a/edx_proctoring/utils.py b/edx_proctoring/utils.py index 62c75703da1..e664de4515a 100644 --- a/edx_proctoring/utils.py +++ b/edx_proctoring/utils.py @@ -13,14 +13,17 @@ from eventtracking import tracker from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import BlockUsageLocator from rest_framework.authentication import SessionAuthentication from rest_framework.permissions import IsAuthenticated from rest_framework.views import APIView from django.conf import settings +from django.urls import reverse from django.utils.translation import ugettext as _ from edx_proctoring.models import ProctoredExamStudentAttempt, ProctoredExamStudentAttemptHistory +from edx_proctoring.runtime import get_runtime_service from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus log = logging.getLogger(__name__) @@ -329,3 +332,66 @@ def resolve_exam_url_for_learning_mfe(course_id, content_id): usage_key = UsageKey.from_string(content_id) url = '{}/course/{}/{}'.format(settings.LEARNING_MICROFRONTEND_URL, course_key, usage_key) return url + + +def get_exam_url(course_id, content_id, is_learning_mfe): + """ Helper to build exam url depending if it is requested for the learning MFE app or not. """ + if is_learning_mfe: + return resolve_exam_url_for_learning_mfe(course_id, content_id) + return reverse('jump_to', args=[course_id, content_id]) + + +def get_user_course_outline_details(user, course_id): + """ Helper to get user's course outline details """ + learning_sequences_service = get_runtime_service('learning_sequences') + course_key = CourseKey.from_string(course_id) + details = learning_sequences_service.get_user_course_outline_details( + course_key, user, pytz.utc.localize(datetime.now()) + ) + return details + + +def categorize_inaccessible_exams_by_date(onboarding_exams, details): + """ + Categorize a list of inaccessible onboarding exams based on whether they are + inaccessible because they are in the future, because they are in the past, or because + they are inaccessible for reason unrelated to the exam schedule (e.g. visibility settings, + content gating, etc.) + + Parameters: + * onboarding_exams: a list of onboarding exams + * details: a UserCourseOutlineData returned by the learning sequences API + + Returns: a tuple containing three lists + * non_date_inaccessible_exams: a list of onboarding exams not accessible to the learner for + reasons other than the exam schedule + * future_exams: a list of onboarding exams not accessible to the learner because the exams are released + in the future + * past_due_exams: a list of onboarding exams not accessible to the learner because the exams are past their + due date + """ + non_date_inaccessible_exams = [] + future_exams = [] + past_due_exams = [] + + for onboarding_exam in onboarding_exams: + usage_key = BlockUsageLocator.from_string(onboarding_exam.content_id) + if usage_key not in details.outline.accessible_sequences: + sequence_schedule = details.schedule.sequences.get(usage_key) + + if sequence_schedule: + effective_start = details.schedule.sequences.get(usage_key).effective_start + due_date = get_visibility_check_date(details.schedule, usage_key) + + if effective_start and pytz.utc.localize(datetime.now()) < effective_start: + future_exams.append(onboarding_exam) + elif due_date and pytz.utc.localize(datetime.now()) > due_date: + past_due_exams.append(onboarding_exam) + else: + non_date_inaccessible_exams.append(onboarding_exam) + else: + # if the sequence schedule is not available, then the sequence is not available + # to the learner + non_date_inaccessible_exams.append(onboarding_exam) + + return non_date_inaccessible_exams, future_exams, past_due_exams diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index c5eafbc12e0..0e8e89da2bf 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -4,14 +4,11 @@ import json import logging -from datetime import datetime from urllib.parse import urlencode -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 from rest_framework.negotiation import BaseContentNegotiation @@ -47,6 +44,7 @@ get_exam_by_id, get_last_verified_onboarding_attempts_per_user, get_onboarding_attempt_data_for_learner, + get_onboarding_exam_link, get_proctoring_settings_by_exam_id, get_review_policy_by_exam_id, get_total_allowed_time_for_exam, @@ -101,13 +99,14 @@ ) from edx_proctoring.utils import ( AuthenticatedAPIView, + categorize_inaccessible_exams_by_date, get_exam_type, + get_exam_url, get_time_remaining_for_attempt, - get_visibility_check_date, + get_user_course_outline_details, humanized_time, locate_attempt_by_attempt_code, - obscured_user_id, - resolve_exam_url_for_learning_mfe + obscured_user_id ) ATTEMPTS_PER_PAGE = 25 @@ -275,18 +274,7 @@ def get(self, request, course_id, content_id=None): # proctored exam we need to navigate them to it with a link if (exam['is_proctored'] and attempt_data and attempt_data['attempt_status'] in ProctoredExamStudentAttemptStatus.onboarding_errors): - onboarding_exam = ProctoredExam.objects.filter( - course_id=course_id, is_active=True, is_practice_exam=True - ).first() - if onboarding_exam: - if is_learning_mfe: - onboarding_link = resolve_exam_url_for_learning_mfe( - course_id, - onboarding_exam.content_id - ) - else: - onboarding_link = reverse('jump_to', args=[course_id, onboarding_exam.content_id]) - exam['onboarding_link'] = onboarding_link + exam['onboarding_link'] = get_onboarding_exam_link(course_id, request.user, is_learning_mfe) exam.update({'attempt': attempt_data}) except ProctoredExamNotFoundException: exam = {} @@ -496,12 +484,13 @@ class StudentOnboardingStatusView(ProctoredAPIView): HTTP GET: returns the learner's onboarding status relative to the given course_id HTTP GET - /edx_proctoring/v1/user_onboarding/status?course_id={course_id}&username={username} + /edx_proctoring/v1/user_onboarding/status?course_id={course_id}&username={username}&is_learning_mfe={} **Query Parameters** * 'course_id': The unique identifier for the course. * 'username': Optional. If not given, the endpoint will return the user's own status. ** In order to view other users' statuses, the user must be course or global staff. + * 'is_learning_mfe': Optional. If set to True, onboarding link will point to the learning mfe application page. **Response Values** * 'onboarding_status': String specifying the learner's onboarding status. @@ -526,6 +515,7 @@ def get(self, request): # pylint: disable=too-many-statements username = request.GET.get('username') course_id = request.GET.get('course_id') + is_learning_mfe = request.GET.get('is_learning_mfe') in ['1', 'true', 'True'] if not course_id: # This parameter is currently required, as the onboarding experience is tied @@ -553,20 +543,16 @@ def get(self, request): # pylint: disable=too-many-statements data={'detail': _('There is no onboarding exam related to this course id.')} ) + # If there are multiple onboarding exams, use the first exam accessible to the user backend = get_backend_provider(name=onboarding_exams[0].backend) - # If there are multiple onboarding exams, use the first exam accessible to the user - learning_sequences_service = get_runtime_service('learning_sequences') - course_key = CourseKey.from_string(course_id) user = get_user_model().objects.get(username=(username or request.user.username)) - details = learning_sequences_service.get_user_course_outline_details( - course_key, user, pytz.utc.localize(datetime.now()) - ) + details = get_user_course_outline_details(user, course_id) # in order to gather information about future or past due exams, we must determine which exams # are inaccessible for other reasons (e.g. visibility settings, content gating, etc.), so that # we can correctly filter them out of the onboarding exams in the course - categorized_exams = self._categorize_inaccessible_exams_by_date(onboarding_exams, details) + categorized_exams = categorize_inaccessible_exams_by_date(onboarding_exams, details) (non_date_inaccessible_exams, future_exams, past_due_exams) = categorized_exams # remove onboarding exams not accessible to learners @@ -593,7 +579,7 @@ def get(self, request): # pylint: disable=too-many-statements if currently_available_exams: onboarding_exam = currently_available_exams[0] - data['onboarding_link'] = reverse('jump_to', args=[course_id, onboarding_exam.content_id]) + data['onboarding_link'] = get_exam_url(course_id, onboarding_exam.content_id, is_learning_mfe) elif future_exams: onboarding_exam = future_exams[0] else: @@ -669,52 +655,6 @@ def get(self, request): # pylint: disable=too-many-statements return Response(data) - def _categorize_inaccessible_exams_by_date(self, onboarding_exams, details): - """ - Categorize a list of inaccessible onboarding exams based on whether they are - inaccessible because they are in the future, because they are in the past, or because - they are inaccessible for reason unrelated to the exam schedule (e.g. visibility settings, - content gating, etc.) - - Parameters: - * onboarding_exams: a list of onboarding exams - * details: a UserCourseOutlineData returned by the learning sequences API - - Returns: a tuple containing three lists - * non_date_inaccessible_exams: a list of onboarding exams not accesible to the learner for - reasons other than the exam schedule - * future_exams: a list of onboarding exams not accessible to the learner because the exams are released - in the future - * past_due_exams: a list of onboarding exams not accessible to the learner because the exams are past their - due date - """ - non_date_inaccessible_exams = [] - future_exams = [] - past_due_exams = [] - - for onboarding_exam in onboarding_exams: - usage_key = BlockUsageLocator.from_string(onboarding_exam.content_id) - - if usage_key not in details.outline.accessible_sequences: - sequence_schedule = details.schedule.sequences.get(usage_key) - - if sequence_schedule: - effective_start = details.schedule.sequences.get(usage_key).effective_start - due_date = get_visibility_check_date(details.schedule, usage_key) - - if effective_start and pytz.utc.localize(datetime.now()) < effective_start: - future_exams.append(onboarding_exam) - elif due_date and pytz.utc.localize(datetime.now()) > due_date: - past_due_exams.append(onboarding_exam) - else: - non_date_inaccessible_exams.append(onboarding_exam) - else: - # if the sequence schedule is not available, then the sequence is not available - # to the learner - non_date_inaccessible_exams.append(onboarding_exam) - - return non_date_inaccessible_exams, future_exams, past_due_exams - class StudentOnboardingStatusByCourseView(ProctoredAPIView): """ diff --git a/package.json b/package.json index 5d47be52bdf..aa8380efaa2 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.17.3", + "version": "3.18.0", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test"