Skip to content

Commit 02fafbf

Browse files
authored
Merge branch 'master' into pwnage101/ENT-11569
2 parents aabafbc + d94afc5 commit 02fafbf

14 files changed

Lines changed: 271 additions & 274 deletions

File tree

cms/envs/common.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,6 @@
256256

257257
MARKETING_EMAILS_OPT_IN = False
258258

259-
############################# MICROFRONTENDS ###################################
260-
COURSE_AUTHORING_MICROFRONTEND_URL = None
261-
262259
############################# SET PATH INFORMATION #############################
263260
PROJECT_ROOT = path(__file__).abspath().dirname().dirname() # /edx-platform/cms
264261
CMS_ROOT = REPO_ROOT / "cms" # noqa: F405

lms/djangoapps/instructor/tests/test_api_v2.py

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ def setUp(self):
8181
self.admin = AdminFactory.create()
8282
self.instructor = InstructorFactory.create(course_key=self.course_key)
8383
self.staff = StaffFactory.create(course_key=self.course_key)
84+
self.django_staff_user = UserFactory.create(is_staff=True)
8485
self.data_researcher = UserFactory.create()
8586
CourseDataResearcherRole(self.course_key).add_users(self.data_researcher)
8687
CourseInstructorRole(self.proctored_course.id).add_users(self.instructor)
@@ -118,60 +119,93 @@ def _get_url(self, course_id=None):
118119
course_id = str(self.course_key)
119120
return reverse('instructor_api_v2:course_metadata', kwargs={'course_id': course_id})
120121

122+
@override_settings(COURSE_AUTHORING_MICROFRONTEND_URL='http://localhost:2001/authoring')
123+
@override_settings(ADMIN_CONSOLE_MICROFRONTEND_URL='http://localhost:2025/admin-console')
121124
def test_get_course_metadata_as_instructor(self):
122125
"""
123126
Test that an instructor can retrieve comprehensive course metadata.
124127
"""
125128
self.client.force_authenticate(user=self.instructor)
126129
response = self.client.get(self._get_url())
127130

128-
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
131+
assert response.status_code == status.HTTP_200_OK
129132
data = response.data
130133

131134
# Verify basic course information
132-
self.assertEqual(data['course_id'], str(self.course_key)) # noqa: PT009
133-
self.assertEqual(data['display_name'], 'Demonstration Course') # noqa: PT009
134-
self.assertEqual(data['org'], 'edX') # noqa: PT009
135-
self.assertEqual(data['course_number'], 'DemoX') # noqa: PT009
136-
self.assertEqual(data['course_run'], 'Demo_Course') # noqa: PT009
137-
self.assertEqual(data['pacing'], 'instructor') # noqa: PT009
135+
assert data['course_id'] == str(self.course_key)
136+
assert data['display_name'] == 'Demonstration Course'
137+
assert data['org'] == 'edX'
138+
assert data['course_number'] == 'DemoX'
139+
assert data['course_run'] == 'Demo_Course'
140+
assert data['pacing'] == 'instructor'
138141

139142
# Verify enrollment counts structure
140-
self.assertIn('enrollment_counts', data) # noqa: PT009
141-
self.assertIn('total', data['enrollment_counts']) # noqa: PT009
142-
self.assertIn('total_enrollment', data) # noqa: PT009
143-
self.assertGreaterEqual(data['total_enrollment'], 3) # noqa: PT009
143+
assert 'enrollment_counts' in data
144+
assert 'total' in data['enrollment_counts']
145+
assert 'total_enrollment' in data
146+
assert data['total_enrollment'] >= 3
144147

145148
# Verify role-based enrollment counts are present
146-
self.assertIn('learner_count', data) # noqa: PT009
147-
self.assertIn('staff_count', data) # noqa: PT009
148-
self.assertEqual(data['total_enrollment'], data['learner_count'] + data['staff_count']) # noqa: PT009
149+
assert 'learner_count' in data
150+
assert 'staff_count' in data
151+
assert data['total_enrollment'] == data['learner_count'] + data['staff_count']
149152

150153
# Verify permissions structure
151-
self.assertIn('permissions', data) # noqa: PT009
154+
assert 'permissions' in data
152155
permissions_data = data['permissions']
153-
self.assertIn('admin', permissions_data) # noqa: PT009
154-
self.assertIn('instructor', permissions_data) # noqa: PT009
155-
self.assertIn('staff', permissions_data) # noqa: PT009
156-
self.assertIn('forum_admin', permissions_data) # noqa: PT009
157-
self.assertIn('finance_admin', permissions_data) # noqa: PT009
158-
self.assertIn('sales_admin', permissions_data) # noqa: PT009
159-
self.assertIn('data_researcher', permissions_data) # noqa: PT009
156+
assert 'admin' in permissions_data
157+
assert 'instructor' in permissions_data
158+
assert 'staff' in permissions_data
159+
assert 'forum_admin' in permissions_data
160+
assert 'finance_admin' in permissions_data
161+
assert 'sales_admin' in permissions_data
162+
assert 'data_researcher' in permissions_data
160163

161164
# Verify sections structure
162-
self.assertIn('tabs', data) # noqa: PT009
163-
self.assertIsInstance(data['tabs'], list) # noqa: PT009
165+
assert 'tabs' in data
166+
assert isinstance(data['tabs'], list)
164167

165168
# Verify other metadata fields
166-
self.assertIn('num_sections', data) # noqa: PT009
167-
self.assertIn('tabs', data) # noqa: PT009
168-
self.assertIn('grade_cutoffs', data) # noqa: PT009
169-
self.assertIn('course_errors', data) # noqa: PT009
170-
self.assertIn('studio_url', data) # noqa: PT009
171-
self.assertIn('disable_buttons', data) # noqa: PT009
172-
self.assertIn('has_started', data) # noqa: PT009
173-
self.assertIn('has_ended', data) # noqa: PT009
174-
self.assertIn('analytics_dashboard_message', data) # noqa: PT009
169+
assert 'num_sections' in data
170+
assert 'grade_cutoffs' in data
171+
assert 'course_errors' in data
172+
assert 'studio_url' in data
173+
assert 'disable_buttons' in data
174+
assert 'has_started' in data
175+
assert 'has_ended' in data
176+
assert 'analytics_dashboard_message' in data
177+
assert 'studio_grading_url' in data
178+
assert 'admin_console_url' in data
179+
180+
assert data['studio_grading_url'] == f'http://localhost:2001/authoring/course/{self.course.id}/settings/grading'
181+
assert data['admin_console_url'] == 'http://localhost:2025/admin-console/authz'
182+
183+
@override_settings(ADMIN_CONSOLE_MICROFRONTEND_URL='http://localhost:2025/admin-console')
184+
def test_admin_console_url_requires_instructor_access(self):
185+
"""
186+
Test that the admin console URL is only available to users with instructor access.
187+
"""
188+
# data researcher has access to course but is not an instructor
189+
self.client.force_authenticate(user=self.data_researcher)
190+
response = self.client.get(self._get_url())
191+
192+
assert response.status_code == status.HTTP_200_OK
193+
assert 'admin_console_url' in response.data
194+
data = response.data
195+
assert data['admin_console_url'] is None
196+
197+
@override_settings(ADMIN_CONSOLE_MICROFRONTEND_URL='http://localhost:2025/admin-console')
198+
def test_django_staff_user_without_instructor_access_can_see_admin_console_url(self):
199+
"""
200+
Test that Django staff users without instructor access can see the admin console URL.
201+
"""
202+
self.client.force_authenticate(user=self.django_staff_user)
203+
response = self.client.get(self._get_url())
204+
205+
assert response.status_code == status.HTTP_200_OK
206+
assert 'admin_console_url' in response.data
207+
data = response.data
208+
assert data['admin_console_url'] == 'http://localhost:2025/admin-console/authz'
175209

176210
def test_get_course_metadata_as_staff(self):
177211
"""

lms/djangoapps/instructor/views/api_v2.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,8 @@ def get(self, request, course_id):
234234
"grade_cutoffs": "A is 0.9, B is 0.8, C is 0.7, D is 0.6",
235235
"course_errors": [],
236236
"studio_url": "https://studio.example.com/course/course-v1:edX+DemoX+2024",
237+
# May be null if user does not have access:
238+
"admin_console_url": "http://apps.local.openedx.io:2025/admin-console/authz",
237239
"permissions": {
238240
"admin": false,
239241
"instructor": true,

lms/djangoapps/instructor/views/serializers_v2.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from lms.djangoapps.instructor.access import FORUM_ROLES, ROLES
3535
from lms.djangoapps.instructor.views.instructor_dashboard import get_analytics_dashboard_message
3636
from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
37+
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
3738
from xmodule.modulestore.django import modulestore
3839

3940
from .tools import DashboardError, get_student_from_identifier, parse_datetime
@@ -77,6 +78,9 @@ class CourseInformationSerializerV2(serializers.Serializer):
7778
studio_grading_url = serializers.SerializerMethodField(
7879
help_text="URL to the Studio grading settings page for the course (null if not configured)"
7980
)
81+
admin_console_url = serializers.SerializerMethodField(
82+
help_text="URL to the admin console (requires instructor access and MFE configuration, null if not accessible)"
83+
)
8084
permissions = serializers.SerializerMethodField(help_text="User permissions for instructor dashboard features")
8185
tabs = serializers.SerializerMethodField(help_text="List of course tabs with configuration and display information")
8286
disable_buttons = serializers.SerializerMethodField(
@@ -462,10 +466,27 @@ def get_gradebook_url(self, data):
462466
def get_studio_grading_url(self, data):
463467
"""Get Studio MFE grading settings URL for the course."""
464468
course_key = data['course'].id
465-
mfe_base_url = getattr(settings, 'COURSE_AUTHORING_MICROFRONTEND_URL', None)
466-
if mfe_base_url:
467-
return f'{mfe_base_url}/course/{course_key}/settings/grading'
468-
return None
469+
mfe_base_url = configuration_helpers.get_value(
470+
'COURSE_AUTHORING_MICROFRONTEND_URL',
471+
getattr(settings, 'COURSE_AUTHORING_MICROFRONTEND_URL', None)
472+
)
473+
if not mfe_base_url:
474+
return None
475+
return f'{mfe_base_url}/course/{course_key}/settings/grading'
476+
477+
def get_admin_console_url(self, data):
478+
"""Get admin console URL (requires instructor access and MFE configuration, null if not accessible)."""
479+
request = data['request']
480+
has_instructor_access = has_access(request.user, 'instructor', data['course'])
481+
mfe_base_url = configuration_helpers.get_value(
482+
'ADMIN_CONSOLE_MICROFRONTEND_URL',
483+
getattr(settings, 'ADMIN_CONSOLE_MICROFRONTEND_URL', None)
484+
)
485+
486+
has_permissions = request.user.is_staff or has_instructor_access
487+
if not mfe_base_url or not has_permissions:
488+
return None
489+
return f'{mfe_base_url}/authz'
469490

470491
def get_disable_buttons(self, data):
471492
"""Check if buttons should be disabled for large courses."""

lms/djangoapps/mfe_config_api/tests/test_views.py

Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,45 @@ def side_effect(key, default=None):
297297
# Value in original MFE_CONFIG not overridden by catalog config should be preserved
298298
self.assertEqual(data["PRESERVED_SETTING"], "preserved") # noqa: PT009
299299

300+
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
301+
def test_legacy_overrides_instructor_dashboard(self, configuration_helpers_mock):
302+
"""Legacy help-tokens SUPPORT_URL is included for instructor-dashboard when no explicit override is set."""
303+
def side_effect(key, default=None):
304+
if key == "MFE_CONFIG":
305+
return {"LMS_BASE_URL": "https://courses.example.com"}
306+
if key == "MFE_CONFIG_OVERRIDES":
307+
return {}
308+
return default
309+
configuration_helpers_mock.get_value.side_effect = side_effect
310+
311+
response = self.client.get(f"{self.mfe_config_api_url}?mfe=instructor-dashboard")
312+
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
313+
data = response.json()
314+
self.assertEqual( # noqa: PT009
315+
data["SUPPORT_URL"],
316+
"https://docs.openedx.org/en/latest/educators/index.html",
317+
)
318+
319+
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
320+
def test_explicit_override_wins_over_legacy_overrides(self, configuration_helpers_mock):
321+
"""An explicit SUPPORT_URL in MFE_CONFIG_OVERRIDES wins over the help-tokens fallback."""
322+
def side_effect(key, default=None):
323+
if key == "MFE_CONFIG":
324+
return {"LMS_BASE_URL": "https://courses.example.com"}
325+
if key == "MFE_CONFIG_OVERRIDES":
326+
return {
327+
"instructor-dashboard": {
328+
"SUPPORT_URL": "https://help.example.com/instructor",
329+
},
330+
}
331+
return default
332+
configuration_helpers_mock.get_value.side_effect = side_effect
333+
334+
response = self.client.get(f"{self.mfe_config_api_url}?mfe=instructor-dashboard")
335+
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
336+
data = response.json()
337+
self.assertEqual(data["SUPPORT_URL"], "https://help.example.com/instructor") # noqa: PT009
338+
300339

301340
class MfeNameToAppIdTests(SimpleTestCase):
302341
"""Tests for the mfe_name_to_app_id helper."""
@@ -317,6 +356,12 @@ def test_mapped_alias(self):
317356
"org.openedx.frontend.app.authoring",
318357
)
319358

359+
def test_instructor_dashboard(self):
360+
self.assertEqual( # noqa: PT009
361+
mfe_name_to_app_id("instructor-dashboard"),
362+
"org.openedx.frontend.app.instructorDashboard",
363+
)
364+
320365
def test_fallback_for_unknown_name(self):
321366
"""Unknown names fall back to programmatic kebab-to-camelCase conversion."""
322367
self.assertEqual( # noqa: PT009
@@ -420,8 +465,9 @@ def side_effect(key, default=None):
420465
for legacy_key in default_legacy_config:
421466
self.assertIn(legacy_key, common) # noqa: PT009
422467

468+
@patch("lms.djangoapps.mfe_config_api.views.get_legacy_config_overrides", return_value={})
423469
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
424-
def test_apps_from_overrides(self, configuration_helpers_mock):
470+
def test_apps_from_overrides(self, configuration_helpers_mock, _legacy_overrides_mock): # noqa: PT019
425471
"""Each MFE_CONFIG_OVERRIDES entry becomes an app with shared base config + overrides."""
426472
mfe_config_overrides = {
427473
"authn": {
@@ -521,9 +567,10 @@ def side_effect(key, default=None):
521567
self.assertNotIn("BASE_URL", data["commonAppConfig"]) # noqa: PT009
522568
self.assertNotIn("LOGIN_URL", data["commonAppConfig"]) # noqa: PT009
523569

570+
@patch("lms.djangoapps.mfe_config_api.views.get_legacy_config_overrides", return_value={})
524571
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
525-
def test_no_apps_when_no_overrides(self, configuration_helpers_mock):
526-
"""The apps key is omitted when MFE_CONFIG_OVERRIDES is empty."""
572+
def test_no_apps_when_no_overrides(self, configuration_helpers_mock, _legacy_overrides_mock): # noqa: PT019
573+
"""The apps key is omitted when MFE_CONFIG_OVERRIDES is empty and no legacy overrides are present."""
527574
def side_effect(key, default=None):
528575
if key == "MFE_CONFIG":
529576
return {"LMS_BASE_URL": "https://courses.example.com"}
@@ -566,8 +613,9 @@ def side_effect(key, default=None):
566613
self.assertEqual(common["CREDENTIALS_BASE_URL"], "https://credentials.example.com") # noqa: PT009
567614
self.assertEqual(common["STUDIO_BASE_URL"], "https://studio.example.com") # noqa: PT009
568615

616+
@patch("lms.djangoapps.mfe_config_api.views.get_legacy_config_overrides", return_value={})
569617
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
570-
def test_invalid_override_entry_skipped(self, configuration_helpers_mock):
618+
def test_invalid_override_entry_skipped(self, configuration_helpers_mock, _legacy_overrides_mock): # noqa: PT019
571619
"""Non-dict override entries are silently skipped."""
572620
mfe_config_overrides = {
573621
"authn": {"SOME_KEY": "value"},
@@ -720,3 +768,49 @@ def side_effect(key, default=None):
720768
# Brand new app from FRONTEND_SITE_CONFIG is appended
721769
brand_new = apps_by_id["org.openedx.frontend.app.brand.new"]["config"]
722770
self.assertEqual(brand_new["BRAND_NEW_KEY"], "value") # noqa: PT009
771+
772+
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
773+
def test_legacy_overrides_instructor_dashboard_support_url(self, configuration_helpers_mock):
774+
"""Instructor dashboard gets SUPPORT_URL from help-tokens when no explicit override is set."""
775+
def side_effect(key, default=None):
776+
if key == "MFE_CONFIG":
777+
return {"LMS_BASE_URL": "https://courses.example.com"}
778+
if key == "MFE_CONFIG_OVERRIDES":
779+
return {}
780+
return default
781+
configuration_helpers_mock.get_value.side_effect = side_effect
782+
783+
response = self.client.get(self.url)
784+
data = response.json()
785+
786+
apps_by_id = {app["appId"]: app for app in data["apps"]}
787+
instructor = apps_by_id["org.openedx.frontend.app.instructorDashboard"]
788+
self.assertEqual( # noqa: PT009
789+
instructor["config"]["SUPPORT_URL"],
790+
"https://docs.openedx.org/en/latest/educators/index.html",
791+
)
792+
793+
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
794+
def test_explicit_override_wins_over_legacy_overrides(self, configuration_helpers_mock):
795+
"""An explicit SUPPORT_URL in MFE_CONFIG_OVERRIDES wins over the help-tokens fallback."""
796+
def side_effect(key, default=None):
797+
if key == "MFE_CONFIG":
798+
return {"LMS_BASE_URL": "https://courses.example.com"}
799+
if key == "MFE_CONFIG_OVERRIDES":
800+
return {
801+
"instructor-dashboard": {
802+
"SUPPORT_URL": "https://help.example.com/instructor",
803+
},
804+
}
805+
return default
806+
configuration_helpers_mock.get_value.side_effect = side_effect
807+
808+
response = self.client.get(self.url)
809+
data = response.json()
810+
811+
apps_by_id = {app["appId"]: app for app in data["apps"]}
812+
instructor = apps_by_id["org.openedx.frontend.app.instructorDashboard"]
813+
self.assertEqual( # noqa: PT009
814+
instructor["config"]["SUPPORT_URL"],
815+
"https://help.example.com/instructor",
816+
)

0 commit comments

Comments
 (0)