Skip to content

feat!: bundle only real defaults in app config - #1688

Merged
arbrandes merged 3 commits into
openedx:masterfrom
arbrandes:arbrandes/issue-1687-frontend-base-2.0
Aug 29, 2026
Merged

feat!: bundle only real defaults in app config#1688
arbrandes merged 3 commits into
openedx:masterfrom
arbrandes:arbrandes/issue-1687-frontend-base-2.0

Conversation

@arbrandes

@arbrandes arbrandes commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Description

Every key an app declares in its own config is a key commonAppConfig can never supply, since app config is the highest-precedence layer. Authn declared twenty-nine, so values an operator set platform-wide through MFE_CONFIG silently lost to bundled defaults. Three are genuine defaults and move to App.defaultConfig (frontend-base ADR 0017), which resolves below commonAppConfig. Two others were read nowhere, fifteen were no-ops against reads that already guard for absence, and the nine placeholders whose reads did not guard now do.

The peer dependency moves to @openedx/frontend-base 2.0.0-alpha.4, the first release carrying App.defaultConfig. The second commit is a mechanical lint:fix for the semicolon member-delimiter rule 2.0 enables, and the third covers the new guards.

Part of #1687. Follows openedx/frontend-app-catalog#156.

LLM usage notice

Built with assistance from Claude.

arbrandes and others added 2 commits August 29, 2026 08:59
Every key an app declares in its own `config` is a key `commonAppConfig` can
never supply, since app config is the highest-precedence layer. Authn declared
twenty-nine, so values an operator set platform-wide through `MFE_CONFIG`
silently lost to bundled defaults.

Three survive as genuine defaults and move to `App.defaultConfig` (frontend-base
ADR 0017, added by openedx/frontend-base#298), which resolves below
`commonAppConfig`: `DISABLE_ENTERPRISE_LOGIN`, whose absence would quietly
re-enable the enterprise flow, and `LOGO_URL` and `LOGO_WHITE_URL`, which carry
the same default frontend-base ships for the shell header.

Two of the rest were read nowhere at all, `FAVICON_URL` and `LOGO_TRADEMARK_URL`.
Fifteen were no-ops against reads that already guard for absence. The remaining
nine were placeholders whose reads did not guard, and those reads now do:
`SEARCH_CATALOG_URL` was a latent `TypeError` in the embedded welcome-page flow;
`INFO_EMAIL` produced `mailto:undefined`; the activation and password-reset
support links rendered dead anchors; and the four `BANNER_IMAGE_*` keys asked the
browser for `/undefined`.

`MARKETING_SITE_BASE_URL` wrapped the logo in a link at ten near-identical sites
and defaulted to a devstack host. Those collapse into one `BrandLogo`, which
renders the logo unlinked when no marketing site is configured, rather than
pointing it at the app's own origin.

The peer dependency moves to `@openedx/frontend-base` 2.0.0-alpha.4, the first
release carrying `App.defaultConfig`.

Three test defects surfaced while auditing and are fixed here, since the audit is
what makes the boolean contract real: `AccountActivationMessage.test.jsx` called
`mergeAppConfig` without its `appId`, so its overrides never landed and the
confirmation suite asserted the activation copy; boolean flags were overridden
with env-era strings that strict `=== false` reads can never honour; and
`SHOW_CONFIGURABLE_EDX_FIELDS` was set in four tests but read nowhere.

Part of openedx#1687.

Co-Authored-By: Claude <noreply@anthropic.com>
The shared ESLint config in frontend-base 2.0 requires semicolons rather than
commas between TypeScript type members. Mechanical `npm run lint:fix`.

Part of openedx#1687.

Co-Authored-By: Claude <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.64%. Comparing base (c9bfd49) to head (804c7ac).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1688      +/-   ##
==========================================
+ Coverage   92.03%   93.64%   +1.60%     
==========================================
  Files          93       94       +1     
  Lines        2047     2076      +29     
  Branches      588      603      +15     
==========================================
+ Hits         1884     1944      +60     
+ Misses        160      129      -31     
  Partials        3        3              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arbrandes arbrandes linked an issue Aug 29, 2026 that may be closed by this pull request
`BaseContainer.test.jsx` passed `wrappingComponent` to React Testing Library,
an enzyme option RTL ignores, so no breakpoint ever matched and neither layout
rendered. Its two assertions were `toBeDefined()` on a `querySelector` that
returned `null`, which passes for anything. With a real wrapper the image and
welcome-page layouts are exercised for the first time, and both go from a
quarter covered to whole.

The remaining new tests cover the guards the audit added: banner backgrounds
with and without an image configured, the activation and password-reset support
links in both their linked and plain-text forms, the forgot-password support
address, and `ProgressiveProfilingPageModal.handleSubmit`, which was untested
altogether and is where the `SEARCH_CATALOG_URL` crash landed.

Coverage goes from 92.00% to 93.54%.

Part of openedx#1687.

Co-Authored-By: Claude <noreply@anthropic.com>
@arbrandes
arbrandes merged commit d524623 into openedx:master Aug 29, 2026
6 checks passed
@arbrandes
arbrandes deleted the arbrandes/issue-1687-frontend-base-2.0 branch August 29, 2026 13:19
arbrandes added a commit that referenced this pull request Aug 29, 2026
Re-releases #1688, whose `feat!` header alone triggered no release.

BREAKING CHANGE: App config no longer bundles twenty-nine keys, so operator
values from MFE_CONFIG reach the app. Requires frontend-base 2.0.0-alpha.4.

Co-Authored-By: Claude <noreply@anthropic.com>
@openedx-semantic-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 2.0.0-alpha.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade to frontend-base 2.0

2 participants