diff --git a/lms/djangoapps/mfe_config_api/tests/test_views.py b/lms/djangoapps/mfe_config_api/tests/test_views.py index fef7574f5932..fcf1f1ad29e8 100644 --- a/lms/djangoapps/mfe_config_api/tests/test_views.py +++ b/lms/djangoapps/mfe_config_api/tests/test_views.py @@ -11,12 +11,23 @@ from rest_framework import status from rest_framework.test import APITestCase +# Default legacy configuration values, used in tests to build a correct expected response +default_legacy_config = { + "COURSE_ABOUT_TWITTER_ACCOUNT": "@YourPlatformTwitterAccount", + "NON_BROWSABLE_COURSES": False, + "ENABLE_COURSE_SORTING_BY_START_DATE": True, + "HOMEPAGE_COURSE_MAX": None, + "HOMEPAGE_PROMO_VIDEO_YOUTUBE_ID": None, + "ENABLE_COURSE_DISCOVERY": False, +} + @ddt.ddt class MFEConfigTestCase(APITestCase): """ Test the use case that exposes the site configuration with the mfe api. """ + def setUp(self): self.mfe_config_api_url = reverse("mfe_config_api:config") return super().setUp() @@ -31,12 +42,15 @@ def test_get_mfe_config(self, configuration_helpers_mock): - The status of the response of the request is a HTTP_200_OK. - The json of the response of the request is equal to the mocked configuration. """ - configuration_helpers_mock.get_value.return_value = {"EXAMPLE_VAR": "value"} - response = self.client.get(self.mfe_config_api_url) + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return {"EXAMPLE_VAR": "value"} + return default + configuration_helpers_mock.get_value.side_effect = side_effect - configuration_helpers_mock.get_value.assert_called_once_with("MFE_CONFIG", settings.MFE_CONFIG) + response = self.client.get(self.mfe_config_api_url) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), {"EXAMPLE_VAR": "value"}) + self.assertEqual(response.json(), {**default_legacy_config, "EXAMPLE_VAR": "value"}) @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") def test_get_mfe_config_with_queryparam(self, configuration_helpers_mock): @@ -48,49 +62,54 @@ def test_get_mfe_config_with_queryparam(self, configuration_helpers_mock): and once with the parameters ("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES). - The json of the response is the merge of both mocked configurations. """ - configuration_helpers_mock.get_value.side_effect = [ - {"EXAMPLE_VAR": "value", "OTHER": "other"}, - {"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, - ] + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return {"EXAMPLE_VAR": "value", "OTHER": "other"} + if key == "MFE_CONFIG_OVERRIDES": + return {"mymfe": {"EXAMPLE_VAR": "mymfe_value"}} + return default + configuration_helpers_mock.get_value.side_effect = side_effect response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe") self.assertEqual(response.status_code, status.HTTP_200_OK) calls = [call("MFE_CONFIG", settings.MFE_CONFIG), call("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES)] configuration_helpers_mock.get_value.assert_has_calls(calls) - self.assertEqual(response.json(), {"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"}) + self.assertEqual( + response.json(), {**default_legacy_config, "EXAMPLE_VAR": "mymfe_value", "OTHER": "other"} + ) @ddt.unpack @ddt.data( dict( mfe_config={}, mfe_config_overrides={}, - expected_response={}, + expected_response={**default_legacy_config}, ), dict( mfe_config={"EXAMPLE_VAR": "value"}, mfe_config_overrides={}, - expected_response={"EXAMPLE_VAR": "value"}, + expected_response={**default_legacy_config, "EXAMPLE_VAR": "value"}, ), dict( mfe_config={}, mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, - expected_response={"EXAMPLE_VAR": "mymfe_value"}, + expected_response={**default_legacy_config, "EXAMPLE_VAR": "mymfe_value"}, ), dict( mfe_config={"EXAMPLE_VAR": "value"}, mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, - expected_response={"EXAMPLE_VAR": "mymfe_value"}, + expected_response={**default_legacy_config, "EXAMPLE_VAR": "mymfe_value"}, ), dict( mfe_config={"EXAMPLE_VAR": "value", "OTHER": "other"}, mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}}, - expected_response={"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"}, + expected_response={**default_legacy_config, "EXAMPLE_VAR": "mymfe_value", "OTHER": "other"}, ), dict( mfe_config={"EXAMPLE_VAR": "value"}, mfe_config_overrides={"yourmfe": {"EXAMPLE_VAR": "yourmfe_value"}}, - expected_response={"EXAMPLE_VAR": "value"}, + expected_response={**default_legacy_config, "EXAMPLE_VAR": "value"}, ), dict( mfe_config={"EXAMPLE_VAR": "value"}, @@ -98,7 +117,7 @@ def test_get_mfe_config_with_queryparam(self, configuration_helpers_mock): "yourmfe": {"EXAMPLE_VAR": "yourmfe_value"}, "mymfe": {"EXAMPLE_VAR": "mymfe_value"}, }, - expected_response={"EXAMPLE_VAR": "mymfe_value"}, + expected_response={**default_legacy_config, "EXAMPLE_VAR": "mymfe_value"}, ), ) @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") @@ -119,7 +138,13 @@ def test_get_mfe_config_with_queryparam_multiple_configs( and once with the parameters ("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES). - The json of the response is the expected_response passed by ddt.data. """ - configuration_helpers_mock.get_value.side_effect = [mfe_config, mfe_config_overrides] + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return mfe_config + if key == "MFE_CONFIG_OVERRIDES": + return mfe_config_overrides + return default + configuration_helpers_mock.get_value.side_effect = side_effect response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -136,7 +161,7 @@ def test_get_mfe_config_from_django_settings(self): - The json response is equal to MFE_CONFIG in lms/envs/test.py""" response = self.client.get(self.mfe_config_api_url) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), settings.MFE_CONFIG) + self.assertEqual(response.json(), default_legacy_config | settings.MFE_CONFIG) def test_get_mfe_config_with_queryparam_from_django_settings(self): """Test that when there is no site configuration, the API with queryparam takes the django settings. @@ -147,7 +172,7 @@ def test_get_mfe_config_with_queryparam_from_django_settings(self): """ response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe") self.assertEqual(response.status_code, status.HTTP_200_OK) - expected = {**settings.MFE_CONFIG, **settings.MFE_CONFIG_OVERRIDES["mymfe"]} + expected = default_legacy_config | settings.MFE_CONFIG | settings.MFE_CONFIG_OVERRIDES["mymfe"] self.assertEqual(response.json(), expected) @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") @@ -162,3 +187,108 @@ def test_404_get_mfe_config(self, configuration_helpers_mock): response = self.client.get(self.mfe_config_api_url) configuration_helpers_mock.get_value.assert_not_called() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") + def test_get_mfe_config_for_catalog(self, configuration_helpers_mock): + """Test the mfe config by explicitly using catalog mfe as an example. + + Expected result: + - The configuration_helpers get_value is called for each catalog-specific configuration. + - The catalog-specific values are included in the response. + """ + mfe_config = {"BASE_URL": "https://catalog.example.com", "COURSE_ABOUT_TWITTER_ACCOUNT": "@TestAccount"} + mfe_config_overrides = { + "catalog": { + "SOME_SETTING": "catalog_value", + "NON_BROWSABLE_COURSES": True, + } + } + + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return mfe_config + if key == "MFE_CONFIG_OVERRIDES": + return mfe_config_overrides + if key == "ENABLE_COURSE_SORTING_BY_START_DATE": + return True + if key == "homepage_promo_video_youtube_id": + return None + if key == "HOMEPAGE_COURSE_MAX": + return 8 + return default + + configuration_helpers_mock.get_value.side_effect = side_effect + + response = self.client.get(f"{self.mfe_config_api_url}?mfe=catalog") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json() + self.assertEqual(data["BASE_URL"], "https://catalog.example.com") + self.assertEqual(data["SOME_SETTING"], "catalog_value") + self.assertEqual(data["ENABLE_COURSE_SORTING_BY_START_DATE"], True) + self.assertEqual(data["HOMEPAGE_PROMO_VIDEO_YOUTUBE_ID"], None) + self.assertEqual(data["HOMEPAGE_COURSE_MAX"], 8) + self.assertEqual(data["COURSE_ABOUT_TWITTER_ACCOUNT"], "@TestAccount") + self.assertEqual(data["NON_BROWSABLE_COURSES"], True) + self.assertEqual(data["ENABLE_COURSE_DISCOVERY"], False) + + @patch("lms.djangoapps.mfe_config_api.views.configuration_helpers") + def test_config_order_of_precedence(self, configuration_helpers_mock): + """Test the precedence of configuration values by explicitly using catalog MFE as an example. + + Expected result: + - Values should be taken in this order (highest to lowest precedence): + 1. MFE_CONFIG_OVERRIDES from site conf + 2. MFE_CONFIG_OVERRIDES from settings + 3. MFE_CONFIG from site conf + 4. MFE_CONFIG from settings + 5. Plain site configuration + 6. Plain settings + """ + mfe_config = { + "HOMEPAGE_COURSE_MAX": 10, + "ENABLE_COURSE_SORTING_BY_START_DATE": False, + "PRESERVED_SETTING": "preserved" + } + mfe_config_overrides = { + "catalog": { + "HOMEPAGE_COURSE_MAX": 15, + } + } + + def side_effect(key, default=None): + if key == "MFE_CONFIG": + return mfe_config + if key == "MFE_CONFIG_OVERRIDES": + return mfe_config_overrides + if key == "HOMEPAGE_COURSE_MAX": + return 5 # Plain site configuration + if key == "homepage_promo_video_youtube_id": + return "site-conf-youtube-id" + return default + + configuration_helpers_mock.get_value.side_effect = side_effect + + with override_settings( + HOMEPAGE_COURSE_MAX=3, # Plain settings (lowest precedence) + FEATURES={ # Settings FEATURES + "ENABLE_COURSE_SORTING_BY_START_DATE": True, + "ENABLE_COURSE_DISCOVERY": True, + } + ): + response = self.client.get(f"{self.mfe_config_api_url}?mfe=catalog") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + data = response.json() + + # MFE_CONFIG_OVERRIDES from site conf (highest precedence) + self.assertEqual(data["HOMEPAGE_COURSE_MAX"], 15) + + # MFE_CONFIG from site conf takes precedence over plain site configuration and settings + self.assertEqual(data["ENABLE_COURSE_SORTING_BY_START_DATE"], False) + + # Plain site configuration takes precedence over plain settings + self.assertEqual(data["HOMEPAGE_PROMO_VIDEO_YOUTUBE_ID"], "site-conf-youtube-id") + + # Value in original MFE_CONFIG not overridden by catalog config should be preserved + self.assertEqual(data["PRESERVED_SETTING"], "preserved") diff --git a/lms/djangoapps/mfe_config_api/views.py b/lms/djangoapps/mfe_config_api/views.py index da875393dc68..3bff483ba2d1 100644 --- a/lms/djangoapps/mfe_config_api/views.py +++ b/lms/djangoapps/mfe_config_api/views.py @@ -32,6 +32,16 @@ def get(self, request): """ Return the MFE configuration, optionally including MFE-specific overrides. + This configuration currently also pulls specific settings from site configuration or + django settings. This is a temporary change as a part of the migration of some legacy + pages to MFEs. This is a temporary compatibility layer which will eventually be deprecated. + + See [Link to DEPR ticket] for more details. todo: add link + + The compatability means that settings from the legacy locations will continue to work but + the settings listed below in the `_get_legacy_config` function should be added to the MFE + config by operators. + **Usage** Get common config: @@ -52,6 +62,8 @@ def get(self, request): "LOGOUT_URL": "https://courses.example.com/logout", "STUDIO_BASE_URL": "https://studio.example.com", "LOGO_URL": "https://courses.example.com/logo.png", + "ENABLE_COURSE_SORTING_BY_START_DATE": True, + "HOMEPAGE_COURSE_MAX": 10, ... and so on } ``` @@ -60,12 +72,49 @@ def get(self, request): if not settings.ENABLE_MFE_CONFIG_API: return HttpResponseNotFound() - mfe_config = configuration_helpers.get_value('MFE_CONFIG', settings.MFE_CONFIG) - if request.query_params.get('mfe'): - mfe = str(request.query_params.get('mfe')) + # Get values from django settings (level 6) or site configuration (level 5) + legacy_config = self._get_legacy_config() + + # Get values from mfe configuration, either from django settings (level 4) or site configuration (level 3) + mfe_config = configuration_helpers.get_value("MFE_CONFIG", settings.MFE_CONFIG) + + # Get values from mfe overrides, either from django settings (level 2) or site configuration (level 1) + mfe_config_overrides = {} + if request.query_params.get("mfe"): + mfe = str(request.query_params.get("mfe")) app_config = configuration_helpers.get_value( - 'MFE_CONFIG_OVERRIDES', + "MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES, ) - mfe_config.update(app_config.get(mfe, {})) - return JsonResponse(mfe_config, status=status.HTTP_200_OK) + mfe_config_overrides = app_config.get(mfe, {}) + + # Merge the three configs in the order of precedence + merged_config = legacy_config | mfe_config | mfe_config_overrides + + return JsonResponse(merged_config, status=status.HTTP_200_OK) + + @staticmethod + def _get_legacy_config() -> dict: + """ + Return legacy configuration values available in either site configuration or django settings. + """ + return { + "ENABLE_COURSE_SORTING_BY_START_DATE": configuration_helpers.get_value( + "ENABLE_COURSE_SORTING_BY_START_DATE", + settings.FEATURES["ENABLE_COURSE_SORTING_BY_START_DATE"] + ), + "HOMEPAGE_PROMO_VIDEO_YOUTUBE_ID": configuration_helpers.get_value( + "homepage_promo_video_youtube_id", + None + ), + "HOMEPAGE_COURSE_MAX": configuration_helpers.get_value( + "HOMEPAGE_COURSE_MAX", + settings.HOMEPAGE_COURSE_MAX + ), + "COURSE_ABOUT_TWITTER_ACCOUNT": configuration_helpers.get_value( + "course_about_twitter_account", + settings.PLATFORM_TWITTER_ACCOUNT + ), + "NON_BROWSABLE_COURSES": not settings.FEATURES.get("COURSES_ARE_BROWSABLE"), + "ENABLE_COURSE_DISCOVERY": settings.FEATURES["ENABLE_COURSE_DISCOVERY"], + }