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

Author: ChrisChV

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(<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' });

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<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,36 +46,54 @@ const CompareContainersWidgetInner = ({
   upstreamBlockId,
   downstreamBlockId,
   parent,
+  state,
   onRowClick,
   onBackBtnClick,
   localUpdateAlertCount,
   localUpdateAlertBlockName,
 }: 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,
     error: containerTitleError,
   } = 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}

src/container-comparison/ContainerRow.test.tsx

@@ -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();

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<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) => (

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

src/container-comparison/index.scss

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

src/container-comparison/messages.ts

@@ -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;

src/container-comparison/utils.ts

@@ -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,

src/course-outline/section-card/SectionCard.tsx

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

src/course-outline/subsection-card/SubsectionCard.tsx

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