feat: Deleted/Added level diff in sync modal [FC-0097] (#2549)

- Adds the Deleted and the Added level diff in sync container modal.
- Fix the icons in the sync container modal
This commit is contained in:
Chris Chávez
2025-10-22 15:53:12 -05:00
committed by GitHub
parent 82a3c2815b
commit 9f1604110b
12 changed files with 145 additions and 46 deletions

View File

@@ -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(<CompareContainersWidget
upstreamBlockId={mockGetContainerMetadata.sectionId}
downstreamBlockId={mockGetCourseContainerChildren.sectionId}
/>);
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(<CompareContainersWidget
upstreamBlockId={mockGetContainerMetadata.sectionId}
downstreamBlockId="block-v1:UNIX+UX1+2025_T3+type@section+block@0-new"
/>);
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' });

View File

@@ -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<ContainerChild>) => 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 <div className="m-auto"><LoadingSpinner /></div>;
}
if (state === 'added') {
return (
<Stack className="align-items-center justify-content-center bg-light-200 text-gray-800">
<Icon src={Add} className="big-icon" />
<FormattedMessage
{...messages.newContainer}
values={{
containerType: getBlockType(upstreamBlockId),
}}
/>
</Stack>
);
}
return result[0]?.map((child) => (
<ContainerRow
key={child.id}
@@ -87,10 +108,24 @@ const CompareContainersWidgetInner = ({
}, [result]);
const renderAfterChildren = useCallback(() => {
if (!result[1]) {
if (!result[1] && state !== 'removed') {
return <div className="m-auto"><LoadingSpinner /></div>;
}
if (state === 'removed') {
return (
<Stack className="align-items-center justify-content-center bg-light-200 text-gray-800">
<Icon src={Delete} className="big-icon" />
<FormattedMessage
{...messages.deletedContainer}
values={{
containerType: getBlockType(upstreamBlockId),
}}
/>
</Stack>
);
}
return result[1]?.map((child) => (
<ContainerRow
key={child.id}
@@ -134,12 +169,21 @@ const CompareContainersWidgetInner = ({
);
}, [parent]);
if (isError || isLibError || isContainerTitleError) {
let beforeTitle: string | undefined | null = data?.displayName;
let afterTitle = containerData?.publishedDisplayName;
if (!data && state === 'added') {
beforeTitle = containerData?.publishedDisplayName;
}
if (!containerData && state === 'removed') {
afterTitle = data?.displayName;
}
if (isError || (isLibError && state !== 'removed') || (isContainerTitleError && state !== 'removed')) {
return <ErrorAlert error={error || libError || containerTitleError} />;
}
return (
<div className="row justify-content-center">
<div className="compare-changes-widget row justify-content-center">
{localUpdateAlertCount > 0 && (
<Alert variant="info">
<FormattedMessage
@@ -153,15 +197,15 @@ const CompareContainersWidgetInner = ({
</Alert>
)}
<div className="col col-6 p-1">
<Card className="p-4">
<ChildrenPreview title={getTitleComponent(data?.displayName)} side="Before">
<Card className="compare-card p-4">
<ChildrenPreview title={getTitleComponent(beforeTitle)} side="Before">
{renderBeforeChildren()}
</ChildrenPreview>
</Card>
</div>
<div className="col col-6 p-1">
<Card className="p-4">
<ChildrenPreview title={getTitleComponent(containerData?.publishedDisplayName)} side="After">
<Card className="compare-card p-4">
<ChildrenPreview title={getTitleComponent(afterTitle)} side="After">
{renderAfterChildren()}
</ChildrenPreview>
</Card>
@@ -181,12 +225,14 @@ export const CompareContainersWidget = ({
isReadyToSyncIndividually = false,
}: ContainerInfoProps) => {
const [currentContainerState, setCurrentContainerState] = useState<ContainerInfoProps & {
parent: ContainerInfoProps[];
state?: ContainerState;
parent:(ContainerInfoProps & { state?: ContainerState })[];
}>({
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}

View File

@@ -29,20 +29,6 @@ describe('<ContainerRow />', () => {
)).toBeInTheDocument();
});
test('is not clickable when state !== modified', async () => {
const onClick = jest.fn();
render(<ContainerRow
title="Test title"
containerType="subsection"
side="Before"
state="removed"
onClick={onClick}
/>);
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();

View File

@@ -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<CourseContainerChildrenData> {
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) => (

View File

@@ -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),
})
);

View File

@@ -0,0 +1,10 @@
.compare-changes-widget {
.compare-card {
min-height: 350px;
}
.big-icon {
height: 68px;
width: 68px;
}
}

View File

@@ -86,6 +86,16 @@ const messages = defineMessages({
defaultMessage: 'The only change is to {count, plural, one {text block <b>{blockName}</b> which has been edited} other {<b>{count} text blocks</b> 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;

View File

@@ -132,7 +132,7 @@ export function diffPreviewContainerChildren<A extends CourseContainerChildBase,
}
export function isRowClickable(state?: ContainerState, blockType?: ContainerType) {
return state === 'modified' && blockType && [
return state && blockType && ['modified', 'added', 'removed'].includes(state) && [
ContainerType.Section,
ContainerType.Subsection,
ContainerType.Unit,

View File

@@ -146,6 +146,7 @@ const SectionCard = ({
upstreamBlockVersionSynced: upstreamInfo.versionSynced,
isReadyToSyncIndividually: upstreamInfo.isReadyToSyncIndividually,
isContainer: true,
blockType: 'section',
};
}, [upstreamInfo]);

View File

@@ -128,6 +128,7 @@ const SubsectionCard = ({
upstreamBlockVersionSynced: upstreamInfo.versionSynced,
isReadyToSyncIndividually: upstreamInfo.isReadyToSyncIndividually,
isContainer: true,
blockType: 'subsection',
};
}, [upstreamInfo]);

View File

@@ -105,6 +105,7 @@ const UnitCard = ({
upstreamBlockVersionSynced: upstreamInfo.versionSynced,
isReadyToSyncIndividually: upstreamInfo.isReadyToSyncIndividually,
isContainer: true,
blockType: 'unit',
};
}, [upstreamInfo]);

View File

@@ -31,6 +31,7 @@
@import "group-configurations/GroupConfigurations";
@import "optimizer-page/scan-results/ScanResults";
@import "legacy-libraries-migration/";
@import "container-comparison/";
// To apply the glow effect to the selected Section/Subsection, in the Course Outline
div.row:has(> div > div.highlight) {