diff --git a/codecov.yml b/codecov.yml index 3202455e6..38a7b1695 100644 --- a/codecov.yml +++ b/codecov.yml @@ -11,4 +11,5 @@ coverage: ignore: - "src/grading-settings/grading-scale/react-ranger.js" - "src/generic/DraggableList/verticalSortableList.ts" + - "src/container-comparison/data/api.mock.ts" - "src/index.js" diff --git a/src/container-comparison/CompareContainersWidget.test.tsx b/src/container-comparison/CompareContainersWidget.test.tsx index 4e15022a8..4a40b0ba3 100644 --- a/src/container-comparison/CompareContainersWidget.test.tsx +++ b/src/container-comparison/CompareContainersWidget.test.tsx @@ -1,64 +1,79 @@ import userEvent from '@testing-library/user-event'; -import { mockGetContainerChildren, mockGetContainerMetadata } from '../library-authoring/data/api.mocks'; -import { initializeMocks, render, screen } from '../testUtils'; +import MockAdapter from 'axios-mock-adapter/types'; +import { getLibraryContainerApiUrl } from '@src/library-authoring/data/api'; +import { mockGetContainerChildren, mockGetContainerMetadata } from '@src/library-authoring/data/api.mocks'; +import { initializeMocks, render, screen } from '@src/testUtils'; import { CompareContainersWidget } from './CompareContainersWidget'; import { mockGetCourseContainerChildren } from './data/api.mock'; mockGetCourseContainerChildren.applyMock(); mockGetContainerChildren.applyMock(); +let axiosMock: MockAdapter; describe('CompareContainersWidget', () => { beforeEach(() => { - initializeMocks(); + ({ axiosMock } = initializeMocks()); }); test('renders the component with a title', async () => { + const url = getLibraryContainerApiUrl(mockGetContainerMetadata.sectionId); + axiosMock.onGet(url).reply(200, { publishedDisplayName: 'Test Title' }); render(); expect((await screen.findAllByText('Test Title')).length).toEqual(2); - expect((await screen.findAllByText('subsection block 0')).length).toEqual(2); + expect((await screen.findAllByText('subsection block 0')).length).toEqual(1); + expect((await screen.findAllByText('subsection block 00')).length).toEqual(1); expect((await screen.findAllByText('This subsection will be modified')).length).toEqual(3); expect((await screen.findAllByText('This subsection was modified')).length).toEqual(3); - expect((await screen.findAllByText('subsection block 1')).length).toEqual(2); - expect((await screen.findAllByText('subsection block 2')).length).toEqual(2); + expect((await screen.findAllByText('subsection block 1')).length).toEqual(1); + expect((await screen.findAllByText('subsection block 2')).length).toEqual(1); + expect((await screen.findAllByText('subsection block 11')).length).toEqual(1); + expect((await screen.findAllByText('subsection block 22')).length).toEqual(1); }); test('renders loading spinner when data is pending', async () => { + const url = getLibraryContainerApiUrl(mockGetContainerMetadata.sectionIdLoading); + axiosMock.onGet(url).reply(() => new Promise(() => {})); render(); - expect((await screen.findAllByText('Test Title')).length).toEqual(2); const spinner = await screen.findAllByRole('status'); - expect(spinner.length).toEqual(2); + expect(spinner.length).toEqual(4); expect(spinner[0].textContent).toEqual('Loading...'); expect(spinner[1].textContent).toEqual('Loading...'); + expect(spinner[2].textContent).toEqual('Loading...'); + expect(spinner[3].textContent).toEqual('Loading...'); }); test('calls onRowClick when a row is clicked and updates diff view', async () => { + // mocks title + axiosMock.onGet(getLibraryContainerApiUrl(mockGetContainerMetadata.sectionId)).reply(200, { publishedDisplayName: 'Test Title' }); + axiosMock.onGet( + getLibraryContainerApiUrl('lct:org1:Demo_course_generated:subsection:subsection-0'), + ).reply(200, { publishedDisplayName: 'subsection block 0' }); + const user = userEvent.setup(); render(); expect((await screen.findAllByText('Test Title')).length).toEqual(2); - let blocks = await screen.findAllByText('subsection block 0'); - expect(blocks.length).toEqual(2); - await user.click(blocks[0]); - // Breadcrumbs - const breadcrumbs = await screen.findAllByRole('button', { name: 'subsection block 0' }); - expect(breadcrumbs.length).toEqual(2); + // left i.e. before side block + let block = await screen.findByText('subsection block 00'); + await user.click(block); + // Breadcrumbs - shows old and new name + expect(await screen.findByRole('button', { name: 'subsection block 00' })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument(); const removedRows = await screen.findAllByText('This unit was removed'); // clicking on removed or added rows does not updated the page. await user.click(removedRows[0]); // Still in same page - expect((await screen.findAllByRole('button', { name: 'subsection block 0' })).length).toEqual(2); + expect(await screen.findByRole('button', { name: 'subsection block 00' })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument(); // Back breadcrumb const backbtns = await screen.findAllByRole('button', { name: 'Back' }); @@ -67,11 +82,12 @@ describe('CompareContainersWidget', () => { // Go back await user.click(backbtns[0]); expect((await screen.findAllByText('Test Title')).length).toEqual(2); - blocks = await screen.findAllByText('subsection block 0'); - expect(blocks.length).toEqual(2); + // right i.e. after side block + block = await screen.findByText('subsection block 0'); // After side click also works - await user.click(blocks[1]); - expect((await screen.findAllByRole('button', { name: 'subsection block 0' })).length).toEqual(2); + await user.click(block); + expect(await screen.findByRole('button', { name: 'subsection block 00' })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument(); }); }); diff --git a/src/container-comparison/CompareContainersWidget.tsx b/src/container-comparison/CompareContainersWidget.tsx index ca515d5d9..ad83d495f 100644 --- a/src/container-comparison/CompareContainersWidget.tsx +++ b/src/container-comparison/CompareContainersWidget.tsx @@ -4,8 +4,9 @@ import { import { ArrowBack } from '@openedx/paragon/icons'; import { useCallback, useMemo, useState } from 'react'; import { ContainerType } from '@src/generic/key-utils'; +import ErrorAlert from '@src/generic/alert-error'; import { LoadingSpinner } from '@src/generic/Loading'; -import { useContainerChildren } from '@src/library-authoring/data/apiHooks'; +import { useContainer, useContainerChildren } from '@src/library-authoring/data/apiHooks'; import { useIntl } from '@edx/frontend-platform/i18n'; import ChildrenPreview from './ChildrenPreview'; import ContainerRow from './ContainerRow'; @@ -15,7 +16,6 @@ import { diffPreviewContainerChildren, isRowClickable } from './utils'; import messages from './messages'; interface ContainerInfoProps { - title: string; upstreamBlockId: string; downstreamBlockId: string; } @@ -30,7 +30,6 @@ interface Props extends ContainerInfoProps { * Actual implementation of the displaying diff between children of containers. */ const CompareContainersWidgetInner = ({ - title, upstreamBlockId, downstreamBlockId, parent, @@ -38,11 +37,17 @@ const CompareContainersWidgetInner = ({ onBackBtnClick, }: Props) => { const intl = useIntl(); - const { data, isPending } = useCourseContainerChildren(downstreamBlockId); + const { data, isError, error } = useCourseContainerChildren(downstreamBlockId); const { data: libData, - isPending: libPending, + isError: isLibError, + error: libError, } = useContainerChildren(upstreamBlockId, true); + const { + data: containerData, + isError: isContainerTitleError, + error: containerTitleError, + } = useContainer(upstreamBlockId); const result = useMemo(() => { if (!data || !libData) { @@ -52,7 +57,7 @@ const CompareContainersWidgetInner = ({ }, [data, libData]); const renderBeforeChildren = useCallback(() => { - if (isPending) { + if (!result[0]) { return
; } @@ -67,10 +72,10 @@ const CompareContainersWidgetInner = ({ onClick={() => onRowClick(child)} /> )); - }, [isPending, result]); + }, [result]); const renderAfterChildren = useCallback(() => { - if (libPending || isPending) { + if (!result[1]) { return
; } @@ -84,9 +89,13 @@ const CompareContainersWidgetInner = ({ onClick={() => onRowClick(child)} /> )); - }, [libPending, isPending, result]); + }, [result]); + + const getTitleComponent = useCallback((title?: string | null) => { + if (!title) { + return
; + } - const getTitleComponent = () => { if (parent.length === 0) { return title; } @@ -95,6 +104,7 @@ const CompareContainersWidgetInner = ({ ariaLabel={intl.formatMessage(messages.breadcrumbAriaLabel)} links={[ { + // This raises failed prop-type error as label expects a string but it works without any issues label: Back, onClick: onBackBtnClick, variant: 'link', @@ -110,20 +120,24 @@ const CompareContainersWidgetInner = ({ linkAs={Button} /> ); - }; + }, [parent]); + + if (isError || isLibError || isContainerTitleError) { + return ; + } return (
- + {renderBeforeChildren()}
- + {renderAfterChildren()} @@ -137,11 +151,10 @@ const CompareContainersWidgetInner = ({ * and allows the user to select the container to view. This is a wrapper component that maintains current * source state. Actual implementation of the diff view is done by CompareContainersWidgetInner. */ -export const CompareContainersWidget = ({ title, upstreamBlockId, downstreamBlockId }: ContainerInfoProps) => { +export const CompareContainersWidget = ({ upstreamBlockId, downstreamBlockId }: ContainerInfoProps) => { const [currentContainerState, setCurrentContainerState] = useState({ - title, upstreamBlockId, downstreamBlockId, parent: [], @@ -153,11 +166,9 @@ export const CompareContainersWidget = ({ title, upstreamBlockId, downstreamBloc } setCurrentContainerState((prev) => ({ - title: row.name, upstreamBlockId: row.id!, downstreamBlockId: row.downstreamId!, parent: [...prev.parent, { - title: prev.title, upstreamBlockId: prev.upstreamBlockId, downstreamBlockId: prev.downstreamBlockId, }], @@ -172,7 +183,6 @@ export const CompareContainersWidget = ({ title, upstreamBlockId, downstreamBloc } const prevParent = prev.parent[prev.parent.length - 1]; return { - title: prevParent!.title, upstreamBlockId: prevParent!.upstreamBlockId, downstreamBlockId: prevParent!.downstreamBlockId, parent: prev.parent.slice(0, -1), @@ -182,7 +192,6 @@ export const CompareContainersWidget = ({ title, upstreamBlockId, downstreamBloc return ( { const numChildren: number = 3; let blockType: string; + let displayName: string; switch (containerId) { case mockGetCourseContainerChildren.unitId: blockType = 'text'; + displayName = 'unit block 00'; break; case mockGetCourseContainerChildren.sectionId: blockType = 'subsection'; + displayName = 'Test Title'; break; case mockGetCourseContainerChildren.subsectionId: blockType = 'unit'; + displayName = 'subsection block 00'; break; case mockGetCourseContainerChildren.unitIdLoading: case mockGetCourseContainerChildren.sectionIdLoading: @@ -25,6 +30,7 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi return new Promise(() => { }); default: blockType = 'unit'; + displayName = 'subsection block 00'; break; } const children = Array(numChildren).fill(mockGetCourseContainerChildren.childTemplate).map((child, idx) => ( @@ -32,7 +38,7 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi ...child, // Generate a unique ID for each child block to avoid "duplicate key" errors in tests id: `block-v1:UNIX+UX1+2025_T3+type@${blockType}+block@${idx}`, - name: `${blockType} block ${idx}`, + name: `${blockType} block ${idx}${idx}`, blockType, upstreamLink: { upstreamRef: `lct:org1:Demo_course_generated:${blockType}:${blockType}-${idx}`, @@ -47,6 +53,7 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi canPasteComponent: true, isPublished: false, children, + displayName, }); } mockGetCourseContainerChildren.unitId = 'block-v1:UNIX+UX1+2025_T3+type@unit+block@0'; diff --git a/src/container-comparison/messages.ts b/src/container-comparison/messages.ts index 1e08b4cbc..29e927694 100644 --- a/src/container-comparison/messages.ts +++ b/src/container-comparison/messages.ts @@ -1,6 +1,11 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ + error: { + id: 'course-authoring.container-comparison.diff.error.message', + defaultMessage: 'Unexpected error: Failed to fetch container data', + description: 'Generic error message', + }, removedDiffBeforeMessage: { id: 'course-authoring.container-comparison.diff.before.removed-message', defaultMessage: 'This {blockType} will be removed in the new version', diff --git a/src/course-unit/data/types.ts b/src/course-unit/data/types.ts index 5531ddc19..3a2b2fcc0 100644 --- a/src/course-unit/data/types.ts +++ b/src/course-unit/data/types.ts @@ -42,4 +42,5 @@ export interface CourseContainerChildrenData { canPasteComponent: boolean; children: ContainerChildData[], isPublished: boolean; + displayName: string; } diff --git a/src/course-unit/preview-changes/index.tsx b/src/course-unit/preview-changes/index.tsx index 44e620cb4..8e2e0530f 100644 --- a/src/course-unit/preview-changes/index.tsx +++ b/src/course-unit/preview-changes/index.tsx @@ -141,7 +141,6 @@ export const PreviewLibraryXBlockChanges = ({ if (blockData.isContainer) { return (