Co-teacher bug fixes for new homepage #65802
Merged
Conversation
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
…ake-new-section-appear-in-list-after-accepting-coteacher-invite
lfryemason
reviewed
May 14, 2025
| ); | ||
|
|
||
| // Update sectionsLoading state when asyncLoadComplete changes | ||
| React.useEffect(() => { |
Contributor
There was a problem hiding this comment.
Why do we need to use the state and a useEffect here instead of just using the redux value from asyncLoadComplete?
| section[key as keyof Section] === undefined && | ||
| prevSection[key as keyof Section] !== undefined | ||
| prevSection[key as keyof Section] !== undefined && | ||
| !window.location.pathname.includes('/teacher_dashboard/home') |
Contributor
There was a problem hiding this comment.
I don't love basing logic on the URL. If there's another solution using props or arguments, I think that would be much more resilient, but understand if this is by far the easiest way.
Nokondi
commented
May 15, 2025
| await screen.findByText(i18n.deleteSection()); | ||
| const deleteModalButton = screen.getByLabelText(i18n.delete()); | ||
| fireEvent.click(deleteModalButton); | ||
| await act(async () => fireEvent.click(deleteModalButton)); |
Contributor
Author
There was a problem hiding this comment.
This fixes an error that was popping up during the test due to an async update happening during this event.
lfryemason
approved these changes
May 15, 2025
| coteacherInvite, | ||
| coteacherInviteForPl, | ||
| // This prop is used to allow asyncLoadSectionData to be run in a way that might remove data | ||
| destructiveLoad = false, |
Contributor
There was a problem hiding this comment.
Fantastic! I also love the name that you went with
Nokondi
deleted the
make-new-section-appear-in-list-after-accepting-coteacher-invite
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This update fixes a couple of issues with the co-teacher notification on the new teacher homepage and adds a spinner while sections are loading asynchronously.
setSectionsmethod inteacherSectionsReduxto prevent the errorSET_SECTIONS called multiple times in a way that would remove datafrom appearing when reloading data on the new teacher homepage. This was preventing async loading and was also preventing the section list from updating unless the page was fully refreshed.Recording.2025-05-14.100618.mp4
Links
TEACH-1849
TEACH-1683
TEACH-1851
Testing story
Added a new unit test for the spinner and updated other unit tests to account for the async loading.
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: