Skip to content

Commit 5935899

Browse files
feat: update logic for instructor tabs to match router routing (#38380)
1 parent c43c720 commit 5935899

3 files changed

Lines changed: 52 additions & 47 deletions

File tree

lms/djangoapps/instructor/tests/test_api_v2.py

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,8 @@ def test_tabs_log_warning_when_mfe_url_not_set(self):
564564
for tab in tabs:
565565
self.assertFalse(tab['url'].startswith('None'), f"Tab URL should not start with 'None': {tab['url']}") # noqa: PT009 # pylint: disable=line-too-long
566566
self.assertTrue( # noqa: PT009
567-
tab['url'].startswith('/instructor/'),
568-
f"Tab URL should start with '/instructor/': {tab['url']}"
567+
tab['url'].startswith(f'/{self.course.id}/'),
568+
f"Tab URL should start with '/{self.course.id}/': {tab['url']}"
569569
)
570570

571571
def test_pacing_self_for_self_paced_course(self):
@@ -597,26 +597,26 @@ class BuildTabUrlTest(SimpleTestCase):
597597
going through the full API stack.
598598
"""
599599

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

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

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

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

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

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

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

647649

lms/djangoapps/instructor/views/serializers_v2.py

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"""
77

88
import logging
9+
from urllib.parse import urlparse
910

1011
from django.conf import settings
1112
from django.utils.html import escape
@@ -83,30 +84,43 @@ class CourseInformationSerializerV2(serializers.Serializer):
8384
)
8485

8586
@staticmethod
86-
def _build_tab_url(setting_name, *path_parts):
87+
def _build_tab_url(setting_name, *path_parts, strip_url=True):
8788
"""
8889
Build a tab URL from a Django setting and path parts.
8990
90-
Retrieves the base URL from `setting_name`, strips any trailing slash,
91+
Retrieves the base URL from `setting_name`, optionally strips the protocol and host,
9192
then joins the provided path parts (stripping their leading/trailing
9293
slashes) with `/` separators — behaving like ``os.path.join`` for URLs.
9394
9495
Logs a warning and falls back to a relative URL if the setting is unset.
9596
97+
Args:
98+
setting_name: Django setting name containing the base URL
99+
*path_parts: Path components to append to the base URL
100+
strip_url: If True, strips protocol/host and uses only the path component.
101+
If False, uses the full URL. Defaults to True.
102+
96103
Example:
97104
98-
_build_tab_url('INSTRUCTOR_MICROFRONTEND_URL', 'instructor', course_key, 'grading')
99-
# => 'http://localhost:2003/instructor/course-v1:.../grading'
105+
_build_tab_url('INSTRUCTOR_MICROFRONTEND_URL', course_key, 'grading')
106+
# => '/instructor-dashboard/course-v1:.../grading' (with strip_url=True)
100107
101-
_build_tab_url('COMMUNICATIONS_MICROFRONTEND_URL', 'courses', course_key, 'bulk_email')
108+
_build_tab_url('COMMUNICATIONS_MICROFRONTEND_URL', 'courses', course_key, 'bulk_email', strip_url=False)
102109
# => 'http://localhost:1984/communications/courses/course-v1:.../bulk_email'
103110
"""
104111
base_url = getattr(settings, setting_name, None)
105112
if base_url is None:
106-
log.warning('%s is not configured.', setting_name)
107-
base_url = ''
108-
parts = [base_url.rstrip('/')] + [str(part).strip('/') for part in path_parts]
109-
return '/'.join(parts)
113+
log.warning("%s is not configured.", setting_name)
114+
base_part = ""
115+
elif strip_url and base_url:
116+
# Extract only the path component from the URL
117+
base_part = urlparse(base_url).path
118+
else:
119+
# Use the full URL as-is
120+
base_part = base_url
121+
122+
parts = [base_part.rstrip("/")] + [str(part).strip("/") for part in path_parts]
123+
return "/".join(parts)
110124

111125
def get_tabs(self, data):
112126
"""Get serialized course tabs."""
@@ -139,7 +153,6 @@ def get_tabs(self, data):
139153
'title': _('Course Info'),
140154
'url': self._build_tab_url(
141155
'INSTRUCTOR_MICROFRONTEND_URL',
142-
'instructor',
143156
course_key,
144157
'course_info'
145158
),
@@ -150,7 +163,6 @@ def get_tabs(self, data):
150163
'title': _('Enrollments'),
151164
'url': self._build_tab_url(
152165
'INSTRUCTOR_MICROFRONTEND_URL',
153-
'instructor',
154166
course_key,
155167
'enrollments'
156168
),
@@ -161,7 +173,6 @@ def get_tabs(self, data):
161173
'title': _('Course Team'),
162174
'url': self._build_tab_url(
163175
'INSTRUCTOR_MICROFRONTEND_URL',
164-
'instructor',
165176
course_key,
166177
'course_team'
167178
),
@@ -172,7 +183,6 @@ def get_tabs(self, data):
172183
'title': _('Grading'),
173184
'url': self._build_tab_url(
174185
'INSTRUCTOR_MICROFRONTEND_URL',
175-
'instructor',
176186
course_key,
177187
'grading'
178188
),
@@ -183,7 +193,6 @@ def get_tabs(self, data):
183193
'title': _('Cohorts'),
184194
'url': self._build_tab_url(
185195
'INSTRUCTOR_MICROFRONTEND_URL',
186-
'instructor',
187196
course_key,
188197
'cohorts'
189198
),
@@ -192,25 +201,23 @@ def get_tabs(self, data):
192201
])
193202

194203
if access['staff'] and is_bulk_email_feature_enabled(course_key):
195-
tabs.append({
196-
'tab_id': 'bulk_email',
197-
'title': _('Bulk Email'),
198-
'url': self._build_tab_url(
199-
'COMMUNICATIONS_MICROFRONTEND_URL',
200-
'courses',
201-
course_key,
202-
'bulk_email'
203-
),
204-
'sort_order': 100,
205-
})
204+
tabs.append(
205+
{
206+
"tab_id": "bulk_email",
207+
"title": _("Bulk Email"),
208+
"url": self._build_tab_url(
209+
"COMMUNICATIONS_MICROFRONTEND_URL", "courses", course_key, "bulk_email", strip_url=False
210+
),
211+
"sort_order": 100,
212+
}
213+
)
206214

207215
if access['instructor'] and is_enabled_for_course(course_key):
208216
tabs.append({
209217
'tab_id': 'date_extensions',
210218
'title': _('Date Extensions'),
211219
'url': self._build_tab_url(
212220
'INSTRUCTOR_MICROFRONTEND_URL',
213-
'instructor',
214221
course_key,
215222
'date_extensions'
216223
),
@@ -223,7 +230,6 @@ def get_tabs(self, data):
223230
'title': _('Data Downloads'),
224231
'url': self._build_tab_url(
225232
'INSTRUCTOR_MICROFRONTEND_URL',
226-
'instructor',
227233
course_key,
228234
'data_downloads'
229235
),
@@ -243,7 +249,6 @@ def get_tabs(self, data):
243249
'title': _('Open Responses'),
244250
'url': self._build_tab_url(
245251
'INSTRUCTOR_MICROFRONTEND_URL',
246-
'instructor',
247252
course_key,
248253
'open_responses'
249254
),
@@ -260,7 +265,6 @@ def get_tabs(self, data):
260265
'title': _('Certificates'),
261266
'url': self._build_tab_url(
262267
'INSTRUCTOR_MICROFRONTEND_URL',
263-
'instructor',
264268
course_key,
265269
'certificates'
266270
),
@@ -282,7 +286,6 @@ def get_tabs(self, data):
282286
'title': _('Special Exams'),
283287
'url': self._build_tab_url(
284288
'INSTRUCTOR_MICROFRONTEND_URL',
285-
'instructor',
286289
course_key,
287290
'special_exams'
288291
),

lms/envs/devstack.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing
398398
AUTHN_MICROFRONTEND_URL = 'http://localhost:1999'
399399
AUTHN_MICROFRONTEND_DOMAIN = 'localhost:1999'
400400
EXAMS_DASHBOARD_MICROFRONTEND_URL = 'http://localhost:2020'
401-
INSTRUCTOR_MICROFRONTEND_URL = 'http://localhost:2003'
401+
INSTRUCTOR_MICROFRONTEND_URL = 'http://localhost:2003/instructor-dashboard'
402402
CATALOG_MICROFRONTEND_URL = 'http://localhost:1998/catalog'
403403

404404
################### FRONTEND APPLICATION DISCUSSIONS ###################

0 commit comments

Comments
 (0)