Remove Legacy UI Waffle Flags [DEPR]#3069
Open
farhan wants to merge 4 commits into
Open
Conversation
7 tasks
farhan
changed the title
farhan
force-pushed
the
remove-legacy-waffle-flags
branch
from
4c64b12 to
22d72bb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3069 +/- ##
==========================================
+ Coverage 95.62% 95.63% +0.01%
==========================================
Files 1392 1392
Lines 33131 33072 -59
Branches 7707 7691 -16
==========================================
- Hits 31681 31630 -51
+ Misses 1385 1379 -6
+ Partials 65 63 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
farhan
force-pushed
the
remove-legacy-waffle-flags
branch
3 times, most recently
from
7244e03 to
16407a2
Compare
farhan
force-pushed
the
remove-legacy-waffle-flags
branch
2 times, most recently
from
cd34b30 to
64e2fa1
Compare
farhan
force-pushed
the
remove-legacy-waffle-flags
branch
4 times, most recently
from
c630456 to
31907fb
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR removes multiple waffle-flag / config gated “legacy vs MFE” code paths and standardizes navigation to always use the MFE routes (e.g., /course/:courseId/...), while also cleaning up related flags, config, and tests.
Changes:
- Removed usage of several waffle flags and config toggles across navigation, breadcrumbs, and unit-page components, defaulting behavior to MFE routes/UI.
- Deleted legacy unit-page/new-design branching logic and related helpers/config.
- Updated tests and mocks to reflect the removal of the deprecated flags and legacy fallbacks.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/textbooks/hooks.tsx | Always links breadcrumbs to MFE course routes; removes waffle/config branching. |
| src/textbooks/Textbooks.tsx | Adjusts ConnectionErrorAlert import path. |
| src/studio-home/card-item/index.tsx | Removes waffle-flag gating for destination URL selection. |
| src/plugin-slots/CourseAuthoringUnitSidebarSlot/index.tsx | Removes conditional sidebar class logic; always applies sidebar. |
| src/pages-and-resources/pages/PageSettingButton.jsx | Removes waffle-flag based legacy/new link logic; always uses MFE route for supported legacy links. |
| src/pages-and-resources/pages/PageSettingButton.test.jsx | Removes legacy/flag-branch test cases; updates expectations. |
| src/pages-and-resources/pages/PageCard.test.jsx | Adjusts waffle flags API mock response. |
| src/index.jsx | Removes unit-page related config keys. |
| src/header/hooks.tsx | Always uses MFE URLs for content/tools menu items; removes several waffle-flag branches. |
| src/header/hooks.test.tsx | Updates tests to match always-MFE URL behavior; removes flag-specific setup. |
| src/header/Header.tsx | Removes waffle-flag dependency; always uses MFE outline link and sets isNewHomePage true. |
| src/editors/containers/VideoEditor/components/VideoSettingsModal/index.tsx | Removes useNewVideoUploadsPage prop and gating; always renders back button when not library. |
| src/editors/containers/VideoEditor/components/VideoSettingsModal/index.test.tsx | Updates test to reflect removal of useNewVideoUploadsPage. |
| src/editors/containers/VideoEditor/components/VideoEditorModal.tsx | Removes waffle-flag plumbing to VideoSettingsModal. |
| src/data/apiHooks.test.tsx | Updates waffle-flag test to assert useReactMarkdownEditor instead of removed flags. |
| src/data/apiHooks.mock.ts | Updates documentation comment for mock usage examples. |
| src/data/api.ts | Removes deprecated waffle-flag defaults. |
| src/custom-pages/CustomPages.tsx | Removes waffle-flag gated breadcrumb link; always uses MFE route. |
| src/custom-pages/CustomPages.test.tsx | Adjusts waffle flags API mock response. |
| src/course-unit/xblock-container-iframe/index.tsx | Removes legacy manage-tags modal path; always uses new flow (sidebar + iframe message). |
| src/course-unit/utils.ts | Removes isUnitPageNewDesignEnabled helper and related config import. |
| src/course-unit/unit-sidebar/unit-info/UnitVisibilityInfo.tsx | Removes legacy visibility UI and prop; always renders new visibility content. |
| src/course-unit/unit-sidebar/unit-info/PublishControls.tsx | Stops passing legacy-only openVisibleModal prop. |
| src/course-unit/unit-sidebar/UnitSidebar.tsx | Removes legacy sidebar branching; always uses new sidebar. |
| src/course-unit/header-title/HeaderTitle.tsx | Removes legacy settings/configure modal UI; keeps inline title editing. |
| src/course-unit/header-title/HeaderTitle.test.tsx | Removes config setup tied to unit-page-new-design flag. |
| src/course-unit/header-navigations/HeaderNavigations.tsx | Removes new-design feature gate; always shows new-design buttons/behavior. |
| src/course-unit/header-navigations/HeaderNavigations.test.tsx | Removes config setup tied to unit-page-new-design flag. |
| src/course-unit/breadcrumbs/Breadcrumbs.tsx | Removes waffle/config-based URL adaptation; always uses MFE URLs. |
| src/course-unit/breadcrumbs/Breadcrumbs.test.tsx | Removes legacy-fallback test and updates waffle mock response. |
| src/course-unit/add-component/AddComponent.tsx | Removes useNewPdfEditor gating; always treats PDF creation as new flow. |
| src/course-unit/add-component/AddComponent.test.tsx | Removes test validating legacy PDF-editor path. |
| src/course-unit/CourseUnit.tsx | Removes unit-page-new-design gating for StatusBar; always renders for vertical units. |
| src/course-unit/CourseUnit.test.tsx | Removes config toggles for unit-page-new-design across test cases. |
| src/course-checklist/ChecklistSection/ChecklistSection.test.jsx | Adjusts waffle flags API mock response. |
| src/course-checklist/ChecklistSection/ChecklistItemComment.jsx | Removes waffle/config gating; always links to MFE outline anchor. |
| src/course-checklist/ChecklistSection/ChecklistItemBody.jsx | Simplifies update links; always uses MFE outline URL and removes waffle dependency. |
| src/CourseAuthoringContext.tsx | Removes legacy unit-page URL branching and full-page navigation; always uses SPA navigation to MFE unit route. |
| plugins/course-apps/proctoring/Settings.test.jsx | Updates expected network call count after removing waffle flag fetch. |
| .env.development | Removes deprecated env flags for unit page / new video upload page. |
| .env | Removes deprecated env flags for unit page / unit page new design. |
Comments suppressed due to low confidence (3)
src/textbooks/Textbooks.tsx:1
- This switches from the project’s
@src/...import alias style to a relative import. To keep imports consistent (and avoid brittleness when files move), prefer using the existing alias import (e.g.,@src/generic/ConnectionErrorAlert) unless there’s a specific reason the alias cannot be used here.
src/pages-and-resources/pages/PageSettingButton.jsx:1 - The identifier
determineLinkDestinationreads like a function, but it’s a memoized value (string/null). Renaming it to something likelinkDestination/destinationHrefwould better match its actual type and reduce confusion for future readers.
src/header/Header.tsx:1 - Passing
isNewHomePagewith no value hard-codes it totrue. If the legacy home page behavior is fully removed, consider deleting the prop entirely from the downstream component API (and any related branching) to reduce dead/obsolete configuration surface area. If legacy behavior is still needed in some contexts, this should remain conditional rather than always-on.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
farhan
force-pushed
the
remove-legacy-waffle-flags
branch
from
23d3018 to
65f4d7a
Compare
5 tasks
Removes all conditional routing based on deprecated waffle flags (useNewCourseOutlinePage, useNewUnitPage, useNewFilesUploadsPage, useNewTextbooksPage, useNewCustomPages, useNewVideoUploadsPage, useNewImportPage, useNewExportPage, useNewUpdatesPage, useNewHomePage, useNewPdfEditor). MFE paths are now always used. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes the 11 flags that were cleaned from all consumers in Phase 1: useNewHomePage, useNewCourseOutlinePage, useNewUnitPage, useNewUpdatesPage, useNewFilesUploadsPage, useNewVideoUploadsPage, useNewTextbooksPage, useNewCustomPages, useNewImportPage, useNewExportPage, useNewPdfEditor. WaffleFlagName type is auto-updated via keyof typeof waffleFlagDefaults. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updates apiHooks.test.tsx to use useReactMarkdownEditor as the example flag (still present, default true). Removes useNewCourseOutlinePage and useNewVideoUploadsPage from all test mocks and HTTP response fixtures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| } | ||
| return `${getConfig().STUDIO_BASE_URL}/container/${locator}`; | ||
| }; | ||
| const getUnitUrl = (locator: string) => `/course/${courseId}/container/${locator}`; |
| } else { | ||
| window.location.assign(url); | ||
| } | ||
| navigate(getUnitUrl(locator)); |
| searchButtonAction={meiliSearchEnabled ? openSearchModal : undefined} | ||
| containerProps={containerProps} | ||
| isNewHomePage={waffleFlags.useNewHomePage} | ||
| isNewHomePage |
| ...(getConfig().ENABLE_TAGGING_TAXONOMY_PAGES === 'true' | ||
| ? [{ | ||
| href: `${studioBaseUrl}/course/${courseId}#export-tags`, | ||
| href: `${getConfig().STUDIO_BASE_URL}/course/${courseId}#export-tags`, |
Comment on lines
+25
to
+29
| const determineLinkDestination = useMemo(() => ( | ||
| legacyLink?.includes('textbooks') || legacyLink?.includes('tabs') | ||
| ? `/course/${courseId}/${id.replace('_', '-')}` | ||
| : null | ||
| ), [legacyLink, courseId, id]); |
farhan
force-pushed
the
remove-legacy-waffle-flags
branch
from
ae90a5d to
3a28254
Compare
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
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.
Closes: #3063
Description
See ticket for the detailed implemented in this PR
Followup tickets:
#3090
#3086
Waffle Flags Removed
useNewCourseOutlinePage,useNewCustomPages,useNewExportPage,useNewFilesUploadsPage,useNewHomePage,useNewImportPage,useNewPdfEditor,useNewTextbooksPage,useNewUnitPage,useNewUpdatesPage,useNewVideoUploadsPageTesting instructions
Phase 1a:
Files cleaner:
card-item/index.tsx,CustomPages.tsx,ChecklistItemComment.jsx,Textbooks.jsx,textbooks/hooks.jsxflags removed:
useNewCourseOutlinePageTest URLs:
[index.tsx]— course card navigates to/course/<id>[CustomPages.tsx]— "Content" breadcrumb stays in MFE[ChecklistItemComment.jsx]— assignment deadline links point to MFE outline[Textbooks.jsx],[hooks.jsx]— "Content" breadcrumb stays in MFEPhase 1b:
Files cleaned:
Header.tsx,ChecklistItemBody.jsx,Breadcrumbs.tsx,AddComponent.tsxflags removed:
useNewHomePage,useNewCourseOutlinePage,useNewUnitPage,useNewPdfEditor,useNewCertificatesPageremovedTest URLs:
[Header.tsx]—isNewHomePagealwaystrue[Header.tsx]— outline link in header always MFE path[ChecklistItemBody.jsx]— certificate/outline "Update" links point to MFE routes[ChecklistItemBody.jsx]— page loads in MFE[Breadcrumbs.tsx],[AddComponent.tsx]— React editor opens inlinePhase 1c:
Files cleaned:
CourseAuthoringContext.tsx,VideoEditorModal.tsx,VideoSettingsModal/index.tsx,header/hooks.tsx,PageSettingButton.jsxflags removed:
useNewUnitPage,useNewCourseOutlinePage,useNewUpdatesPage,useNewFilesUploadsPage,useNewImportPage,useNewExportPage,useNewCertificatesPage,useNewVideoUploadsPage,useNewTextbooksPage,useNewCustomPagesremovedTest URLs:
[CourseAuthoringContext.tsx]— clicking a unit block in the outline opens/course/<id>/container/<locator>(MFE unit page)[hooks.tsx]— Header → Content → "Outline" link stays in MFE[hooks.tsx]— Header → Content → "Updates" link stays in MFE[hooks.tsx]— Header → Content → "Files & Uploads" link stays in MFE[hooks.tsx]— Header → Tools → "Import" link stays in MFE[hooks.tsx]— Header → Tools → "Export" link stays in MFE[hooks.tsx]— Header → Settings → "Certificates" appears only whenENABLE_CERTIFICATE_PAGE === 'true'[VideoEditorModal.tsx],[VideoSettingsModal/index.tsx]— "Upload from video uploads page" button is always visible (non-library courses)[PageSettingButton.jsx]— Textbooks and Custom Pages settings icons navigate to MFE pathsBest Practices Checklist
We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:
.ts,.tsx).propTypesanddefaultPropsin any new or modified code.src/testUtils.tsx(specificallyinitializeMocks)apiHooks.tsin this repo for examples.messages.tsfiles have adescriptionfor translators to use.../in import paths. To import from parent folders, use@src, e.g.import { initializeMocks } from '@src/testUtils';instead offrom '../../../../testUtils'Screen shots:
Tested both new and legacy flows for the PDF editor, unit page, and video uploads page waffle flags. Despite flags being set to false in the serializer, the new React-based flows are consistently rendered apparently due to hard codings. These flags required dedicated testing because their values are controlled via waffle flags in edx-platform, and changing them could directly impact which editor or page flow is served to course authors in Studio.
Screenshots attached showing the new UI in the enabled state — safe to remove these flag checks.