Skip to content

ulmoBackport: removed hard-coded waffle flag checks for email and push notifications#1387

Merged
awais-ansari merged 1 commit intorelease/ulmofrom
aansari/refactor-notifications-flags
Nov 24, 2025
Merged

ulmoBackport: removed hard-coded waffle flag checks for email and push notifications#1387
awais-ansari merged 1 commit intorelease/ulmofrom
aansari/refactor-notifications-flags

Conversation

@awais-ansari
Copy link
Contributor

@awais-ansari awais-ansari commented Nov 24, 2025

Issue
The flags SHOW_PUSH_CHANNEL and SHOW_EMAIL_CHANNEL explicitly check for the string 'true'. As a result, the Boolean value True was incorrectly treated as an "off" state due to this hard-coded check. Waffle flags should operate as Boolean values instead. It will cause any community instance that uses a Boolean value.

  • SHOW_PUSH_CHANNEL: process.env.SHOW_PUSH_CHANNEL === 'true'
  • ...(getConfig().SHOW_EMAIL_CHANNEL === 'true' && { EMAIL: 'email' }),

Fix
The logic for the waffle flags in the account MFE has been corrected. If a flag is empty, missing, or not available in the config, it is now considered false. Any flag with a non-empty string value is considered true.

For Example

  • If SHOW_PUSH_CHANNEL = '', it will be treated as the flag being off.
  • If SHOW_PUSH_CHANNEL = 'true', it will be treated as the flag being on.

@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.63%. Comparing base (cab1a24) to head (6af95e4).
⚠️ Report is 1 commits behind head on release/ulmo.

Additional details and impacted files
@@               Coverage Diff                @@
##           release/ulmo    #1387      +/-   ##
================================================
+ Coverage         68.25%   68.63%   +0.37%     
================================================
  Files               120      119       -1     
  Lines              2372     2359      -13     
  Branches            655      654       -1     
================================================
  Hits               1619     1619              
+ Misses              707      694      -13     
  Partials             46       46              

☔ View full report in Codecov by Sentry.
📢 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.

@awais-ansari awais-ansari merged commit 3682791 into release/ulmo Nov 24, 2025
8 checks passed
@awais-ansari awais-ansari deleted the aansari/refactor-notifications-flags branch November 24, 2025 12:10
@farhaanbukhsh
Copy link
Member

farhaanbukhsh commented Nov 24, 2025

We should make similar changes in https://github.com/openedx/tutor-contrib-notifications/blob/main/tutornotifications/patches/mfe-lms-production-settings so that it is inline with notification enabling plugin. cc: @bmtcril

Added openedx/tutor-contrib-platform-notifications#31

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.

4 participants