refactor: course outline to remove redux#3073
Draft
navinkarkera wants to merge 85 commits into
Draft
Conversation
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @navinkarkera! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
navinkarkera
changed the title
navinkarkera
force-pushed
the
navin/fAL-3835/outline-refactor
branch
from
f28d857 to
b13cc19
Compare
…o context - Add intent-level drag handlers (previewSections, cancelReorderPreview, commitSectionReorder, commitSubsectionReorder, commitUnitReorder) - Use preview/commit snapshot pattern with rollback on failure - Await refreshed sections before clearing subsection/unit reorder preview - Add optimistic section preview on drag drop - Remove setSections/restoreSectionList from public API - Remove duplicate useCourseOutlineIndex hook from apiHooks
After successful section/subsection/unit reorder, sync React Query outline index cache so committed tree does not snap back to pre-drop state. CourseOutlineStateContext.tsx: - Replace acceptReorderPreview with acceptReorderAndSyncSections / acceptReorderAndSyncSectionOrder helpers that update query cache on success. - Use effectiveOutlineIndexData (prefer query cache) over outlineIndexData. - Read sections from query-backed data with Redux fallback. apiHooks.ts: - Extract replaceSectionInOutlineIndex, appendSectionToOutlineIndex, insertDuplicatedSectionInOutlineIndex, invalidateParentQueriesAndSync helpers. - Remove remaining Redux dispatch imports (addSection, updateSectionList, duplicateSection); write to query cache instead. CourseOutline.test.tsx: - Update courseId fixture to realistic format.
- await configure mutations and keep modal open on failure - scope publish mutations by courseId for save-status tracking
hook now returns OutlineModals props object instead of rendered JSX
Route local course-outline imports through public state/data index files. Move reorder preview math to callers so commit handlers own mutation and cache-sync work. Also remove stale hook deps and an unsafe outline data cast.
- Create src/course-outline/data/queryKeys.ts as single source of truth for all course-outline React Query keys, replacing three separate factories in apiHooks.ts, outlineIndexQuery.ts, and mutationKeys.ts. - Extract updateCourseStructure helper in outlineIndexCacheUtils.ts to replace repeated 4-level immutable spread pattern. - Fix useCourseItemData cache priming: replace nested forEach(async) with recursive primeChildCache using for...of for deterministic order and arbitrary depth support. Add guards for edge cases (undefined childInfo, missing children). - Delete mutationKeys.ts; update all imports to use queryKeys.ts. - Merge query key consolidation: courseOutlineQueryKeys, index, and mutations keys now live in one module and are consumed via courseOutlineQueryKeys.index(courseId) and courseOutlineQueryKeys.mutations.savingOperation(...).
- buildOutlineIndex typed wrapper around buildTestOutline - Migrate CourseOutline.test.tsx off 3201-line static fixture - Centralized jest.mock handles in testSetup.tsx with SetupCardTestMocksOptions - CardHeader/SectionCard migrated to renderCard/setupCardTestMocks - SubsectionCard/UnitCard deduplicated common mock blocks - Delete src/course-outline/__mocks__/courseOutlineIndex.ts (static fixture) - Remove courseOutlineIndexMock re-export from __mocks__/index.ts 142/142 targeted tests pass. No new TS/lint/format issues.
Extract 8 shared card tests into describeCard factory (card-test-factory.tsx). Refactor SectionCard, SubsectionCard, UnitCard test files to call factory, keeping only unique tests. Extract 4 shared sidebar menu tests into describeSidebarMenus helper in InfoSidebar.test.tsx. 70/70 tests pass. No new lint/format/typecheck issues.
Replace three card components (SectionCard/SubsectionCard/UnitCard) with a single recursive OutlineNode component driven by depth prop. Card files become thin wrappers preserving test imports. OutlineTree uses a recursive renderNode function with RenderContext for move-computation parameter passing. Consolidate reorder helpers: - applySubsectionReorderMove + applyUnitReorderMove → applyReorderMove - moveSubsectionOver + moveUnitOver → moveItemOver - moveSubsection + moveUnit → moveItem 174/174 tests pass. No new lint/format/type issues.
Extract useModalState<T> generic hook — refactor useConfigureDialog and useHighlightsModal to delegate toggle+data state to it. Extract useItemFieldSync hook — replace three didMountRef+useEffect blocks in SubsectionSettings. Extract useDefaultTab hook — replace identical default-tab effects in SectionInfoSidebar, SubsectionInfoSidebar, UnitInfoSidebar. 43/43 affected tests pass. No new lint/format/type issues.
- Replace CourseOutlineState interface with OutlineLoadingStatus type - Replace AccessManagedXBlockDataTypes with Pick<XBlockBase, ...> - Remove cancelReorderPreview alias in useOutlineReorderState - Fix useCreateBlockSidebar payload type (CreateCourseXBlockType & ParentIds) - Simplify OutlineModals props (import hook output types directly) - Pass highlights/configure hook objects directly in CourseOutline - Multiple low-impact dedups: pickDefined, forEachDismissedKey, PageWrap layout, dead error fields, createBlock helper 143/143 affected tests + 3/3 CourseAuthoringRoutes pass.
navinkarkera
force-pushed
the
navin/fAL-3835/outline-refactor
branch
from
7114b80 to
e0e5b00
Compare
Add three-layer guard against phantom blur-after-Escape mutation: - skipBlurSubmitRef flag set by Escape keydown, checked in handleEditSubmit - pointerDownBeforeBlurRef to distinguish real user blur from programmatic - Phantom blur guard: close without saving when blur has no pointerdown and no usable relatedTarget (preserves Tab-away save) - Tests for Escape cancel, click-away blur save, phantom blur no-mutate, and Tab-relatedTarget save
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
3 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.
CourseAuthoringContextCourseOutlineStateContextDescription
Describe what this pull request changes, and why. Include implications for people using this change.
Design decisions and their rationales should be documented in the repo (docstring / ADR), per
OEP-19, and can be linked here.
Useful information to include:
"Developer", and "Operator".
Supporting information
Link to other information about the change, such as GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.
Testing instructions
Please provide detailed step-by-step instructions for manually testing this change.
Other information
Include anything else that will help reviewers and consumers understand the change.
Best 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'