feat: send course access error along to course home MFE

We previously only sent it to the courseware MFE. Also unify the
naming of this access error to 'course_access' in courseware and
course home APIs.
This commit is contained in:
Michael Terry
2021-07-22 16:23:46 -04:00
parent 23f0fc489f
commit 29159891c3
6 changed files with 107 additions and 29 deletions

View File

@@ -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()

View File

@@ -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'

View File

@@ -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,
}

View File

@@ -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()

View File

@@ -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 """

View File

@@ -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?