From 060f7d46182565d8e8ef15c2991cdb5ff6cd3092 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 27 Feb 2026 19:53:46 +0530 Subject: [PATCH] refactor(course-outline): replace thunks with react query, add typed APIs, and improve type usages (#2900) * Replaces configure xblock and section highlights redux functions with react-query. * Replaces section highlights thunks with react query * Replaces duplicate block thunks * Removes add subsection, unit redux functions * Replaces scrollTo redux state to react-query based state. * Replaces paste unit block redux functions * Removes a lot of redux related functions as a result. * Reduces API calls without compromising data integrity. --- src/CourseAuthoringContext.tsx | 61 +---- src/course-outline/CourseOutline.test.tsx | 113 ++++++--- src/course-outline/CourseOutline.tsx | 19 +- .../OutlineAddChildButtons.test.tsx | 16 +- src/course-outline/OutlineAddChildButtons.tsx | 36 +-- src/course-outline/data/api.ts | 132 ++++------ src/course-outline/data/apiHooks.ts | 157 +++++++++++- src/course-outline/data/selectors.ts | 1 - src/course-outline/data/slice.ts | 56 +---- src/course-outline/data/thunk.ts | 230 +----------------- src/course-outline/data/types.ts | 40 ++- src/course-outline/hooks.jsx | 112 ++++++--- .../outline-sidebar/AddSidebar.test.tsx | 7 +- .../outline-sidebar/AddSidebar.tsx | 28 +-- src/course-outline/page-alerts/PageAlerts.jsx | 35 +-- .../page-alerts/PageAlerts.test.jsx | 18 +- .../section-card/SectionCard.test.tsx | 3 - .../section-card/SectionCard.tsx | 12 +- .../subsection-card/SubsectionCard.test.tsx | 5 +- .../subsection-card/SubsectionCard.tsx | 20 +- src/course-outline/unit-card/UnitCard.tsx | 9 +- src/course-unit/header-title/HeaderTitle.tsx | 23 +- src/course-unit/hooks.tsx | 17 +- .../xblock-container-iframe/index.tsx | 42 ++-- .../xblock-container-iframe/types.ts | 40 +-- .../xblock-container-iframe/utils.ts | 11 +- src/data/apiHooks.ts | 35 +++ src/data/types.ts | 40 ++- src/generic/configure-modal/AdvancedTab.tsx | 2 +- ...Modal.test.jsx => ConfigureModal.test.tsx} | 23 +- ...{ConfigureModal.jsx => ConfigureModal.tsx} | 148 +++++------ src/generic/configure-modal/UnitTab.tsx | 12 +- .../index.jsx | 1 - .../index.jsx | 3 - 34 files changed, 705 insertions(+), 802 deletions(-) rename src/generic/configure-modal/{ConfigureModal.test.jsx => ConfigureModal.test.tsx} (94%) rename src/generic/configure-modal/{ConfigureModal.jsx => ConfigureModal.tsx} (72%) diff --git a/src/CourseAuthoringContext.tsx b/src/CourseAuthoringContext.tsx index 9832ba147..a31f88715 100644 --- a/src/CourseAuthoringContext.tsx +++ b/src/CourseAuthoringContext.tsx @@ -4,18 +4,14 @@ import { } from 'react'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import { useCreateCourseBlock } from '@src/course-outline/data/apiHooks'; -import { getCourseItem } from '@src/course-outline/data/api'; -import { useDispatch, useSelector } from 'react-redux'; -import { - addSection, addSubsection, addUnit, updateSavingStatus, -} from '@src/course-outline/data/slice'; +import { useSelector } from 'react-redux'; import { useNavigate } from 'react-router'; import { getOutlineIndexData } from '@src/course-outline/data/selectors'; import { useToggleWithValue } from '@src/hooks'; import { SelectionState, type UnitXBlock, type XBlock } from '@src/data/types'; import { CourseDetailsData } from './data/api'; import { useCourseDetails, useWaffleFlags } from './data/apiHooks'; -import { RequestStatus, RequestStatusType } from './data/constants'; +import { RequestStatusType } from './data/constants'; type ModalState = { value?: XBlock | UnitXBlock; @@ -30,10 +26,8 @@ export type CourseAuthoringContextData = { courseDetails?: CourseDetailsData; courseDetailStatus: RequestStatusType; canChangeProviders: boolean; - handleAddSection: ReturnType; - handleAddSubsection: ReturnType; handleAddAndOpenUnit: ReturnType; - handleAddUnit: ReturnType; + handleAddBlock: ReturnType; openUnitPage: (locator: string) => void; getUnitUrl: (locator: string) => string; isUnlinkModalOpen: boolean; @@ -66,7 +60,6 @@ export const CourseAuthoringProvider = ({ children, courseId, }: CourseAuthoringProviderProps) => { - const dispatch = useDispatch(); const navigate = useNavigate(); const waffleFlags = useWaffleFlags(); const { data: courseDetails, status: courseDetailStatus } = useCourseDetails(courseId); @@ -114,47 +107,11 @@ export const CourseAuthoringProvider = ({ window.location.assign(url); } }; - - const addSectionToCourse = /* istanbul ignore next */ async (locator: string) => { - try { - const data = await getCourseItem(locator); - // Page should scroll to newly added section. - data.shouldScroll = true; - dispatch(addSection(data)); - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; - - const addSubsectionToCourse = /* istanbul ignore next */ async (locator: string, parentLocator: string) => { - try { - const data = await getCourseItem(locator); - // Page should scroll to newly added subsection. - data.shouldScroll = true; - dispatch(addSubsection({ parentLocator, data })); - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; - - const addUnitToCourse = /* istanbul ignore next */ async (locator: string, parentLocator: string) => { - try { - const data = await getCourseItem(locator); - // Page should scroll to newly added subsection. - data.shouldScroll = true; - dispatch(addUnit({ parentLocator, data })); - } catch { - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; - - const handleAddSection = useCreateCourseBlock(addSectionToCourse); - const handleAddSubsection = useCreateCourseBlock(addSubsectionToCourse); /** * import a unit block from library and redirect user to this unit page. */ - const handleAddAndOpenUnit = useCreateCourseBlock(openUnitPage); - const handleAddUnit = useCreateCourseBlock(addUnitToCourse); + const handleAddAndOpenUnit = useCreateCourseBlock(courseId, openUnitPage); + const handleAddBlock = useCreateCourseBlock(courseId); const context = useMemo(() => ({ courseId, @@ -162,9 +119,7 @@ export const CourseAuthoringProvider = ({ courseDetails, courseDetailStatus, canChangeProviders, - handleAddSection, - handleAddSubsection, - handleAddUnit, + handleAddBlock, handleAddAndOpenUnit, getUnitUrl, openUnitPage, @@ -184,9 +139,7 @@ export const CourseAuthoringProvider = ({ courseDetails, courseDetailStatus, canChangeProviders, - handleAddSection, - handleAddSubsection, - handleAddUnit, + handleAddBlock, handleAddAndOpenUnit, getUnitUrl, openUnitPage, diff --git a/src/course-outline/CourseOutline.test.tsx b/src/course-outline/CourseOutline.test.tsx index db0c5bb54..638df9c77 100644 --- a/src/course-outline/CourseOutline.test.tsx +++ b/src/course-outline/CourseOutline.test.tsx @@ -36,10 +36,9 @@ import { fetchCourseBestPracticesQuery, fetchCourseLaunchQuery, fetchCourseOutlineIndexQuery, syncDiscussionsTopics, - updateCourseSectionHighlightsQuery, } from './data/thunk'; import { - courseOutlineIndexMock, + courseOutlineIndexMock as originalCourseOutlineIndexMock, courseOutlineIndexWithoutSections, courseBestPracticesMock, courseLaunchMock, @@ -71,6 +70,7 @@ const getContainerKey = jest.fn().mockReturnValue('lct:org:lib:unit:1'); const getContainerType = jest.fn().mockReturnValue('unit'); const clearSelection = jest.fn(); let selectedContainerId: string | undefined; +let courseOutlineIndexMock = cloneDeep(originalCourseOutlineIndexMock); window.HTMLElement.prototype.scrollIntoView = jest.fn(); jest.mock('@src/course-outline/outline-sidebar/OutlineSidebarContext', () => ({ @@ -96,13 +96,6 @@ jest.mock('@src/help-urls/hooks', () => ({ }), })); -jest.mock('@edx/frontend-platform/i18n', () => ({ - ...jest.requireActual('@edx/frontend-platform/i18n'), - useIntl: () => ({ - formatMessage: (message) => message.defaultMessage, - }), -})); - jest.mock('./data/api', () => ({ ...jest.requireActual('./data/api'), getTagsCount: () => jest.fn().mockResolvedValue({}), @@ -163,6 +156,8 @@ describe('', () => { beforeEach(async () => { const mocks = initializeMocks(); selectedContainerId = undefined; + // restore index mock + courseOutlineIndexMock = cloneDeep(originalCourseOutlineIndexMock); jest.mocked(useLocation).mockReturnValue({ pathname: mockPathname, @@ -300,7 +295,7 @@ describe('', () => { expect(alertElements.find( (el) => el.classList.contains('alert-content'), )).toHaveTextContent( - pageAlertMessages.alertFailedGeneric.defaultMessage, + 'Unable to save changes. Please try again.', ); }); @@ -404,6 +399,7 @@ describe('', () => { }); it('adds new subsection correctly', async () => { + const user = userEvent.setup(); const { findAllByTestId } = renderComponent(); const [section] = await findAllByTestId('section-card'); let subsections = await within(section).findAllByTestId('subsection-card'); @@ -428,10 +424,14 @@ describe('', () => { axiosMock .onGet(getXBlockApiUrl(courseSubsectionMock.id)) .reply(200, courseSubsectionMock); + const firstSectionData = courseOutlineIndexMock.courseStructure.childInfo.children[0]; + // @ts-ignore + firstSectionData.childInfo.children.push(courseSubsectionMock); + axiosMock + .onGet(getXBlockApiUrl(firstSectionData.id)) + .reply(200, firstSectionData); const newSubsectionButton = await within(section).findByRole('button', { name: 'New subsection' }); - await act(async () => { - fireEvent.click(newSubsectionButton); - }); + await user.click(newSubsectionButton); subsections = await within(section).findAllByTestId('subsection-card'); expect(subsections.length).toBe(3); @@ -540,6 +540,7 @@ describe('', () => { }); it('adds a section from library correctly', async () => { + const user = userEvent.setup(); getContainerKey.mockReturnValue('lct:org:lib:section:1'); getContainerKey.mockReturnValue('section'); renderComponent(); @@ -552,11 +553,14 @@ describe('', () => { locator: 'block-v1:UNIX+UX1+2025_T3+type@chapter+block@chaptersdafdd', courseKey: 'course-v1:UNIX+UX1+2025_T3', }); + axiosMock + .onGet(getXBlockApiUrl('block-v1:UNIX+UX1+2025_T3+type@chapter+block@chaptersdafdd')) + .reply(200, courseSectionMock); const addSectionFromLibraryButton = await screen.findByRole('button', { name: /use section from library/i, }); - fireEvent.click(addSectionFromLibraryButton); + await user.click(addSectionFromLibraryButton); // click dummy button to execute onComponentSelected prop. const dummyBtn = await screen.findByRole('button', { name: 'Dummy button' }); @@ -705,26 +709,54 @@ describe('', () => { }); it('check edit title works for section, subsection and unit', async () => { - const { findAllByTestId } = renderComponent(); + const user = userEvent.setup(); + renderComponent(); + const [section] = courseOutlineIndexMock.courseStructure.childInfo.children; const checkEditTitle = async (element, item, newName, elementName) => { axiosMock.reset(); axiosMock .onPost(getCourseItemApiUrl(item.id)) .reply(200, { dummy: 'value' }); + + if (item.id === section.id) { + // return normal section data the first time to keep original name first + axiosMock + .onGet(getXBlockApiUrl(section.id)) + // @ts-ignore + .replyOnce(section); + } + // mock section, subsection and unit name and check within the elements. // this is done to avoid adding conditions to this mock. + axiosMock - .onGet(getXBlockApiUrl(item.id)) + .onGet(getXBlockApiUrl(section.id)) .reply(200, { - ...item, + ...section, display_name: newName, + childInfo: { + children: [ + { + ...section.childInfo.children[0], + display_name: newName, + childInfo: { + children: [ + { + ...section.childInfo.children[0].childInfo.children[0], + display_name: newName, + }, + ], + }, + }, + ], + }, }); const editButton = await within(element).findByTestId(`${elementName}-edit-button`); fireEvent.click(editButton); const editField = await within(element).findByTestId(`${elementName}-edit-field`); fireEvent.change(editField, { target: { value: newName } }); - await act(async () => fireEvent.blur(editField)); + await user.keyboard('{enter}'); expect( axiosMock.history.post[axiosMock.history.post.length - 1].data, ).toBe(JSON.stringify({ @@ -737,8 +769,7 @@ describe('', () => { }; // check section - const [section] = courseOutlineIndexMock.courseStructure.childInfo.children; - const [sectionElement] = await findAllByTestId('section-card'); + const [sectionElement] = await screen.findAllByTestId('section-card'); await checkEditTitle(sectionElement, section, 'New section name', 'section'); // check subsection @@ -1627,15 +1658,14 @@ describe('', () => { }); it('check update highlights when update highlights query is successfully', async () => { - const { getByRole } = renderComponent(); + const user = userEvent.setup(); + renderComponent(); const section = courseOutlineIndexMock.courseStructure.childInfo.children[0]; const highlights = [ 'New Highlight 1', 'New Highlight 2', 'New Highlight 3', - 'New Highlight 4', - 'New Highlight 5', ]; axiosMock @@ -1653,12 +1683,21 @@ describe('', () => { ...section, highlights, }); - - await executeThunk(updateCourseSectionHighlightsQuery(section.id, highlights), store.dispatch); - - await waitFor(() => { - expect(getByRole('button', { name: '5 Section highlights' })).toBeInTheDocument(); + const highlightBtn = await screen.findAllByRole('button', { name: '0 Section highlights' }); + await user.click(highlightBtn[0]); + const dialog = await screen.findByRole('dialog'); + fireEvent.change(await within(dialog).findByRole('textbox', { name: 'Highlight 1' }), { + target: { value: 'New Highlight 1' }, }); + fireEvent.change(await within(dialog).findByRole('textbox', { name: 'Highlight 2' }), { + target: { value: 'New Highlight 2' }, + }); + fireEvent.change(await within(dialog).findByRole('textbox', { name: 'Highlight 3' }), { + target: { value: 'New Highlight 3' }, + }); + await user.click(await within(dialog).findByRole('button', { name: 'Save' })); + + expect(await screen.findByRole('button', { name: '3 Section highlights' })).toBeInTheDocument(); }); it('check whether section move up and down options work correctly', async () => { @@ -2270,6 +2309,7 @@ describe('', () => { }); it('check whether unit copy & paste option works correctly', async () => { + const user = userEvent.setup(); renderComponent(); // get first section -> first subsection -> first unit element const [section] = courseOutlineIndexMock.courseStructure.childInfo.children; @@ -2328,13 +2368,6 @@ describe('', () => { const lastUnitElement = (await within(subsectionElement).findAllByTestId('unit-card')).slice(-1)[0]; expect(lastUnitElement).toHaveTextContent(unit.displayName); - // check pasteFileNotices in store - expect(store.getState().courseOutline.pasteFileNotices).toEqual({ - newFiles: ['some.css'], - conflictingFiles: ['con.css'], - errorFiles: ['error.css'], - }); - let alerts = await screen.findAllByRole('alert'); // Exclude processing notification toast alerts = alerts.filter((el) => !el.classList.contains('toast-container')); @@ -2343,18 +2376,18 @@ describe('', () => { // check alerts for errorFiles let dismissBtn = await within(alerts[0]).findByText('Dismiss'); - fireEvent.click(dismissBtn); + await user.click(dismissBtn); // check alerts for conflictingFiles dismissBtn = await within(alerts[1]).findByText('Dismiss'); - fireEvent.click(dismissBtn); + await user.click(dismissBtn); // check alerts for newFiles dismissBtn = await within(alerts[2]).findByText('Dismiss'); - fireEvent.click(dismissBtn); + await user.click(dismissBtn); - // check pasteFileNotices in store - expect(store.getState().courseOutline.pasteFileNotices).toEqual({}); + // check that all alerts are gone + expect((screen.queryAllByRole('alert')).length).toEqual(0); }); it('should show toats on export tags', async () => { diff --git a/src/course-outline/CourseOutline.tsx b/src/course-outline/CourseOutline.tsx index 6708c5d6d..95d75627c 100644 --- a/src/course-outline/CourseOutline.tsx +++ b/src/course-outline/CourseOutline.tsx @@ -71,10 +71,8 @@ const CourseOutline = () => { const { courseId, courseUsageKey, - handleAddSubsection, - handleAddUnit, + handleAddBlock, handleAddAndOpenUnit, - handleAddSection, isUnlinkModalOpen, closeUnlinkModal, currentSelection, @@ -97,6 +95,7 @@ const CourseOutline = () => { isDisabledReindexButton, isHighlightsModalOpen, isConfigureModalOpen, + isConfigureOpPending, isDeleteModalOpen, closeHighlightsModal, handleConfigureModalClose, @@ -109,14 +108,17 @@ const CourseOutline = () => { handleEnableHighlightsSubmit, handleInternetConnectionFailed, handleOpenHighlightsModal, + isSectionHighlightsUpdatePending, handleHighlightsFormSubmit, handleConfigureItemSubmit, handleDeleteItemSubmit, handleDuplicateSectionSubmit, handleDuplicateSubsectionSubmit, handleDuplicateUnitSubmit, + isDuplicatingItem, handleVideoSharingOptionChange, handlePasteClipboardClick, + isPasting, notificationDismissUrl, discussionsSettings, discussionsIncontextLearnmoreUrl, @@ -129,7 +131,6 @@ const CourseOutline = () => { handleSubsectionDragAndDrop, handleUnitDragAndDrop, errors, - resetScrollState, handleUnlinkItemSubmit, } = useCourseOutline({ courseId }); @@ -386,7 +387,6 @@ const CourseOutline = () => { onDuplicateSubmit={handleDuplicateSectionSubmit} isSectionsExpanded={isSectionsExpanded} onOrderChange={updateSectionOrderByIndex} - resetScrollState={resetScrollState} > { onOpenConfigureModal={openConfigureModal} onOrderChange={updateSubsectionOrderByIndex} onPasteClick={handlePasteClipboardClick} - resetScrollState={resetScrollState} > { // Show processing toast if any mutation is running isShow={ isShowProcessingNotification - || handleAddUnit.isPending + || handleAddBlock.isPending || handleAddAndOpenUnit.isPending - || handleAddSubsection.isPending - || handleAddSection.isPending + || isConfigureOpPending + || isSectionHighlightsUpdatePending + || isDuplicatingItem + || isPasting } // HACK: Use saving as default title till we have a need for better messages title={processingNotificationTitle || NOTIFICATION_MESSAGES.saving} diff --git a/src/course-outline/OutlineAddChildButtons.test.tsx b/src/course-outline/OutlineAddChildButtons.test.tsx index 2b0ff929f..7455ff8c7 100644 --- a/src/course-outline/OutlineAddChildButtons.test.tsx +++ b/src/course-outline/OutlineAddChildButtons.test.tsx @@ -14,10 +14,8 @@ jest.mock('@src/studio-home/data/selectors', () => ({ }), })); -const handleAddSection = { mutateAsync: jest.fn() }; -const handleAddSubsection = { mutateAsync: jest.fn() }; const handleAddAndOpenUnit = { mutateAsync: jest.fn() }; -const handleAddUnit = { mutateAsync: jest.fn() }; +const handleAddBlock = { mutateAsync: jest.fn() }; const courseUsageKey = 'some/usage/key'; const setCurrentSelection = jest.fn(); jest.mock('@src/CourseAuthoringContext', () => ({ @@ -25,10 +23,8 @@ jest.mock('@src/CourseAuthoringContext', () => ({ courseId: 5, courseUsageKey, getUnitUrl: (id: string) => `/some/${id}`, - handleAddSection, - handleAddSubsection, handleAddAndOpenUnit, - handleAddUnit, + handleAddBlock, setCurrentSelection, }), })); @@ -81,9 +77,11 @@ jest.mock('@src/course-outline/outline-sidebar/OutlineSidebarContext', () => ({ it('calls appropriate new handlers', async () => { const parentLocator = `parent-of-${containerType}`; + const grandParentLocator = `grandparent-of-${containerType}`; render(, { extraWrapper: OutlineSidebarProvider }); const newBtn = await screen.findByRole('button', { name: `New ${containerType}` }); @@ -91,17 +89,18 @@ jest.mock('@src/course-outline/outline-sidebar/OutlineSidebarContext', () => ({ await userEvent.click(newBtn); switch (containerType) { case ContainerType.Section: - await waitFor(() => expect(handleAddSection.mutateAsync).toHaveBeenCalledWith({ + await waitFor(() => expect(handleAddBlock.mutateAsync).toHaveBeenCalledWith({ type: ContainerType.Chapter, parentLocator: courseUsageKey, displayName: 'Section', })); break; case ContainerType.Subsection: - await waitFor(() => expect(handleAddSubsection.mutateAsync).toHaveBeenCalledWith({ + await waitFor(() => expect(handleAddBlock.mutateAsync).toHaveBeenCalledWith({ type: ContainerType.Sequential, parentLocator, displayName: 'Subsection', + sectionId: parentLocator, })); break; case ContainerType.Unit: @@ -109,6 +108,7 @@ jest.mock('@src/course-outline/outline-sidebar/OutlineSidebarContext', () => ({ type: ContainerType.Vertical, parentLocator, displayName: 'Unit', + sectionId: grandParentLocator, })); break; default: diff --git a/src/course-outline/OutlineAddChildButtons.tsx b/src/course-outline/OutlineAddChildButtons.tsx index ef8c583b4..562c489cb 100644 --- a/src/course-outline/OutlineAddChildButtons.tsx +++ b/src/course-outline/OutlineAddChildButtons.tsx @@ -28,9 +28,7 @@ const AddPlaceholder = ({ parentLocator }: { parentLocator?: string }) => { const intl = useIntl(); const { isCurrentFlowOn, currentFlow, stopCurrentFlow } = useOutlineSidebarContext(); const { - handleAddSection, - handleAddSubsection, - handleAddUnit, + handleAddBlock, handleAddAndOpenUnit, } = useCourseAuthoringContext(); @@ -58,10 +56,7 @@ const AddPlaceholder = ({ parentLocator }: { parentLocator?: string }) => { > - {(handleAddSection.isPending - || handleAddSubsection.isPending - || handleAddAndOpenUnit.isPending - || handleAddUnit.isPending) && ( + {(handleAddAndOpenUnit.isPending || handleAddBlock.isPending) && ( )}

{getTitle()}

@@ -86,11 +81,11 @@ interface BaseProps { btnClasses?: string; btnSize?: 'sm' | 'md' | 'lg' | 'inline'; parentLocator: string; + grandParentLocator?: string; } interface NewChildButtonsProps extends BaseProps { handleUseFromLibraryClick?: () => void; - grandParentLocator?: string; } const NewOutlineAddChildButtons = ({ @@ -113,8 +108,7 @@ const NewOutlineAddChildButtons = ({ const intl = useIntl(); const { courseUsageKey, - handleAddSection, - handleAddSubsection, + handleAddBlock, handleAddAndOpenUnit, } = useCourseAuthoringContext(); const { startCurrentFlow } = useOutlineSidebarContext(); @@ -132,7 +126,7 @@ const NewOutlineAddChildButtons = ({ newButton: messages.newSectionButton, importButton: messages.useSectionFromLibraryButton, }; - onNewCreateContent = () => handleAddSection.mutateAsync({ + onNewCreateContent = () => handleAddBlock.mutateAsync({ type: ContainerType.Chapter, parentLocator: courseUsageKey, displayName: COURSE_BLOCK_NAMES.chapter.name, @@ -144,10 +138,11 @@ const NewOutlineAddChildButtons = ({ newButton: messages.newSubsectionButton, importButton: messages.useSubsectionFromLibraryButton, }; - onNewCreateContent = () => handleAddSubsection.mutateAsync({ + onNewCreateContent = () => handleAddBlock.mutateAsync({ type: ContainerType.Sequential, parentLocator, displayName: COURSE_BLOCK_NAMES.sequential.name, + sectionId: parentLocator, }); flowType = ContainerType.Subsection; break; @@ -160,6 +155,7 @@ const NewOutlineAddChildButtons = ({ type: ContainerType.Vertical, parentLocator, displayName: COURSE_BLOCK_NAMES.vertical.name, + sectionId: grandParentLocator, }); flowType = ContainerType.Unit; break; @@ -226,6 +222,7 @@ const LegacyOutlineAddChildButtons = ({ btnClasses = 'mt-4 border-gray-500 rounded-0', btnSize, parentLocator, + grandParentLocator, onClickCard, }: BaseProps) => { // WARNING: Do not use "useStudioHome" to get "librariesV2Enabled" flag below, @@ -237,8 +234,7 @@ const LegacyOutlineAddChildButtons = ({ const intl = useIntl(); const { courseUsageKey, - handleAddSection, - handleAddSubsection, + handleAddBlock, handleAddAndOpenUnit, } = useCourseAuthoringContext(); const [ @@ -263,12 +259,12 @@ const LegacyOutlineAddChildButtons = ({ importButton: messages.useSectionFromLibraryButton, modalTitle: messages.sectionPickerModalTitle, }; - onNewCreateContent = () => handleAddSection.mutateAsync({ + onNewCreateContent = () => handleAddBlock.mutateAsync({ type: ContainerType.Chapter, parentLocator: courseUsageKey, displayName: COURSE_BLOCK_NAMES.chapter.name, }); - onUseLibraryContent = (selected: SelectedComponent) => handleAddSection.mutateAsync({ + onUseLibraryContent = (selected: SelectedComponent) => handleAddBlock.mutateAsync({ type: COMPONENT_TYPES.libraryV2, category: ContainerType.Chapter, parentLocator: courseUsageKey, @@ -283,16 +279,18 @@ const LegacyOutlineAddChildButtons = ({ importButton: messages.useSubsectionFromLibraryButton, modalTitle: messages.subsectionPickerModalTitle, }; - onNewCreateContent = () => handleAddSubsection.mutateAsync({ + onNewCreateContent = () => handleAddBlock.mutateAsync({ type: ContainerType.Sequential, parentLocator, displayName: COURSE_BLOCK_NAMES.sequential.name, + sectionId: parentLocator, }); - onUseLibraryContent = (selected: SelectedComponent) => handleAddSubsection.mutateAsync({ + onUseLibraryContent = (selected: SelectedComponent) => handleAddBlock.mutateAsync({ type: COMPONENT_TYPES.libraryV2, category: ContainerType.Sequential, parentLocator, libraryContentKey: selected.usageKey, + sectionId: parentLocator, }); visibleTabs = [ContentType.subsections]; query = ['block_type = "subsection"']; @@ -307,12 +305,14 @@ const LegacyOutlineAddChildButtons = ({ type: ContainerType.Vertical, parentLocator, displayName: COURSE_BLOCK_NAMES.vertical.name, + sectionId: grandParentLocator, }); onUseLibraryContent = (selected: SelectedComponent) => handleAddAndOpenUnit.mutateAsync({ type: COMPONENT_TYPES.libraryV2, category: ContainerType.Vertical, parentLocator, libraryContentKey: selected.usageKey, + sectionId: grandParentLocator, }); visibleTabs = [ContentType.units]; query = ['block_type = "unit"']; diff --git a/src/course-outline/data/api.ts b/src/course-outline/data/api.ts index f162c55a8..b8aebc718 100644 --- a/src/course-outline/data/api.ts +++ b/src/course-outline/data/api.ts @@ -1,7 +1,15 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { XBlock } from '@src/data/types'; -import { CourseOutline, CourseDetails, CourseItemUpdateResult } from './types'; +import { + CourseOutline, + CourseDetails, + CourseItemUpdateResult, + ConfigureSectionData, + ConfigureSubsectionData, + ConfigureUnitData, + StaticFileNotices, +} from './types'; const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; @@ -212,23 +220,15 @@ export async function publishCourseItem(itemId: string): Promise} */ -export async function configureCourseSection( - sectionId: string, - isVisibleToStaffOnly: boolean, - startDatetime: string, -): Promise { +export async function configureCourseSection(variables: ConfigureSectionData): Promise { const { data } = await getAuthenticatedHttpClient() - .post(getCourseItemApiUrl(sectionId), { + .post(getCourseItemApiUrl(variables.sectionId), { publish: 'republish', metadata: { // The backend expects metadata.visible_to_staff_only to either true or null - visible_to_staff_only: isVisibleToStaffOnly ? true : null, - start: startDatetime, + visible_to_staff_only: variables.isVisibleToStaffOnly ? true : null, + start: variables.startDatetime, }, }); @@ -236,66 +236,30 @@ export async function configureCourseSection( } /** - * Configure course section - * @param {string} itemId - * @param {string} isVisibleToStaffOnly - * @param {string} releaseDate - * @param {string} graderType - * @param {string} dueDate - * @param {boolean} isProctoredExam, - * @param {boolean} isOnboardingExam, - * @param {boolean} isPracticeExam, - * @param {string} examReviewRules, - * @param {boolean} isTimeLimited - * @param {number} defaultTimeLimitMin - * @param {string} hideAfterDue - * @param {string} showCorrectness - * @param {boolean} isPrereq, - * @param {string} prereqUsageKey, - * @param {number} prereqMinScore, - * @param {number} prereqMinCompletion, - * @returns {Promise} + * Configure course subsection */ -export async function configureCourseSubsection( - itemId: string, - isVisibleToStaffOnly: string, - releaseDate: string, - graderType: string, - dueDate: string, - isTimeLimited: boolean, - isProctoredExam: boolean, - isOnboardingExam: boolean, - isPracticeExam: boolean, - examReviewRules: string, - defaultTimeLimitMin: number, - hideAfterDue: string, - showCorrectness: string, - isPrereq: boolean, - prereqUsageKey: string, - prereqMinScore: number, - prereqMinCompletion: number, -): Promise { +export async function configureCourseSubsection(variables: ConfigureSubsectionData): Promise { const { data } = await getAuthenticatedHttpClient() - .post(getCourseItemApiUrl(itemId), { + .post(getCourseItemApiUrl(variables.itemId), { publish: 'republish', - graderType, - isPrereq, - prereqUsageKey, - prereqMinScore, - prereqMinCompletion, + graderType: variables.graderType, + isPrereq: variables.isPrereq, + prereqUsageKey: variables.prereqUsageKey, + prereqMinScore: variables.prereqMinScore, + prereqMinCompletion: variables.prereqMinCompletion, metadata: { // The backend expects metadata.visible_to_staff_only to either true or null - visible_to_staff_only: isVisibleToStaffOnly ? true : null, - due: dueDate, - hide_after_due: hideAfterDue, - show_correctness: showCorrectness, - is_practice_exam: isPracticeExam, - is_time_limited: isTimeLimited, - is_proctored_enabled: isProctoredExam || isPracticeExam || isOnboardingExam, - exam_review_rules: examReviewRules, - default_time_limit_minutes: defaultTimeLimitMin, - is_onboarding_exam: isOnboardingExam, - start: releaseDate, + visible_to_staff_only: variables.isVisibleToStaffOnly ? true : null, + due: variables.dueDate, + hide_after_due: variables.hideAfterDue, + show_correctness: variables.showCorrectness, + is_practice_exam: variables.isPracticeExam, + is_time_limited: variables.isTimeLimited, + is_proctored_enabled: variables.isProctoredExam || variables.isPracticeExam || variables.isOnboardingExam, + exam_review_rules: variables.examReviewRules, + default_time_limit_minutes: variables.defaultTimeLimitMin, + is_onboarding_exam: variables.isOnboardingExam, + start: variables.releaseDate, }, }); return data; @@ -303,26 +267,16 @@ export async function configureCourseSubsection( /** * Configure course unit - * @param {string} unitId - * @param {boolean} isVisibleToStaffOnly - * @param {object} groupAccess - * @param {boolean} discussionEnabled - * @returns {Promise} */ -export async function configureCourseUnit( - unitId: string, - isVisibleToStaffOnly: boolean, - groupAccess: object, - discussionEnabled: boolean, -): Promise { +export async function configureCourseUnit(variables: ConfigureUnitData): Promise { const { data } = await getAuthenticatedHttpClient() - .post(getCourseItemApiUrl(unitId), { + .post(getCourseItemApiUrl(variables.unitId), { publish: 'republish', metadata: { // The backend expects metadata.visible_to_staff_only to either true or null - visible_to_staff_only: isVisibleToStaffOnly ? true : null, - group_access: groupAccess, - discussion_enabled: discussionEnabled, + visible_to_staff_only: variables.isVisibleToStaffOnly ? true : null, + group_access: variables.groupAccess, + discussion_enabled: variables.discussionEnabled, }, }); @@ -361,7 +315,10 @@ export async function deleteCourseItem(itemId: string): Promise { /** * Duplicate course section */ -export async function duplicateCourseItem(itemId: string, parentId: string): Promise { +export async function duplicateCourseItem(itemId: string, parentId: string): Promise<{ + courseKey: string; + locator: string; +}> { const { data } = await getAuthenticatedHttpClient() .post(getXBlockBaseApiUrl(), { duplicate_source_locator: itemId, @@ -467,7 +424,12 @@ export async function setVideoSharingOption( * @param {string} parentLocator * @returns {Promise} */ -export async function pasteBlock(parentLocator: string): Promise { +export async function pasteBlock(parentLocator: string): Promise<{ + locator: string; + courseKey: string; + staticFileNotices: StaticFileNotices; + upstreamRef: string; +}> { const { data } = await getAuthenticatedHttpClient() .post(getXBlockBaseApiUrl(), { parent_locator: parentLocator, diff --git a/src/course-outline/data/apiHooks.ts b/src/course-outline/data/apiHooks.ts index 088b107dd..64289eea8 100644 --- a/src/course-outline/data/apiHooks.ts +++ b/src/course-outline/data/apiHooks.ts @@ -1,12 +1,21 @@ import { containerComparisonQueryKeys } from '@src/container-comparison/data/apiHooks'; +import { addSection, duplicateSection, updateSectionList } from '@src/course-outline/data/slice'; +import { + ConfigureSectionData, + ConfigureSubsectionData, + ConfigureUnitData, + StaticFileNotices, +} from '@src/course-outline/data/types'; +import { createGlobalState } from '@src/data/apiHooks'; import type { XBlockBase, XblockChildInfo } from '@src/data/types'; -import { getCourseKey } from '@src/generic/key-utils'; +import { getBlockType, getCourseKey } from '@src/generic/key-utils'; import { handleResponseErrors } from '@src/generic/saving-error-alert'; import { ParentIds } from '@src/generic/types'; import { QueryClient, skipToken, useMutation, useQuery, useQueryClient, } from '@tanstack/react-query'; +import { useDispatch } from 'react-redux'; import { createCourseXblock, type CreateCourseXBlockType, @@ -15,6 +24,12 @@ import { getCourseDetails, getCourseItem, publishCourseItem, + configureCourseSection, + configureCourseSubsection, + configureCourseUnit, + updateCourseSectionHighlights, + duplicateCourseItem, + pasteBlock, } from './api'; export const courseOutlineQueryKeys = { @@ -27,6 +42,14 @@ export const courseOutlineQueryKeys = { ...courseOutlineQueryKeys.course(itemId ? getCourseKey(itemId) : undefined), itemId, ], + scrollToCourseItemId: (courseId?: string) => [ + ...courseOutlineQueryKeys.course(courseId), + 'scroll', + ], + pasteFileNotices: (courseId?: string) => [ + ...courseOutlineQueryKeys.course(courseId), + 'pasteFileNotices', + ], courseDetails: (courseId?: string) => [ ...courseOutlineQueryKeys.course(courseId), 'details', @@ -42,6 +65,14 @@ export const courseOutlineQueryKeys = { ], }; +type ScrollState = { + id?: string; +}; + +export const useScrollState = createGlobalState(courseOutlineQueryKeys.scrollToCourseItemId, { + id: undefined, +}); + /** * Invalidate parent Subsection and Section data. * @@ -72,23 +103,35 @@ type CreateCourseXBlockMutationProps = CreateCourseXBlockType & ParentIds; * @returns Mutation object for creating course blocks */ export const useCreateCourseBlock = ( + courseKey: string, callback?: ((locator: string, parentLocator: string) => Promise), ) => { const queryClient = useQueryClient(); + const { setData } = useScrollState(courseKey); + const dispatch = useDispatch(); return useMutation({ mutationFn: (variables: CreateCourseXBlockMutationProps) => createCourseXblock(variables), - onSettled: async (data: { locator: string; }, _err, variables) => { + onSuccess: async (data: { locator: string; }, variables) => { await callback?.(data.locator, variables.parentLocator); queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(data.locator)), }); - invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e)); + await invalidateParentQueries(queryClient, variables); + // scroll to newly added block + setData({ id: data.locator }); + // if newly created block is chapter or section, fetch and add it to store + // all other types are handled by invalidateParentQueries and useCourseItemData + if (getBlockType(data.locator) === 'chapter') { + const newBlock = await getCourseItem(data.locator); + dispatch(addSection(newBlock)); + } }, }); }; export const useCourseItemData = (itemId?: string, initialData?: T, enabled: boolean = true) => { const queryClient = useQueryClient(); + const dispatch = useDispatch(); return useQuery({ initialData, queryKey: courseOutlineQueryKeys.courseItemId(itemId), @@ -111,6 +154,14 @@ export const useCourseItemData = (itemId?: string, initial } }); } + // We update redux store section list to update children list in outline. + // Even though each block has its own hook to fetch data, new child blocks or deleted blocks + // won't be detected as the child blocks are rendered in the outline from the top level + // sectionList from redux store. + if (['chapter', 'section'].includes(data.category)) { + const payload = { [data.id]: data }; + dispatch(updateSectionList(payload)); + } return data; } : skipToken, }); @@ -172,3 +223,103 @@ export const useDeleteCourseItem = () => { }, }); }; + +export const useConfigureSection = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (variables: ConfigureSectionData & ParentIds) => configureCourseSection(variables), + onSettled: (_data, _err, variables) => { + queryClient.invalidateQueries({ + queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.sectionId)), + }); + invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e)); + }, + }); +}; + +export const useConfigureSubsection = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (variables: ConfigureSubsectionData & ParentIds) => configureCourseSubsection(variables), + onSettled: (_data, _err, variables) => { + queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.itemId)) }); + invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e)); + }, + }); +}; + +export const useConfigureUnit = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (variables: ConfigureUnitData & ParentIds) => configureCourseUnit(variables), + onSettled: (_data, _err, variables) => { + queryClient.invalidateQueries({ queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.unitId)) }); + invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e)); + }, + }); +}; + +export const useUpdateCourseSectionHighlights = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (variables: { + sectionId: string; + highlights: string[]; + } & ParentIds) => updateCourseSectionHighlights(variables.sectionId, variables.highlights), + onSettled: (_data, _err, variables) => { + queryClient.invalidateQueries({ + queryKey: courseOutlineQueryKeys.courseDetails(getCourseKey(variables.sectionId)), + }); + invalidateParentQueries(queryClient, variables).catch((e) => handleResponseErrors(e)); + }, + }); +}; + +export const useDuplicateItem = (courseKey: string) => { + const queryClient = useQueryClient(); + const dispatch = useDispatch(); + const { setData } = useScrollState(courseKey); + return useMutation({ + mutationFn: (variables: { + itemId: string; + parentId: string; + } & ParentIds) => duplicateCourseItem(variables.itemId, variables.parentId), + onSuccess: async (data, variables) => { + await invalidateParentQueries(queryClient, variables); + // add duplicated section to store, subsection and unit are handled by invalidateParentQueries + if (getBlockType(variables.itemId) === 'chapter') { + const duplicatedItem = await getCourseItem(data.locator); + dispatch(duplicateSection({ id: variables.itemId, duplicatedItem })); + } + // scroll to newly added block + setData({ id: data.locator }); + }, + }); +}; + +export const usePasteFileNotices = createGlobalState( + courseOutlineQueryKeys.pasteFileNotices, + { + newFiles: [], + conflictingFiles: [], + errorFiles: [], + }, +); + +export const usePasteItem = (courseId?: string) => { + const queryClient = useQueryClient(); + const { setData: setScrollState } = useScrollState(courseId); + const { setData } = usePasteFileNotices(courseId); + return useMutation({ + mutationFn: (variables: { + parentLocator: string; + } & ParentIds) => pasteBlock(variables.parentLocator), + onSuccess: async (data, variables) => { + await invalidateParentQueries(queryClient, variables); + // set pasteFileNotices + setData(data.staticFileNotices); + // scroll to pasted block + setScrollState({ id: data.locator }); + }, + }); +}; diff --git a/src/course-outline/data/selectors.ts b/src/course-outline/data/selectors.ts index b5ab44022..68eb04cdc 100644 --- a/src/course-outline/data/selectors.ts +++ b/src/course-outline/data/selectors.ts @@ -7,6 +7,5 @@ export const getCourseActions = (state) => state.courseOutline.actions; export const getCustomRelativeDatesActiveFlag = (state) => state.courseOutline.isCustomRelativeDatesActive; export const getProctoredExamsFlag = (state) => state.courseOutline.enableProctoredExams; export const getTimedExamsFlag = (state) => state.courseOutline.enableTimedExams; -export const getPasteFileNotices = (state) => state.courseOutline.pasteFileNotices; export const getErrors = (state) => state.courseOutline.errors; export const getCreatedOn = (state) => state.courseOutline.createdOn; diff --git a/src/course-outline/data/slice.ts b/src/course-outline/data/slice.ts index 9b23f65a0..5d475e382 100644 --- a/src/course-outline/data/slice.ts +++ b/src/course-outline/data/slice.ts @@ -47,9 +47,8 @@ const initialState = { }, enableProctoredExams: false, enableTimedExams: false, - pasteFileNotices: {}, createdOn: null, -} satisfies CourseOutlineState as unknown as CourseOutlineState; +} satisfies CourseOutlineState; const slice = createSlice({ name: 'courseOutline', @@ -133,27 +132,6 @@ const slice = createSlice({ payload, ]; }, - resetScrollField: (state) => { - state.sectionsList = state.sectionsList.map((section) => { - section.shouldScroll = false; - section.childInfo.children.map((subsection) => { - subsection.shouldScroll = false; - return subsection; - }); - return section; - }); - }, - addSubsection: (state: CourseOutlineState, { payload }) => { - state.sectionsList = state.sectionsList.map((section) => { - if (section.id === payload.parentLocator) { - section.childInfo.children = [ - ...section.childInfo.children.filter(child => child.id !== payload.data.id), // Filter to avoid duplicates - payload.data, - ]; - } - return section; - }); - }, deleteSection: (state: CourseOutlineState, { payload }) => { state.sectionsList = state.sectionsList.filter( ({ id }) => id !== payload.itemId, @@ -170,25 +148,6 @@ const slice = createSlice({ return section; }); }, - // FIXME: This is a temporary measure to add unit using redux even while we are - // actively trying to get rid of it. - // To remove this and other add functions, we need to migrate course outline data - // to a react-query and perform optimistic updates to add/remove content. - addUnit: /* istanbul ignore next */ (state: CourseOutlineState, { payload }) => { - state.sectionsList = state.sectionsList.map((section) => { - section.childInfo.children = section.childInfo.children.map((subsection) => { - if (subsection.id !== payload.parentLocator) { - return subsection; - } - subsection.childInfo.children = [ - ...subsection.childInfo.children.filter(({ id }) => id !== payload.data.id), - payload.data, - ]; - return subsection; - }); - return section; - }); - }, deleteUnit: (state: CourseOutlineState, { payload }) => { state.sectionsList = state.sectionsList.map((section) => { if (section.id !== payload.sectionId) { @@ -214,20 +173,11 @@ const slice = createSlice({ return [...result, currentValue]; }, []); }, - setPasteFileNotices: (state: CourseOutlineState, { payload }) => { - state.pasteFileNotices = payload; - }, - removePasteFileNotices: (state: CourseOutlineState, { payload }) => { - const pasteFileNotices = { ...state.pasteFileNotices }; - payload.forEach((key: string | number) => delete pasteFileNotices[key]); - state.pasteFileNotices = pasteFileNotices; - }, }, }); export const { addSection, - addSubsection, fetchOutlineIndexSuccess, updateOutlineIndexLoadingStatus, updateReindexLoadingStatus, @@ -242,13 +192,9 @@ export const { deleteSection, deleteSubsection, deleteUnit, - addUnit, duplicateSection, reorderSectionList, - setPasteFileNotices, - removePasteFileNotices, dismissError, - resetScrollField, } = slice.actions; export const { diff --git a/src/course-outline/data/thunk.ts b/src/course-outline/data/thunk.ts index 8441715cf..ae7664c97 100644 --- a/src/course-outline/data/thunk.ts +++ b/src/course-outline/data/thunk.ts @@ -11,21 +11,15 @@ import { } from '../utils/getChecklistForStatusBar'; import { getErrorDetails } from '../utils/getErrorDetails'; import { - duplicateCourseItem, enableCourseHighlightsEmails, getCourseBestPractices, getCourseLaunch, getCourseOutlineIndex, getCourseItem, - configureCourseSection, - configureCourseSubsection, - configureCourseUnit, restartIndexingOnCourse, - updateCourseSectionHighlights, setSectionOrderList, setVideoSharingOption, setCourseItemOrderList, - pasteBlock, dismissNotification, createDiscussionsTopics, } from './api'; import { @@ -39,9 +33,7 @@ import { updateSavingStatus, updateSectionList, updateFetchSectionLoadingStatus, - duplicateSection, reorderSectionList, - setPasteFileNotices, updateCourseLaunchQueryStatus, } from './slice'; @@ -201,32 +193,13 @@ export function fetchCourseReindexQuery(reindexLink: string) { /** * Fetches course sections and optionally scrolls to a specific subsection/unit. */ -export function fetchCourseSectionQuery(sectionIds: string[], scrollToId?: { - subsectionId: string, - unitId?: string, -}) { +export function fetchCourseSectionQuery(sectionIds: string[]) { return async (dispatch) => { dispatch(updateFetchSectionLoadingStatus({ status: RequestStatus.IN_PROGRESS })); try { const sections = {}; const results = await Promise.all(sectionIds.map((sectionId) => getCourseItem(sectionId))); results.forEach(section => { - if (scrollToId) { - const targetSubsection = section?.childInfo?.children?.find( - subsection => subsection.id === scrollToId.subsectionId, - ); - - if (targetSubsection) { - if (scrollToId.unitId) { - const targetUnit = targetSubsection?.childInfo?.children?.find(unit => unit.id === scrollToId.unitId); - if (targetUnit) { - targetUnit.shouldScroll = true; - } - } else { - targetSubsection.shouldScroll = true; - } - } - } sections[section.id] = section; }); dispatch(updateSectionList(sections)); @@ -240,186 +213,6 @@ export function fetchCourseSectionQuery(sectionIds: string[], scrollToId?: { }; } -export function updateCourseSectionHighlightsQuery(sectionId: string, highlights: string[]) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); - dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.saving)); - - try { - await updateCourseSectionHighlights(sectionId, highlights).then(async (result) => { - if (result) { - await dispatch(fetchCourseSectionQuery([sectionId])); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - dispatch(hideProcessingNotification()); - } - }); - } catch { - dispatch(hideProcessingNotification()); - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; -} - -export function configureCourseItemQuery(sectionId: string, configureFn: () => Promise) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); - dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.saving)); - - try { - await configureFn().then(async (result) => { - if (result) { - await dispatch(fetchCourseSectionQuery([sectionId])); - dispatch(hideProcessingNotification()); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - } - }); - } catch { - dispatch(hideProcessingNotification()); - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; -} - -export function configureCourseSectionQuery(sectionId: string, isVisibleToStaffOnly: boolean, startDatetime: string) { - return async (dispatch) => { - dispatch(configureCourseItemQuery( - sectionId, - async () => configureCourseSection(sectionId, isVisibleToStaffOnly, startDatetime), - )); - }; -} - -export function configureCourseSubsectionQuery( - itemId: string, - sectionId: string, - isVisibleToStaffOnly: string, - releaseDate: string, - graderType: string, - dueDate: string, - isTimeLimited: boolean, - isProctoredExam: boolean, - isOnboardingExam: boolean, - isPracticeExam: boolean, - examReviewRules: string, - defaultTimeLimitMin: number, - hideAfterDue: string, - showCorrectness: string, - isPrereq: boolean, - prereqUsageKey: string, - prereqMinScore: number, - prereqMinCompletion: number, -) { - return async (dispatch) => { - dispatch(configureCourseItemQuery( - sectionId, - async () => configureCourseSubsection( - itemId, - isVisibleToStaffOnly, - releaseDate, - graderType, - dueDate, - isTimeLimited, - isProctoredExam, - isOnboardingExam, - isPracticeExam, - examReviewRules, - defaultTimeLimitMin, - hideAfterDue, - showCorrectness, - isPrereq, - prereqUsageKey, - prereqMinScore, - prereqMinCompletion, - ), - )); - }; -} - -export function configureCourseUnitQuery( - itemId: string, - sectionId: string, - isVisibleToStaffOnly: boolean, - groupAccess: object, - discussionEnabled: boolean, -) { - return async (dispatch) => { - dispatch(configureCourseItemQuery( - sectionId, - async () => configureCourseUnit(itemId, isVisibleToStaffOnly, groupAccess, discussionEnabled), - )); - }; -} - -/** - * Generic function to duplicate any course item. See wrapper functions below for specific implementations. - * @param {string} itemId - * @param {string} parentLocator - * @param {(locator) => Promise} duplicateFn - */ -function duplicateCourseItemQuery( - itemId: string, - parentLocator: string, - duplicateFn: (locator: string) => Promise, -) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); - dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.duplicating)); - - try { - await duplicateCourseItem(itemId, parentLocator).then(async (result) => { - if (result) { - await duplicateFn(result.locator); - dispatch(hideProcessingNotification()); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - } - }); - } catch { - dispatch(hideProcessingNotification()); - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; -} - -export function duplicateSectionQuery(sectionId: string, courseBlockId: string) { - return async (dispatch) => { - dispatch(duplicateCourseItemQuery( - sectionId, - courseBlockId, - async (locator) => { - const duplicatedItem = await getCourseItem(locator); - // Page should scroll to newly duplicated item. - duplicatedItem.shouldScroll = true; - dispatch(duplicateSection({ id: sectionId, duplicatedItem })); - }, - )); - }; -} - -export function duplicateSubsectionQuery(subsectionId: string, sectionId: string) { - return async (dispatch) => { - dispatch(duplicateCourseItemQuery( - subsectionId, - sectionId, - async (itemId: string) => dispatch(fetchCourseSectionQuery([sectionId], { - subsectionId: itemId, // To scroll to the newly duplicated subsection - })), - )); - }; -} - -export function duplicateUnitQuery(unitId: string, subsectionId: string, sectionId: string) { - return async (dispatch) => { - dispatch(duplicateCourseItemQuery( - unitId, - subsectionId, - async (itemId: string) => dispatch(fetchCourseSectionQuery([sectionId], { - subsectionId, - unitId: itemId, // To scroll to the newly duplicated unit - })), - )); - }; -} - function setBlockOrderListQuery( parentId: string, blockIds: string[], @@ -515,27 +308,6 @@ export function setUnitOrderListQuery( }; } -export function pasteClipboardContent(parentLocator: string, sectionId: string) { - return async (dispatch) => { - dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); - dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.pasting)); - - try { - await pasteBlock(parentLocator).then(async (result: any) => { - if (result) { - dispatch(fetchCourseSectionQuery([sectionId], { subsectionId: parentLocator, unitId: result.locator })); - dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); - dispatch(hideProcessingNotification()); - dispatch(setPasteFileNotices(result?.staticFileNotices)); - } - }); - } catch { - dispatch(hideProcessingNotification()); - dispatch(updateSavingStatus({ status: RequestStatus.FAILED })); - } - }; -} - export function dismissNotificationQuery(url: string) { return async (dispatch) => { dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); diff --git a/src/course-outline/data/types.ts b/src/course-outline/data/types.ts index 55323104f..4e927216d 100644 --- a/src/course-outline/data/types.ts +++ b/src/course-outline/data/types.ts @@ -74,7 +74,6 @@ export interface CourseOutlineState { actions: XBlockActions; enableProctoredExams: boolean; enableTimedExams: boolean; - pasteFileNotices: object; createdOn: null | Date; } @@ -90,3 +89,42 @@ export interface CourseItemUpdateResult { displayName?: string; } } + +export interface ConfigureSectionData { + sectionId: string, + isVisibleToStaffOnly: boolean, + startDatetime: string, +} + +export interface ConfigureSubsectionData { + itemId: string, + isVisibleToStaffOnly: boolean, + releaseDate: string, + graderType: string, + dueDate: string, + isTimeLimited: boolean, + isProctoredExam: boolean, + isOnboardingExam: boolean, + isPracticeExam: boolean, + examReviewRules: string, + defaultTimeLimitMin: number, + hideAfterDue: string, + showCorrectness: string, + isPrereq: boolean, + prereqUsageKey: string, + prereqMinScore: number, + prereqMinCompletion: number, +} + +export interface ConfigureUnitData { + unitId: string, + isVisibleToStaffOnly: boolean, + groupAccess: object, + discussionEnabled: boolean, +} + +export type StaticFileNotices = { + conflictingFiles: string[], + errorFiles: string[], + newFiles: string[], +}; diff --git a/src/course-outline/hooks.jsx b/src/course-outline/hooks.jsx index 87d46fd5b..6d8197de5 100644 --- a/src/course-outline/hooks.jsx +++ b/src/course-outline/hooks.jsx @@ -12,13 +12,21 @@ import { ContainerType, getBlockType } from '@src/generic/key-utils'; import { useOutlineSidebarContext } from '@src/course-outline/outline-sidebar/OutlineSidebarContext'; import { useUnlinkDownstream } from '@src/generic/unlink-modal'; import { useQueryClient } from '@tanstack/react-query'; -import { courseOutlineQueryKeys, useDeleteCourseItem } from '@src/course-outline/data/apiHooks'; +import { + courseOutlineQueryKeys, + useConfigureSection, + useConfigureSubsection, + useConfigureUnit, + useDeleteCourseItem, + useDuplicateItem, + usePasteItem, + useUpdateCourseSectionHighlights, +} from '@src/course-outline/data/apiHooks'; import { COURSE_BLOCK_NAMES } from './constants'; import { deleteSection, deleteSubsection, deleteUnit, - resetScrollField, updateSavingStatus, } from './data/slice'; import { @@ -33,23 +41,15 @@ import { getCreatedOn, } from './data/selectors'; import { - duplicateSectionQuery, - duplicateSubsectionQuery, - duplicateUnitQuery, enableCourseHighlightsEmailsQuery, fetchCourseBestPracticesQuery, fetchCourseLaunchQuery, fetchCourseOutlineIndexQuery, fetchCourseReindexQuery, - updateCourseSectionHighlightsQuery, - configureCourseSectionQuery, - configureCourseSubsectionQuery, - configureCourseUnitQuery, setSectionOrderListQuery, setVideoSharingOptionQuery, setSubsectionOrderListQuery, setUnitOrderListQuery, - pasteClipboardContent, dismissNotificationQuery, syncDiscussionsTopics, } from './data/thunk'; @@ -57,7 +57,7 @@ import { const useCourseOutline = ({ courseId }) => { const dispatch = useDispatch(); const { - handleAddSection, + handleAddBlock, setCurrentSelection, currentSelection, currentUnlinkModalData, @@ -99,18 +99,19 @@ const useCourseOutline = ({ courseId }) => { const isSavingStatusFailed = savingStatus === RequestStatus.FAILED || genericSavingStatus === RequestStatus.FAILED; - const handlePasteClipboardClick = (parentLocator, sectionId) => { - dispatch(pasteClipboardContent(parentLocator, sectionId)); - }; - - const resetScrollState = () => { - dispatch(resetScrollField()); + const { mutate: pasteClipboardContent, isPending: isPasting } = usePasteItem(courseId); + const handlePasteClipboardClick = (parentLocator, subsectionId, sectionId) => { + pasteClipboardContent({ + parentLocator, + subsectionId, + sectionId, + }); }; const headerNavigationsActions = { handleNewSection: () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises - handleAddSection.mutateAsync({ + handleAddBlock.mutateAsync({ type: ContainerType.Chapter, parentLocator: courseStructure?.id, displayName: COURSE_BLOCK_NAMES.chapter.name, @@ -147,9 +148,16 @@ const useCourseOutline = ({ courseId }) => { openHighlightsModal(); }; + const { + mutate: updateCourseSectionHighlights, + isPending: isSectionHighlightsUpdatePending, + } = useUpdateCourseSectionHighlights(); const handleHighlightsFormSubmit = (highlights) => { const dataToSend = Object.values(highlights).filter(Boolean); - dispatch(updateCourseSectionHighlightsQuery(currentSelection?.currentId, dataToSend)); + updateCourseSectionHighlights({ + sectionId: currentSelection?.currentId, + highlights: dataToSend, + }); closeHighlightsModal(); }; @@ -180,17 +188,41 @@ const useCourseOutline = ({ courseId }) => { }); }, [currentUnlinkModalData, unlinkDownstream, closeUnlinkModal]); - const handleConfigureItemSubmit = (...arg) => { + const { + mutate: configureCourseSection, + isPending: isSectionConfigurePending, + } = useConfigureSection(); + const { + mutate: configureCourseSubsection, + isPending: isSubsectionConfigurePending, + } = useConfigureSubsection(); + const { + mutate: configureCourseUnit, + isPending: isUnitConfigurePending, + } = useConfigureUnit(); + const isConfigureOpPending = isSectionConfigurePending || isSubsectionConfigurePending || isUnitConfigurePending; + const handleConfigureItemSubmit = (variables) => { const category = getBlockType(currentSelection.currentId); switch (category) { case COURSE_BLOCK_NAMES.chapter.id: - dispatch(configureCourseSectionQuery(currentSelection?.sectionId, ...arg)); + configureCourseSection({ + sectionId: currentSelection?.sectionId, + ...variables, + }); break; case COURSE_BLOCK_NAMES.sequential.id: - dispatch(configureCourseSubsectionQuery(currentSelection?.currentId, currentSelection?.sectionId, ...arg)); + configureCourseSubsection({ + itemId: currentSelection?.currentId, + sectionId: currentSelection?.sectionId, + ...variables, + }); break; case COURSE_BLOCK_NAMES.vertical.id: - dispatch(configureCourseUnitQuery(currentSelection?.currentId, currentSelection?.sectionId, ...arg)); + configureCourseUnit({ + unitId: currentSelection?.currentId, + sectionId: currentSelection?.sectionId, + ...variables, + }); break; default: // istanbul ignore next @@ -264,20 +296,35 @@ const useCourseOutline = ({ courseId }) => { deleteSubsection, ]); + const { + mutate: duplicateItem, + isPending: isDuplicatingItem, + } = useDuplicateItem(courseId); const handleDuplicateSectionSubmit = () => { - dispatch(duplicateSectionQuery(currentSelection?.sectionId, courseStructure.id)); + duplicateItem({ + itemId: currentSelection?.currentId, + parentId: courseStructure.id, + sectionId: currentSelection?.sectionId, + subsectionId: currentSelection?.subsectionId, + }); }; const handleDuplicateSubsectionSubmit = () => { - dispatch(duplicateSubsectionQuery(currentSelection?.subsectionId, currentSelection?.sectionId)); + duplicateItem({ + itemId: currentSelection?.currentId, + parentId: currentSelection?.sectionId, + sectionId: currentSelection?.sectionId, + subsectionId: currentSelection?.subsectionId, + }); }; const handleDuplicateUnitSubmit = () => { - dispatch(duplicateUnitQuery( - currentSelection?.currentId, - currentSelection?.subsectionId, - currentSelection?.sectionId, - )); + duplicateItem({ + itemId: currentSelection?.currentId, + parentId: currentSelection?.subsectionId, + sectionId: currentSelection?.sectionId, + subsectionId: currentSelection?.subsectionId, + }); }; const handleVideoSharingOptionChange = (value) => { @@ -360,12 +407,14 @@ const useCourseOutline = ({ courseId }) => { isConfigureModalOpen, openConfigureModal, handleConfigureModalClose, + isConfigureOpPending, headerNavigationsActions, handleEnableHighlightsSubmit, handleHighlightsFormSubmit, handleConfigureItemSubmit, statusBarData, isEnableHighlightsModalOpen, + isSectionHighlightsUpdatePending, openEnableHighlightsModal, closeEnableHighlightsModal, isInternetConnectionAlertFailed: isSavingStatusFailed, @@ -380,9 +429,11 @@ const useCourseOutline = ({ courseId }) => { handleDeleteItemSubmit, handleDuplicateSectionSubmit, handleDuplicateSubsectionSubmit, + isDuplicatingItem, handleDuplicateUnitSubmit, handleVideoSharingOptionChange, handlePasteClipboardClick, + isPasting, notificationDismissUrl, discussionsSettings, discussionsIncontextLearnmoreUrl, @@ -396,7 +447,6 @@ const useCourseOutline = ({ courseId }) => { handleSubsectionDragAndDrop, handleUnitDragAndDrop, errors, - resetScrollState, handleUnlinkItemSubmit, }; }; diff --git a/src/course-outline/outline-sidebar/AddSidebar.test.tsx b/src/course-outline/outline-sidebar/AddSidebar.test.tsx index 3dbc44ffc..bb69f6f88 100644 --- a/src/course-outline/outline-sidebar/AddSidebar.test.tsx +++ b/src/course-outline/outline-sidebar/AddSidebar.test.tsx @@ -21,7 +21,7 @@ import type { ContainerType } from '@src/generic/key-utils'; import { XBlock } from '@src/data/types'; import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; import { snakeCaseKeys } from '@src/editors/utils'; -import { getXBlockBaseApiUrl } from '@src/course-outline/data/api'; +import { getXBlockApiUrl, getXBlockBaseApiUrl } from '@src/course-outline/data/api'; import MockAdapter from 'axios-mock-adapter/types'; import { AddSidebar } from './AddSidebar'; @@ -199,10 +199,13 @@ describe('AddSidebar', () => { const sectionId = 'block-v1:UNIX+UX1+2025_T3+type@chapter+block@chapter123'; axiosMock.onPost(getXBlockBaseApiUrl()) .reply(200, { locator: sectionId }); + axiosMock.onGet(getXBlockApiUrl(sectionId)) + .reply(200, {}); renderComponent(); const subsection = await screen.findByRole('button', { name: 'Subsection' }); await user.click(subsection); + await waitFor(() => expect(axiosMock.history.post.length).toBeGreaterThan(1)); // should add a section first expect(axiosMock.history.post[0].data).toEqual(JSON.stringify(snakeCaseKeys({ type: 'chapter', @@ -250,6 +253,8 @@ describe('AddSidebar', () => { .reply(200, { locator: subsectionId }); axiosMock.onPost(getXBlockBaseApiUrl(), unitBody) .reply(200, { locator: unitId }); + axiosMock.onGet(getXBlockApiUrl(sectionId)) + .reply(200, {}); renderComponent(); const unit = await screen.findByRole('button', { name: 'Unit' }); diff --git a/src/course-outline/outline-sidebar/AddSidebar.tsx b/src/course-outline/outline-sidebar/AddSidebar.tsx index ac04263b2..8915d5da9 100644 --- a/src/course-outline/outline-sidebar/AddSidebar.tsx +++ b/src/course-outline/outline-sidebar/AddSidebar.tsx @@ -50,8 +50,7 @@ type AddContentButtonProps = { const AddContentButton = ({ name, blockType } : AddContentButtonProps) => { const { courseUsageKey, - handleAddSection, - handleAddSubsection, + handleAddBlock, handleAddAndOpenUnit, } = useCourseAuthoringContext(); const { @@ -65,7 +64,7 @@ const AddContentButton = ({ name, blockType } : AddContentButtonProps) => { let subsectionParentId = lastEditableSubsection?.data?.id; const addSection = (onSuccess?: (data: { locator: string; }) => void) => { - handleAddSection.mutate({ + handleAddBlock.mutate({ type: ContainerType.Chapter, parentLocator: courseUsageKey, displayName: COURSE_BLOCK_NAMES.chapter.name, @@ -82,10 +81,11 @@ const AddContentButton = ({ name, blockType } : AddContentButtonProps) => { }; const addSubsection = (sectionId: string, onSuccess?: (data: { locator: string; }) => void) => { - handleAddSubsection.mutate({ + handleAddBlock.mutate({ type: ContainerType.Sequential, parentLocator: sectionId, displayName: COURSE_BLOCK_NAMES.sequential.name, + sectionId, }, { onSuccess: (data: { locator: string; }) => { // istanbul ignore next @@ -146,8 +146,7 @@ const AddContentButton = ({ name, blockType } : AddContentButtonProps) => { }, [ blockType, courseUsageKey, - handleAddSection, - handleAddSubsection, + handleAddBlock, handleAddAndOpenUnit, currentFlow, sectionParentId, @@ -155,7 +154,7 @@ const AddContentButton = ({ name, blockType } : AddContentButtonProps) => { lastEditableSubsection, ]); - const disabled = handleAddSection.isPending || handleAddSubsection.isPending || handleAddAndOpenUnit.isPending; + const disabled = handleAddBlock.isPending || handleAddAndOpenUnit.isPending; return ( { const ShowLibraryContent = () => { const { courseUsageKey, - handleAddSection, - handleAddSubsection, - handleAddUnit, + handleAddBlock, } = useCourseAuthoringContext(); const { isCurrentFlowOn, @@ -233,7 +230,7 @@ const ShowLibraryContent = () => { const onComponentSelected: ComponentSelectedEvent = useCallback(async ({ usageKey, blockType }) => { switch (blockType) { case 'section': - await handleAddSection.mutateAsync({ + await handleAddBlock.mutateAsync({ type: COMPONENT_TYPES.libraryV2, category: ContainerType.Chapter, parentLocator: courseUsageKey, @@ -243,11 +240,12 @@ const ShowLibraryContent = () => { case 'subsection': sectionParentId = currentFlow?.parentLocator || sectionParentId; if (sectionParentId) { - await handleAddSubsection.mutateAsync({ + await handleAddBlock.mutateAsync({ type: COMPONENT_TYPES.libraryV2, category: ContainerType.Sequential, parentLocator: sectionParentId, libraryContentKey: usageKey, + sectionId: sectionParentId, }); } break; @@ -257,7 +255,7 @@ const ShowLibraryContent = () => { ); subsectionParentId = currentFlow?.parentLocator || subsectionParentId; if (subsectionParentId) { - await handleAddUnit.mutateAsync({ + await handleAddBlock.mutateAsync({ type: COMPONENT_TYPES.libraryV2, category: ContainerType.Vertical, parentLocator: subsectionParentId, @@ -273,9 +271,7 @@ const ShowLibraryContent = () => { stopCurrentFlow(); }, [ courseUsageKey, - handleAddSection, - handleAddSubsection, - handleAddUnit, + handleAddBlock, lastEditableSection, lastEditableSubsection, currentFlow, diff --git a/src/course-outline/page-alerts/PageAlerts.jsx b/src/course-outline/page-alerts/PageAlerts.jsx index ce430d6e4..738926304 100644 --- a/src/course-outline/page-alerts/PageAlerts.jsx +++ b/src/course-outline/page-alerts/PageAlerts.jsx @@ -11,9 +11,10 @@ import { } from '@openedx/paragon/icons'; import { uniqBy } from 'lodash'; import PropTypes from 'prop-types'; -import React, { useState } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import { useState } from 'react'; +import { useDispatch } from 'react-redux'; import { Link, useNavigate } from 'react-router-dom'; +import { usePasteFileNotices } from '@src/course-outline/data/apiHooks'; import CourseOutlinePageAlertsSlot from '../../plugin-slots/CourseOutlinePageAlertsSlot'; import advancedSettingsMessages from '../../advanced-settings/messages'; import { OutOfSyncAlert } from '../../course-libraries/OutOfSyncAlert'; @@ -23,8 +24,7 @@ import ErrorAlert from '../../editors/sharedComponents/ErrorAlerts/ErrorAlert'; import AlertMessage from '../../generic/alert-message'; import AlertProctoringError from '../../generic/AlertProctoringError'; import { API_ERROR_TYPES } from '../constants'; -import { getPasteFileNotices } from '../data/selectors'; -import { dismissError, removePasteFileNotices } from '../data/slice'; +import { dismissError } from '../data/slice'; import messages from './messages'; const PageAlerts = ({ @@ -48,7 +48,7 @@ const PageAlerts = ({ const [showDiscussionAlert, setShowDiscussionAlert] = useState( localStorage.getItem(discussionAlertDismissKey) === null, ); - const { newFiles, conflictingFiles, errorFiles } = useSelector(getPasteFileNotices); + const { data: pasteFileNotices, setData: setPasteFileNotices } = usePasteFileNotices(courseId); const [showOutOfSyncAlert, setShowOutOfSyncAlert] = useState(false); const navigate = useNavigate(); @@ -247,16 +247,16 @@ const PageAlerts = ({ const newFilesPasteAlert = () => { const onDismiss = () => { - dispatch(removePasteFileNotices(['newFiles'])); + setPasteFileNotices({ ...pasteFileNotices, newFiles: [] }); }; - if (newFiles?.length) { + if (pasteFileNotices?.newFiles?.length) { return ( { const onDismiss = () => { - dispatch(removePasteFileNotices(['errorFiles'])); + setPasteFileNotices({ ...pasteFileNotices, errorFiles: [] }); }; - if (errorFiles?.length) { + if (pasteFileNotices?.errorFiles?.length) { return ( { const onDismiss = () => { - dispatch(removePasteFileNotices(['conflictingFiles'])); + setPasteFileNotices({ ...pasteFileNotices, conflictingFiles: [] }); }; - if (conflictingFiles?.length) { + if (pasteFileNotices?.conflictingFiles?.length) { return ( ({ }), })); -jest.mock('react-redux', () => ({ - ...jest.requireActual('react-redux'), - useSelector: jest.fn(), +let mockNotices = {}; +jest.mock('@src/course-outline/data/apiHooks', () => ({ + ...jest.requireActual('@src/course-outline/data/apiHooks'), + usePasteFileNotices: () => ({ data: mockNotices }), })); jest.mock('../../course-libraries/data/apiHooks', () => ({ @@ -72,7 +72,7 @@ describe('', () => { }, }); store = initializeStore(); - useSelector.mockReturnValue({}); + mockNotices = {}; }); it('renders null when no alerts are present', async () => { @@ -174,11 +174,11 @@ describe('', () => { }); it('renders new & error files alert', async () => { - useSelector.mockReturnValue({ + mockNotices = { newFiles: ['periodic-table.css'], conflictingFiles: [], errorFiles: ['error.css'], - }); + }; renderComponent(); expect(screen.queryByText(messages.newFileAlertTitle.defaultMessage)).toBeInTheDocument(); expect(screen.queryByText(messages.errorFileAlertTitle.defaultMessage)).toBeInTheDocument(); @@ -189,11 +189,11 @@ describe('', () => { }); it('renders conflicting files alert', async () => { - useSelector.mockReturnValue({ + mockNotices = { newFiles: [], conflictingFiles: ['some.css', 'some.js'], errorFiles: [], - }); + }; renderComponent(); expect(screen.queryByText(messages.conflictingFileAlertTitle.defaultMessage)).toBeInTheDocument(); expect(screen.queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute( diff --git a/src/course-outline/section-card/SectionCard.test.tsx b/src/course-outline/section-card/SectionCard.test.tsx index a1757852c..6d7ef1502 100644 --- a/src/course-outline/section-card/SectionCard.test.tsx +++ b/src/course-outline/section-card/SectionCard.test.tsx @@ -26,8 +26,6 @@ jest.mock('@src/course-unit/data/apiHooks', () => ({ jest.mock('@src/CourseAuthoringContext', () => ({ useCourseAuthoringContext: () => ({ courseId: 5, - handleAddSubsectionFromLibrary: jest.fn(), - handleNewSubsectionSubmit: jest.fn(), setCurrentSelection, }), })); @@ -99,7 +97,6 @@ const renderComponent = (props?: object, entry = '/course/:courseId') => render( isSectionsExpanded isSelfPaced={false} isCustomRelativeDatesActive={false} - resetScrollState={jest.fn()} {...props} > children diff --git a/src/course-outline/section-card/SectionCard.tsx b/src/course-outline/section-card/SectionCard.tsx index 029f99aab..fac67b817 100644 --- a/src/course-outline/section-card/SectionCard.tsx +++ b/src/course-outline/section-card/SectionCard.tsx @@ -22,8 +22,9 @@ import type { XBlock } from '@src/data/types'; import { invalidateLinksQuery } from '@src/course-libraries/data/apiHooks'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; import { useOutlineSidebarContext } from '@src/course-outline/outline-sidebar/OutlineSidebarContext'; -import { courseOutlineQueryKeys, useCourseItemData } from '@src/course-outline/data/apiHooks'; +import { courseOutlineQueryKeys, useCourseItemData, useScrollState } from '@src/course-outline/data/apiHooks'; import moment from 'moment'; +import { handleResponseErrors } from '@src/generic/saving-error-alert'; import messages from './messages'; interface SectionCardProps { @@ -39,7 +40,6 @@ interface SectionCardProps { index: number, canMoveItem: (oldIndex: number, newIndex: number) => boolean, onOrderChange: (oldIndex: number, newIndex: number) => void, - resetScrollState: () => void, } const SectionCard = ({ @@ -55,7 +55,6 @@ const SectionCard = ({ onDuplicateSubmit, isSectionsExpanded, onOrderChange, - resetScrollState, }: SectionCardProps) => { const currentRef = useRef(null); const { activeId, overId } = useContext(DragContext); @@ -68,6 +67,7 @@ const SectionCard = ({ const queryClient = useQueryClient(); // Set initialData state from course outline and subsequently depend on its own state const { data: section = initialData } = useCourseItemData(initialData.id, initialData); + const { data: scrollState, resetData: resetScrollState } = useScrollState(courseId); const isScrolledToElement = locatorId === section?.id; // Expand the section if a search result should be shown/scrolled to @@ -153,13 +153,13 @@ const SectionCard = ({ }, [activeId, overId]); useEffect(() => { - if (currentRef.current && (section.shouldScroll || isScrolledToElement)) { + if (currentRef.current && (scrollState?.id === section.id || isScrolledToElement)) { // Align element closer to the top of the screen if scrolling for search result const alignWithTop = !!isScrolledToElement; scrollToElement(currentRef.current, alignWithTop, true); - resetScrollState(); + resetScrollState().catch((error) => handleResponseErrors(error)); } - }, [isScrolledToElement]); + }, [isScrolledToElement, scrollState, resetScrollState]); useEffect(() => { // If the locatorId is set/changed, we need to make sure that the section is expanded diff --git a/src/course-outline/subsection-card/SubsectionCard.test.tsx b/src/course-outline/subsection-card/SubsectionCard.test.tsx index 1aaf9ea35..3d4f6cd2a 100644 --- a/src/course-outline/subsection-card/SubsectionCard.test.tsx +++ b/src/course-outline/subsection-card/SubsectionCard.test.tsx @@ -31,8 +31,7 @@ jest.mock('@src/CourseAuthoringContext', () => ({ useCourseAuthoringContext: () => ({ courseId: 5, handleAddAndOpenUnit: handleOnAddUnitFromLibrary, - handleAddSubsection: {}, - handleAddSection: {}, + handleAddBlock: {}, setCurrentSelection, }), })); @@ -127,7 +126,6 @@ const renderComponent = (props?: object, entry = '/course/:courseId') => render( onDuplicateSubmit={jest.fn()} onOpenConfigureModal={jest.fn()} onPasteClick={jest.fn()} - resetScrollState={jest.fn()} isSectionsExpanded={false} {...props} > @@ -344,6 +342,7 @@ describe('', () => { type: COMPONENT_TYPES.libraryV2, parentLocator: 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0', category: 'vertical', + sectionId: 'block-v1:UNIX+UX1+2025_T3+type@section+block@0', libraryContentKey: containerKey, }); }); diff --git a/src/course-outline/subsection-card/SubsectionCard.tsx b/src/course-outline/subsection-card/SubsectionCard.tsx index 967796da8..c782d05a8 100644 --- a/src/course-outline/subsection-card/SubsectionCard.tsx +++ b/src/course-outline/subsection-card/SubsectionCard.tsx @@ -24,8 +24,9 @@ import type { XBlock } from '@src/data/types'; import { invalidateLinksQuery } from '@src/course-libraries/data/apiHooks'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; import { useOutlineSidebarContext } from '@src/course-outline/outline-sidebar/OutlineSidebarContext'; -import { courseOutlineQueryKeys, useCourseItemData } from '@src/course-outline/data/apiHooks'; +import { courseOutlineQueryKeys, useCourseItemData, useScrollState } from '@src/course-outline/data/apiHooks'; import moment from 'moment'; +import { handleResponseErrors } from '@src/generic/saving-error-alert'; import messages from './messages'; interface SubsectionCardProps { @@ -41,8 +42,11 @@ interface SubsectionCardProps { getPossibleMoves: (index: number, step: number) => void, onOrderChange: (section: XBlock, moveDetails: any) => void, onOpenConfigureModal: () => void, - onPasteClick: (parentLocator: string, sectionId: string) => void, - resetScrollState: () => void, + onPasteClick: ( + parentLocator: string, + subsectionId: string, + sectionId: string + ) => void, } const SubsectionCard = ({ @@ -59,7 +63,6 @@ const SubsectionCard = ({ onOrderChange, onOpenConfigureModal, onPasteClick, - resetScrollState, }: SubsectionCardProps) => { const currentRef = useRef(null); const intl = useIntl(); @@ -77,6 +80,7 @@ const SubsectionCard = ({ // Set initialData state from course outline and subsequently depend on its own state const { data: section = initialSectionData } = useCourseItemData(initialSectionData.id, initialSectionData); const { data: subsection = initialData } = useCourseItemData(initialData.id, initialData); + const { data: scrollState, resetData: resetScrollState } = useScrollState(courseId); const isScrolledToElement = locatorId === subsection.id; const { @@ -188,7 +192,7 @@ const SubsectionCard = ({ onOrderChange(section, moveDownDetails); }; - const handlePasteButtonClick = () => onPasteClick(id, section.id); + const handlePasteButtonClick = () => onPasteClick(id, id, section.id); const titleComponent = ( { // if this items has been newly added, scroll to it. - if (currentRef.current && (subsection.shouldScroll || isScrolledToElement)) { + if (currentRef.current && (scrollState?.id === subsection.id || isScrolledToElement)) { // Align element closer to the top of the screen if scrolling for search result const alignWithTop = !!isScrolledToElement; scrollToElement(currentRef.current, alignWithTop, true); - resetScrollState(); + resetScrollState().catch((error) => handleResponseErrors(error)); } - }, [isScrolledToElement]); + }, [isScrolledToElement, scrollState, resetScrollState]); useEffect(() => { // If the locatorId is set/changed, we need to make sure that the subsection is expanded diff --git a/src/course-outline/unit-card/UnitCard.tsx b/src/course-outline/unit-card/UnitCard.tsx index ed90f1072..11be7bcc4 100644 --- a/src/course-outline/unit-card/UnitCard.tsx +++ b/src/course-outline/unit-card/UnitCard.tsx @@ -22,8 +22,9 @@ import { PreviewLibraryXBlockChanges } from '@src/course-unit/preview-changes'; import { invalidateLinksQuery } from '@src/course-libraries/data/apiHooks'; import type { UnitXBlock, XBlock } from '@src/data/types'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; -import { courseOutlineQueryKeys, useCourseItemData } from '@src/course-outline/data/apiHooks'; +import { courseOutlineQueryKeys, useCourseItemData, useScrollState } from '@src/course-outline/data/apiHooks'; import moment from 'moment'; +import { handleResponseErrors } from '@src/generic/saving-error-alert'; import { useOutlineSidebarContext } from '../outline-sidebar/OutlineSidebarContext'; interface UnitCardProps { @@ -76,6 +77,7 @@ const UnitCard = ({ initialSubsectionData, ); const { data: unit = initialData } = useCourseItemData(initialData.id, initialData); + const { data: scrollState, resetData: resetScrollState } = useScrollState(courseId); const isScrolledToElement = locatorId === unit.id; const { @@ -211,12 +213,13 @@ const UnitCard = ({ useEffect(() => { // if this items has been newly added, scroll to it. - if (currentRef.current && (unit.shouldScroll || isScrolledToElement)) { + if (currentRef.current && (scrollState?.id === unit.id || isScrolledToElement)) { // Align element closer to the top of the screen if scrolling for search result const alignWithTop = !!isScrolledToElement; scrollToElement(currentRef.current, alignWithTop, true); + resetScrollState().catch((error) => handleResponseErrors(error)); } - }, [isScrolledToElement]); + }, [isScrolledToElement, scrollState, resetScrollState]); if (!isHeaderVisible) { return null; diff --git a/src/course-unit/header-title/HeaderTitle.tsx b/src/course-unit/header-title/HeaderTitle.tsx index 623c099e9..758469cb9 100644 --- a/src/course-unit/header-title/HeaderTitle.tsx +++ b/src/course-unit/header-title/HeaderTitle.tsx @@ -11,6 +11,7 @@ import { import ConfigureModal from '@src/generic/configure-modal/ConfigureModal'; import { COURSE_BLOCK_NAMES } from '@src/constants'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { ConfigureUnitData } from '@src/course-outline/data/types'; import { getCourseUnitData } from '../data/selectors'; import { updateQueryPendingStatus } from '../data/slice'; import messages from './messages'; @@ -21,13 +22,7 @@ type HeaderTitleProps = { isTitleEditFormOpen: boolean; handleTitleEdit: () => void; handleTitleEditSubmit: (title: string) => void; - handleConfigureSubmit: ( - id: string, - isVisible: boolean, - groupAccess: boolean, - isDiscussionEnabled: boolean, - closeModalFn: (value: boolean) => void - ) => void; + handleConfigureSubmit: (variables: ConfigureUnitData & { closeModalFn?: () => void }) => void; }; /** @@ -56,14 +51,12 @@ const HeaderTitle = ({ COURSE_BLOCK_NAMES.component.id, ].includes(currentItemData.category); - const onConfigureSubmit = (...arg) => { - handleConfigureSubmit( - currentItemData.id, - arg[0], - arg[1], - arg[2], - closeConfigureModal, - ); + const onConfigureSubmit = (variables: Omit) => { + handleConfigureSubmit({ + ...variables, + unitId: currentItemData.id, + closeModalFn: closeConfigureModal, + }); }; useEffect(() => { diff --git a/src/course-unit/hooks.tsx b/src/course-unit/hooks.tsx index 5db2e7037..82559a51b 100644 --- a/src/course-unit/hooks.tsx +++ b/src/course-unit/hooks.tsx @@ -14,6 +14,7 @@ import { useEventListener } from '@src/generic/hooks'; import { useIframe } from '@src/generic/hooks/context/hooks'; import { COURSE_BLOCK_NAMES, iframeMessageTypes } from '@src/constants'; +import { ConfigureUnitData } from '@src/course-outline/data/types'; import { messageTypes, PUBLISH_TYPES } from './constants'; import { createNewCourseXBlock, @@ -99,19 +100,17 @@ export const useCourseUnit = ({ dispatch(changeEditTitleFormOpen(!isTitleEditFormOpen)); }; - const handleConfigureSubmit = (id, isVisible, groupAccess, isDiscussionEnabled, closeModalFn) => { + const handleConfigureSubmit = (variables: ConfigureUnitData & { closeModalFn?: () => void }) => { dispatch(editCourseUnitVisibilityAndData( - id, + variables.unitId, PUBLISH_TYPES.republish, - isVisible, - groupAccess, - isDiscussionEnabled, - () => sendMessageToIframe(messageTypes.completeManageXBlockAccess, { locator: id }), + variables.isVisibleToStaffOnly, + variables.groupAccess, + variables.discussionEnabled, + () => sendMessageToIframe(messageTypes.completeManageXBlockAccess, { locator: variables.unitId }), blockId, )); - if (typeof closeModalFn === 'function') { - closeModalFn(); - } + variables.closeModalFn?.(); }; const handleTitleEditSubmit = (displayName) => { diff --git a/src/course-unit/xblock-container-iframe/index.tsx b/src/course-unit/xblock-container-iframe/index.tsx index d77768e07..c24872969 100644 --- a/src/course-unit/xblock-container-iframe/index.tsx +++ b/src/course-unit/xblock-container-iframe/index.tsx @@ -25,6 +25,8 @@ import VideoSelectorPage from '@src/editors/VideoSelectorPage'; import EditorPage from '@src/editors/EditorPage'; import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; +import { ConfigureUnitData } from '@src/course-outline/data/types'; +import { AccessManagedXBlockDataTypes } from '@src/data/types'; import { messageTypes } from '../constants'; import { fetchCourseSectionVerticalData, @@ -37,7 +39,6 @@ import { import messages from './messages'; import { XBlockContainerIframeProps, - AccessManagedXBlockDataTypes, } from './types'; import { formatAccessManagedXBlockData, getIframeUrl, getLegacyEditModalUrl } from './utils'; import { useUnitSidebarContext } from '../unit-sidebar/UnitSidebarContext'; @@ -69,7 +70,10 @@ const XBlockContainerIframe: FC = ({ const [blockType, setBlockType] = useState(''); const { useVideoGalleryFlow } = useWaffleFlags(courseId); const [newBlockId, setNewBlockId] = useState(''); - const [accessManagedXBlockData, setAccessManagedXBlockData] = useState({}); + const [ + accessManagedXBlockData, + setAccessManagedXBlockData, + ] = useState(undefined); const [iframeOffset, setIframeOffset] = useState(0); const [deleteXBlockId, setDeleteXBlockId] = useState(null); const [unlinkXBlockId, setUnlinkXBlockId] = useState(null); @@ -146,10 +150,14 @@ const XBlockContainerIframe: FC = ({ } }; - const onManageXBlockAccessSubmit = (...args: any[]) => { + const onManageXBlockAccessSubmit = (variables: Omit) => { if (configureXBlockId) { - handleConfigureSubmit(configureXBlockId, ...args, closeConfigureModal); - setAccessManagedXBlockData({}); + handleConfigureSubmit({ + unitId: configureXBlockId, + ...variables, + closeModalFn: closeConfigureModal, + }); + setAccessManagedXBlockData(undefined); } }; @@ -288,19 +296,17 @@ const XBlockContainerIframe: FC = ({ /> )} - {Object.keys(accessManagedXBlockData).length ? ( - { - closeConfigureModal(); - setAccessManagedXBlockData({}); - }} - onConfigureSubmit={onManageXBlockAccessSubmit} - currentItemData={accessManagedXBlockData as AccessManagedXBlockDataTypes} - isSelfPaced={false} - /> - ) : null} + { + closeConfigureModal(); + setAccessManagedXBlockData(undefined); + }} + onConfigureSubmit={onManageXBlockAccessSubmit} + currentItemData={accessManagedXBlockData} + isSelfPaced={false} + />