diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/serializers.py b/lms/djangoapps/course_home_api/course_metadata/v1/serializers.py index 61955cd36d..de793b34f0 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/serializers.py +++ b/lms/djangoapps/course_home_api/course_metadata/v1/serializers.py @@ -33,15 +33,17 @@ class CourseHomeMetadataSerializer(VerifiedModeSerializerMixin, serializers.Seri """ Serializer for the Course Home Course Metadata """ + can_load_courseware = serializers.BooleanField() + celebrations = serializers.DictField() + course_access = serializers.DictField() course_id = serializers.CharField() - username = serializers.CharField() is_enrolled = serializers.BooleanField() is_self_paced = serializers.BooleanField() is_staff = serializers.BooleanField() number = serializers.CharField() org = serializers.CharField() original_user_is_staff = serializers.BooleanField() + start = serializers.DateTimeField() # used for certain access denied errors tabs = CourseTabSerializer(many=True) title = serializers.CharField() - can_load_courseware = serializers.BooleanField() - celebrations = serializers.DictField() + username = serializers.CharField() 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 ffea36d9e1..06f4455f07 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 @@ -8,6 +8,7 @@ from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.roles import CourseInstructorRole from lms.djangoapps.courseware.toggles import ( COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES, COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_STREAK_CELEBRATION, @@ -91,3 +92,58 @@ class CourseHomeMetadataTests(BaseCourseHomeTests): celebrations = response.json()['celebrations'] assert celebrations['streak_length_to_celebrate'] == 3 assert celebrations['streak_discount_experiment_enabled'] is True + + @ddt.data( + # Who has access to MFE courseware? + { + # Enrolled learners should have access. + 'enroll_user': True, + 'instructor_role': False, + 'masquerade_role': None, + 'expect_course_access': True, + }, + { + # Un-enrolled learners should NOT have access. + 'enroll_user': False, + 'instructor_role': False, + 'masquerade_role': None, + 'expect_course_access': False, + }, + { + # Un-enrolled instructors should have access. + 'enroll_user': False, + 'instructor_role': True, + 'masquerade_role': None, + 'expect_course_access': True, + }, + { + # Un-enrolled instructors masquerading as students should have access. + 'enroll_user': False, + 'instructor_role': True, + 'masquerade_role': 'student', + 'expect_course_access': True, + }, + ) + @ddt.unpack + def test_course_access(self, enroll_user, instructor_role, masquerade_role, expect_course_access): + """ + Test that course_access is calculated correctly based on + access to MFE and access to the course itself. + """ + if enroll_user: + CourseEnrollment.enroll(self.user, self.course.id, 'audit') + if instructor_role: + CourseInstructorRole(self.course.id).add_users(self.user) + if masquerade_role: + self.update_masquerade(role=masquerade_role) + + response = self.client.get(self.url) + + assert response.status_code == 200 + if expect_course_access: + assert response.data['course_access']['has_access'] + else: + assert not response.data['course_access']['has_access'] + + # Start date is used when handling some errors, so make sure it is present too + assert response.data['start'] == self.course.start.isoformat() + 'Z' 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 d6e8f905d8..b069f73b63 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/views.py +++ b/lms/djangoapps/course_home_api/course_metadata/v1/views.py @@ -6,20 +6,19 @@ from opaque_keys.edx.keys import CourseKey from rest_framework.generics import RetrieveAPIView from rest_framework.response import Response -from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=reimported - from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.djangoapps.courseware_api.utils import get_celebrations_dict -from openedx.core.djangoapps.courseware_api.views import CoursewareMeta -from common.djangoapps.student.models import CourseEnrollment, UserCelebration # lint-amnesty, pylint: disable=unused-import +from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_api.api import course_detail from lms.djangoapps.course_home_api.course_metadata.v1.serializers import CourseHomeMetadataSerializer from lms.djangoapps.courseware.access import has_access +from lms.djangoapps.courseware.courses import check_course_access from lms.djangoapps.courseware.masquerade import setup_masquerade from lms.djangoapps.courseware.tabs import get_course_tab_list +from lms.djangoapps.courseware.toggles import courseware_mfe_is_visible class CourseHomeMetadataView(RetrieveAPIView): @@ -71,22 +70,38 @@ class CourseHomeMetadataView(RetrieveAPIView): def get(self, request, *args, **kwargs): course_key_string = kwargs.get('course_key_string') course_key = CourseKey.from_string(course_key_string) + original_user_is_global_staff = self.request.user.is_staff original_user_is_staff = has_access(request.user, 'staff', course_key).has_access + course = course_detail(request, request.user.username, course_key) + + # We must compute course load access *before* setting up masquerading, + # else course staff (who are not enrolled) will not be able view + # their course from the perspective of a learner. + load_access = check_course_access( + course, + request.user, + 'load', + check_if_enrolled=True, + check_if_authenticated=True, + ) + _, request.user = setup_masquerade( request, course_key, - staff_access=has_access(request.user, 'staff', course_key), + staff_access=original_user_is_staff, reset_masquerade_data=True, ) 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 and enrollment.is_active) - courseware_meta = CoursewareMeta(course_key, request, request.user.username) - can_load_courseware = courseware_meta.is_microfrontend_enabled_for_user() + can_load_courseware = courseware_mfe_is_visible( + course_key=course_key, + is_global_staff=original_user_is_global_staff, + is_course_staff=original_user_is_staff + ) browser_timezone = self.request.query_params.get('browser_timezone', None) celebrations = get_celebrations_dict(request.user, enrollment, course, browser_timezone) @@ -98,10 +113,12 @@ class CourseHomeMetadataView(RetrieveAPIView): 'original_user_is_staff': original_user_is_staff, 'number': course.display_number_with_default, 'org': course.display_org_with_default, + 'start': course.start, 'tabs': get_course_tab_list(request.user, course), 'title': course.display_name_with_default, 'is_self_paced': getattr(course, 'self_paced', False), 'is_enrolled': user_is_enrolled, + 'course_access': load_access.to_json(), 'can_load_courseware': can_load_courseware, 'celebrations': celebrations, } diff --git a/openedx/core/djangoapps/courseware_api/serializers.py b/openedx/core/djangoapps/courseware_api/serializers.py index 68b7836eb1..12b8af98d8 100644 --- a/openedx/core/djangoapps/courseware_api/serializers.py +++ b/openedx/core/djangoapps/courseware_api/serializers.py @@ -106,7 +106,8 @@ class CourseInfoSerializer(serializers.Serializer): # pylint: disable=abstract- original_user_is_staff = serializers.BooleanField() can_view_legacy_courseware = serializers.BooleanField() is_staff = serializers.BooleanField() - can_load_courseware = serializers.DictField() + course_access = serializers.DictField() + can_load_courseware = serializers.DictField(source='course_access') # can be removed once MFE updates notes = serializers.DictField() marketing_url = serializers.CharField() celebrations = serializers.DictField() diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index c9dd69d040..cacd2d8e27 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -198,9 +198,9 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): # multiple checks use this handler check_public_access.assert_called() assert response.data['enrollment']['mode'] is None - assert response.data['can_load_courseware']['has_access'] + assert response.data['course_access']['has_access'] else: - assert not response.data['can_load_courseware']['has_access'] + assert not response.data['course_access']['has_access'] @ddt.data( # Who has access to MFE courseware? @@ -210,7 +210,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "student", "enroll_user": True, "masquerade_role": None, - "expect_can_load_courseware": True, + "expect_course_access": True, }, { # Un-enrolled learners should NOT have access. @@ -218,7 +218,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "student", "enroll_user": False, "masquerade_role": None, - "expect_can_load_courseware": False, + "expect_course_access": False, }, { # Un-enrolled instructors should have access. @@ -226,7 +226,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "instructor", "enroll_user": False, "masquerade_role": None, - "expect_can_load_courseware": True, + "expect_course_access": True, }, { # Un-enrolled instructors masquerading as students should have access. @@ -234,7 +234,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "instructor", "enroll_user": False, "masquerade_role": "student", - "expect_can_load_courseware": True, + "expect_course_access": True, }, { # If MFE is not visible, enrolled learners shouldn't have access. @@ -242,7 +242,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "student", "enroll_user": True, "masquerade_role": None, - "expect_can_load_courseware": False, + "expect_course_access": False, }, { # If MFE is not visible, instructors shouldn't have access. @@ -250,7 +250,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "instructor", "enroll_user": False, "masquerade_role": None, - "expect_can_load_courseware": False, + "expect_course_access": False, }, { # If MFE is not visible, masquerading instructors shouldn't have access. @@ -258,20 +258,20 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): "username": "instructor", "enroll_user": False, "masquerade_role": "student", - "expect_can_load_courseware": False, + "expect_course_access": False, }, ) @ddt.unpack - def test_can_load_courseware( + def test_course_access( self, mfe_is_visible: bool, username: str, enroll_user: bool, masquerade_role: Optional[str], - expect_can_load_courseware: bool, + expect_course_access: bool, ): """ - Test that can_load_courseware is calculated correctly based on + Test that course_access is calculated correctly based on access to MFE and access to the course itself. """ user = User.objects.get(username=username) @@ -289,10 +289,13 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): response = self.client.get(self.url) assert response.status_code == 200 - if expect_can_load_courseware: - assert response.data['can_load_courseware']['has_access'] + if expect_course_access: + assert response.data['course_access']['has_access'] else: - assert not response.data['can_load_courseware']['has_access'] + assert not response.data['course_access']['has_access'] + + # Remove this once can_load_courseware is also removed + assert response.data['course_access'] == response.data['can_load_courseware'] def test_streak_data_in_response(self): """ Test that metadata endpoint returns data for the streak celebration """ diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index 55f71ba481..dc27952df1 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -86,7 +86,6 @@ class CoursewareMeta: self.request.user, 'load', check_if_enrolled=True, - check_survey_complete=False, check_if_authenticated=True, ) self.original_user_is_staff = has_access(self.request.user, 'staff', self.overview).has_access @@ -166,7 +165,7 @@ class CoursewareMeta: return course.license @property - def can_load_courseware(self) -> dict: + def course_access(self) -> dict: """ Can the user load this course in the learning micro-frontend?