feat: add feat flag for courses dropdown & webinars #2666
Conversation
asadali145
force-pushed
the
asad/add-feat-flag
branch
from
25d91af
to
0a55890
Compare
asadali145
changed the title
mitxpro/settings.py
Outdated
| @@ -1432,4 +1432,10 @@ | |||
| description="Comma separated string of course/program runs/Ids that support digital credentials", | |||
| ) | |||
|
|
|||
| ENABLE_COURSE_DROPDOWN = get_bool( | |||
There was a problem hiding this comment.
The standard name format to define feature flags is FEATURE_*. Which in this case could be FEATURE_COURSE_DROPDOWN.
The reason for this is that the manipulation of the setting from the environment file automatically handles the features and adds them in setting.FEATURES dict which we can then use anywhere. An example of adding that can be seen in 0ade363
mitxpro/utils.py
Outdated
| @@ -594,4 +594,5 @@ def get_js_settings(request: HttpRequest): | |||
| }, | |||
| "digital_credentials": settings.FEATURES.get("DIGITAL_CREDENTIALS", False), | |||
| "digital_credentials_supported_runs": settings.DIGITAL_CREDENTIALS_SUPPORTED_RUNS, | |||
| "enable_course_dropdown": settings.ENABLE_COURSE_DROPDOWN, | |||
There was a problem hiding this comment.
This will be changed accordingly as per the last comment.
| it("does not have a CatalogMenu component when feature flag is disabled", () => { | ||
| SETTINGS.enable_course_dropdown = false | ||
| assert.isNotOk( | ||
| shallow(<TopAppBar currentUser={user} location={null} errorPageHeader={null} courseTopics={courseTopics} />) | ||
| .find("CatalogMenu") | ||
| .exists() | ||
| ) | ||
| }) | ||
|
|
There was a problem hiding this comment.
I think we can merge this test and the one below with a parameterized test that validates the UI when the flag is on and off.
static/js/containers/App.js
Outdated
| const courseTopicsQuery = SETTINGS.enable_course_dropdown ? catalog.courseTopicsQuery() : [] | ||
|
|
||
| const mapPropsToConfig = () => [users.currentUserQuery(), courseTopicsQuery] | ||
|
|
There was a problem hiding this comment.
Can this be simplified like:
const mapPropsToConfig = () => SETTINGS.enable_course_dropdown ? [users.currentUserQuery(), courseTopicsQuery]: [users.currentUserQuery()]
static/js/containers/HeaderApp.js
Outdated
| const mapPropsToConfig = () => errorPageHeader ? [] : [users.currentUserQuery(), catalog.courseTopicsQuery()] | ||
| const courseTopicsQuery = SETTINGS.enable_course_dropdown ? catalog.courseTopicsQuery() : [] | ||
|
|
||
| const mapPropsToConfig = () => errorPageHeader ? [] : [users.currentUserQuery(), courseTopicsQuery] |
There was a problem hiding this comment.
Same comment as above to simplify, If possible
Pre-Flight checklist
app.jsonWhat are the relevant tickets?
None but this discussion.
What's this PR do?
Adds a feature flag for the course topics dropdown.
How should this be manually tested?
ENABLE_COURSE_DROPDOWNto True and the header course topics dropdown should be enabled.ENABLE_COURSE_DROPDOWNto False and the header course topics dropdown should be disabled.Screenshots (if appropriate)