Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions lms/djangoapps/instructor/tests/test_api_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,8 +564,8 @@ def test_tabs_log_warning_when_mfe_url_not_set(self):
for tab in tabs:
self.assertFalse(tab['url'].startswith('None'), f"Tab URL should not start with 'None': {tab['url']}") # noqa: PT009 # pylint: disable=line-too-long
self.assertTrue( # noqa: PT009
tab['url'].startswith('/instructor/'),
f"Tab URL should start with '/instructor/': {tab['url']}"
tab['url'].startswith(f'/{self.course.id}/'),
f"Tab URL should start with '/{self.course.id}/': {tab['url']}"
)

def test_pacing_self_for_self_paced_course(self):
Expand Down Expand Up @@ -597,26 +597,26 @@ class BuildTabUrlTest(SimpleTestCase):
going through the full API stack.
"""

def _build(self, setting_name, *parts):
return CourseInformationSerializerV2._build_tab_url(setting_name, *parts) # pylint: disable=protected-access
def _build(self, setting_name, *parts, strip_url=True):
return CourseInformationSerializerV2._build_tab_url(setting_name, *parts, strip_url=strip_url) # pylint: disable=protected-access

@override_settings(INSTRUCTOR_MICROFRONTEND_URL='http://localhost:2003')
@override_settings(INSTRUCTOR_MICROFRONTEND_URL='http://localhost:2003/instructor-dashboard')
def test_joins_base_and_path_parts(self):
"""Parts are joined with '/' separators."""
result = self._build('INSTRUCTOR_MICROFRONTEND_URL', 'instructor', 'course-v1:edX+DemoX+Demo', 'grading')
self.assertEqual(result, 'http://localhost:2003/instructor/course-v1:edX+DemoX+Demo/grading') # noqa: PT009
result = self._build('INSTRUCTOR_MICROFRONTEND_URL', 'course-v1:edX+DemoX+Demo', 'grading')
self.assertEqual(result, '/instructor-dashboard/course-v1:edX+DemoX+Demo/grading') # noqa: PT009

@override_settings(INSTRUCTOR_MICROFRONTEND_URL='http://localhost:2003/')
@override_settings(INSTRUCTOR_MICROFRONTEND_URL='http://localhost:2003/instructor-dashboard/')
def test_strips_trailing_slash_from_base(self):
"""A trailing slash on the base URL does not produce a double slash."""
result = self._build('INSTRUCTOR_MICROFRONTEND_URL', 'instructor', 'course-v1:edX+DemoX+Demo', 'grading')
self.assertEqual(result, 'http://localhost:2003/instructor/course-v1:edX+DemoX+Demo/grading') # noqa: PT009
result = self._build('INSTRUCTOR_MICROFRONTEND_URL', 'course-v1:edX+DemoX+Demo', 'grading')
self.assertEqual(result, '/instructor-dashboard/course-v1:edX+DemoX+Demo/grading') # noqa: PT009

@override_settings(INSTRUCTOR_MICROFRONTEND_URL='http://localhost:2003')
@override_settings(INSTRUCTOR_MICROFRONTEND_URL='http://localhost:2003/instructor-dashboard')
def test_strips_slashes_from_path_parts(self):
"""Leading and trailing slashes on path parts are stripped before joining."""
result = self._build('INSTRUCTOR_MICROFRONTEND_URL', '/instructor/', '/course-v1:edX+DemoX+Demo/', '/grading/')
self.assertEqual(result, 'http://localhost:2003/instructor/course-v1:edX+DemoX+Demo/grading') # noqa: PT009
result = self._build('INSTRUCTOR_MICROFRONTEND_URL', '/course-v1:edX+DemoX+Demo/', '/grading/')
self.assertEqual(result, '/instructor-dashboard/course-v1:edX+DemoX+Demo/grading') # noqa: PT009

@override_settings(COMMUNICATIONS_MICROFRONTEND_URL=None)
def test_logs_warning_and_returns_relative_url_when_setting_is_none(self):
Expand All @@ -633,15 +633,17 @@ def test_logs_warning_and_returns_relative_url_when_setting_is_none(self):
def test_logs_warning_when_setting_does_not_exist(self):
"""When the setting name is not defined at all, behavior matches the None case."""
with self.assertLogs('lms.djangoapps.instructor.views.serializers_v2', level='WARNING') as cm:
result = self._build('NONEXISTENT_MFE_URL', 'instructor', 'course-v1:edX+DemoX+Demo', 'grading')
result = self._build('NONEXISTENT_MFE_URL', 'course-v1:edX+DemoX+Demo', 'grading')

self.assertTrue(any('NONEXISTENT_MFE_URL is not configured' in msg for msg in cm.output)) # noqa: PT009
self.assertEqual(result, '/instructor/course-v1:edX+DemoX+Demo/grading') # noqa: PT009
self.assertEqual(result, '/course-v1:edX+DemoX+Demo/grading') # noqa: PT009

@override_settings(COMMUNICATIONS_MICROFRONTEND_URL='http://localhost:1984/communications/')
def test_base_with_subpath_and_trailing_slash(self):
"""Base URL with a subpath and trailing slash is joined cleanly."""
result = self._build('COMMUNICATIONS_MICROFRONTEND_URL', 'courses', 'course-v1:edX+DemoX+Demo', 'bulk_email')
result = self._build(
"COMMUNICATIONS_MICROFRONTEND_URL", "courses", "course-v1:edX+DemoX+Demo", "bulk_email", strip_url=False
)
self.assertEqual(result, 'http://localhost:1984/communications/courses/course-v1:edX+DemoX+Demo/bulk_email') # noqa: PT009 # pylint: disable=line-too-long


Expand Down
63 changes: 33 additions & 30 deletions lms/djangoapps/instructor/views/serializers_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"""

import logging
from urllib.parse import urlparse

from django.conf import settings
from django.utils.html import escape
Expand Down Expand Up @@ -83,30 +84,43 @@ class CourseInformationSerializerV2(serializers.Serializer):
)

@staticmethod
def _build_tab_url(setting_name, *path_parts):
def _build_tab_url(setting_name, *path_parts, strip_url=True):
"""
Build a tab URL from a Django setting and path parts.

Retrieves the base URL from `setting_name`, strips any trailing slash,
Retrieves the base URL from `setting_name`, optionally strips the protocol and host,
then joins the provided path parts (stripping their leading/trailing
slashes) with `/` separators — behaving like ``os.path.join`` for URLs.

Logs a warning and falls back to a relative URL if the setting is unset.

Args:
setting_name: Django setting name containing the base URL
*path_parts: Path components to append to the base URL
strip_url: If True, strips protocol/host and uses only the path component.
If False, uses the full URL. Defaults to True.

Example:

_build_tab_url('INSTRUCTOR_MICROFRONTEND_URL', 'instructor', course_key, 'grading')
# => 'http://localhost:2003/instructor/course-v1:.../grading'
_build_tab_url('INSTRUCTOR_MICROFRONTEND_URL', course_key, 'grading')
# => '/instructor-dashboard/course-v1:.../grading' (with strip_url=True)

_build_tab_url('COMMUNICATIONS_MICROFRONTEND_URL', 'courses', course_key, 'bulk_email')
_build_tab_url('COMMUNICATIONS_MICROFRONTEND_URL', 'courses', course_key, 'bulk_email', strip_url=False)
# => 'http://localhost:1984/communications/courses/course-v1:.../bulk_email'
"""
base_url = getattr(settings, setting_name, None)
if base_url is None:
log.warning('%s is not configured.', setting_name)
base_url = ''
parts = [base_url.rstrip('/')] + [str(part).strip('/') for part in path_parts]
return '/'.join(parts)
log.warning("%s is not configured.", setting_name)
base_part = ""
elif strip_url and base_url:
# Extract only the path component from the URL
base_part = urlparse(base_url).path
else:
# Use the full URL as-is
base_part = base_url

parts = [base_part.rstrip("/")] + [str(part).strip("/") for part in path_parts]
return "/".join(parts)

def get_tabs(self, data):
"""Get serialized course tabs."""
Expand Down Expand Up @@ -139,7 +153,6 @@ def get_tabs(self, data):
'title': _('Course Info'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'course_info'
),
Expand All @@ -150,7 +163,6 @@ def get_tabs(self, data):
'title': _('Enrollments'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'enrollments'
),
Expand All @@ -161,7 +173,6 @@ def get_tabs(self, data):
'title': _('Course Team'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'course_team'
),
Expand All @@ -172,7 +183,6 @@ def get_tabs(self, data):
'title': _('Grading'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'grading'
),
Expand All @@ -183,7 +193,6 @@ def get_tabs(self, data):
'title': _('Cohorts'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'cohorts'
),
Expand All @@ -192,25 +201,23 @@ def get_tabs(self, data):
])

if access['staff'] and is_bulk_email_feature_enabled(course_key):
tabs.append({
'tab_id': 'bulk_email',
'title': _('Bulk Email'),
'url': self._build_tab_url(
'COMMUNICATIONS_MICROFRONTEND_URL',
'courses',
course_key,
'bulk_email'
),
'sort_order': 100,
})
tabs.append(
{
"tab_id": "bulk_email",
"title": _("Bulk Email"),
"url": self._build_tab_url(
"COMMUNICATIONS_MICROFRONTEND_URL", "courses", course_key, "bulk_email", strip_url=False
),
"sort_order": 100,
}
)

if access['instructor'] and is_enabled_for_course(course_key):
tabs.append({
'tab_id': 'date_extensions',
'title': _('Date Extensions'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'date_extensions'
),
Expand All @@ -223,7 +230,6 @@ def get_tabs(self, data):
'title': _('Data Downloads'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'data_downloads'
),
Expand All @@ -243,7 +249,6 @@ def get_tabs(self, data):
'title': _('Open Responses'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'open_responses'
),
Expand All @@ -260,7 +265,6 @@ def get_tabs(self, data):
'title': _('Certificates'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'certificates'
),
Expand All @@ -282,7 +286,6 @@ def get_tabs(self, data):
'title': _('Special Exams'),
'url': self._build_tab_url(
'INSTRUCTOR_MICROFRONTEND_URL',
'instructor',
course_key,
'special_exams'
),
Expand Down
2 changes: 1 addition & 1 deletion lms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing
AUTHN_MICROFRONTEND_URL = 'http://localhost:1999'
AUTHN_MICROFRONTEND_DOMAIN = 'localhost:1999'
EXAMS_DASHBOARD_MICROFRONTEND_URL = 'http://localhost:2020'
INSTRUCTOR_MICROFRONTEND_URL = 'http://localhost:2003'
INSTRUCTOR_MICROFRONTEND_URL = 'http://localhost:2003/instructor-dashboard'
CATALOG_MICROFRONTEND_URL = 'http://localhost:1998/catalog'

################### FRONTEND APPLICATION DISCUSSIONS ###################
Expand Down
Loading