refactor: migrate remaining FEATURES-as-dict flags (batch 12) - #39000
Merged
Conversation
`common/djangoapps/util/file.py` read the flag via
`settings.FEATURES.get('ENABLE_COURSE_FILENAME_CCX_SUFFIX', False)`. The
setting is defined only in lms/envs/common.py, but this reader lives in
common/ and is reachable from CMS, so use
`getattr(settings, 'ENABLE_COURSE_FILENAME_CCX_SUFFIX', False)` to avoid an
AttributeError under Studio settings. Convert the test's whole-dict
`@override_settings(FEATURES={...})` to
`@override_settings(ENABLE_COURSE_FILENAME_CCX_SUFFIX=True)`. Verified the
CCX-suffix tests pass under both lms.envs.test and cms.envs.test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The learner_home view tests carried seven `@patch.dict(settings.FEATURES, ENTERPRISE_ENABLED=False)` decorators. Note this is the kwarg form, so it set a FEATURES key literally named 'ENTERPRISE_ENABLED' — which nothing in the codebase reads (the enterprise dashboard is gated by ENABLE_ENTERPRISE_INTEGRATION). The nearby module constant `ENTERPRISE_ENABLED = "ENABLE_ENTERPRISE_INTEGRATION"` was never referenced by these kwargs, so the override has always been a no-op; it looks like the author intended to disable ENABLE_ENTERPRISE_INTEGRATION. This change is scoped to getting off the FEATURES dict, not fixing the latent no-op: convert the decorators to the equivalent `@override_settings(ENTERPRISE_ENABLED=False)` (behavior identical — still sets an unread flat attribute) and drop the now-dead, misleading constant. A follow-up could decide whether these tests meant to assert the enterprise-disabled path via ENABLE_ENTERPRISE_INTEGRATION. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…TH off FEATURES-as-dict
Three bulk_email test classes carried
`@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True,
'REQUIRE_COURSE_EMAIL_AUTH': False})`. Neither FEATURES key is read
anywhere in the platform or the installed packages anymore — bulk email
enablement is now governed by the BulkEmailFlag waffle flag and the
CourseAuthorization model — so these class-level overrides are vestigial
no-ops (and, being class-level `patch.dict`, only ever wrapped the
`test_*` methods, not setUp).
Both flags sit in a single decorator dict, so they migrate together:
convert to `@override_settings(ENABLE_INSTRUCTOR_EMAIL=True,
REQUIRE_COURSE_EMAIL_AUTH=False)`. Behavior is unchanged (still unread);
this just removes the FEATURES-dict usage. The classes subclass
SharedModuleStoreTestCase, so the class-level override_settings is valid.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feanil
marked this pull request as ready for review
August 19, 2026 13:00
kdmccormick
approved these changes
Aug 19, 2026
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.
The tail end of the FEATURES-as-dict migration: the flags the earlier quoted-name picker never surfaced (whole-dict
override_settings(FEATURES={...}), kwarg-formpatch.dict, and acommon/-hosted reader). Found during a full audit of what's left before theFeaturesProxybridge andFEATURESdict can be deleted.Flags (one commit per flag, except co-located pairs)
common/djangoapps/util/file.py. Setting is LMS-only but the reader is reachable from CMS, so migrated togetattr(settings, 'ENABLE_COURSE_FILENAME_CCX_SUFFIX', False); tests converted from whole-dictoverride_settings(FEATURES={...})to flat. Verified under bothlms.envs.testandcms.envs.test.@patch.dict(settings.FEATURES, ENTERPRISE_ENABLED=False)in learner_home tests. This kwarg set a FEATURES key literally namedENTERPRISE_ENABLED, which nothing reads (the enterprise dashboard is gated byENABLE_ENTERPRISE_INTEGRATION) — a latent no-op. Converted to the equivalent@override_settings(ENTERPRISE_ENABLED=False)and dropped a dead, misleading module constant. Behavior unchanged; a follow-up can decide whether these tests meantENABLE_ENTERPRISE_INTEGRATION.BulkEmailFlagwaffle +CourseAuthorizationmodel), so these are vestigial no-ops. Migrated together to@override_settings(...).Two of these (
ENTERPRISE_ENABLED,ENABLE_INSTRUCTOR_EMAIL/REQUIRE_COURSE_EMAIL_AUTH) are no-op test overrides with no reader; they're migrated anyway so theFEATURESdict can eventually be deleted. See the per-commit messages for details.