From 2f8572d540f53dd2b628de1bbb0fb7c9c062eb2b Mon Sep 17 00:00:00 2001 From: Carla Duarte Date: Tue, 11 May 2021 17:00:50 -0400 Subject: [PATCH] fix: handle unenrolled users in course home APIs (AA-790) --- .../course_metadata/v1/tests/test_views.py | 10 ++++++++++ .../course_home_api/course_metadata/v1/views.py | 2 +- .../course_home_api/dates/v1/tests/test_views.py | 11 ++++++++--- lms/djangoapps/course_home_api/dates/v1/views.py | 7 ++++++- .../course_home_api/outline/v1/tests/test_views.py | 7 ++++++- .../course_home_api/progress/v1/tests/test_views.py | 10 +++++++--- lms/djangoapps/course_home_api/progress/v1/views.py | 13 ++++++++----- 7 files changed, 46 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py index 4272ae5c70..672a8e99e9 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py @@ -46,6 +46,16 @@ class CourseHomeMetadataTests(BaseCourseHomeTests): # 'Course', 'Wiki', 'Progress' tabs assert len(response.data.get('tabs', [])) == 4 + @ddt.data(True, False) + def test_get_authenticated_not_enrolled(self, has_previously_enrolled): + if has_previously_enrolled: + # Create an enrollment, then unenroll to set is_active to False + CourseEnrollment.enroll(self.user, self.course.id) + CourseEnrollment.unenroll(self.user, self.course.id) + response = self.client.get(self.url) + assert response.status_code == 200 + assert response.data['is_enrolled'] is False + def test_get_authenticated_staff_user(self): self.client.logout() self.client.login(username=self.staff_user.username, password='bar') diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/views.py b/lms/djangoapps/course_home_api/course_metadata/v1/views.py index 4417f3d44a..d6e8f905d8 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/views.py +++ b/lms/djangoapps/course_home_api/course_metadata/v1/views.py @@ -83,7 +83,7 @@ class CourseHomeMetadataView(RetrieveAPIView): username = request.user.username if request.user.username else None course = course_detail(request, request.user.username, course_key) enrollment = CourseEnrollment.get_enrollment(request.user, course_key_string) - user_is_enrolled = bool(enrollment) + user_is_enrolled = bool(enrollment and enrollment.is_active) courseware_meta = CoursewareMeta(course_key, request, request.user.username) can_load_courseware = courseware_meta.is_microfrontend_enabled_for_user() diff --git a/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py b/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py index d867e60ed5..94b0ff7a67 100644 --- a/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py @@ -41,10 +41,14 @@ class DatesTabTestViews(BaseCourseHomeTests): @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_DATES_TAB, active=True) - def test_get_authenticated_user_not_enrolled(self): + @ddt.data(True, False) + def test_get_authenticated_user_not_enrolled(self, has_previously_enrolled): + if has_previously_enrolled: + # Create an enrollment, then unenroll to set is_active to False + CourseEnrollment.enroll(self.user, self.course.id) + CourseEnrollment.unenroll(self.user, self.course.id) response = self.client.get(self.url) - assert response.status_code == 200 - assert not response.data.get('learner_is_full_access') + assert response.status_code == 401 @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_DATES_TAB, active=True) @@ -63,6 +67,7 @@ class DatesTabTestViews(BaseCourseHomeTests): @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_DATES_TAB, active=True) def test_banner_data_is_returned(self): + CourseEnrollment.enroll(self.user, self.course.id) response = self.client.get(self.url) assert response.status_code == 200 self.assertContains(response, 'missed_deadlines') diff --git a/lms/djangoapps/course_home_api/dates/v1/views.py b/lms/djangoapps/course_home_api/dates/v1/views.py index 7d87e44792..e35744305d 100644 --- a/lms/djangoapps/course_home_api/dates/v1/views.py +++ b/lms/djangoapps/course_home_api/dates/v1/views.py @@ -11,6 +11,7 @@ from rest_framework.generics import RetrieveAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_home_api.dates.v1.serializers import DatesTabSerializer from lms.djangoapps.course_home_api.toggles import course_home_mfe_dates_tab_is_active from lms.djangoapps.courseware.access import has_access @@ -83,14 +84,18 @@ class DatesTabView(RetrieveAPIView): monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) + is_staff = bool(has_access(request.user, 'staff', course_key)) _, request.user = setup_masquerade( request, course_key, - staff_access=has_access(request.user, 'staff', course_key), + staff_access=is_staff, reset_masquerade_data=True, ) + if not CourseEnrollment.is_enrolled(request.user, course_key) and not is_staff: + return Response('User not enrolled.', status=401) + blocks = get_course_date_blocks(course, request.user, request, include_access=True, include_past_dates=True) learner_is_full_access = not ContentTypeGatingConfig.enabled_for_enrollment( diff --git a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py index 6974178ace..d9daead6c7 100644 --- a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py @@ -78,7 +78,12 @@ class OutlineTabTestViews(BaseCourseHomeTests): @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_OUTLINE_TAB, active=True) - def test_get_authenticated_user_not_enrolled(self): + @ddt.data(True, False) + def test_get_authenticated_user_not_enrolled(self, has_previously_enrolled): + if has_previously_enrolled: + # Create an enrollment, then unenroll to set is_active to False + CourseEnrollment.enroll(self.user, self.course.id) + CourseEnrollment.unenroll(self.user, self.course.id) response = self.client.get(self.url) assert response.status_code == 200 diff --git a/lms/djangoapps/course_home_api/progress/v1/tests/test_views.py b/lms/djangoapps/course_home_api/progress/v1/tests/test_views.py index ef1e42546a..26d64ec081 100644 --- a/lms/djangoapps/course_home_api/progress/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/progress/v1/tests/test_views.py @@ -61,10 +61,14 @@ class ProgressTabTestViews(BaseCourseHomeTests): @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True) - def test_get_authenticated_user_not_enrolled(self): + @ddt.data(True, False) + def test_get_authenticated_user_not_enrolled(self, has_previously_enrolled): + if has_previously_enrolled: + # Create an enrollment, then unenroll to set is_active to False + CourseEnrollment.enroll(self.user, self.course.id) + CourseEnrollment.unenroll(self.user, self.course.id) response = self.client.get(self.url) - # expecting a redirect - assert response.status_code == 302 + assert response.status_code == 401 @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True) diff --git a/lms/djangoapps/course_home_api/progress/v1/views.py b/lms/djangoapps/course_home_api/progress/v1/views.py index 375d536a95..3b099defb7 100644 --- a/lms/djangoapps/course_home_api/progress/v1/views.py +++ b/lms/djangoapps/course_home_api/progress/v1/views.py @@ -95,8 +95,7 @@ class ProgressTabView(RetrieveAPIView): **Returns** * 200 on success with above fields. - * 302 if the user is not enrolled. - * 401 if the user is not authenticated. + * 401 if the user is not authenticated or not enrolled. * 404 if the course is not available or cannot be seen. """ @@ -119,20 +118,24 @@ class ProgressTabView(RetrieveAPIView): monitoring_utils.set_custom_attribute('course_id', course_key_string) monitoring_utils.set_custom_attribute('user_id', request.user.id) monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) + is_staff = bool(has_access(request.user, 'staff', course_key)) _, request.user = setup_masquerade( request, course_key, - staff_access=has_access(request.user, 'staff', course_key), + staff_access=is_staff, reset_masquerade_data=True ) - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) + course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) course_overview = CourseOverview.get_from_id(course_key) enrollment = CourseEnrollment.get_enrollment(request.user, course_key) enrollment_mode = getattr(enrollment, 'mode', None) + if not (enrollment and enrollment.is_active) and not is_staff: + return Response('User not enrolled.', status=401) + # The block structure is used for both the course_grade and has_scheduled content fields # So it is called upfront and reused for optimization purposes collected_block_structure = get_block_structure_manager(course_key).get_collected() @@ -185,7 +188,7 @@ class ProgressTabView(RetrieveAPIView): 'verification_data': verification_data, } context = self.get_serializer_context() - context['staff_access'] = bool(has_access(request.user, 'staff', course)) + context['staff_access'] = is_staff context['course_key'] = course_key # course_overview and enrollment will be used by VerifiedModeSerializerMixin context['course_overview'] = course_overview