From 25160347b376ffc9c911b69d359667997522fc74 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 25 Sep 2025 21:43:58 +0530 Subject: [PATCH] feat: container library-course sync diff prevew [FC-0097] (#2464) Container sync preview implemented --- src/container-comparison/ChildrenPreview.tsx | 26 ++ .../CompareContainersWidget.test.tsx | 77 +++++ .../CompareContainersWidget.tsx | 193 ++++++++++++ .../ContainerRow.test.tsx | 99 ++++++ src/container-comparison/ContainerRow.tsx | 102 +++++++ src/container-comparison/data/api.mock.ts | 73 +++++ src/container-comparison/data/apiHooks.ts | 26 ++ src/container-comparison/messages.ts | 71 +++++ src/container-comparison/types.ts | 31 ++ src/container-comparison/utils.test.ts | 283 ++++++++++++++++++ src/container-comparison/utils.ts | 130 ++++++++ src/course-outline/data/apiHooks.ts | 2 +- src/course-outline/hooks.jsx | 5 + .../section-card/SectionCard.test.tsx | 18 +- .../subsection-card/SubsectionCard.test.tsx | 10 +- .../unit-card/UnitCard.test.tsx | 15 +- src/course-unit/data/{api.js => api.ts} | 91 ++---- src/course-unit/data/thunk.js | 22 +- src/course-unit/data/types.ts | 45 +++ .../preview-changes/index.test.tsx | 2 +- src/course-unit/preview-changes/index.tsx | 36 ++- src/data/types.ts | 11 + src/generic/key-utils.test.ts | 17 ++ src/generic/key-utils.ts | 16 + .../CompareChangesWidget.tsx | 48 +-- .../component-comparison/messages.ts | 5 - src/library-authoring/data/api.mocks.ts | 3 +- 27 files changed, 1318 insertions(+), 139 deletions(-) create mode 100644 src/container-comparison/ChildrenPreview.tsx create mode 100644 src/container-comparison/CompareContainersWidget.test.tsx create mode 100644 src/container-comparison/CompareContainersWidget.tsx create mode 100644 src/container-comparison/ContainerRow.test.tsx create mode 100644 src/container-comparison/ContainerRow.tsx create mode 100644 src/container-comparison/data/api.mock.ts create mode 100644 src/container-comparison/data/apiHooks.ts create mode 100644 src/container-comparison/messages.ts create mode 100644 src/container-comparison/types.ts create mode 100644 src/container-comparison/utils.test.ts create mode 100644 src/container-comparison/utils.ts rename src/course-unit/data/{api.js => api.ts} (56%) create mode 100644 src/course-unit/data/types.ts diff --git a/src/container-comparison/ChildrenPreview.tsx b/src/container-comparison/ChildrenPreview.tsx new file mode 100644 index 000000000..f9c94bebf --- /dev/null +++ b/src/container-comparison/ChildrenPreview.tsx @@ -0,0 +1,26 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; +import { Stack } from '@openedx/paragon'; +import messages from './messages'; + +interface Props { + title: React.ReactNode; + children: React.ReactNode; + side: 'Before' | 'After'; +} + +const ChildrenPreview = ({ title, children, side }: Props) => { + const intl = useIntl(); + const sideTitle = side === 'Before' + ? intl.formatMessage(messages.diffBeforeTitle) + : intl.formatMessage(messages.diffAfterTitle); + + return ( + + {sideTitle} + {title} + {children} + + ); +}; + +export default ChildrenPreview; diff --git a/src/container-comparison/CompareContainersWidget.test.tsx b/src/container-comparison/CompareContainersWidget.test.tsx new file mode 100644 index 000000000..4e15022a8 --- /dev/null +++ b/src/container-comparison/CompareContainersWidget.test.tsx @@ -0,0 +1,77 @@ +import userEvent from '@testing-library/user-event'; +import { mockGetContainerChildren, mockGetContainerMetadata } from '../library-authoring/data/api.mocks'; +import { initializeMocks, render, screen } from '../testUtils'; +import { CompareContainersWidget } from './CompareContainersWidget'; +import { mockGetCourseContainerChildren } from './data/api.mock'; + +mockGetCourseContainerChildren.applyMock(); +mockGetContainerChildren.applyMock(); + +describe('CompareContainersWidget', () => { + beforeEach(() => { + initializeMocks(); + }); + + test('renders the component with a title', async () => { + render(); + expect((await screen.findAllByText('Test Title')).length).toEqual(2); + expect((await screen.findAllByText('subsection block 0')).length).toEqual(2); + 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); + }); + + test('renders loading spinner when data is pending', async () => { + render(); + expect((await screen.findAllByText('Test Title')).length).toEqual(2); + const spinner = await screen.findAllByRole('status'); + expect(spinner.length).toEqual(2); + expect(spinner[0].textContent).toEqual('Loading...'); + expect(spinner[1].textContent).toEqual('Loading...'); + }); + + test('calls onRowClick when a row is clicked and updates diff view', async () => { + 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); + + 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); + + // Back breadcrumb + const backbtns = await screen.findAllByRole('button', { name: 'Back' }); + expect(backbtns.length).toEqual(2); + + // 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); + + // After side click also works + await user.click(blocks[1]); + expect((await screen.findAllByRole('button', { name: 'subsection block 0' })).length).toEqual(2); + }); +}); diff --git a/src/container-comparison/CompareContainersWidget.tsx b/src/container-comparison/CompareContainersWidget.tsx new file mode 100644 index 000000000..ca515d5d9 --- /dev/null +++ b/src/container-comparison/CompareContainersWidget.tsx @@ -0,0 +1,193 @@ +import { + Breadcrumb, Button, Card, Icon, Stack, +} from '@openedx/paragon'; +import { ArrowBack } from '@openedx/paragon/icons'; +import { useCallback, useMemo, useState } from 'react'; +import { ContainerType } from '@src/generic/key-utils'; +import { LoadingSpinner } from '@src/generic/Loading'; +import { useContainerChildren } from '@src/library-authoring/data/apiHooks'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import ChildrenPreview from './ChildrenPreview'; +import ContainerRow from './ContainerRow'; +import { useCourseContainerChildren } from './data/apiHooks'; +import { ContainerChild, ContainerChildBase, WithState } from './types'; +import { diffPreviewContainerChildren, isRowClickable } from './utils'; +import messages from './messages'; + +interface ContainerInfoProps { + title: string; + upstreamBlockId: string; + downstreamBlockId: string; +} + +interface Props extends ContainerInfoProps { + parent: ContainerInfoProps[]; + onRowClick: (row: WithState) => void; + onBackBtnClick: () => void; +} + +/** + * Actual implementation of the displaying diff between children of containers. + */ +const CompareContainersWidgetInner = ({ + title, + upstreamBlockId, + downstreamBlockId, + parent, + onRowClick, + onBackBtnClick, +}: Props) => { + const intl = useIntl(); + const { data, isPending } = useCourseContainerChildren(downstreamBlockId); + const { + data: libData, + isPending: libPending, + } = useContainerChildren(upstreamBlockId, true); + + const result = useMemo(() => { + if (!data || !libData) { + return [undefined, undefined]; + } + return diffPreviewContainerChildren(data.children, libData as ContainerChildBase[]); + }, [data, libData]); + + const renderBeforeChildren = useCallback(() => { + if (isPending) { + return
; + } + + return result[0]?.map((child) => ( + onRowClick(child)} + /> + )); + }, [isPending, result]); + + const renderAfterChildren = useCallback(() => { + if (libPending || isPending) { + return
; + } + + return result[1]?.map((child) => ( + onRowClick(child)} + /> + )); + }, [libPending, isPending, result]); + + const getTitleComponent = () => { + if (parent.length === 0) { + return title; + } + return ( + Back, + onClick: onBackBtnClick, + variant: 'link', + className: 'px-0 text-gray-900', + }, + { + label: title, + variant: 'link', + className: 'px-0 text-gray-900', + disabled: true, + }, + ]} + linkAs={Button} + /> + ); + }; + + return ( +
+
+ + + {renderBeforeChildren()} + + +
+
+ + + {renderAfterChildren()} + + +
+
+ ); +}; + +/** + * CompareContainersWidget component. Displays a diff of set of child containers from two different sources + * 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) => { + const [currentContainerState, setCurrentContainerState] = useState({ + title, + upstreamBlockId, + downstreamBlockId, + parent: [], + }); + + const onRowClick = (row: WithState) => { + if (!isRowClickable(row.state, row.blockType as ContainerType)) { + return; + } + + setCurrentContainerState((prev) => ({ + title: row.name, + upstreamBlockId: row.id!, + downstreamBlockId: row.downstreamId!, + parent: [...prev.parent, { + title: prev.title, + upstreamBlockId: prev.upstreamBlockId, + downstreamBlockId: prev.downstreamBlockId, + }], + })); + }; + + const onBackBtnClick = () => { + setCurrentContainerState((prev) => { + // istanbul ignore if: this should never happen + if (prev.parent.length < 1) { + return prev; + } + const prevParent = prev.parent[prev.parent.length - 1]; + return { + title: prevParent!.title, + upstreamBlockId: prevParent!.upstreamBlockId, + downstreamBlockId: prevParent!.downstreamBlockId, + parent: prev.parent.slice(0, -1), + }; + }); + }; + + return ( + + ); +}; diff --git a/src/container-comparison/ContainerRow.test.tsx b/src/container-comparison/ContainerRow.test.tsx new file mode 100644 index 000000000..aa80a888e --- /dev/null +++ b/src/container-comparison/ContainerRow.test.tsx @@ -0,0 +1,99 @@ +import userEvent from '@testing-library/user-event'; +import { + fireEvent, initializeMocks, render, screen, +} from '../testUtils'; +import ContainerRow from './ContainerRow'; +import messages from './messages'; + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + test('renders with default props', async () => { + render(); + expect(await screen.findByText('Test title')).toBeInTheDocument(); + }); + + test('renders with modified state', async () => { + render(); + expect(await screen.findByText( + messages.modifiedDiffBeforeMessage.defaultMessage.replace('{blockType}', 'subsection'), + )).toBeInTheDocument(); + }); + + test('renders with removed state', async () => { + render(); + expect(await screen.findByText( + messages.removedDiffAfterMessage.defaultMessage.replace('{blockType}', 'subsection'), + )).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(); + render(); + const titleDiv = await screen.findByText('Test title'); + const card = titleDiv.closest('.clickable'); + expect(card).not.toBe(null); + await user.click(card!); + expect(onClick).toHaveBeenCalled(); + }); + + test('calls onClick when pressed enter or space', async () => { + const onClick = jest.fn(); + const user = userEvent.setup(); + render(); + const titleDiv = await screen.findByText('Test title'); + const card = titleDiv.closest('.clickable'); + expect(card).not.toBe(null); + fireEvent.select(card!); + await user.keyboard('{enter}'); + expect(onClick).toHaveBeenCalled(); + }); + + test('renders with originalName', async () => { + render(); + expect(await screen.findByText(messages.renamedDiffBeforeMessage.defaultMessage.replace('{name}', 'Modified name'))).toBeInTheDocument(); + }); + + test('renders with moved state', async () => { + render(); + expect(await screen.findByText( + messages.movedDiffAfterMessage.defaultMessage.replace('{blockType}', 'subsection'), + )).toBeInTheDocument(); + }); + + test('renders with added state', async () => { + render(); + expect(await screen.findByText( + messages.addedDiffAfterMessage.defaultMessage.replace('{blockType}', 'subsection'), + )).toBeInTheDocument(); + }); +}); diff --git a/src/container-comparison/ContainerRow.tsx b/src/container-comparison/ContainerRow.tsx new file mode 100644 index 000000000..723001ade --- /dev/null +++ b/src/container-comparison/ContainerRow.tsx @@ -0,0 +1,102 @@ +import { + ActionRow, Card, Icon, Stack, +} from '@openedx/paragon'; +import type { MessageDescriptor } from 'react-intl'; +import { useMemo } from 'react'; +import { + Cached, ChevronRight, Delete, Done, Plus, +} from '@openedx/paragon/icons'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import { getItemIcon } from '@src/generic/block-type-utils'; +import { ContainerType } from '@src/generic/key-utils'; +import { COMPONENT_TYPES } from '@src/generic/block-type-utils/constants'; +import messages from './messages'; +import { ContainerState } from './types'; +import { isRowClickable } from './utils'; + +export interface ContainerRowProps { + title: string; + containerType: ContainerType | keyof typeof COMPONENT_TYPES | string; + state?: ContainerState; + side: 'Before' | 'After'; + originalName?: string; + onClick?: () => void; +} + +const ContainerRow = ({ + title, containerType, state, side, originalName, onClick, +}: ContainerRowProps) => { + const isClickable = isRowClickable(state, containerType as ContainerType); + const stateContext = useMemo(() => { + let message: MessageDescriptor | undefined; + switch (state) { + case 'added': + message = side === 'Before' ? messages.addedDiffBeforeMessage : messages.addedDiffAfterMessage; + return ['text-white bg-success-500', Plus, message]; + case 'modified': + message = side === 'Before' ? messages.modifiedDiffBeforeMessage : messages.modifiedDiffAfterMessage; + return ['text-white bg-warning-900', Cached, message]; + case 'removed': + message = side === 'Before' ? messages.removedDiffBeforeMessage : messages.removedDiffAfterMessage; + return ['text-white bg-danger-600', Delete, message]; + case 'locallyRenamed': + message = side === 'Before' ? messages.renamedDiffBeforeMessage : messages.renamedDiffAfterMessage; + return ['bg-light-300 text-light-300 ', Done, message]; + case 'moved': + message = side === 'Before' ? messages.movedDiffBeforeMessage : messages.movedDiffAfterMessage; + return ['bg-light-300 text-light-300', Done, message]; + default: + return ['bg-light-300 text-light-300', Done, message]; + } + }, [state, side]); + + return ( + { + if (e.key === 'Enter' || e.key === ' ') { + onClick?.(); + } + }} + className="mb-2 rounded shadow-sm border border-light-100" + > + +
+ +
+ + + + + {title} + + {stateContext[2] ? ( + + + + ) : ( +   + )} + + + {isClickable && } + + + + ); +}; + +export default ContainerRow; diff --git a/src/container-comparison/data/api.mock.ts b/src/container-comparison/data/api.mock.ts new file mode 100644 index 000000000..61afa6e70 --- /dev/null +++ b/src/container-comparison/data/api.mock.ts @@ -0,0 +1,73 @@ +import { CourseContainerChildrenData } from '@src/course-unit/data/types'; +import * as unitApi from '@src/course-unit/data/api'; + +/** + * Mock for `getLibraryContainerChildren()` + * + * This mock returns a fixed response for the given container ID. + */ +export async function mockGetCourseContainerChildren(containerId: string): Promise { + const numChildren: number = 3; + let blockType: string; + switch (containerId) { + case mockGetCourseContainerChildren.unitId: + blockType = 'text'; + break; + case mockGetCourseContainerChildren.sectionId: + blockType = 'subsection'; + break; + case mockGetCourseContainerChildren.subsectionId: + blockType = 'unit'; + break; + case mockGetCourseContainerChildren.unitIdLoading: + case mockGetCourseContainerChildren.sectionIdLoading: + case mockGetCourseContainerChildren.subsectionIdLoading: + return new Promise(() => { }); + default: + blockType = 'unit'; + break; + } + const children = Array(numChildren).fill(mockGetCourseContainerChildren.childTemplate).map((child, idx) => ( + { + ...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}`, + blockType, + upstreamLink: { + upstreamRef: `lct:org1:Demo_course_generated:${blockType}:${blockType}-${idx}`, + versionSynced: 1, + versionAvailable: 2, + versionDeclined: null, + isModified: false, + }, + } + )); + return Promise.resolve({ + canPasteComponent: true, + isPublished: false, + children, + }); +} +mockGetCourseContainerChildren.unitId = 'block-v1:UNIX+UX1+2025_T3+type@unit+block@0'; +mockGetCourseContainerChildren.subsectionId = 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0'; +mockGetCourseContainerChildren.sectionId = 'block-v1:UNIX+UX1+2025_T3+type@section+block@0'; +mockGetCourseContainerChildren.unitIdLoading = 'block-v1:UNIX+UX1+2025_T3+type@unit+block@loading'; +mockGetCourseContainerChildren.subsectionIdLoading = 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@loading'; +mockGetCourseContainerChildren.sectionIdLoading = 'block-v1:UNIX+UX1+2025_T3+type@section+block@loading'; +mockGetCourseContainerChildren.childTemplate = { + id: 'block-v1:UNIX+UX1+2025_T3+type@unit+block@1', + name: 'Unit 1 remote edit - local edit', + blockType: 'unit', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-1-2a1741', + versionSynced: 1, + versionAvailable: 2, + versionDeclined: null, + isModified: false, + }, +}; +/** Apply this mock. Returns a spy object that can tell you if it's been called. */ +mockGetCourseContainerChildren.applyMock = () => { + jest.spyOn(unitApi, 'getCourseContainerChildren').mockImplementation(mockGetCourseContainerChildren); +}; diff --git a/src/container-comparison/data/apiHooks.ts b/src/container-comparison/data/apiHooks.ts new file mode 100644 index 000000000..92ea4aecf --- /dev/null +++ b/src/container-comparison/data/apiHooks.ts @@ -0,0 +1,26 @@ +import { useQuery } from '@tanstack/react-query'; +import { getCourseContainerChildren } from '@src/course-unit/data/api'; +import { getCourseKey } from '@src/generic/key-utils'; + +export const containerComparisonQueryKeys = { + all: ['containerComparison'], + /** + * Base key for a course + */ + course: (courseKey: string) => [...containerComparisonQueryKeys.all, courseKey], + /** + * Key for a single container + */ + container: (usageKey: string) => { + const courseKey = getCourseKey(usageKey); + return [...containerComparisonQueryKeys.course(courseKey), usageKey]; + }, +}; + +export const useCourseContainerChildren = (usageKey?: string) => ( + useQuery({ + enabled: !!usageKey, + queryFn: () => getCourseContainerChildren(usageKey!), + queryKey: containerComparisonQueryKeys.container(usageKey!), + }) +); diff --git a/src/container-comparison/messages.ts b/src/container-comparison/messages.ts new file mode 100644 index 000000000..1e08b4cbc --- /dev/null +++ b/src/container-comparison/messages.ts @@ -0,0 +1,71 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + removedDiffBeforeMessage: { + id: 'course-authoring.container-comparison.diff.before.removed-message', + defaultMessage: 'This {blockType} will be removed in the new version', + description: 'Description for removed component in before section of diff preview', + }, + removedDiffAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.removed-message', + defaultMessage: 'This {blockType} was removed', + description: 'Description for removed component in after section of diff preview', + }, + modifiedDiffBeforeMessage: { + id: 'course-authoring.container-comparison.diff.before.modified-message', + defaultMessage: 'This {blockType} will be modified', + description: 'Description for modified component in before section of diff preview', + }, + modifiedDiffAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.modified-message', + defaultMessage: 'This {blockType} was modified', + description: 'Description for modified component in after section of diff preview', + }, + addedDiffBeforeMessage: { + id: 'course-authoring.container-comparison.diff.before.added-message', + defaultMessage: 'This {blockType} will be added in the new version', + description: 'Description for added component in before section of diff preview', + }, + addedDiffAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.added-message', + defaultMessage: 'This {blockType} was added', + description: 'Description for added component in after section of diff preview', + }, + renamedDiffBeforeMessage: { + id: 'course-authoring.container-comparison.diff.before.renamed-message', + defaultMessage: 'Original Library Name: {name}', + description: 'Description for renamed component in before section of diff preview', + }, + renamedDiffAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.renamed-message', + defaultMessage: 'This {blockType} will remain renamed', + description: 'Description for renamed component in after section of diff preview', + }, + movedDiffBeforeMessage: { + id: 'course-authoring.container-comparison.diff.before.moved-message', + defaultMessage: 'This {blockType} will be moved in the new version', + description: 'Description for moved component in before section of diff preview', + }, + movedDiffAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.moved-message', + defaultMessage: 'This {blockType} was moved', + description: 'Description for moved component in after section of diff preview', + }, + breadcrumbAriaLabel: { + id: 'course-authoring.container-comparison.diff.breadcrumb.ariaLabel', + defaultMessage: 'Title breadcrumb', + description: 'Aria label text for breadcrumb in diff preview', + }, + diffBeforeTitle: { + id: 'course-authoring.container-comparison.diff.before.title', + defaultMessage: 'Before', + description: 'Before section title text', + }, + diffAfterTitle: { + id: 'course-authoring.container-comparison.diff.after.title', + defaultMessage: 'After', + description: 'After section title text', + }, +}); + +export default messages; diff --git a/src/container-comparison/types.ts b/src/container-comparison/types.ts new file mode 100644 index 000000000..4b01770a7 --- /dev/null +++ b/src/container-comparison/types.ts @@ -0,0 +1,31 @@ +import { UpstreamInfo } from '@src/data/types'; + +export type ContainerState = 'removed' | 'added' | 'modified' | 'childrenModified' | 'locallyRenamed' | 'moved'; + +export type WithState = T & { state?: ContainerState, originalName?: string }; +export type WithIndex = T & { index: number }; + +export type CourseContainerChildBase = { + name: string; + id: string; + upstreamLink: UpstreamInfo; + blockType: string; +}; + +export type ContainerChildBase = { + displayName: string; + id: string; + containerType?: string; + blockType?: string; +} & ({ + containerType: string; +} | { + blockType: string; +}); + +export type ContainerChild = { + name: string; + id?: string; + downstreamId?: string; + blockType: string; +}; diff --git a/src/container-comparison/utils.test.ts b/src/container-comparison/utils.test.ts new file mode 100644 index 000000000..2b732b41e --- /dev/null +++ b/src/container-comparison/utils.test.ts @@ -0,0 +1,283 @@ +import { ContainerChildBase, CourseContainerChildBase } from './types'; +import { diffPreviewContainerChildren } from './utils'; + +export const getMockCourseContainerData = ( + type: 'added|deleted' | 'moved|deleted' | 'all', +): [CourseContainerChildBase[], ContainerChildBase[]] => { + switch (type) { + case 'moved|deleted': + return [ + [ + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@1', + name: 'Unit 1 remote edit - local edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-1-2a1741', + versionSynced: 11, + versionAvailable: 11, + versionDeclined: null, + isModified: true, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@2', + name: 'New unit remote edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:new-unit-remote-7eb9d1', + versionSynced: 7, + versionAvailable: 7, + versionDeclined: null, + isModified: false, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@3', + name: 'Unit with tags', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + versionSynced: 2, + versionAvailable: 2, + versionDeclined: null, + isModified: false, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@4', + name: 'One more unit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:one-more-unit-745176', + versionSynced: 1, + versionAvailable: 1, + versionDeclined: null, + isModified: false, + }, + }, + ], + [ + { + id: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + displayName: 'Unit with tags', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:unit-1-2a1741', + displayName: 'Unit 1 remote edit 2', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:one-more-unit-745176', + displayName: 'One more unit', + containerType: 'unit', + }, + ], + ] as [CourseContainerChildBase[], ContainerChildBase[]]; + case 'added|deleted': + return [ + [ + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@1', + name: 'Unit 1 remote edit - local edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-1-2a1741', + versionSynced: 11, + versionAvailable: 11, + versionDeclined: null, + isModified: true, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@2', + name: 'New unit remote edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:new-unit-remote-7eb9d1', + versionSynced: 7, + versionAvailable: 7, + versionDeclined: null, + isModified: false, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@3', + name: 'Unit with tags', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + versionSynced: 2, + versionAvailable: 2, + versionDeclined: null, + isModified: false, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@4', + name: 'One more unit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:one-more-unit-745176', + versionSynced: 1, + versionAvailable: 1, + versionDeclined: null, + isModified: false, + }, + }, + ], + [ + { + id: 'lct:UNIX:CS1:unit:unit-1-2a1741', + displayName: 'Unit 1 remote edit 2', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + displayName: 'Unit with tags', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:added-unit-1', + displayName: 'Added unit', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:one-more-unit-745176', + displayName: 'One more unit', + containerType: 'unit', + }, + ], + ] as [CourseContainerChildBase[], ContainerChildBase[]]; + case 'all': + return [ + [ + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@1', + name: 'Unit 1 remote edit - local edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-1-2a1741', + versionSynced: 11, + versionAvailable: 11, + versionDeclined: null, + isModified: true, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@2', + name: 'New unit remote edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:new-unit-remote-7eb9d1', + versionSynced: 7, + versionAvailable: 7, + versionDeclined: null, + isModified: false, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@3', + name: 'Unit with tags', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + versionSynced: 2, + versionAvailable: 2, + versionDeclined: null, + isModified: false, + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@4', + name: 'One more unit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:one-more-unit-745176', + versionSynced: 1, + versionAvailable: 1, + versionDeclined: null, + isModified: false, + }, + }, + ], + [ + { + id: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + displayName: 'Unit with tags', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:added-unit-1', + displayName: 'Added unit', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:one-more-unit-745176', + displayName: 'One more unit', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:unit-1-2a1741', + displayName: 'Unit 1 remote edit 2', + containerType: 'unit', + }, + ], + ] as [CourseContainerChildBase[], ContainerChildBase[]]; + default: + throw new Error(); + } +}; + +describe('diffPreviewContainerChildren', () => { + it('should handle moved and deleted', () => { + const [a, b] = getMockCourseContainerData('moved|deleted'); + const result = diffPreviewContainerChildren(a as CourseContainerChildBase[], b); + expect(result[0].length).toEqual(result[1].length); + // renamed takes precendence over moved + expect(result[0][0].state).toEqual('locallyRenamed'); + expect(result[1][2].state).toEqual('locallyRenamed'); + expect(result[0][1].state).toEqual('removed'); + expect(result[1][1].state).toEqual('removed'); + expect(result[1][2].name).toEqual(a[0].name); + }); + + it('should handle add and delete', () => { + const [a, b] = getMockCourseContainerData('added|deleted'); + const result = diffPreviewContainerChildren(a as CourseContainerChildBase[], b); + expect(result[0].length).toEqual(result[1].length); + // No change, state=undefined + expect(result[0][0].state).toEqual('locallyRenamed'); + expect(result[0][0].originalName).toEqual(b[0].displayName); + expect(result[1][0].state).toEqual('locallyRenamed'); + + // Deleted entry + expect(result[0][1].state).toEqual('removed'); + expect(result[1][1].state).toEqual('removed'); + expect(result[1][0].name).toEqual(a[0].name); + expect(result[0][3].name).toEqual(result[1][3].name); + expect(result[0][3].state).toEqual('added'); + expect(result[1][3].state).toEqual('added'); + }); + + it('should handle add, delete and moved', () => { + const [a, b] = getMockCourseContainerData('all'); + const result = diffPreviewContainerChildren(a as CourseContainerChildBase[], b); + expect(result[0].length).toEqual(result[1].length); + // renamed takes precendence over moved + expect(result[0][0].state).toEqual('locallyRenamed'); + expect(result[1][4].state).toEqual('locallyRenamed'); + expect(result[1][4].id).toEqual(result[0][0].id); + + // Deleted entry + expect(result[0][1].state).toEqual('removed'); + expect(result[1][1].state).toEqual('removed'); + expect(result[1][1].name).toEqual(result[0][1].name); + + // added entry + expect(result[0][2].state).toEqual('added'); + expect(result[1][2].state).toEqual('added'); + expect(result[1][2].id).toEqual(result[0][2].id); + }); +}); diff --git a/src/container-comparison/utils.ts b/src/container-comparison/utils.ts new file mode 100644 index 000000000..7a15155d7 --- /dev/null +++ b/src/container-comparison/utils.ts @@ -0,0 +1,130 @@ +import { UpstreamInfo } from '@src/data/types'; +import { ContainerType, normalizeContainerType } from '@src/generic/key-utils'; +import { + ContainerChild, + ContainerChildBase, + ContainerState, + CourseContainerChildBase, + WithIndex, + WithState, +} from './types'; + +export function checkIsReadyToSync(link: UpstreamInfo): boolean { + return (link.versionSynced < (link.versionAvailable || 0)) + || (link.versionSynced < (link.versionDeclined || 0)) + || ((link.readyToSyncChildren?.length || 0) > 0); +} + +/** + * Compares two arrays of container children (`a` and `b`) to determine the differences between them. + * It generates two lists indicating which elements have been added, modified, moved, or removed. + */ +export function diffPreviewContainerChildren( + a: A[], + b: B[], + idKey: string = 'id', +): [WithState[], WithState[]] { + const mapA = new Map>(); + const mapB = new Map>(); + for (let index = 0; index < a.length; index++) { + const element = a[index]; + mapA.set(element.upstreamLink?.upstreamRef, { ...element, index }); + } + const updatedA: WithState[] = Array(a.length); + const addedA: Array> = []; + const updatedB: WithState[] = []; + for (let index = 0; index < b.length; index++) { + const newVersion = b[index]; + const oldVersion = mapA.get(newVersion.id); + if (!oldVersion) { + // This is a newly added component + addedA.push({ + id: newVersion.id, + name: newVersion.displayName, + blockType: (newVersion.containerType || newVersion.blockType)!, + index, + }); + updatedB.push({ + name: newVersion.displayName, + blockType: (newVersion.blockType || newVersion.containerType)!, + id: newVersion.id, + state: 'added', + }); + } else { + // It was present in previous version + let state: ContainerState | undefined; + const displayName = oldVersion.upstreamLink.isModified ? oldVersion.name : newVersion.displayName; + let originalName: string | undefined; + if (index !== oldVersion.index) { + // has moved from its position + state = 'moved'; + } + if (displayName !== newVersion.displayName && displayName === oldVersion.name) { + // Has been renamed + state = 'locallyRenamed'; + originalName = newVersion.displayName; + } + if (checkIsReadyToSync(oldVersion.upstreamLink)) { + // has a new version ready to sync + state = 'modified'; + } + // Insert in its original index + updatedA.splice(oldVersion.index, 1, { + name: oldVersion.name, + blockType: normalizeContainerType(oldVersion.blockType), + id: oldVersion.upstreamLink.upstreamRef, + downstreamId: oldVersion.id, + state, + originalName, + }); + updatedB.push({ + name: displayName, + blockType: (newVersion.blockType || newVersion.containerType)!, + id: newVersion.id, + downstreamId: oldVersion.id, + state, + }); + // Delete it from mapA as it is processed. + mapA.delete(newVersion.id); + } + } + + // If there are remaining items in mapA, it means they were deleted in newVersion; + mapA.forEach((oldVersion) => { + updatedA.splice(oldVersion.index, 1, { + name: oldVersion.name, + blockType: normalizeContainerType(oldVersion.blockType), + id: oldVersion.upstreamLink.upstreamRef, + downstreamId: oldVersion.id, + state: 'removed', + }); + updatedB.splice(oldVersion.index, 0, { + id: oldVersion.upstreamLink.upstreamRef, + name: oldVersion.name, + blockType: normalizeContainerType(oldVersion.blockType), + downstreamId: oldVersion.id, + state: 'removed', + }); + }); + + // Create a map for id with index of newly updatedB array + for (let index = 0; index < updatedB.length; index++) { + const element = updatedB[index]; + mapB.set(element[idKey], { ...element, index }); + } + + // Use new mapB for getting new index for added elements + addedA.forEach((addedRow) => { + updatedA.splice(mapB.get(addedRow.id)?.index!, 0, { ...addedRow, state: 'added' }); + }); + + return [updatedA, updatedB]; +} + +export function isRowClickable(state?: ContainerState, blockType?: ContainerType) { + return state === 'modified' && blockType && [ + ContainerType.Section, + ContainerType.Subsection, + ContainerType.Unit, + ].includes(blockType); +} diff --git a/src/course-outline/data/apiHooks.ts b/src/course-outline/data/apiHooks.ts index 6d9db3661..e4686f3c6 100644 --- a/src/course-outline/data/apiHooks.ts +++ b/src/course-outline/data/apiHooks.ts @@ -22,7 +22,7 @@ export const useCreateCourseBlock = ( ) => useMutation({ mutationFn: createCourseXblock, onSettled: async (data) => { - callback?.(data.locator, data.parent_locator); + callback?.(data?.locator, data.parent_locator); }, }); diff --git a/src/course-outline/hooks.jsx b/src/course-outline/hooks.jsx index 4ad0210c9..9ccfcde83 100644 --- a/src/course-outline/hooks.jsx +++ b/src/course-outline/hooks.jsx @@ -3,6 +3,7 @@ import { useDispatch, useSelector } from 'react-redux'; import { useNavigate } from 'react-router-dom'; import { useToggle } from '@openedx/paragon'; import { getConfig } from '@edx/frontend-platform'; +import { useQueryClient } from '@tanstack/react-query'; import moment from 'moment'; import { getSavingStatus as getGenericSavingStatus } from '@src/generic/data/selectors'; @@ -64,9 +65,11 @@ import { } from './data/thunk'; import { useCreateCourseBlock } from './data/apiHooks'; import { getCourseItem } from './data/api'; +import { containerComparisonQueryKeys } from '../container-comparison/data/apiHooks'; const useCourseOutline = ({ courseId }) => { const dispatch = useDispatch(); + const queryClient = useQueryClient(); const navigate = useNavigate(); const waffleFlags = useWaffleFlags(courseId); @@ -245,6 +248,8 @@ const useCourseOutline = ({ courseId }) => { const handleEditSubmit = (itemId, sectionId, displayName) => { dispatch(editCourseItemQuery(itemId, sectionId, displayName)); + // Invalidate container diff queries to update sync diff preview + queryClient.invalidateQueries({ queryKey: containerComparisonQueryKeys.course(courseId) }); }; const handleDeleteItemSubmit = () => { diff --git a/src/course-outline/section-card/SectionCard.test.tsx b/src/course-outline/section-card/SectionCard.test.tsx index ec73b0f2e..35e83057f 100644 --- a/src/course-outline/section-card/SectionCard.test.tsx +++ b/src/course-outline/section-card/SectionCard.test.tsx @@ -17,11 +17,11 @@ jest.mock('@src/course-unit/data/apiHooks', () => ({ })); const unit = { - id: 'unit-1', + id: 'block-v1:UNIX+UX1+2025_T3+type@unit+block@0', }; const subsection = { - id: '123', + id: 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0', displayName: 'Subsection Name', category: 'sequential', published: true, @@ -43,7 +43,7 @@ const subsection = { } satisfies Partial as XBlock; const section = { - id: '123', + id: 'block-v1:UNIX+UX1+2025_T3+type@section+block@0', displayName: 'Section Name', category: 'chapter', published: true, @@ -71,6 +71,8 @@ const section = { readyToSync: true, upstreamRef: 'lct:org1:lib1:section:1', versionSynced: 1, + versionAvailable: 2, + versionDeclined: null, errorMessage: null, }, } satisfies Partial as XBlock; @@ -186,7 +188,9 @@ describe('', () => { const collapsedSections = { ...section }; // @ts-ignore-next-line collapsedSections.isSectionsExpanded = false; - renderComponent(collapsedSections, `/course/:courseId?show=${subsection.id}`); + // url encode subsection.id + const subsectionIdUrl = encodeURIComponent(subsection.id); + renderComponent(collapsedSections, `/course/:courseId?show=${subsectionIdUrl}`); const cardSubsections = await screen.findByTestId('section-card__subsections'); const newSubsectionButton = await screen.findByRole('button', { name: 'New subsection' }); @@ -198,7 +202,9 @@ describe('', () => { const collapsedSections = { ...section }; // @ts-ignore-next-line collapsedSections.isSectionsExpanded = false; - renderComponent(collapsedSections, `/course/:courseId?show=${unit.id}`); + // url encode subsection.id + const unitIdUrl = encodeURIComponent(unit.id); + renderComponent(collapsedSections, `/course/:courseId?show=${unitIdUrl}`); const cardSubsections = await screen.findByTestId('section-card__subsections'); const newSubsectionButton = await screen.findByRole('button', { name: 'New subsection' }); @@ -230,7 +236,6 @@ describe('', () => { // Should open compare preview modal expect(screen.getByRole('heading', { name: /preview changes: section name/i })).toBeInTheDocument(); - expect(screen.getByText('Preview not available for container changes at this time')).toBeInTheDocument(); // Click on accept changes const acceptChangesButton = screen.getByText(/accept changes/i); @@ -250,7 +255,6 @@ describe('', () => { // Should open compare preview modal expect(screen.getByRole('heading', { name: /preview changes: section name/i })).toBeInTheDocument(); - expect(screen.getByText('Preview not available for container changes at this time')).toBeInTheDocument(); // Click on ignore changes const ignoreChangesButton = screen.getByRole('button', { name: /ignore changes/i }); diff --git a/src/course-outline/subsection-card/SubsectionCard.test.tsx b/src/course-outline/subsection-card/SubsectionCard.test.tsx index f3203671e..969852070 100644 --- a/src/course-outline/subsection-card/SubsectionCard.test.tsx +++ b/src/course-outline/subsection-card/SubsectionCard.test.tsx @@ -52,7 +52,7 @@ const unit = { }; const subsection: XBlock = { - id: '123', + id: 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0', displayName: 'Subsection Name', category: 'sequential', published: true, @@ -75,12 +75,14 @@ const subsection: XBlock = { readyToSync: true, upstreamRef: 'lct:org1:lib1:subsection:1', versionSynced: 1, + versionAvailable: 2, + versionDeclined: null, errorMessage: null, }, } satisfies Partial as XBlock; const section: XBlock = { - id: '123', + id: 'block-v1:UNIX+UX1+2025_T3+type@section+block@0', displayName: 'Section Name', published: true, visibilityState: 'live', @@ -321,7 +323,7 @@ describe('', () => { expect(handleOnAddUnitFromLibrary).toHaveBeenCalled(); expect(handleOnAddUnitFromLibrary).toHaveBeenCalledWith({ type: COMPONENT_TYPES.libraryV2, - parentLocator: '123', + parentLocator: 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0', category: 'vertical', libraryContentKey: containerKey, }); @@ -338,7 +340,6 @@ describe('', () => { // Should open compare preview modal expect(screen.getByRole('heading', { name: /preview changes: subsection name/i })).toBeInTheDocument(); - expect(screen.getByText('Preview not available for container changes at this time')).toBeInTheDocument(); // Click on accept changes const acceptChangesButton = screen.getByText(/accept changes/i); @@ -358,7 +359,6 @@ describe('', () => { // Should open compare preview modal expect(screen.getByRole('heading', { name: /preview changes: subsection name/i })).toBeInTheDocument(); - expect(screen.getByText('Preview not available for container changes at this time')).toBeInTheDocument(); // Click on ignore changes const ignoreChangesButton = screen.getByRole('button', { name: /ignore changes/i }); diff --git a/src/course-outline/unit-card/UnitCard.test.tsx b/src/course-outline/unit-card/UnitCard.test.tsx index e52ca99a1..e6f8c605c 100644 --- a/src/course-outline/unit-card/UnitCard.test.tsx +++ b/src/course-outline/unit-card/UnitCard.test.tsx @@ -19,7 +19,7 @@ jest.mock('@src/course-unit/data/apiHooks', () => ({ })); const section = { - id: '1', + id: 'block-v1:UNIX+UX1+2025_T3+type@section+block@0', displayName: 'Section Name', published: true, visibilityState: 'live', @@ -34,7 +34,7 @@ const section = { } satisfies Partial as XBlock; const subsection = { - id: '12', + id: 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0', displayName: 'Subsection Name', published: true, visibilityState: 'live', @@ -48,7 +48,7 @@ const subsection = { } satisfies Partial as XBlock; const unit = { - id: '123', + id: 'block-v1:UNIX+UX1+2025_T3+type@unit+block@0', displayName: 'unit Name', category: 'vertical', published: true, @@ -65,6 +65,8 @@ const unit = { readyToSync: true, upstreamRef: 'lct:org1:lib1:unit:1', versionSynced: 1, + versionAvailable: 2, + versionDeclined: null, errorMessage: null, }, } satisfies Partial as XBlock; @@ -107,7 +109,10 @@ describe('', () => { const { findByTestId } = renderComponent(); expect(await findByTestId('unit-card-header')).toBeInTheDocument(); - expect(await findByTestId('unit-card-header__title-link')).toHaveAttribute('href', '/some/123'); + expect(await findByTestId('unit-card-header__title-link')).toHaveAttribute( + 'href', + '/some/block-v1:UNIX+UX1+2025_T3+type@unit+block@0', + ); }); it('hides header based on isHeaderVisible flag', async () => { @@ -198,7 +203,6 @@ describe('', () => { // Should open compare preview modal expect(screen.getByRole('heading', { name: /preview changes: unit name/i })).toBeInTheDocument(); - expect(screen.getByText('Preview not available for container changes at this time')).toBeInTheDocument(); // Click on accept changes const acceptChangesButton = screen.getByText(/accept changes/i); @@ -218,7 +222,6 @@ describe('', () => { // Should open compare preview modal expect(screen.getByRole('heading', { name: /preview changes: unit name/i })).toBeInTheDocument(); - expect(screen.getByText('Preview not available for container changes at this time')).toBeInTheDocument(); // Click on ignore changes const ignoreChangesButton = screen.getByRole('button', { name: /ignore changes/i }); diff --git a/src/course-unit/data/api.js b/src/course-unit/data/api.ts similarity index 56% rename from src/course-unit/data/api.js rename to src/course-unit/data/api.ts index 5c32cdde6..e4abc9619 100644 --- a/src/course-unit/data/api.js +++ b/src/course-unit/data/api.ts @@ -3,24 +3,22 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { PUBLISH_TYPES } from '../constants'; +import { CourseContainerChildrenData, CourseOutlineData, MoveInfoData } from './types'; import { isUnitImportedFromLib, normalizeCourseSectionVerticalData, updateXBlockBlockIdToId } from './utils'; const getStudioBaseUrl = () => getConfig().STUDIO_BASE_URL; -export const getXBlockBaseApiUrl = (itemId) => `${getStudioBaseUrl()}/xblock/${itemId}`; -export const getCourseSectionVerticalApiUrl = (itemId) => `${getStudioBaseUrl()}/api/contentstore/v1/container_handler/${itemId}`; -export const getCourseVerticalChildrenApiUrl = (itemId) => `${getStudioBaseUrl()}/api/contentstore/v1/container/vertical/${itemId}/children`; -export const getCourseOutlineInfoUrl = (courseId) => `${getStudioBaseUrl()}/course/${courseId}?format=concise`; +export const getXBlockBaseApiUrl = (itemId: string) => `${getStudioBaseUrl()}/xblock/${itemId}`; +export const getCourseSectionVerticalApiUrl = (itemId: string) => `${getStudioBaseUrl()}/api/contentstore/v1/container_handler/${itemId}`; +export const getCourseVerticalChildrenApiUrl = (itemId: string) => `${getStudioBaseUrl()}/api/contentstore/v1/container/${itemId}/children`; +export const getCourseOutlineInfoUrl = (courseId: string) => `${getStudioBaseUrl()}/course/${courseId}?format=concise`; export const postXBlockBaseApiUrl = () => `${getStudioBaseUrl()}/xblock/`; -export const libraryBlockChangesUrl = (blockId) => `${getStudioBaseUrl()}/api/contentstore/v2/downstreams/${blockId}/sync`; +export const libraryBlockChangesUrl = (blockId: string) => `${getStudioBaseUrl()}/api/contentstore/v2/downstreams/${blockId}/sync`; /** * Edit course unit display name. - * @param {string} unitId - * @param {string} displayName - * @returns {Promise} */ -export async function editUnitDisplayName(unitId, displayName) { +export async function editUnitDisplayName(unitId: string, displayName: string): Promise { const { data } = await getAuthenticatedHttpClient() .post(getXBlockBaseApiUrl(unitId), { metadata: { @@ -33,10 +31,8 @@ export async function editUnitDisplayName(unitId, displayName) { /** * Fetch vertical block data from the container_handler endpoint. - * @param {string} unitId - * @returns {Promise} */ -export async function getVerticalData(unitId) { +export async function getVerticalData(unitId: string): Promise { const { data } = await getAuthenticatedHttpClient() .get(getCourseSectionVerticalApiUrl(unitId)); @@ -65,7 +61,15 @@ export async function createCourseXblock({ boilerplate, stagedContent, libraryContentKey, -}) { +}: { + type: string, + category?: string, + parentLocator: string, + displayName?: string, + boilerplate?: string, + stagedContent?: string, + libraryContentKey?: string, +}): Promise { const body = { type, boilerplate, @@ -85,14 +89,14 @@ export async function createCourseXblock({ /** * Handles the visibility and data of a course unit, such as publishing, resetting to default values, * and toggling visibility to students. - * @param {string} unitId - The ID of the course unit. - * @param {string} type - The action type (e.g., PUBLISH_TYPES.discardChanges). - * @param {boolean} isVisible - The visibility status for students. - * @param {boolean} groupAccess - Access group key set. - * @param {boolean} isDiscussionEnabled - Indicates whether the discussion feature is enabled. - * @returns {Promise} A promise that resolves with the response data. */ -export async function handleCourseUnitVisibilityAndData(unitId, type, isVisible, groupAccess, isDiscussionEnabled) { +export async function handleCourseUnitVisibilityAndData( + unitId: string, + type: string, + isVisible: boolean, + groupAccess: boolean, + isDiscussionEnabled: boolean, +): Promise { const body = { publish: groupAccess ? null : type, ...(type === PUBLISH_TYPES.republish ? { @@ -111,24 +115,20 @@ export async function handleCourseUnitVisibilityAndData(unitId, type, isVisible, } /** - * Get an object containing course section vertical children data. - * @param {string} itemId - * @returns {Promise} + * Get an object containing course vertical children data. */ -export async function getCourseVerticalChildren(itemId) { +export async function getCourseContainerChildren(itemId: string): Promise { const { data } = await getAuthenticatedHttpClient() .get(getCourseVerticalChildrenApiUrl(itemId)); const camelCaseData = camelCaseObject(data); - return updateXBlockBlockIdToId(camelCaseData); + return updateXBlockBlockIdToId(camelCaseData) as CourseContainerChildrenData; } /** * Delete a unit item. - * @param {string} itemId - * @returns {Promise} */ -export async function deleteUnitItem(itemId) { +export async function deleteUnitItem(itemId: string): Promise { const { data } = await getAuthenticatedHttpClient() .delete(getXBlockBaseApiUrl(itemId)); @@ -137,11 +137,8 @@ export async function deleteUnitItem(itemId) { /** * Duplicate a unit item. - * @param {string} itemId - * @param {string} XBlockId - * @returns {Promise} */ -export async function duplicateUnitItem(itemId, XBlockId) { +export async function duplicateUnitItem(itemId: string, XBlockId: string): Promise { const { data } = await getAuthenticatedHttpClient() .post(postXBlockBaseApiUrl(), { parent_locator: itemId, @@ -151,45 +148,23 @@ export async function duplicateUnitItem(itemId, XBlockId) { return data; } -/** - * @typedef {Object} courseOutline - * @property {string} id - The unique identifier of the course. - * @property {string} displayName - The display name of the course. - * @property {string} category - The category of the course (e.g., "course"). - * @property {boolean} hasChildren - Whether the course has child items. - * @property {boolean} unitLevelDiscussions - Indicates if unit-level discussions are available. - * @property {Object} childInfo - Information about the child elements of the course. - * @property {string} childInfo.category - The category of the child (e.g., "chapter"). - * @property {string} childInfo.display_name - The display name of the child element. - * @property {Array} childInfo.children - List of children within the child_info (could be empty). - */ - /** * Get an object containing course outline data. - * @param {string} courseId - The identifier of the course. - * @returns {Promise} - The course outline data. */ -export async function getCourseOutlineInfo(courseId) { +export async function getCourseOutlineInfo(courseId: string): Promise { const { data } = await getAuthenticatedHttpClient() .get(getCourseOutlineInfoUrl(courseId)); return camelCaseObject(data); } -/** - * @typedef {Object} moveInfo - * @property {string} moveSourceLocator - The locator of the source block being moved. - * @property {string} parentLocator - The locator of the parent block where the source is being moved to. - * @property {number} sourceIndex - The index position of the source block. - */ - /** * Move a unit item to new unit. * @param {string} sourceLocator - The ID of the item to be moved. * @param {string} targetParentLocator - The ID of the XBlock associated with the item. * @returns {Promise} - The move information. */ -export async function patchUnitItem(sourceLocator, targetParentLocator) { +export async function patchUnitItem(sourceLocator: string, targetParentLocator: string): Promise { const { data } = await getAuthenticatedHttpClient() .patch(postXBlockBaseApiUrl(), { parent_locator: targetParentLocator, @@ -203,7 +178,7 @@ export async function patchUnitItem(sourceLocator, targetParentLocator) { * Accept the changes from upstream library block in course * @param {string} blockId - The ID of the item to be updated from library. */ -export async function acceptLibraryBlockChanges(blockId) { +export async function acceptLibraryBlockChanges(blockId: string) { await getAuthenticatedHttpClient() .post(libraryBlockChangesUrl(blockId)); } @@ -212,7 +187,7 @@ export async function acceptLibraryBlockChanges(blockId) { * Ignore the changes from upstream library block in course * @param {string} blockId - The ID of the item to be updated from library. */ -export async function ignoreLibraryBlockChanges(blockId) { +export async function ignoreLibraryBlockChanges(blockId: string) { await getAuthenticatedHttpClient() .delete(libraryBlockChangesUrl(blockId)); } diff --git a/src/course-unit/data/thunk.js b/src/course-unit/data/thunk.js index 9374a3d07..e4a1ec69d 100644 --- a/src/course-unit/data/thunk.js +++ b/src/course-unit/data/thunk.js @@ -3,17 +3,17 @@ import { camelCaseObject } from '@edx/frontend-platform'; import { hideProcessingNotification, showProcessingNotification, -} from '../../generic/processing-notification/data/slice'; -import { handleResponseErrors } from '../../generic/saving-error-alert'; -import { RequestStatus } from '../../data/constants'; -import { NOTIFICATION_MESSAGES } from '../../constants'; -import { updateModel, updateModels } from '../../generic/model-store'; +} from '@src/generic/processing-notification/data/slice'; +import { handleResponseErrors } from '@src/generic/saving-error-alert'; +import { RequestStatus } from '@src/data/constants'; +import { NOTIFICATION_MESSAGES } from '@src/constants'; +import { updateModel, updateModels } from '@src/generic/model-store'; import { messageTypes } from '../constants'; import { editUnitDisplayName, getVerticalData, createCourseXblock, - getCourseVerticalChildren, + getCourseContainerChildren, handleCourseUnitVisibilityAndData, deleteUnitItem, duplicateUnitItem, @@ -126,7 +126,7 @@ export function editCourseUnitVisibilityAndData( } const courseSectionVerticalData = await getVerticalData(blockId); dispatch(fetchCourseSectionVerticalDataSuccess(courseSectionVerticalData)); - const courseVerticalChildrenData = await getCourseVerticalChildren(blockId); + const courseVerticalChildrenData = await getCourseContainerChildren(blockId); dispatch(updateCourseVerticalChildren(courseVerticalChildrenData)); dispatch(hideProcessingNotification()); dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); @@ -163,7 +163,7 @@ export function createNewCourseXBlock(body, callback, blockId, sendMessageToIfra localStorage.removeItem('staticFileNotices'); } } - const courseVerticalChildrenData = await getCourseVerticalChildren(blockId); + const courseVerticalChildrenData = await getCourseContainerChildren(blockId); dispatch(updateCourseVerticalChildren(courseVerticalChildrenData)); dispatch(hideProcessingNotification()); if (callback) { @@ -190,11 +190,11 @@ export function fetchCourseVerticalChildrenData(itemId, isSplitTestType, skipPag } try { - const courseVerticalChildrenData = await getCourseVerticalChildren(itemId); + const courseVerticalChildrenData = await getCourseContainerChildren(itemId); if (isSplitTestType) { const blockIds = courseVerticalChildrenData.children.map(child => child.blockId); const childrenDataArray = await Promise.all( - blockIds.map(blockId => getCourseVerticalChildren(blockId)), + blockIds.map(blockId => getCourseContainerChildren(blockId)), ); const allChildren = childrenDataArray.reduce( (acc, data) => acc.concat(data.children || []), @@ -239,7 +239,7 @@ export function duplicateUnitItemQuery(itemId, xblockId, callback) { callback(courseKey, locator); const courseSectionVerticalData = await getVerticalData(itemId); dispatch(fetchCourseSectionVerticalDataSuccess(courseSectionVerticalData)); - const courseVerticalChildrenData = await getCourseVerticalChildren(itemId); + const courseVerticalChildrenData = await getCourseContainerChildren(itemId); dispatch(updateCourseVerticalChildren(courseVerticalChildrenData)); dispatch(hideProcessingNotification()); dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); diff --git a/src/course-unit/data/types.ts b/src/course-unit/data/types.ts new file mode 100644 index 000000000..5531ddc19 --- /dev/null +++ b/src/course-unit/data/types.ts @@ -0,0 +1,45 @@ +import { UpstreamInfo, XBlock } from '@src/data/types'; +import { ContainerType } from '@src/generic/key-utils'; +import { COMPONENT_TYPES } from '@src/generic/block-type-utils/constants'; + +export interface MoveInfoData { + /** + * The locator of the source block being moved. + */ + moveSourceLocator: string; + /** + * The locator of the parent block where the source is being moved to. + */ + parentLocator: string; + /** + * The index position of the source block. + */ + sourceIndex: number; +} + +export interface CourseOutlineData { + id: string; + displayName: string; + category: string; + hasChildren: boolean; + unitLevelDiscussions: boolean; + childInfo: { + category: string; + displayName: string; + children: XBlock[]; + } +} + +export interface ContainerChildData { + blockId: string; + blockType: ContainerType | keyof typeof COMPONENT_TYPES; + id: string; + name: string; + upstreamLink: UpstreamInfo; +} + +export interface CourseContainerChildrenData { + canPasteComponent: boolean; + children: ContainerChildData[], + isPublished: boolean; +} diff --git a/src/course-unit/preview-changes/index.test.tsx b/src/course-unit/preview-changes/index.test.tsx index c475fed83..dd0f5f626 100644 --- a/src/course-unit/preview-changes/index.test.tsx +++ b/src/course-unit/preview-changes/index.test.tsx @@ -13,7 +13,7 @@ import { messageTypes } from '../constants'; import { libraryBlockChangesUrl } from '../data/api'; import { ToastActionData } from '../../generic/toast-context'; -const usageKey = 'some-id'; +const usageKey = 'block-v1:UNIX+UX1+2025_T3+type@unit+block@1'; const defaultEventData: LibraryChangesMessageData = { displayName: 'Test block', downstreamBlockId: usageKey, diff --git a/src/course-unit/preview-changes/index.tsx b/src/course-unit/preview-changes/index.tsx index dceedc701..1ef8ab088 100644 --- a/src/course-unit/preview-changes/index.tsx +++ b/src/course-unit/preview-changes/index.tsx @@ -5,17 +5,18 @@ import { import { Warning } from '@openedx/paragon/icons'; import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; -import { useEventListener } from '../../generic/hooks'; -import { messageTypes } from '../constants'; -import CompareChangesWidget from '../../library-authoring/component-comparison/CompareChangesWidget'; -import { useAcceptLibraryBlockChanges, useIgnoreLibraryBlockChanges } from '../data/apiHooks'; -import AlertMessage from '../../generic/alert-message'; -import { useIframe } from '../../generic/hooks/context/hooks'; -import DeleteModal from '../../generic/delete-modal/DeleteModal'; +import { CompareContainersWidget } from '@src/container-comparison/CompareContainersWidget'; +import { useEventListener } from '@src/generic/hooks'; +import CompareChangesWidget from '@src/library-authoring/component-comparison/CompareChangesWidget'; +import AlertMessage from '@src/generic/alert-message'; +import { useIframe } from '@src/generic/hooks/context/hooks'; +import DeleteModal from '@src/generic/delete-modal/DeleteModal'; +import { ToastContext } from '@src/generic/toast-context'; +import LoadingButton from '@src/generic/loading-button'; +import Loading from '@src/generic/Loading'; import messages from './messages'; -import { ToastContext } from '../../generic/toast-context'; -import LoadingButton from '../../generic/loading-button'; -import Loading from '../../generic/Loading'; +import { useAcceptLibraryBlockChanges, useIgnoreLibraryBlockChanges } from '../data/apiHooks'; +import { messageTypes } from '../constants'; export interface LibraryChangesMessageData { displayName: string, @@ -55,12 +56,21 @@ export const PreviewLibraryXBlockChanges = ({ if (!blockData) { return ; } + if (blockData.isContainer) { + return ( + + ); + } + return ( ); }, [blockData]); @@ -109,13 +119,15 @@ export const PreviewLibraryXBlockChanges = ({ {title} - + + {!blockData.isContainer && ( + )} {getBody()} diff --git a/src/data/types.ts b/src/data/types.ts index dc41af3b1..111dc5785 100644 --- a/src/data/types.ts +++ b/src/data/types.ts @@ -48,11 +48,22 @@ export interface XBlockPrereqs { blockDisplayName: string; } +export interface UpstreamChildrenInfo { + name: string; + upstream: string; + id: string; +} + export interface UpstreamInfo { readyToSync: boolean, upstreamRef: string, versionSynced: number, + versionAvailable: number | null, + versionDeclined: number | null, errorMessage: string | null, + isModified?: boolean, + hasTopLevelParent?: boolean, + readyToSyncChildren?: UpstreamChildrenInfo[], } export interface XBlock { diff --git a/src/generic/key-utils.test.ts b/src/generic/key-utils.test.ts index 3a62821b3..60e4fe4ec 100644 --- a/src/generic/key-utils.test.ts +++ b/src/generic/key-utils.test.ts @@ -1,9 +1,11 @@ import { buildCollectionUsageKey, + ContainerType, getBlockType, getLibraryId, isLibraryKey, isLibraryV1Key, + normalizeContainerType, } from './key-utils'; describe('component utils', () => { @@ -100,4 +102,19 @@ describe('component utils', () => { }); } }); + + describe('normalizeContainerType', () => { + for (const [containerType, expected] of [ + [ContainerType.Vertical, ContainerType.Unit], + [ContainerType.Sequential, ContainerType.Subsection], + [ContainerType.Chapter, ContainerType.Section], + [ContainerType.Unit, ContainerType.Unit], + [ContainerType.Section, ContainerType.Section], + [ContainerType.Subsection, ContainerType.Subsection], + ] as const) { + it(`returns '${expected}' for '${containerType}'`, () => { + expect(normalizeContainerType(containerType)).toStrictEqual(expected); + }); + } + }); }); diff --git a/src/generic/key-utils.ts b/src/generic/key-utils.ts index cdf9876f4..a98bce9ab 100644 --- a/src/generic/key-utils.ts +++ b/src/generic/key-utils.ts @@ -101,3 +101,19 @@ export enum ContainerType { */ Components = 'components', } + +/** + * Normalize a container type to the standard version. For example, 'sequential' will be normalized to 'subsection'. + */ +export function normalizeContainerType(containerType: ContainerType | string) { + switch (containerType) { + case ContainerType.Chapter: + return ContainerType.Section; + case ContainerType.Sequential: + return ContainerType.Subsection; + case ContainerType.Vertical: + return ContainerType.Unit; + default: + return containerType; + } +} diff --git a/src/library-authoring/component-comparison/CompareChangesWidget.tsx b/src/library-authoring/component-comparison/CompareChangesWidget.tsx index aaca0df27..976826a44 100644 --- a/src/library-authoring/component-comparison/CompareChangesWidget.tsx +++ b/src/library-authoring/component-comparison/CompareChangesWidget.tsx @@ -1,26 +1,15 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Tab, Tabs } from '@openedx/paragon'; -import { IframeProvider } from '../../generic/hooks/context/iFrameContext'; +import { IframeProvider } from '@src/generic/hooks/context/iFrameContext'; import { LibraryBlock, type VersionSpec } from '../LibraryBlock'; import messages from './messages'; -const PreviewNotAvailable = () => { - const intl = useIntl(); - - return ( -
- {intl.formatMessage(messages.previewNotAvailable)} -
- ); -}; - interface Props { usageKey: string; oldVersion?: VersionSpec; newVersion?: VersionSpec; - isContainer?: boolean; } /** @@ -35,37 +24,32 @@ const CompareChangesWidget = ({ usageKey, oldVersion = 'published', newVersion = 'draft', - isContainer = false, }: Props) => { const intl = useIntl(); return ( -
+
- {isContainer ? () : ( - - - - )} + + +
- {isContainer ? () : ( - - - - )} + + +
diff --git a/src/library-authoring/component-comparison/messages.ts b/src/library-authoring/component-comparison/messages.ts index 8da23e3d2..89275918a 100644 --- a/src/library-authoring/component-comparison/messages.ts +++ b/src/library-authoring/component-comparison/messages.ts @@ -17,11 +17,6 @@ const messages = defineMessages({ defaultMessage: 'Compare Changes', description: 'Title used for the compare changes dialog', }, - previewNotAvailable: { - id: 'course-authoring.library-authoring.component-comparison.preview-not-available', - defaultMessage: 'Preview not available for container changes at this time', - description: 'Message shown when preview is not available.', - }, }); export default messages; diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index c5d3a796b..44151f34b 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -602,6 +602,7 @@ mockGetContainerMetadata.applyMock = () => { */ export async function mockGetContainerChildren(containerId: string): Promise { let numChildren: number; + let blockType = 'html'; switch (containerId) { case mockGetContainerMetadata.unitId: case mockGetContainerMetadata.sectionId: @@ -618,7 +619,6 @@ export async function mockGetContainerChildren(containerId: string): Promise