From 40c1faa07b21aeb60940dfc36aa22d8c44cb6bdb Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 24 Apr 2026 11:19:35 -0400 Subject: [PATCH] feat: add instructor dashboard SUPPORT_URL legacy fallback to MFE_CONFIG Adds a legacy config override layer so the instructor dashboard gets its SUPPORT_URL from the help-tokens package when no explicit MFE_CONFIG_OVERRIDES entry is set. This supports the frontend-base help button (openedx/frontend-base#245). Also adds instructor-dashboard to MFE_NAME_TO_APP_ID and refactors get_mfe_config_overrides into separate legacy/explicit/merged helpers. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../mfe_config_api/tests/test_views.py | 102 +++++++++++++++++- lms/djangoapps/mfe_config_api/views.py | 62 +++++++++-- 2 files changed, 154 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/mfe_config_api/tests/test_views.py b/lms/djangoapps/mfe_config_api/tests/test_views.py index ddfc67498f01..85fdeb2005d7 100644 --- a/lms/djangoapps/mfe_config_api/tests/test_views.py +++ b/lms/djangoapps/mfe_config_api/tests/test_views.py @@ -297,6 +297,45 @@ def side_effect(key, default=None): # Value in original MFE_CONFIG not overridden by catalog config should be preserved self.assertEqual(data["PRESERVED_SETTING"], "preserved") # noqa: PT009 + @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") + def test_legacy_overrides_instructor_dashboard(self, configuration_helpers_mock): + """Legacy help-tokens SUPPORT_URL is included for instructor-dashboard when no explicit override is set.""" + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return {"LMS_BASE_URL": "https://courses.example.com"} + if key == "MFE_CONFIG_OVERRIDES": + return {} + return default + configuration_helpers_mock.get_value.side_effect = side_effect + + response = self.client.get(f"{self.mfe_config_api_url}?mfe=instructor-dashboard") + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + data = response.json() + self.assertEqual( # noqa: PT009 + data["SUPPORT_URL"], + "https://docs.openedx.org/en/latest/educators/index.html", + ) + + @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") + def test_explicit_override_wins_over_legacy_overrides(self, configuration_helpers_mock): + """An explicit SUPPORT_URL in MFE_CONFIG_OVERRIDES wins over the help-tokens fallback.""" + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return {"LMS_BASE_URL": "https://courses.example.com"} + if key == "MFE_CONFIG_OVERRIDES": + return { + "instructor-dashboard": { + "SUPPORT_URL": "https://help.example.com/instructor", + }, + } + return default + configuration_helpers_mock.get_value.side_effect = side_effect + + response = self.client.get(f"{self.mfe_config_api_url}?mfe=instructor-dashboard") + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + data = response.json() + self.assertEqual(data["SUPPORT_URL"], "https://help.example.com/instructor") # noqa: PT009 + class MfeNameToAppIdTests(SimpleTestCase): """Tests for the mfe_name_to_app_id helper.""" @@ -317,6 +356,12 @@ def test_mapped_alias(self): "org.openedx.frontend.app.authoring", ) + def test_instructor_dashboard(self): + self.assertEqual( # noqa: PT009 + mfe_name_to_app_id("instructor-dashboard"), + "org.openedx.frontend.app.instructorDashboard", + ) + def test_fallback_for_unknown_name(self): """Unknown names fall back to programmatic kebab-to-camelCase conversion.""" self.assertEqual( # noqa: PT009 @@ -420,8 +465,9 @@ def side_effect(key, default=None): for legacy_key in default_legacy_config: self.assertIn(legacy_key, common) # noqa: PT009 + @patch("lms.djangoapps.mfe_config_api.views.get_legacy_config_overrides", return_value={}) @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") - def test_apps_from_overrides(self, configuration_helpers_mock): + def test_apps_from_overrides(self, configuration_helpers_mock, _legacy_overrides_mock): # noqa: PT019 """Each MFE_CONFIG_OVERRIDES entry becomes an app with shared base config + overrides.""" mfe_config_overrides = { "authn": { @@ -521,9 +567,10 @@ def side_effect(key, default=None): self.assertNotIn("BASE_URL", data["commonAppConfig"]) # noqa: PT009 self.assertNotIn("LOGIN_URL", data["commonAppConfig"]) # noqa: PT009 + @patch("lms.djangoapps.mfe_config_api.views.get_legacy_config_overrides", return_value={}) @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") - def test_no_apps_when_no_overrides(self, configuration_helpers_mock): - """The apps key is omitted when MFE_CONFIG_OVERRIDES is empty.""" + def test_no_apps_when_no_overrides(self, configuration_helpers_mock, _legacy_overrides_mock): # noqa: PT019 + """The apps key is omitted when MFE_CONFIG_OVERRIDES is empty and no legacy overrides are present.""" def side_effect(key, default=None): if key == "MFE_CONFIG": return {"LMS_BASE_URL": "https://courses.example.com"} @@ -566,8 +613,9 @@ def side_effect(key, default=None): self.assertEqual(common["CREDENTIALS_BASE_URL"], "https://credentials.example.com") # noqa: PT009 self.assertEqual(common["STUDIO_BASE_URL"], "https://studio.example.com") # noqa: PT009 + @patch("lms.djangoapps.mfe_config_api.views.get_legacy_config_overrides", return_value={}) @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") - def test_invalid_override_entry_skipped(self, configuration_helpers_mock): + def test_invalid_override_entry_skipped(self, configuration_helpers_mock, _legacy_overrides_mock): # noqa: PT019 """Non-dict override entries are silently skipped.""" mfe_config_overrides = { "authn": {"SOME_KEY": "value"}, @@ -720,3 +768,49 @@ def side_effect(key, default=None): # Brand new app from FRONTEND_SITE_CONFIG is appended brand_new = apps_by_id["org.openedx.frontend.app.brand.new"]["config"] self.assertEqual(brand_new["BRAND_NEW_KEY"], "value") # noqa: PT009 + + @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") + def test_legacy_overrides_instructor_dashboard_support_url(self, configuration_helpers_mock): + """Instructor dashboard gets SUPPORT_URL from help-tokens when no explicit override is set.""" + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return {"LMS_BASE_URL": "https://courses.example.com"} + if key == "MFE_CONFIG_OVERRIDES": + return {} + return default + configuration_helpers_mock.get_value.side_effect = side_effect + + response = self.client.get(self.url) + data = response.json() + + apps_by_id = {app["appId"]: app for app in data["apps"]} + instructor = apps_by_id["org.openedx.frontend.app.instructorDashboard"] + self.assertEqual( # noqa: PT009 + instructor["config"]["SUPPORT_URL"], + "https://docs.openedx.org/en/latest/educators/index.html", + ) + + @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") + def test_explicit_override_wins_over_legacy_overrides(self, configuration_helpers_mock): + """An explicit SUPPORT_URL in MFE_CONFIG_OVERRIDES wins over the help-tokens fallback.""" + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return {"LMS_BASE_URL": "https://courses.example.com"} + if key == "MFE_CONFIG_OVERRIDES": + return { + "instructor-dashboard": { + "SUPPORT_URL": "https://help.example.com/instructor", + }, + } + return default + configuration_helpers_mock.get_value.side_effect = side_effect + + response = self.client.get(self.url) + data = response.json() + + apps_by_id = {app["appId"]: app for app in data["apps"]} + instructor = apps_by_id["org.openedx.frontend.app.instructorDashboard"] + self.assertEqual( # noqa: PT009 + instructor["config"]["SUPPORT_URL"], + "https://help.example.com/instructor", + ) diff --git a/lms/djangoapps/mfe_config_api/views.py b/lms/djangoapps/mfe_config_api/views.py index b49dd9515cc5..5af0f242adfe 100644 --- a/lms/djangoapps/mfe_config_api/views.py +++ b/lms/djangoapps/mfe_config_api/views.py @@ -2,11 +2,14 @@ MFE API Views for useful information related to mfes. """ +from configparser import Error as ConfigParserError + import edx_api_doc_tools as apidocs from django.conf import settings from django.http import HttpResponseNotFound, JsonResponse from django.utils.decorators import method_decorator from django.views.decorators.cache import cache_page +from help_tokens.core import HelpUrlExpert from rest_framework import status from rest_framework.exceptions import NotFound from rest_framework.permissions import AllowAny @@ -48,6 +51,7 @@ "course-authoring": "org.openedx.frontend.app.authoring", "discussions": "org.openedx.frontend.app.discussions", "gradebook": "org.openedx.frontend.app.gradebook", + "instructor-dashboard": "org.openedx.frontend.app.instructorDashboard", "learner-dashboard": "org.openedx.frontend.app.learnerDashboard", "learner-record": "org.openedx.frontend.app.learnerRecord", "learning": "org.openedx.frontend.app.learning", @@ -93,30 +97,76 @@ def get_mfe_config() -> dict: return mfe_config -def get_mfe_config_overrides() -> dict: - """Return all MFE-specific overrides from settings or site configuration. +def resolve_help_token(token: str) -> str | None: + """Resolve a help-tokens token to a URL, returning None if the token cannot be resolved.""" + try: + return HelpUrlExpert.the_one().url_for_token(token) + except (KeyError, ConfigParserError): + return None + + +def get_legacy_config_overrides() -> dict: + """Return per-app legacy configuration overrides. + + Same shape as get_explicit_mfe_config_overrides(): a dict keyed by MFE name, + where each value is a dict of config values. + + This is a compatibility layer for per-app values that historically + came from legacy systems (e.g., help-tokens). + """ + overrides: dict[str, dict] = {} + + instructor_help_url = resolve_help_token("instructor") + if instructor_help_url: + overrides["instructor-dashboard"] = {"SUPPORT_URL": instructor_help_url} + + return overrides + + +def get_explicit_mfe_config_overrides() -> dict: + """Return MFE-specific overrides from settings or site configuration. Returns: A dictionary keyed by MFE name, where each value is a dict of per-MFE overrides. Non-dict entries are filtered out. """ - mfe_config_overrides = ( + raw_overrides = ( configuration_helpers.get_value( "MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES, ) or {} ) - if not isinstance(mfe_config_overrides, dict): + if not isinstance(raw_overrides, dict): return {} return { - name: overrides - for name, overrides in mfe_config_overrides.items() + mfe_name: overrides + for mfe_name, overrides in raw_overrides.items() if isinstance(overrides, dict) } +def get_mfe_config_overrides() -> dict: + """Return all MFE-specific overrides, merging legacy fallbacks with explicit settings. + + Legacy per-app fallbacks (e.g., from help-tokens) are included at the lowest + precedence; explicit MFE_CONFIG_OVERRIDES from settings or site configuration + take priority. + + Returns: + A dictionary keyed by MFE name, where each value is a dict of + per-MFE overrides. + """ + legacy_overrides = get_legacy_config_overrides() + explicit_overrides = get_explicit_mfe_config_overrides() + all_mfe_names = set(legacy_overrides) | set(explicit_overrides) + return { + mfe_name: legacy_overrides.get(mfe_name, {}) | explicit_overrides.get(mfe_name, {}) + for mfe_name in all_mfe_names + } + + def get_frontend_site_config() -> dict: """Return frontend site configuration from settings or site configuration.