Skip to content

fix: read DISPLAY_FEEDBACK_WIDGET from getConfig instead of process.env#552

Open
eemaanamir wants to merge 1 commit intoopenedx:release/teakfrom
edly-io:develop-teak-wikilearn
Open

fix: read DISPLAY_FEEDBACK_WIDGET from getConfig instead of process.env#552
eemaanamir wants to merge 1 commit intoopenedx:release/teakfrom
edly-io:develop-teak-wikilearn

Conversation

@eemaanamir
Copy link

Summary

This PR updates the feedback widget initialization to read DISPLAY_FEEDBACK_WIDGET from the runtime configuration (getConfig()) instead of directly from process.env.

Problem

Previously, the value was read using:

process.env.DISPLAY_FEEDBACK_WIDGET

This makes the flag build-time only, meaning it cannot be overridden dynamically in Tutor-based deployments.

As a result, when patching or overriding environment variables via Tutor, the value would not change unless the MFE was rebuilt.

Root Cause

process.env values are inlined at build time by Webpack. In Open edX MFEs, deploy-time configuration is expected to be accessed via:

getConfig()

Using process.env directly prevents runtime overrides and breaks Tutor patching behavior.

Fix

Replaced:

process.env.DISPLAY_FEEDBACK_WIDGET

With:

getConfig().DISPLAY_FEEDBACK_WIDGET

This aligns with standard Open edX MFE configuration patterns and allows the flag to be properly overridden in Tutor environments.

Impact

  • Enables runtime configurability.
  • Restores expected Tutor override behavior.
  • No functional change beyond configuration handling.

Copy link
Contributor

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the requested change, do you mind submitting this PR to master so we can merge it there first?

import { getConfig } from '@edx/frontend-platform';

const lightning = () => {
if (getConfig().DISPLAY_FEEDBACK_WIDGET === "true") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically correct, but the problem is that with getConfig the operator can also supply a boolean. Best to handle both cases (such as is done elsewhere):

Suggested change
if (getConfig().DISPLAY_FEEDBACK_WIDGET === "true") {
if (getConfig().DISPLAY_FEEDBACK_WIDGET.toString().toLowerCase() === "true") {

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants