feat!: Flip Studio MFE Waffle Flags to be On-By-Default#36495
Conversation
47ba960 to
7019761
Compare
3da457a to
d2e91bd
Compare
d2e91bd to
e07c1a9
Compare
|
It looks really good. I really like how it demonstrates the value of having independent functions to answer the question of whether something is enabled, allowing us to switch the toggles underneath. I'm not sure if we document that pattern explicitly anywhere. |
9c7e299 to
f85807b
Compare
7587da1 to
7bf3c0b
Compare
|
Sandbox deployment successful 🚀 |
c9e9baf to
b30ba54
Compare
|
Sandbox deployment successful 🚀 |
f76a8be to
4bab1d1
Compare
|
Sandbox deployment successful 🚀 |
4bab1d1 to
c9f1a5c
Compare
|
Sandbox deployment successful 🚀 |
c9f1a5c to
10447ad
Compare
This makes nearly all of Studio React-by-default by replacing the "opt-in-to-React" flags with a set of parallel "opt-out-of-React-and-use-the-legacy-experience" flags. Here is the mapping: * `contentstore.new_studio_mfe.use_new_unit_page` -> `!legacy_studio.unit_editor` * `new_core_editors.use_new_problem_editor` -> `!legacy_studio.problem_editor` * `new_core_editors.use_new_text_editor` -> `!legacy_studio.text_editor` * `new_core_editors.use_new_video_editor` -> `!legacy_studio.video_editor` * `new_studio_mfe.use_new_home_page` -> `!legacy_studio.home` * `contentstore.new_studio_mfe.use_new_custom_pages` -> `!legacy_studio.custom_pages` * `contentstore.new_studio_mfe.use_new_schedule_details_page` -> `!legacy_studio.schedule_details` * `contentstore.new_studio_mfe.use_new_advanced_settings_page` -> `!legacy_studio.advanced_settings` * `contentstore.new_studio_mfe.use_new_grading_page` -> `!legacy_studio.grading` * `contentstore.new_studio_mfe.use_new_updates_page` -> `!legacy_studio.updates` * `contentstore.new_studio_mfe.use_new_import_page` -> `!legacy_studio.import` * `contentstore.new_studio_mfe.use_new_export_page` -> `!legacy_studio.export` * `contentstore.new_studio_mfe.use_new_files_uploads_page` -> `!legacy_studio.files_uploads` * `contentstore.new_studio_mfe.use_new_course_outline_page` -> `!legacy_studio.course_outline` * `contentstore.new_studio_mfe.use_new_course_team_page` -> `!legacy_studio.course_team` * `contentstore.new_studio_mfe.use_new_certificates_page` -> `!legacy_studio.certificates` * `contentstore.new_studio_mfe.use_new_textbooks_page` -> `!legacy_studio.textbooks` * `contentstore.new_studio_mfe.use_new_group_configurations_page` -> `!legacy_studio.configurations` Part of: openedx#36275
10447ad to
c399772
Compare
This makes nearly all of Studio React-by-default by replacing the "opt-in-to-React" flags with a set of parallel "opt-out-of-React-and-use-the-legacy-experience" flags. Here is the mapping: * `contentstore.new_studio_mfe.use_new_unit_page` -> `!legacy_studio.unit_editor` * `new_core_editors.use_new_problem_editor` -> `!legacy_studio.problem_editor` * `new_core_editors.use_new_text_editor` -> `!legacy_studio.text_editor` * `new_core_editors.use_new_video_editor` -> `!legacy_studio.video_editor` * `new_studio_mfe.use_new_home_page` -> `!legacy_studio.home` * `contentstore.new_studio_mfe.use_new_custom_pages` -> `!legacy_studio.custom_pages` * `contentstore.new_studio_mfe.use_new_schedule_details_page` -> `!legacy_studio.schedule_details` * `contentstore.new_studio_mfe.use_new_advanced_settings_page` -> `!legacy_studio.advanced_settings` * `contentstore.new_studio_mfe.use_new_grading_page` -> `!legacy_studio.grading` * `contentstore.new_studio_mfe.use_new_updates_page` -> `!legacy_studio.updates` * `contentstore.new_studio_mfe.use_new_import_page` -> `!legacy_studio.import` * `contentstore.new_studio_mfe.use_new_export_page` -> `!legacy_studio.export` * `contentstore.new_studio_mfe.use_new_files_uploads_page` -> `!legacy_studio.files_uploads` * `contentstore.new_studio_mfe.use_new_course_outline_page` -> `!legacy_studio.course_outline` * `contentstore.new_studio_mfe.use_new_course_team_page` -> `!legacy_studio.course_team` * `contentstore.new_studio_mfe.use_new_certificates_page` -> `!legacy_studio.certificates` * `contentstore.new_studio_mfe.use_new_textbooks_page` -> `!legacy_studio.textbooks` * `contentstore.new_studio_mfe.use_new_group_configurations_page` -> `!legacy_studio.configurations` Part of: openedx#36275
c399772 to
3c452d0
Compare
|
Sandbox deployment failed 💥 |
|
Sandbox deployment successful 🚀 |
| ) | ||
| # .. toggle_description: Temporarily fall back to the old proctored exam settings view. | ||
| # .. toggle_use_cases: temporary | ||
| # .. toggle_creation_date: 2025-03-14 |
There was a problem hiding this comment.
Nit: The toggle creation dates are all set to the DEPR acceptance date here instead of when they're actually being created. Is that intentional?
There was a problem hiding this comment.
Oh, nvm, of course it is. Because that's part of the whole six month notice + removal cycle.
|
|
||
|
|
||
| def exam_setting_view_enabled(): | ||
| def exam_setting_view_enabled(course_key): |
There was a problem hiding this comment.
What would you think about giving course_key a default value of None where you're currently adding a required param, so that we're less likely to break plugins that might have been calling these functions?
There was a problem hiding this comment.
(Odds are small anyhow, but I'm paranoid.)
There was a problem hiding this comment.
@ormsbee I initially agreed, and I was in the process of making that change for this function and for the use_new_$blah_editor functions. But then I realized that this wouldn't really be preserving backwards compatibility--rather, it would be suppressing backwards incompatibility. If a plugin is calling these functions from the context of a course and isn't passing in the course_key, then that is incorrect. It needs to pass in course_key in order to respect WaffleFlagCourseOverride expectations. So, I think I'd rather leave this, and let plugins break if they are reaching into this module.
There was a problem hiding this comment.
It would still be correct if there weren't course-specific overrides, which is going to be a very common case, since course waffle flags fallback to that behavior if course_key=None. The failure mode would also be, "some users see the wrong thing for a specific course because the plugin doesn't respect course-specific overrides" vs. "all users get a 500 error because the check explodes".
There was a problem hiding this comment.
I'm not going to hold up the PR over it since it's such an edge case, but we should probably message it in the risky-changes slack channel just in case.
|
Sandbox deployment failed 💥 |
There was a problem hiding this comment.
I had one question w.r.t adding a default value for checks like exam_setting_view_enabled where it goes from being no-arg to taking an explicit course_key. I'll wait until you're up, since the cut datetime has been pushed to Friday morning EST anyhow.
Otherwise, LGTM.
ormsbee
left a comment
There was a problem hiding this comment.
Approved, with the caveat that I do think it's worth making a few of the feature check calls more backwards compatible with a default course_key of None, even though it won't respect course overrides.
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
This makes nearly all of Studio React-by-default by replacing the "opt-in-to-React" flags with a set of parallel "opt-out-of-React-and-use-the-legacy-experience" flags. Here is the mapping: * `contentstore.new_studio_mfe.use_new_unit_page` -> `!legacy_studio.unit_editor` * `new_core_editors.use_new_problem_editor` -> `!legacy_studio.problem_editor` * `new_core_editors.use_new_text_editor` -> `!legacy_studio.text_editor` * `new_core_editors.use_new_video_editor` -> `!legacy_studio.video_editor` * `new_studio_mfe.use_new_home_page` -> `!legacy_studio.home` * `contentstore.new_studio_mfe.use_new_custom_pages` -> `!legacy_studio.custom_pages` * `contentstore.new_studio_mfe.use_new_schedule_details_page` -> `!legacy_studio.schedule_details` * `contentstore.new_studio_mfe.use_new_advanced_settings_page` -> `!legacy_studio.advanced_settings` * `contentstore.new_studio_mfe.use_new_grading_page` -> `!legacy_studio.grading` * `contentstore.new_studio_mfe.use_new_updates_page` -> `!legacy_studio.updates` * `contentstore.new_studio_mfe.use_new_import_page` -> `!legacy_studio.import` * `contentstore.new_studio_mfe.use_new_export_page` -> `!legacy_studio.export` * `contentstore.new_studio_mfe.use_new_files_uploads_page` -> `!legacy_studio.files_uploads` * `contentstore.new_studio_mfe.use_new_course_outline_page` -> `!legacy_studio.course_outline` * `contentstore.new_studio_mfe.use_new_course_team_page` -> `!legacy_studio.course_team` * `contentstore.new_studio_mfe.use_new_certificates_page` -> `!legacy_studio.certificates` * `contentstore.new_studio_mfe.use_new_textbooks_page` -> `!legacy_studio.textbooks` * `contentstore.new_studio_mfe.use_new_group_configurations_page` -> `!legacy_studio.configurations` Part of: #36275
This makes nearly all of Studio React-by-default by replacing the "opt-in-to-React" flags with a set of parallel "opt-out-of-React-and-use-the-legacy-experience" flags. Here is the mapping: * `contentstore.new_studio_mfe.use_new_unit_page` -> `!legacy_studio.unit_editor` * `new_core_editors.use_new_problem_editor` -> `!legacy_studio.problem_editor` * `new_core_editors.use_new_text_editor` -> `!legacy_studio.text_editor` * `new_core_editors.use_new_video_editor` -> `!legacy_studio.video_editor` * `new_studio_mfe.use_new_home_page` -> `!legacy_studio.home` * `contentstore.new_studio_mfe.use_new_custom_pages` -> `!legacy_studio.custom_pages` * `contentstore.new_studio_mfe.use_new_schedule_details_page` -> `!legacy_studio.schedule_details` * `contentstore.new_studio_mfe.use_new_advanced_settings_page` -> `!legacy_studio.advanced_settings` * `contentstore.new_studio_mfe.use_new_grading_page` -> `!legacy_studio.grading` * `contentstore.new_studio_mfe.use_new_updates_page` -> `!legacy_studio.updates` * `contentstore.new_studio_mfe.use_new_import_page` -> `!legacy_studio.import` * `contentstore.new_studio_mfe.use_new_export_page` -> `!legacy_studio.export` * `contentstore.new_studio_mfe.use_new_files_uploads_page` -> `!legacy_studio.files_uploads` * `contentstore.new_studio_mfe.use_new_course_outline_page` -> `!legacy_studio.course_outline` * `contentstore.new_studio_mfe.use_new_course_team_page` -> `!legacy_studio.course_team` * `contentstore.new_studio_mfe.use_new_certificates_page` -> `!legacy_studio.certificates` * `contentstore.new_studio_mfe.use_new_textbooks_page` -> `!legacy_studio.textbooks` * `contentstore.new_studio_mfe.use_new_group_configurations_page` -> `!legacy_studio.configurations` Part of: #36275
Description
This makes nearly all of Studio React-by-default by replacing
the "opt-in-to-React" flags with a set of parallel
"opt-out-of-React-and-use-the-legacy-experience" flags.
Here is the mapping:
contentstore.new_studio_mfe.use_new_unit_page->!legacy_studio.unit_editornew_core_editors.use_new_problem_editor->!legacy_studio.problem_editornew_core_editors.use_new_text_editor->!legacy_studio.text_editornew_core_editors.use_new_video_editor->!legacy_studio.video_editornew_studio_mfe.use_new_home_page->!legacy_studio.homecontentstore.new_studio_mfe.use_new_custom_pages->!legacy_studio.custom_pagescontentstore.new_studio_mfe.use_new_schedule_details_page->!legacy_studio.schedule_detailscontentstore.new_studio_mfe.use_new_advanced_settings_page->!legacy_studio.advanced_settingscontentstore.new_studio_mfe.use_new_grading_page->!legacy_studio.gradingcontentstore.new_studio_mfe.use_new_updates_page->!legacy_studio.updatescontentstore.new_studio_mfe.use_new_import_page->!legacy_studio.importcontentstore.new_studio_mfe.use_new_export_page->!legacy_studio.exportcontentstore.new_studio_mfe.use_new_files_uploads_page->!legacy_studio.files_uploadscontentstore.new_studio_mfe.use_new_course_outline_page->!legacy_studio.course_outlinecontentstore.new_studio_mfe.use_new_course_team_page->!legacy_studio.course_teamcontentstore.new_studio_mfe.use_new_certificates_page->!legacy_studio.certificatescontentstore.new_studio_mfe.use_new_textbooks_page->!legacy_studio.textbookscontentstore.new_studio_mfe.use_new_group_configurations_page->!legacy_studio.configurationsSupporting Info
This is part of the plan detailed in: #36275
Merge considerations
I have warned about this in #cc-risky-changes. The waffle flip was extensively communicated using the DEPR process, so there should not be any blocker to merging this.
Test instructions
new_studio_mfeopt-in waffles have been deleted from this sandboxOther Information
Known issues
discussionstoggles rather than thecontentstoretoggles. So, Studio will still default to the really old Pages page in Teak, unless I can sell BTR on the idea of backporting one more waffle-flip.Query count increases
cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py:CourseIndexViewTest.test_number_of_calls_to_dbincreased from 32 to 33cms/djangoapps/contentstore/views/tests/test_course_index.py:TestCourseOutline.test_number_of_calls_to_dbincreased from 29 to 38 😬
In both cases, the new query(ies) were all:
I believe this is all because get_course_authoring_url queries org-level site configuration, and for whatever reason the result isn't cached. Ick.