feat: add FrontendSiteConfigView for frontend-base site configuration#38061
Open
arbrandes wants to merge 1 commit intoopenedx:masterfrom
Open
feat: add FrontendSiteConfigView for frontend-base site configuration#38061arbrandes wants to merge 1 commit intoopenedx:masterfrom
arbrandes wants to merge 1 commit intoopenedx:masterfrom
Conversation
Add a new endpoint at /api/mfe_config/v1/frontend_site that translates legacy MFE_CONFIG / MFE_CONFIG_OVERRIDES settings into the camelCase structure expected by frontend-base's SiteConfig interface. Site-level keys (RequiredSiteConfig/OptionalSiteConfig) are promoted to the top level. Shared app-level config (legacy + unmapped MFE_CONFIG keys) is returned in a commonAppConfig key. Per-app overrides from MFE_CONFIG_OVERRIDES are returned in the apps array with only their override keys, as frontend-base handles merging with commonAppConfig. Also refactors shared config helpers (get_legacy_config, get_mfe_config, get_mfe_config_overrides) out of MFEConfigView for reuse. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
FrontendSiteConfigViewendpoint at/api/mfe_config/v1/frontend_sitethat translates the existing flatMFE_CONFIG/MFE_CONFIG_OVERRIDESDjango settings into the camelCase structure expected by frontend-base'sSiteConfig.MFEConfigViewto use shared helper functions (get_legacy_config,get_mfe_config,get_mfe_config_overrides), reducing duplication.RequiredSiteConfig/OptionalSiteConfigare promoted to the top level under their camelCase names; remaining keys go intocommonAppConfig(pending feat: support commonAppConfig site config option frontend-base#179); eachMFE_CONFIG_OVERRIDESentry becomes an element of theappsarray.Work log (Claude Code sessions)
Session 1: Initial implementation
Started from a scaffold
FrontendSiteConfigViewwith a docstring describing the desired behavior. Built the implementation iteratively:SITE_CONFIG_TRANSLATION_MAPmapping all knownRequiredSiteConfigandOptionalSiteConfigfields from frontend-base'stypes.ts. Keys inMFE_CONFIGmatching the map are promoted to camelCase top-level fields; all others become app configuration.customdict; this was removed in favor of including them directly in app config.MFE_CONFIG_OVERRIDESentry becomes an element of theappsarray with its overrides merged on top of the shared base config.get_legacy_config(),get_mfe_config(), andget_mfe_config_overrides()into shared module-level functions, eliminating duplication betweenMFEConfigViewand the new view. Removed the intermediateget_merged_mfe_config()function in favor of inline composition.get_legacy_config()directly intoapp_base_configsince none of its keys map to frontend-base types, avoiding a split-then-recombine step.SITE_CONFIG_TRANSLATION_MAPkeys from per-app override configs so they don't leak into app configuration.mfe_name_to_app_idhelper — Converts kebab-case MFE names to reverse-domain appIds (e.g.learner-dashboard→org.openedx.frontend.app.learnerDashboard).Session 2: Code review and
commonAppConfigrefactorA code review identified that per-app configs were duplicating all shared keys (legacy + unmapped MFE_CONFIG) into every app entry. This was both wasteful and divergent from frontend-base's design where shared config should be separate.
commonAppConfigas a top-level field containing legacy config + unmappedMFE_CONFIGkeys, shared across all apps. Per-app entries inappsnow carry only their own overrides, since frontend-base mergescommonAppConfiginto each app's config client-side.Session 3: Final review
A final review confirmed the implementation is correct and identified remaining items for consideration:
BASE_URL) are silently discarded — by design, since frontend-base treats these as site-wide, but worth documenting for operators.siteId(aRequiredSiteConfigfield in frontend-base) has no legacy Django equivalent and is not emitted./frontend_site) as a workaround for the parent URL lacking a trailing slash.🤖 Generated with Claude Code