refactor: migrate 6 more flags off FEATURES-as-dict (batch 11)#38910
Open
feanil wants to merge 7 commits into
Open
refactor: migrate 6 more flags off FEATURES-as-dict (batch 11)#38910feanil wants to merge 7 commits into
feanil wants to merge 7 commits into
Conversation
Already a flat setting in both lms/envs/common.py (True) and cms/envs/common.py (False); switch the two subscript readers in shared student user models to settings.X (resolves per-process) and convert the four test overrides.
ORGANIZATIONS_APP is a removed pre-Lilac flag with no reader or settings definition; the only reference was a historical docstring literally containing FEATURES['ORGANIZATIONS_APP'], which kept surfacing in the migration picker. Reword to plain prose so the eventual FEATURES-dict deletion is clean. No behavior change.
Already a flat setting in both lms/envs/common.py (True) and cms/envs/common.py (False); switch the subscript reader in shared student user models to settings.X and convert the six test overrides (patch.dict and whole-dict override_settings(FEATURES=...) forms) to flat overrides.
…RES-as-dict Defined only in lms/envs/common.py (False) but read from shared openedx/core/djangoapps user_authn code, so use getattr(settings, ..., False) matching the prior default.
…s-dict Switch the built-in HtmlBlock reader from settings.FEATURES.get(...) to getattr(settings, self.ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA, False) (the flag is LMS-only, read from shared xmodule code), and convert the test overrides to flat @override_settings. The user-facing 'To enable, set FEATURES[...]' message is intentionally left unchanged: the extracted xblocks_contrib.HtmlBlock (exercised by the same tests via USE_EXTRACTED_HTML_BLOCK) still emits it, and the shared assertion must match both. Flat override_settings still reaches the extracted block's FEATURES.get reader through the FeaturesProxy bridge.
Switch the enterprise_enabled() reader to getattr(settings, ..., False) (LMS-only flag read from openedx/features code reachable from CMS), and convert all test overrides to flat @override_settings. This removes the shared FEATURES_WITH_ENTERPRISE_ENABLED test helper (a settings.FEATURES copy with the flag set) in favor of @override_settings(ENABLE_ENTERPRISE_INTEGRATION=True) at each call site, and pulls the flag out of a DISABLE_START_DATES patch.dict (that skip-list flag stays on the dict for now).
feanil
marked this pull request as ready for review
July 20, 2026 17:10
feanil
requested review from
a team,
farhan,
irtazaakram and
salman2013
as code owners
July 20, 2026 17:10
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
1 participant
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.
Summary
Migrates 6 more feature flags off
settings.FEATURES['X']dict-style access onto flat settings (settings.X) with@override_settings(X=Y)in tests. Follows the pattern established in #38772.This batch targets the subscript readers (
settings.FEATURES['X']) plus one medium flag — several of these were invisible to the earlier.get()-only reference query.Each
refactor:commit migrates a single flag:Plus one
docs:commit removing a staleFEATURES['ORGANIZATIONS_APP']reference from a docstring (that flag is a removed pre-Lilac flag with no reader or setting).Notes on a few flags:
SQUELCH_PII_IN_LOGSandENABLE_MAX_FAILED_LOGIN_ATTEMPTSare already flat settings in bothlms/envs/common.pyandcms/envs/common.py(different per-process defaults); the readers just needed to stop going through the dict.ENABLE_COMBINED_LOGIN_REGISTRATION_FOOTERandENABLE_ENTERPRISE_INTEGRATIONare LMS-only settings read from shared code, so those readers usegetattr(settings, 'X', default).ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA: the reader moves togetattr(settings, ...), but the user-facingTo enable, set FEATURES["..."]message is intentionally left unchanged — the extractedxblocks_contrib.HtmlBlock(exercised by the same tests) still emits it, and flat@override_settingsreaches that external reader through the FeaturesProxy bridge.ENABLE_ENTERPRISE_INTEGRATIONremoves the sharedFEATURES_WITH_ENTERPRISE_ENABLEDtest helper in favor of@override_settings(ENABLE_ENTERPRISE_INTEGRATION=True)at each call site.