Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: [AA-1206] remove redundant fields from API (#29920)

Browse Source 
* feat: [AA-1206] remove redundant fields from API

Part of a larger effort to clean up the MFE BFF endpoints.
Remove the redundant fields username and course_access, both of which are also available in the course home metadata call.
This commit is contained in:
Chris Deery
2022-03-04 09:01:02 -05:00
committed by GitHub
parent 65aa26bd81
commit b61b29196b
5 changed files with 13 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
docs/swagger.yaml
View File

@@ -2557,7 +2557,6 @@ paths:
* pacing: Course pacing. Possible values: instructor, self
* tabs: Course tabs
* user_timezone: User's chosen timezone setting (or null for browser default)
* can_load_course: Whether the user can view the course (AccessResponse object)
* is_staff: Whether the effective user has staff access to the course
* original_user_is_staff: Whether the original user has staff access to the course
* can_view_legacy_courseware: Indicates whether the user is able to see the legacy courseware view

11
lms/djangoapps/courseware/access_response.py
View File

@@ -236,14 +236,3 @@ class AuthenticationRequiredAccessError(AccessError):
developer_message = "User must be authenticated to view the course"
user_message = _("You must be logged in to see this course")
super().__init__(error_code, developer_message, user_message)
class CoursewareMicrofrontendDisabledAccessError(AccessError):
"""
Access denied because the courseware micro-frontend is disabled for this user.
"""
def __init__(self):
error_code = 'microfrontend_disabled'
developer_message = 'Micro-frontend is disabled for this user'
user_message = _('Please view your course in the existing experience')
super().__init__(error_code, developer_message, user_message)

2
openedx/core/djangoapps/courseware_api/serializers.py
View File

@@ -109,7 +109,6 @@ class CourseInfoSerializer(serializers.Serializer): # pylint: disable=abstract-
can_view_legacy_courseware = serializers.BooleanField()
can_access_proctored_exams = serializers.BooleanField()
is_staff = serializers.BooleanField()
course_access = serializers.DictField()
notes = serializers.DictField()
marketing_url = serializers.CharField()
celebrations = serializers.DictField()
@@ -121,7 +120,6 @@ class CourseInfoSerializer(serializers.Serializer): # pylint: disable=abstract-
linkedin_add_to_profile_url = serializers.URLField()
is_integrity_signature_enabled = serializers.BooleanField()
user_needs_integrity_signature = serializers.BooleanField()
username = serializers.CharField()
def __init__(self, *args, **kwargs):
"""

37
openedx/core/djangoapps/courseware_api/tests/test_views.py
View File

@@ -129,9 +129,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
)
@ddt.unpack
@mock.patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True})
@mock.patch('openedx.core.djangoapps.courseware_api.views.CoursewareMeta.is_microfrontend_enabled_for_user')
def test_enrolled_course_metadata(self, logged_in, enrollment_mode, is_microfrontend_enabled_for_user):
is_microfrontend_enabled_for_user.return_value = True
def test_enrolled_course_metadata(self, logged_in, enrollment_mode):
check_public_access = mock.Mock()
check_public_access.return_value = ACCESS_DENIED
with mock.patch('lms.djangoapps.courseware.access_utils.check_public_access', check_public_access):
@@ -177,7 +175,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
if enrollment_mode == 'audit':
assert response.data['verify_identity_url'] is None
assert response.data['verification_status'] == 'none' # lint-amnesty, pylint: disable=literal-comparison
assert response.data['verification_status'] == 'none'
assert response.data['linkedin_add_to_profile_url'] is None
else:
assert response.data['certificate_data']['cert_status'] == 'earned_but_not_available'
@@ -186,7 +184,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
)
# The response contains an absolute URL so this is only checking the path of the final
assert expected_verify_identity_url in response.data['verify_identity_url']
assert response.data['verification_status'] == 'none' # lint-amnesty, pylint: disable=literal-comparison
assert response.data['verification_status'] == 'none'
request = RequestFactory().request()
cert_url = get_certificate_url(course_id=self.course.id, uuid=cert.verify_uuid)
@@ -215,9 +213,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
)
@ddt.unpack
@mock.patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True})
@mock.patch('openedx.core.djangoapps.courseware_api.views.CoursewareMeta.is_microfrontend_enabled_for_user')
def test_unenrolled_course_metadata(self, logged_in, enable_anonymous, is_microfrontend_enabled_for_user):
is_microfrontend_enabled_for_user.return_value = True
def test_unenrolled_course_metadata(self, logged_in, enable_anonymous):
check_public_access = mock.Mock()
check_public_access.return_value = enable_anonymous
with mock.patch('lms.djangoapps.courseware.access_utils.check_public_access', check_public_access):
@@ -229,13 +225,9 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
if enable_anonymous and not logged_in:
# multiple checks use this handler
check_public_access.assert_called()
assert response.data['enrollment']['mode'] is None
assert response.data['course_access']['has_access']
assert response.data['course_goals']['selected_goal'] is None
assert response.data['course_goals']['weekly_learning_goal_enabled'] is False
else:
assert not response.data['course_access']['has_access']
@ddt.data(
# Who has access to MFE courseware?
@@ -245,7 +237,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "student",
"enroll_user": True,
"masquerade_role": None,
"expect_course_access": True,
},
{
# Un-enrolled learners should NOT have access.
@@ -253,7 +244,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "student",
"enroll_user": False,
"masquerade_role": None,
"expect_course_access": False,
},
{
# Un-enrolled instructors should have access.
@@ -261,7 +251,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "instructor",
"enroll_user": False,
"masquerade_role": None,
"expect_course_access": True,
},
{
# Un-enrolled instructors masquerading as students should have access.
@@ -269,7 +258,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "instructor",
"enroll_user": False,
"masquerade_role": "student",
"expect_course_access": True,
},
{
# If MFE is not visible, enrolled learners shouldn't have access.
@@ -277,7 +265,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "student",
"enroll_user": True,
"masquerade_role": None,
"expect_course_access": False,
},
{
# If MFE is not visible, instructors shouldn't have access.
@@ -285,7 +272,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "instructor",
"enroll_user": False,
"masquerade_role": None,
"expect_course_access": False,
},
{
# If MFE is not visible, masquerading instructors shouldn't have access.
@@ -293,7 +279,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
"username": "instructor",
"enroll_user": False,
"masquerade_role": "student",
"expect_course_access": False,
},
)
@ddt.unpack
@@ -303,7 +288,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
username: str,
enroll_user: bool,
masquerade_role: Optional[str],
expect_course_access: bool,
):
"""
Test that course_access is calculated correctly based on
@@ -313,22 +297,13 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
if enroll_user:
CourseEnrollment.enroll(user, self.course.id, 'audit')
patch_mfe_visible = mock.patch(
'openedx.core.djangoapps.courseware_api.views.courseware_mfe_is_visible',
return_value=mfe_is_visible,
)
self.client.login(username=user, password='foo')
if masquerade_role:
self.update_masquerade(role=masquerade_role)
with patch_mfe_visible:
response = self.client.get(self.url)
response = self.client.get(self.url)
assert response.status_code == 200
assert response.data['username'] == masquerade_role or username
if expect_course_access:
assert response.data['course_access']['has_access']
else:
assert not response.data['course_access']['has_access']
def test_streak_data_in_response(self):
""" Test that metadata endpoint returns data for the streak celebration """

54
openedx/core/djangoapps/courseware_api/views.py
View File

@@ -18,6 +18,11 @@ from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response
from rest_framework.views import APIView
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from xmodule.modulestore.search import path_to_location
from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.util.views import expose_header
from lms.djangoapps.edxnotes.helpers import is_feature_enabled
@@ -27,11 +32,8 @@ from lms.djangoapps.course_api.api import course_detail
from lms.djangoapps.course_goals.models import UserActivity
from lms.djangoapps.course_goals.api import get_course_goal
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.access_response import (
CoursewareMicrofrontendDisabledAccessError,
)
from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs
from lms.djangoapps.courseware.courses import check_course_access
from lms.djangoapps.courseware.masquerade import (
is_masquerading_as_specific_student,
setup_masquerade,
@@ -42,7 +44,6 @@ from lms.djangoapps.courseware.module_render import get_module_by_usage_id
from lms.djangoapps.courseware.tabs import get_course_tab_list
from lms.djangoapps.courseware.toggles import (
courseware_legacy_is_visible,
courseware_mfe_is_visible,
course_exit_page_is_active,
)
from lms.djangoapps.courseware.views.views import get_cert_data
@@ -64,10 +65,6 @@ from common.djangoapps.student.models import (
CourseEnrollmentCelebration,
LinkedInAddToProfileConfiguration
)
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.search import path_to_location # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW # lint-amnesty, pylint: disable=wrong-import-order
from .serializers import CourseInfoSerializer
from .utils import serialize_upgrade_info
@@ -84,16 +81,7 @@ class CoursewareMeta:
username or self.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.
self.load_access = check_course_access(
self.overview,
self.request.user,
'load',
check_if_enrolled=True,
check_if_authenticated=True,
)
self.original_user_is_staff = has_access(self.request.user, 'staff', self.overview).has_access
self.original_user_is_global_staff = self.request.user.is_staff
self.course_key = course_key
@@ -115,16 +103,6 @@ class CoursewareMeta:
def __getattr__(self, name):
return getattr(self.overview, name)
def is_microfrontend_enabled_for_user(self):
"""
Can this user see the MFE for this course?
"""
return courseware_mfe_is_visible(
course_key=self.course_key,
is_global_staff=self.original_user_is_global_staff,
is_course_staff=self.original_user_is_staff
)
@property
def enrollment(self):
"""
@@ -161,24 +139,6 @@ class CoursewareMeta:
def license(self):
return self.course.license
@property
def username(self):
return self.effective_user.username
@property
def course_access(self) -> dict:
"""
Can the user load this course in the learning micro-frontend?
Return a JSON-friendly access response.
"""
# Only check whether the MFE is enabled if the user would otherwise be allowed to see it
# This means that if the user was denied access, they'll see a meaningful message first if
# there is one.
if self.load_access and not self.is_microfrontend_enabled_for_user():
return CoursewareMicrofrontendDisabledAccessError().to_json()
return self.load_access.to_json()
@property
def tabs(self):
"""