Skip to content

refactor: migrate 10 more flags off FEATURES-as-dict (batch 10)#38909

Open
feanil wants to merge 10 commits into
masterfrom
feanil/features-dict-batch-10
Open

refactor: migrate 10 more flags off FEATURES-as-dict (batch 10)#38909
feanil wants to merge 10 commits into
masterfrom
feanil/features-dict-batch-10

Conversation

@feanil

@feanil feanil commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates 10 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.

Each commit migrates a single flag:

  • ENABLE_BULK_USER_RETIREMENT
  • ENABLE_CERTIFICATES_IDV_REQUIREMENT
  • ENABLE_COURSE_ASSESSMENT_GRADE_CHANGE_SIGNAL
  • ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES
  • FRONTEND_APP_PUBLISHER_URL
  • IN_CONTEXT_DISCUSSION_ENABLED_DEFAULT
  • MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW
  • MAX_PROBLEM_RESPONSES_COUNT
  • MODE_CREATION_FOR_TESTING
  • THIRD_PARTY_AUTH_HINT

Notes on a few flags:

  • FRONTEND_APP_PUBLISHER_URL and THIRD_PARTY_AUTH_HINT had no flat definition in any envs/common.py, so those commits add an annotated definition (matching the prior inline default) before switching the readers.
  • IN_CONTEXT_DISCUSSION_ENABLED_DEFAULT and MODE_CREATION_FOR_TESTING are only defined in one process's settings but are read from shared code, so those readers use getattr(settings, 'X', default).

feanil added 10 commits July 20, 2026 11:23
The URL was only read via settings.FEATURES.get with an inline False
default and was never defined in a common envs file (only in
cms/envs/devstack.py). Add an annotated flat setting to cms/envs/common.py,
defaulting to None (the idiomatic 'no URL' value, consistent with the
sibling keys in the course settings context) and matching the prior
falsy default, then read it directly at the CMS-only caller.
…as-dict

The setting is only defined in cms/envs/common.py but the reader lives in
shared xmodule code reachable from LMS, so use getattr(settings, ..., True)
(matching the prior inline default) rather than a bare settings lookup that
would AttributeError under LMS.
Already a flat setting in lms/envs/common.py (5000); just switch the
lingering FEATURES.get reader and its test override to the flat setting.
Setting is LMS-only (lms/envs/common.py) but the reader is in shared
common/djangoapps/course_modes/urls.py, so use getattr with the prior
False default. The test's setUp-level patch.dict becomes a class-level
@override_settings (trap: override_settings on setUp conflicts with
CacheIsolationMixin, and the class needs the flag set during URL reset).
The tpa_hint was only read via settings.FEATURES.get with an inline ''
default and was never defined in a common envs file. Add an annotated flat
setting (default '') to openedx/envs/common.py so the shared reader in
common/djangoapps/student/helpers.py can read it directly from both LMS
and CMS, and convert the test overrides.
@feanil
feanil force-pushed the feanil/features-dict-batch-10 branch from 385870f to 4b16493 Compare July 20, 2026 15:23
@feanil
feanil requested a review from kdmccormick July 20, 2026 15:27
@feanil
feanil marked this pull request as ready for review July 20, 2026 15:42
@feanil
feanil requested review from a team, farhan, irtazaakram and salman2013 as code owners July 20, 2026 15:42
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.

1 participant