diff --git a/src/container-comparison/CompareContainersWidget.test.tsx b/src/container-comparison/CompareContainersWidget.test.tsx index 348aff0546..dcfe6cb8ed 100644 --- a/src/container-comparison/CompareContainersWidget.test.tsx +++ b/src/container-comparison/CompareContainersWidget.test.tsx @@ -71,13 +71,6 @@ describe('CompareContainersWidget', () => { 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.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' }); expect(backbtns.length).toEqual(2); @@ -94,6 +87,47 @@ describe('CompareContainersWidget', () => { expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument(); }); + test('should show removed container diff state', 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); + // left i.e. before side block + const block = await screen.findByText('subsection block 00'); + await user.click(block); + + const removedRows = await screen.findAllByText('This unit was removed'); + await user.click(removedRows[0]); + + expect(await screen.findByText('This unit has been removed')).toBeInTheDocument(); + }); + + test('should show new added container diff state', 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(); + const blocks = await screen.findAllByText('This subsection will be added in the new version'); + await user.click(blocks[0]); + + expect(await screen.findByText(/this subsection is new/i)).toBeInTheDocument(); + }); + test('should show alert if the only change is a single text component with local overrides', async () => { const url = getLibraryContainerApiUrl(mockGetContainerMetadata.sectionId); axiosMock.onGet(url).reply(200, { publishedDisplayName: 'Test Title' }); diff --git a/src/container-comparison/CompareContainersWidget.tsx b/src/container-comparison/CompareContainersWidget.tsx index 96fec4a51a..90f58e5f26 100644 --- a/src/container-comparison/CompareContainersWidget.tsx +++ b/src/container-comparison/CompareContainersWidget.tsx @@ -4,10 +4,10 @@ import { Alert, Breadcrumb, Button, Card, Icon, Stack, } from '@openedx/paragon'; -import { ArrowBack } from '@openedx/paragon/icons'; +import { ArrowBack, Add, Delete } from '@openedx/paragon/icons'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; -import { ContainerType } from '@src/generic/key-utils'; +import { ContainerType, getBlockType } from '@src/generic/key-utils'; import ErrorAlert from '@src/generic/alert-error'; import { LoadingSpinner } from '@src/generic/Loading'; import { useContainer, useContainerChildren } from '@src/library-authoring/data/apiHooks'; @@ -16,7 +16,9 @@ import { BoldText } from '@src/utils'; import ChildrenPreview from './ChildrenPreview'; import ContainerRow from './ContainerRow'; import { useCourseContainerChildren } from './data/apiHooks'; -import { ContainerChild, ContainerChildBase, WithState } from './types'; +import { + ContainerChild, ContainerChildBase, ContainerState, WithState, +} from './types'; import { diffPreviewContainerChildren, isRowClickable } from './utils'; import messages from './messages'; @@ -30,6 +32,7 @@ interface Props extends ContainerInfoProps { parent: ContainerInfoProps[]; onRowClick: (row: WithState) => void; onBackBtnClick: () => void; + state?: ContainerState; // This two props are used to show an alert for the changes to text components with local overrides. // They may be removed in the future. localUpdateAlertCount: number; @@ -43,6 +46,7 @@ const CompareContainersWidgetInner = ({ upstreamBlockId, downstreamBlockId, parent, + state, onRowClick, onBackBtnClick, localUpdateAlertCount, @@ -50,11 +54,13 @@ const CompareContainersWidgetInner = ({ }: Props) => { const intl = useIntl(); const { data, isError, error } = useCourseContainerChildren(downstreamBlockId, parent.length === 0); + // There is the case in which the item is removed, but it still exists + // in the library, for that case, we avoid bringing the children. const { data: libData, isError: isLibError, error: libError, - } = useContainerChildren(upstreamBlockId, true); + } = useContainerChildren(state === 'removed' ? undefined : upstreamBlockId, true); const { data: containerData, isError: isContainerTitleError, @@ -62,17 +68,32 @@ const CompareContainersWidgetInner = ({ } = useContainer(upstreamBlockId); const result = useMemo(() => { - if (!data || !libData) { + if ((!data || !libData) && !['added', 'removed'].includes(state || '')) { return [undefined, undefined]; } - return diffPreviewContainerChildren(data.children, libData as ContainerChildBase[]); + + return diffPreviewContainerChildren(data?.children || [], libData as ContainerChildBase[] || []); }, [data, libData]); const renderBeforeChildren = useCallback(() => { - if (!result[0]) { + if (!result[0] && state !== 'added') { return
; } + if (state === 'added') { + return ( + + + + + ); + } + return result[0]?.map((child) => ( { - if (!result[1]) { + if (!result[1] && state !== 'removed') { return
; } + if (state === 'removed') { + return ( + + + + + ); + } + return result[1]?.map((child) => ( ; } return ( -
+
{localUpdateAlertCount > 0 && ( )}
- - + + {renderBeforeChildren()}
- - + + {renderAfterChildren()} @@ -181,12 +225,14 @@ export const CompareContainersWidget = ({ isReadyToSyncIndividually = false, }: ContainerInfoProps) => { const [currentContainerState, setCurrentContainerState] = useState({ - upstreamBlockId, - downstreamBlockId, - parent: [], - }); + upstreamBlockId, + downstreamBlockId, + parent: [], + state: 'modified', + }); const { data } = useCourseContainerChildren(downstreamBlockId, true); let localUpdateAlertBlockName = ''; @@ -213,9 +259,11 @@ export const CompareContainersWidget = ({ setCurrentContainerState((prev) => ({ upstreamBlockId: row.id!, downstreamBlockId: row.downstreamId!, + state: row.state, parent: [...prev.parent, { upstreamBlockId: prev.upstreamBlockId, downstreamBlockId: prev.downstreamBlockId, + state: prev.state, }], })); }; @@ -230,6 +278,7 @@ export const CompareContainersWidget = ({ return { upstreamBlockId: prevParent!.upstreamBlockId, downstreamBlockId: prevParent!.downstreamBlockId, + state: prevParent!.state, parent: prev.parent.slice(0, -1), }; }); @@ -240,6 +289,7 @@ export const CompareContainersWidget = ({ upstreamBlockId={currentContainerState.upstreamBlockId} downstreamBlockId={currentContainerState.downstreamBlockId} parent={currentContainerState.parent} + state={currentContainerState.state} onRowClick={onRowClick} onBackBtnClick={onBackBtnClick} localUpdateAlertCount={localUpdateAlertCount} diff --git a/src/container-comparison/ContainerRow.test.tsx b/src/container-comparison/ContainerRow.test.tsx index dfd811a6f2..669c9a7c78 100644 --- a/src/container-comparison/ContainerRow.test.tsx +++ b/src/container-comparison/ContainerRow.test.tsx @@ -29,20 +29,6 @@ describe('', () => { )).toBeInTheDocument(); }); - test('is not clickable when state !== modified', async () => { - const onClick = jest.fn(); - render(); - const titleDiv = await screen.findByText('Test title'); - const card = titleDiv.closest('.clickable'); - expect(card).toBe(null); - }); - test('calls onClick when clicked', async () => { const onClick = jest.fn(); const user = userEvent.setup(); diff --git a/src/container-comparison/data/api.mock.ts b/src/container-comparison/data/api.mock.ts index e27e23681f..49dccb95fc 100644 --- a/src/container-comparison/data/api.mock.ts +++ b/src/container-comparison/data/api.mock.ts @@ -8,7 +8,7 @@ import * as unitApi from '@src/course-unit/data/api'; * This mock returns a fixed response for the given container ID. */ export async function mockGetCourseContainerChildren(containerId: string): Promise { - const numChildren: number = 3; + let numChildren: number = 3; let blockType: string; let displayName: string; let upstreamReadyToSyncChildrenInfo: UpstreamReadyToSyncChildrenInfo[] = []; @@ -61,8 +61,9 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi case mockGetCourseContainerChildren.subsectionIdLoading: return new Promise(() => { }); default: - blockType = 'unit'; - displayName = 'subsection block 00'; + blockType = 'section'; + displayName = 'section block 00'; + numChildren = 0; break; } const children = Array(numChildren).fill(mockGetCourseContainerChildren.childTemplate).map((child, idx) => ( diff --git a/src/container-comparison/data/apiHooks.ts b/src/container-comparison/data/apiHooks.ts index 053ee47fab..dd78e37a99 100644 --- a/src/container-comparison/data/apiHooks.ts +++ b/src/container-comparison/data/apiHooks.ts @@ -11,7 +11,10 @@ export const containerComparisonQueryKeys = { /** * Key for a single container */ - container: (usageKey: string, getUpstreamInfo: boolean) => { + container: (getUpstreamInfo: boolean, usageKey?: string) => { + if (usageKey === undefined) { + return [undefined, undefined, getUpstreamInfo.toString()]; + } const courseKey = getCourseKey(usageKey); return [...containerComparisonQueryKeys.course(courseKey), usageKey, getUpstreamInfo.toString()]; }, @@ -21,6 +24,7 @@ export const useCourseContainerChildren = (usageKey?: string, getUpstreamInfo?: useQuery({ enabled: !!usageKey, queryFn: () => getCourseContainerChildren(usageKey!, getUpstreamInfo), - queryKey: containerComparisonQueryKeys.container(usageKey!, getUpstreamInfo || false), + // If we first get data with a valid `usageKey` and then the `usageKey` changes to undefined, an error occurs. + queryKey: containerComparisonQueryKeys.container(getUpstreamInfo || false, usageKey), }) ); diff --git a/src/container-comparison/index.scss b/src/container-comparison/index.scss new file mode 100644 index 0000000000..cc47033f0a --- /dev/null +++ b/src/container-comparison/index.scss @@ -0,0 +1,10 @@ +.compare-changes-widget { + .compare-card { + min-height: 350px; + } + + .big-icon { + height: 68px; + width: 68px; + } +} diff --git a/src/container-comparison/messages.ts b/src/container-comparison/messages.ts index 1a8e61d2cb..549824c90d 100644 --- a/src/container-comparison/messages.ts +++ b/src/container-comparison/messages.ts @@ -86,6 +86,16 @@ const messages = defineMessages({ defaultMessage: 'The only change is to {count, plural, one {text block {blockName} which has been edited} other {{count} text blocks which have been edited}} in this course. Accepting will not remove local edits.', description: 'Alert to show if the only change is on text components with local overrides.', }, + newContainer: { + id: 'course-authoring.container-comparison.new-container.text', + defaultMessage: 'This {containerType} is new', + description: 'Text to show in the comparison when a container is new.', + }, + deletedContainer: { + id: 'course-authoring.container-comparison.deleted-container.text', + defaultMessage: 'This {containerType} has been removed', + description: 'Text to show in the comparison when a container is removed.', + }, }); export default messages; diff --git a/src/container-comparison/utils.ts b/src/container-comparison/utils.ts index ab0fa14439..2d4ddb4d21 100644 --- a/src/container-comparison/utils.ts +++ b/src/container-comparison/utils.ts @@ -132,7 +132,7 @@ export function diffPreviewContainerChildren div > div.highlight) {