refactor: complete FEATURES-as-dict migration for callers missed via indirection - #38999
Merged
Conversation
feanil
marked this pull request as ready for review
August 19, 2026 13:01
Completes the migration started in #38903 (batch 6). The production reader in program_enrollments/rest_api/v1/views.py read the flag via `settings.FEATURES.get(ENABLE_ENROLLMENT_RESET_FLAG)`, where ENABLE_ENROLLMENT_RESET_FLAG is a named constant ('ENABLE_ENROLLMENT_RESET') imported from constants.py. Because the flag name never appeared as a quoted literal at the call site, the batch-6 reference grep did not match it, so this reader was missed. The FeaturesProxy bridge kept it working, which is why CI stayed green. Migrate the reader to bare `settings.ENABLE_ENROLLMENT_RESET` (defined in openedx/envs/common.py), convert the test's whole-dict `@override_settings(FEATURES=FEATURES_WITH_ENABLED)` to `@override_settings(ENABLE_ENROLLMENT_RESET=True)`, and drop the now-unused ENABLE_ENROLLMENT_RESET_FLAG constant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…f FEATURES-as-dict
`SearchIndexerBase.indexing_is_enabled()` read the flag via
`settings.FEATURES.get(cls.ENABLE_INDEXING_KEY, False)`, where
ENABLE_INDEXING_KEY is a per-subclass class attribute holding the flag
name ('ENABLE_COURSEWARE_INDEX' for CoursewareSearchIndexer /
CourseAboutSearchIndexer, 'ENABLE_LIBRARY_INDEX' for LibrarySearchIndexer).
Because the flag name is a class attribute rather than a quoted literal
at the call site, batch 7's reference grep never matched this reader, so
ENABLE_COURSEWARE_INDEX's migration (#38904) missed it; ENABLE_LIBRARY_INDEX
was likewise never picked up. The FeaturesProxy bridge kept it working, so
CI stayed green (the companion reader in contentstore/utils.py was already
migrated to flat `settings.ENABLE_COURSEWARE_INDEX`).
Read the dynamic class-attribute key off flat settings with
`getattr(settings, cls.ENABLE_INDEXING_KEY, False)`. Both flags are defined
in cms/envs/common.py (and set True flat in cms/envs/test.py), so the
existing indexer tests exercise the enabled path directly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`TeamsSearchIndexer.search_is_enabled()` read the flag via
`settings.FEATURES.get(cls.ENABLE_SEARCH_KEY, False)`, where
ENABLE_SEARCH_KEY is a class attribute holding the flag name
('ENABLE_TEAMS'). As with the other class-attribute indexers, the flag
name never appears as a quoted literal at the call site, so the earlier
reference grep missed this reader; the FeaturesProxy bridge kept it
working. The companion test already toggles the flag flat via
`@override_settings(ENABLE_TEAMS=False)`.
Read the dynamic key off flat settings with
`getattr(settings, cls.ENABLE_SEARCH_KEY, False)`. ENABLE_TEAMS is defined
in lms/envs/common.py (and openedx/envs/common.py).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The v3 studio-home view still read the flag via
`settings.FEATURES.get('STUDIO_REQUEST_EMAIL', '')`. The earlier
STUDIO_REQUEST_EMAIL migration updated the v1 view
(contentstore/rest_api/v1/views/home.py) and course_creators/admin.py to
flat `settings.STUDIO_REQUEST_EMAIL`, but missed this duplicated v3 copy.
The FeaturesProxy bridge kept it working, so CI stayed green.
Read the flag off flat settings with `settings.STUDIO_REQUEST_EMAIL`
(defined in cms/envs/common.py, default `''` — matching the prior `.get`
default). Tests already override it flat via
`@override_settings(..., STUDIO_REQUEST_EMAIL=...)`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feanil
force-pushed
the
feanil/features-dict-missed-callers
branch
from
August 19, 2026 14:42
29d37e1 to
e5cffc6
Compare
feanil
enabled auto-merge (rebase)
August 19, 2026 15:18
This was referenced Aug 19, 2026
kdmccormick
approved these changes
Aug 19, 2026
Comment on lines
-15
to
-18
| # This flag should only be enabled on sandboxes. | ||
| # It enables the endpoint that wipes all program enrollments. | ||
| ENABLE_ENROLLMENT_RESET_FLAG = 'ENABLE_ENROLLMENT_RESET' | ||
|
|
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
2 participants
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.
Completes migrations that shipped incomplete in earlier merged batches. Each of these readers accessed its flag through indirection — a named constant or a per-subclass class attribute — so the flag name never appeared as a quoted literal at the call site, and the reference greps used in the original batches (which key on the quoted name) never matched them. The
FeaturesProxybridge routedFEATURES.get('X')to the flatsettings.X, so the readers kept working and CI stayed green — masking the miss.This surfaced during a full audit of what's left before the bridge/
FEATURESdict can be deleted.Flags (one commit each)
program_enrollments/rest_api/v1/views.pyread it viasettings.FEATURES.get(ENABLE_ENROLLMENT_RESET_FLAG)(constant fromconstants.py). Migrated to baresettings.ENABLE_ENROLLMENT_RESET; converted the test's whole-dict@override_settings(FEATURES=...)to@override_settings(ENABLE_ENROLLMENT_RESET=True); dropped the now-dead constant. (Missed by refactor: migrate 10 more flags off FEATURES-as-dict (batch 6) #38903.)SearchIndexerBase.indexing_is_enabled()readsettings.FEATURES.get(cls.ENABLE_INDEXING_KEY, False). Migrated togetattr(settings, cls.ENABLE_INDEXING_KEY, False). (COURSEWARE_INDEX missed by refactor: migrate 10 more flags off FEATURES-as-dict (batch 7) #38904; LIBRARY_INDEX never picked up.)TeamsSearchIndexer.search_is_enabled()readsettings.FEATURES.get(cls.ENABLE_SEARCH_KEY, False). Migrated togetattr(settings, cls.ENABLE_SEARCH_KEY, False).settings.FEATURES.get('STUDIO_REQUEST_EMAIL', '')(a plain duplicate-view miss; the v1 view andcourse_creators/admin.pywere already migrated). Migrated tosettings.STUDIO_REQUEST_EMAIL.Each flag already had a flat definition in the appropriate
envs/common.py, and the existing tests exercise the enabled/disabled paths via flat settings (verified locally).