fix: saving alert doesn't disappear even its saved

saving alert doesn't disappear even if its saved and keep showing loading status

src/container-comparison/CompareContainersWidget.tsx

@@ -13,6 +13,7 @@ import { LoadingSpinner } from '@src/generic/Loading';
 import { useContainer, useContainerChildren } from '@src/library-authoring/data/apiHooks';
 import { BoldText } from '@src/utils';
 
+import { Container, LibraryBlockMetadata } from '@src/library-authoring/data/api';
 import ChildrenPreview from './ChildrenPreview';
 import ContainerRow from './ContainerRow';
 import { useCourseContainerChildren } from './data/apiHooks';
@@ -60,7 +61,7 @@ const CompareContainersWidgetInner = ({
     data: libData,
     isError: isLibError,
     error: libError,
-  } = useContainerChildren(state === 'removed' ? undefined : upstreamBlockId, true);
+  } = useContainerChildren<Container | LibraryBlockMetadata>(state === 'removed' ? undefined : upstreamBlockId, true);
   const {
     data: containerData,
     isError: isContainerTitleError,

src/editors/sharedComponents/TinyMceWidget/hooks.ts

@@ -457,6 +457,9 @@ export const editorConfig = ({
       valid_elements: '*[*]',
       // FIXME: this is passing 'utf-8', which is not a valid entity_encoding value. It should be 'named' etc.
       entity_encoding: 'utf-8' as any,
+      // Protect self-closing <script /> tags from being mangled,
+      // to preserve backwards compatibility with content that relied on this behavior
+      protect: [/<script[^>]*\/>/g],
     },
   };
 };

src/grading-settings/GradingSettings.jsx

@@ -87,7 +87,7 @@ const GradingSettings = ({ courseId }) => {
       setIsQueryPending(!isQueryPending);
       window.scrollTo({ top: 0, behavior: 'smooth' });
     }
-  }, [savePending]);
+  }, [savePending, savingStatus]);
 
   if (isLoadingDenied) {
     return (

src/legacy-libraries-migration/ConfirmationView.tsx

@@ -74,13 +74,13 @@ export const ConfirmationView = ({
         {...messages.confirmationViewAlert}
         values={{
           count: legacyLibraries.length,
-          libraryName: destination.title,
           b: BoldText,
         }}
       />
     </Alert>
     {legacyLibraries.map((legacyLib) => (
       <ConfirmationCard
+        key={legacyLib.libraryKey}
         legacyLib={legacyLib}
         destinationName={destination.title}
       />

src/legacy-libraries-migration/LegacyLibMigrationPage.test.tsx

@@ -6,6 +6,7 @@ import {
   render,
   screen,
   waitFor,
+  within,
 } from '@src/testUtils';
 import studioHomeMock from '@src/studio-home/__mocks__/studioHomeMock';
 import { mockGetContentLibraryV2List } from '@src/library-authoring/data/api.mocks';
@@ -184,7 +185,7 @@ describe('<LegacyLibMigrationPage />', () => {
     nextButton.click();
 
     // Should show alert of SelectDestinationView
-    expect(await screen.findByText(/any legacy libraries that are used/i)).toBeInTheDocument();
+    expect(await screen.findByText(/you selected will be migrated to this new library/i)).toBeInTheDocument();
 
     const backButton = screen.getByRole('button', { name: /back/i });
     backButton.click();
@@ -210,7 +211,7 @@ describe('<LegacyLibMigrationPage />', () => {
     nextButton.click();
 
     // Should show alert of SelectDestinationView
-    expect(await screen.findByText(/any legacy libraries that are used/i)).toBeInTheDocument();
+    expect(await screen.findByText(/you selected will be migrated to this new library/i)).toBeInTheDocument();
 
     // The next button is disabled
     expect(nextButton).toBeDisabled();
@@ -224,27 +225,31 @@ describe('<LegacyLibMigrationPage />', () => {
   });
 
   it('should back to select library destination', async () => {
+    const user = userEvent.setup();
     renderPage();
     expect(await screen.findByText('Migrate Legacy Libraries')).toBeInTheDocument();
     expect(await screen.findByText('MBA')).toBeInTheDocument();
 
     const legacyLibrary = screen.getByRole('checkbox', { name: 'MBA' });
-    legacyLibrary.click();
+    await user.click(legacyLibrary);
 
-    const nextButton = screen.getByRole('button', { name: /next/i });
-    nextButton.click();
+    const nextButton = await screen.findByRole('button', { name: /next/i });
+    await user.click(nextButton);
 
     // Should show alert of SelectDestinationView
-    expect(await screen.findByText(/any legacy libraries that are used/i)).toBeInTheDocument();
+    expect(await screen.findByText(/you selected will be migrated to this new library/i)).toBeInTheDocument();
     expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
     const radioButton = screen.getByRole('radio', { name: /test library 1/i });
-    radioButton.click();
+    await user.click(radioButton);
 
-    nextButton.click();
-    expect(await screen.findByText(/these 1 legacy library will be migrated to/i)).toBeInTheDocument();
+    await user.click(nextButton);
+    const alert = await screen.findByRole('alert');
+    expect(await within(alert).findByText(
+      /All content from the legacy library you selected will be migrated to the Content Library you select/i,
+    )).toBeInTheDocument();
 
     const backButton = screen.getByRole('button', { name: /back/i });
-    backButton.click();
+    await user.click(backButton);
 
     expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
     // The selected v2 library remains checked
@@ -269,7 +274,7 @@ describe('<LegacyLibMigrationPage />', () => {
     nextButton.click();
 
     // Should show alert of SelectDestinationView
-    expect(await screen.findByText(/any legacy libraries that are used/i)).toBeInTheDocument();
+    expect(await screen.findByText(/you selected will be migrated to this new library/i)).toBeInTheDocument();
 
     const createButton = await screen.findByRole('button', { name: /create new library/i });
     expect(createButton).toBeInTheDocument();
@@ -336,18 +341,21 @@ describe('<LegacyLibMigrationPage />', () => {
     legacyLibrary3.click();
 
     const nextButton = screen.getByRole('button', { name: /next/i });
-    nextButton.click();
+    await user.click(nextButton);
 
     // Should show alert of SelectDestinationView
-    expect(await screen.findByText(/any legacy libraries that are used/i)).toBeInTheDocument();
+    expect(await screen.findByText(/you selected will be migrated to this new library/i)).toBeInTheDocument();
     expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
     const radioButton = screen.getByRole('radio', { name: /test library 1/i });
-    radioButton.click();
+    await user.click(radioButton);
 
-    nextButton.click();
+    await user.click(nextButton);
 
     // Should show alert of ConfirmationView
-    expect(await screen.findByText(/these 3 legacy libraries will be migrated to/i)).toBeInTheDocument();
+    const alert = await screen.findByRole('alert');
+    expect(await within(alert).findByText(
+      /All content from the 3 legacy libraries you selected will be migrated to the Content Library you select/i,
+    )).toBeInTheDocument();
     expect(screen.getByText('MBA')).toBeInTheDocument();
     expect(screen.getByText('Legacy library 1')).toBeInTheDocument();
     expect(screen.getByText('MBA 1')).toBeInTheDocument();
@@ -390,18 +398,22 @@ describe('<LegacyLibMigrationPage />', () => {
     legacyLibrary3.click();
 
     const nextButton = screen.getByRole('button', { name: /next/i });
-    nextButton.click();
+    await user.click(nextButton);
 
     // Should show alert of SelectDestinationView
-    expect(await screen.findByText(/any legacy libraries that are used/i)).toBeInTheDocument();
+    expect(await screen.findByText(/you selected will be migrated to this new library/i)).toBeInTheDocument();
     expect(await screen.findByText('Test Library 1')).toBeInTheDocument();
     const radioButton = screen.getByRole('radio', { name: /test library 1/i });
-    radioButton.click();
+    await user.click(radioButton);
 
-    nextButton.click();
+    await user.click(nextButton);
 
     // Should show alert of ConfirmationView
-    expect(await screen.findByText(/these 3 legacy libraries will be migrated to/i)).toBeInTheDocument();
+    const alert = await screen.findByRole('alert');
+    expect(await within(alert).findByText(
+      /All content from the 3 legacy libraries you selected will be migrated to the Content Library you select/i,
+      { exact: false },
+    )).toBeInTheDocument();
     expect(screen.getByText('MBA')).toBeInTheDocument();
     expect(screen.getByText('Legacy library 1')).toBeInTheDocument();
     expect(screen.getByText('MBA 1')).toBeInTheDocument();

src/legacy-libraries-migration/messages.ts

@@ -64,17 +64,19 @@ const messages = defineMessages({
   selectDestinationAlert: {
     id: 'legacy-libraries-migration.select-destination.alert.text',
     defaultMessage: 'All content from the'
-      + ' {count, plural, one {{count} legacy library} other {{count} legacy libraries}} you selected will'
-      + ' be migrated to this new library, organized into collections. Any legacy libraries that are used in'
-      + ' problem banks will maintain their link with migrated content the first time they are migrated.',
+      + ' {count, plural, one {legacy library} other {{count} legacy libraries}} you selected will'
+      + ' be migrated to this new library, organized into collections. Legacy library content used in courses will'
+      + ' continue to work as-is. To receive any future changes to migrated content, you must update these'
+      + ' references within your course.',
     description: 'Alert text in the select destination step of the legacy libraries migration page.',
   },
   confirmationViewAlert: {
     id: 'legacy-libraries-migration.select-destination.alert.text',
-    defaultMessage: 'These {count, plural, one {{count} legacy library} other {{count} legacy libraries}}'
-      + ' will be migrated to <b>{libraryName}</b> and organized as collections. Legacy library content used'
-      + ' in courses will continue to work as-is. To receive any future changes to migrated content,'
-      + ' you must update these references within your course.',
+    defaultMessage: 'All content from the'
+      + ' {count, plural, one {legacy library} other {{count} legacy libraries}} you selected will'
+      + ' be migrated to the Content Library you select, organized into collections. Legacy library content used in courses will'
+      + ' continue to work as-is. To receive any future changes to migrated content, you must update these'
+      + ' references within your course.',
     description: 'Alert text in the confirmation step of the legacy libraries migration page.',
   },
   previouslyMigratedAlert: {

src/library-authoring/common/context/SidebarContext.tsx

@@ -72,6 +72,7 @@ export interface DefaultTabs {
 export interface SidebarItemInfo {
   type: SidebarBodyItemId;
   id: string;
+  index?: number;
 }
 
 export enum SidebarActions {
@@ -88,7 +89,7 @@ export type SidebarContextData = {
   openCollectionInfoSidebar: (collectionId: string) => void;
   openComponentInfoSidebar: (usageKey: string) => void;
   openContainerInfoSidebar: (usageKey: string) => void;
-  openItemSidebar: (selectedItemId: string, type: SidebarBodyItemId) => void;
+  openItemSidebar: (selectedItemId: string, type: SidebarBodyItemId, index?: number) => void;
   sidebarItemInfo?: SidebarItemInfo;
   sidebarAction: SidebarActions;
   setSidebarAction: (action: SidebarActions) => void;
@@ -154,35 +155,38 @@ export const SidebarProvider = ({
     setSidebarItemInfo({ id: '', type: SidebarBodyItemId.Info });
   }, []);
 
-  const openComponentInfoSidebar = useCallback((usageKey: string) => {
+  const openComponentInfoSidebar = useCallback((usageKey: string, index?: number) => {
     setSidebarItemInfo({
       id: usageKey,
       type: SidebarBodyItemId.ComponentInfo,
+      index,
     });
   }, []);
 
-  const openCollectionInfoSidebar = useCallback((newCollectionId: string) => {
+  const openCollectionInfoSidebar = useCallback((newCollectionId: string, index?: number) => {
     setSidebarItemInfo({
       id: newCollectionId,
       type: SidebarBodyItemId.CollectionInfo,
+      index,
     });
   }, []);
 
-  const openContainerInfoSidebar = useCallback((usageKey: string) => {
+  const openContainerInfoSidebar = useCallback((usageKey: string, index?: number) => {
     setSidebarItemInfo({
       id: usageKey,
       type: SidebarBodyItemId.ContainerInfo,
+      index,
     });
   }, []);
 
   const { navigateTo } = useLibraryRoutes();
-  const openItemSidebar = useCallback((selectedItemId: string, type: SidebarBodyItemId) => {
-    navigateTo({ selectedItemId });
-    setSidebarItemInfo({ id: selectedItemId, type });
+  const openItemSidebar = useCallback((selectedItemId: string, type: SidebarBodyItemId, index?: number) => {
+    navigateTo({ selectedItemId, index });
+    setSidebarItemInfo({ id: selectedItemId, type, index });
   }, [navigateTo, setSidebarItemInfo]);
 
   // Set the initial sidebar state based on the URL parameters and context.
-  const { selectedItemId } = useParams();
+  const { selectedItemId, index: indexParam } = useParams();
   const { collectionId, containerId } = useLibraryContext();
   const { componentPickerMode } = useComponentPickerContext();
 
@@ -198,12 +202,15 @@ export const SidebarProvider = ({
 
     // Handle selected item id changes
     if (selectedItemId) {
+      // if a item is selected that means we have list of items displayed
+      // which means we can get the index from url and set it.
+      const indexNumber = indexParam ? Number(indexParam) : undefined;
       if (selectedItemId.startsWith('lct:')) {
-        openContainerInfoSidebar(selectedItemId);
+        openContainerInfoSidebar(selectedItemId, indexNumber);
       } else if (selectedItemId.startsWith('lb:')) {
-        openComponentInfoSidebar(selectedItemId);
+        openComponentInfoSidebar(selectedItemId, indexNumber);
       } else {
-        openCollectionInfoSidebar(selectedItemId);
+        openCollectionInfoSidebar(selectedItemId, indexNumber);
       }
     } else if (collectionId) {
       openCollectionInfoSidebar(collectionId);

src/library-authoring/component-info/ComponentInfo.tsx

@@ -111,6 +111,8 @@ const ComponentActions = ({
   const [isPublisherOpen, openPublisher, closePublisher] = useToggle(false);
   const canEdit = canEditComponent(componentId);
 
+  const { sidebarItemInfo } = useSidebarContext();
+
   if (isPublisherOpen) {
     return (
       <ComponentPublisher
@@ -141,7 +143,7 @@ const ComponentActions = ({
         )}
       </div>
       <div className="mt-2">
-        <ComponentMenu usageKey={componentId} />
+        <ComponentMenu usageKey={componentId} index={sidebarItemInfo?.index} />
       </div>
     </div>
   );

src/library-authoring/components/ComponentMenu.tsx

@@ -12,18 +12,23 @@ import { useClipboard } from '@src/generic/clipboard';
 import { getBlockType } from '@src/generic/key-utils';
 import { ToastContext } from '@src/generic/toast-context';
 
-import { useLibraryContext } from '../common/context/LibraryContext';
-import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext';
-import { useRemoveItemsFromCollection } from '../data/apiHooks';
+import { useLibraryContext } from '@src/library-authoring/common/context/LibraryContext';
+import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '@src/library-authoring/common/context/SidebarContext';
+import { useRemoveItemsFromCollection } from '@src/library-authoring/data/apiHooks';
+import containerMessages from '@src/library-authoring/containers/messages';
+import { useLibraryRoutes } from '@src/library-authoring/routes';
+import { useRunOnNextRender } from '@src/utils';
 import { canEditComponent } from './ComponentEditorModal';
 import ComponentDeleter from './ComponentDeleter';
 import ComponentRemover from './ComponentRemover';
 import messages from './messages';
-import containerMessages from '../containers/messages';
-import { useLibraryRoutes } from '../routes';
-import { useRunOnNextRender } from '../../utils';
 
-export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
+interface Props {
+  usageKey: string;
+  index?: number;
+}
+
+export const ComponentMenu = ({ usageKey, index }: Props) => {
   const intl = useIntl();
   const {
     libraryId,
@@ -135,6 +140,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
       {isRemoveModalOpen && (
         <ComponentRemover
           usageKey={usageKey}
+          index={index}
           close={closeRemoveModal}
         />
       )}

src/library-authoring/components/ComponentRemover.tsx

@@ -4,31 +4,38 @@ import { Warning } from '@openedx/paragon/icons';
 
 import DeleteModal from '@src/generic/delete-modal/DeleteModal';
 import { ToastContext } from '@src/generic/toast-context';
-import { useLibraryContext } from '../common/context/LibraryContext';
-import { useSidebarContext } from '../common/context/SidebarContext';
+import { useLibraryContext } from '@src/library-authoring/common/context/LibraryContext';
+import { useSidebarContext } from '@src/library-authoring/common/context/SidebarContext';
 import {
   useContainer,
   useRemoveContainerChildren,
-  useAddItemsToContainer,
   useLibraryBlockMetadata,
-} from '../data/apiHooks';
+  useContainerChildren,
+  useUpdateContainerChildren,
+} from '@src/library-authoring/data/apiHooks';
+import { LibraryBlockMetadata } from '@src/library-authoring/data/api';
 import messages from './messages';
 
 interface Props {
   usageKey: string;
+  index?: number;
   close: () => void;
 }
 
-const ComponentRemover = ({ usageKey, close }: Props) => {
+const ComponentRemover = ({ usageKey, index, close }: Props) => {
   const intl = useIntl();
   const { sidebarItemInfo, closeLibrarySidebar } = useSidebarContext();
-  const { containerId } = useLibraryContext();
+  const { containerId, showOnlyPublished } = useLibraryContext();
   const { showToast } = useContext(ToastContext);
 
   const removeContainerItemMutation = useRemoveContainerChildren(containerId);
-  const addItemToContainerMutation = useAddItemsToContainer(containerId);
+  const updateContainerChildrenMutation = useUpdateContainerChildren(containerId);
   const { data: container, isPending: isPendingParentContainer } = useContainer(containerId);
   const { data: component, isPending } = useLibraryBlockMetadata(usageKey);
+  // Use update api for children if duplicates are present to avoid removing all instances of the child
+  const { data: children } = useContainerChildren<LibraryBlockMetadata>(containerId, showOnlyPublished);
+  const childrenUsageIds = children?.map((child) => child.id);
+  const hasDuplicates = (childrenUsageIds?.filter((child) => child === usageKey).length || 0) > 1;
 
   // istanbul ignore if: loading state
   if (isPending || isPendingParentContainer) {
@@ -36,28 +43,62 @@ const ComponentRemover = ({ usageKey, close }: Props) => {
     return null;
   }
 
+  const restoreComponent = () => {
+    // istanbul ignore if: this should never happen
+    if (!childrenUsageIds) {
+      return;
+    }
+    updateContainerChildrenMutation.mutateAsync(childrenUsageIds).then(() => {
+      showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastSuccess));
+    }).catch(() => {
+      showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastFailed));
+    });
+  };
+
+  const showSuccessToast = () => {
+    showToast(
+      intl.formatMessage(messages.removeComponentFromContainerSuccess),
+      {
+        label: intl.formatMessage(messages.undoRemoveComponentFromContainerToastAction),
+        onClick: restoreComponent,
+      },
+    );
+  };
+
+  const showFailureToast = () => showToast(intl.formatMessage(messages.removeComponentFromContainerFailure));
+
   const removeFromContainer = () => {
-    const restoreComponent = () => {
-      addItemToContainerMutation.mutateAsync([usageKey]).then(() => {
-        showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastSuccess));
-      }).catch(() => {
-        showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastFailed));
-      });
-    };
     removeContainerItemMutation.mutateAsync([usageKey]).then(() => {
       if (sidebarItemInfo?.id === usageKey) {
         // Close sidebar if current component is open
         closeLibrarySidebar();
       }
-      showToast(
-        intl.formatMessage(messages.removeComponentFromContainerSuccess),
-        {
-          label: intl.formatMessage(messages.undoRemoveComponentFromContainerToastAction),
-          onClick: restoreComponent,
-        },
-      );
+      showSuccessToast();
     }).catch(() => {
-      showToast(intl.formatMessage(messages.removeComponentFromContainerFailure));
+      showFailureToast();
+    });
+
+    close();
+  };
+
+  const excludeOneInstance = () => {
+    if (!childrenUsageIds || typeof index === 'undefined') {
+      return;
+    }
+    const updatedKeys = childrenUsageIds.filter((childId, idx) => childId !== usageKey || idx !== index);
+    updateContainerChildrenMutation.mutateAsync(updatedKeys).then(() => {
+      // istanbul ignore if
+      if (sidebarItemInfo?.id === usageKey && sidebarItemInfo?.index === index) {
+        // Close sidebar if current component is open
+        closeLibrarySidebar();
+      }
+      // Already tested as part of removeFromContainer
+      // istanbul ignore next
+      showSuccessToast();
+    }).catch(() => {
+      // Already tested as part of removeFromContainer
+      // istanbul ignore next
+      showFailureToast();
     });
 
     close();
@@ -76,7 +117,7 @@ const ComponentRemover = ({ usageKey, close }: Props) => {
       title={intl.formatMessage(messages.removeComponentWarningTitle)}
       icon={Warning}
       description={removeText}
-      onDeleteSubmit={removeFromContainer}
+      onDeleteSubmit={hasDuplicates ? excludeOneInstance : removeFromContainer}
       btnLabel={intl.formatMessage(messages.componentRemoveButtonLabel)}
       buttonVariant="primary"
     />

src/library-authoring/containers/ContainerCard.tsx

@@ -17,23 +17,24 @@ import { type ContainerHit, Highlight, PublishStatus } from '@src/search-manager
 import { ToastContext } from '@src/generic/toast-context';
 import { useRunOnNextRender } from '@src/utils';
 
-import { useComponentPickerContext } from '../common/context/ComponentPickerContext';
-import { useLibraryContext } from '../common/context/LibraryContext';
-import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '../common/context/SidebarContext';
-import { useRemoveItemsFromCollection } from '../data/apiHooks';
-import { useLibraryRoutes } from '../routes';
+import { useComponentPickerContext } from '@src/library-authoring/common/context/ComponentPickerContext';
+import { useLibraryContext } from '@src/library-authoring/common/context/LibraryContext';
+import { SidebarActions, SidebarBodyItemId, useSidebarContext } from '@src/library-authoring/common/context/SidebarContext';
+import { useRemoveItemsFromCollection } from '@src/library-authoring/data/apiHooks';
+import { useLibraryRoutes } from '@src/library-authoring/routes';
+import BaseCard from '@src/library-authoring/components/BaseCard';
+import AddComponentWidget from '@src/library-authoring/components/AddComponentWidget';
 import messages from './messages';
 import ContainerDeleter from './ContainerDeleter';
 import ContainerRemover from './ContainerRemover';
-import BaseCard from '../components/BaseCard';
-import AddComponentWidget from '../components/AddComponentWidget';
 
 type ContainerMenuProps = {
   containerKey: string;
   displayName: string;
+  index?: number;
 };
 
-export const ContainerMenu = ({ containerKey, displayName } : ContainerMenuProps) => {
+export const ContainerMenu = ({ containerKey, displayName, index } : ContainerMenuProps) => {
   const intl = useIntl();
   const { libraryId, collectionId, containerId } = useLibraryContext();
   const {
@@ -144,6 +145,7 @@ export const ContainerMenu = ({ containerKey, displayName } : ContainerMenuProps
           close={cancelRemove}
           containerKey={containerKey}
           displayName={displayName}
+          index={index}
         />
       )}
     </>

src/library-authoring/containers/ContainerRemover.test.tsx

@@ -0,0 +1,73 @@
+import userEvent from '@testing-library/user-event';
+import type MockAdapter from 'axios-mock-adapter';
+
+import {
+  initializeMocks,
+  render,
+  screen,
+  waitFor,
+} from '@src/testUtils';
+import { ToastProvider } from '@src/generic/toast-context';
+import {
+  getLibraryContainerChildrenApiUrl,
+} from '../data/api';
+import {
+  mockContentLibrary,
+  mockGetContainerChildren,
+} from '../data/api.mocks';
+import ContainerRemover from './ContainerRemover';
+import { LibraryProvider } from '../common/context/LibraryContext';
+
+let axiosMock: MockAdapter;
+
+mockGetContainerChildren.applyMock();
+mockContentLibrary.applyMock();
+
+const mockClose = jest.fn();
+
+const { libraryId } = mockContentLibrary;
+const renderModal = (element: React.ReactNode) => {
+  render(
+    <ToastProvider>
+      <LibraryProvider libraryId={libraryId}>
+        {element}
+      </LibraryProvider>
+    </ToastProvider>,
+  );
+};
+
+jest.mock('react-router-dom', () => ({
+  ...jest.requireActual('react-router-dom'),
+  useParams: jest.fn().mockImplementation(() => ({
+    containerId: mockGetContainerChildren.unitIdWithDuplicate,
+  })),
+}));
+
+describe('<ContainerRemover />', () => {
+  beforeEach(() => {
+    ({ axiosMock } = initializeMocks());
+  });
+
+  it('triggers update container children api call when duplicates are present', async () => {
+    const user = userEvent.setup();
+    const url = getLibraryContainerChildrenApiUrl(mockGetContainerChildren.unitIdWithDuplicate);
+    axiosMock.onPatch(url).reply(200);
+    const result = await mockGetContainerChildren(mockGetContainerChildren.unitIdWithDuplicate);
+    const resultIds = result.map((obj) => obj.id);
+    renderModal(<ContainerRemover
+      close={mockClose}
+      containerKey={result[0].id}
+      displayName="Title"
+      index={0}
+    />);
+    const btn = await screen.findByRole('button', { name: 'Remove' });
+    await user.click(btn);
+
+    await waitFor(() => {
+      expect(axiosMock.history.patch[0].url).toEqual(url);
+    });
+    // Only the first element is removed even though the last element has the same id.
+    expect(JSON.parse(axiosMock.history.patch[0].data).usage_keys).toEqual(resultIds.slice(1));
+    expect(mockClose).toHaveBeenCalled();
+  });
+});

src/library-authoring/containers/ContainerRemover.tsx

@@ -7,32 +7,42 @@ import DeleteModal from '@src/generic/delete-modal/DeleteModal';
 import { ToastContext } from '@src/generic/toast-context';
 import { getBlockType } from '@src/generic/key-utils';
 
-import { useSidebarContext } from '../common/context/SidebarContext';
-import { useLibraryContext } from '../common/context/LibraryContext';
-import { useContainer, useRemoveContainerChildren } from '../data/apiHooks';
-import messages from '../components/messages';
+import { useSidebarContext } from '@src/library-authoring/common/context/SidebarContext';
+import { useLibraryContext } from '@src/library-authoring/common/context/LibraryContext';
+import {
+  useContainer, useContainerChildren, useRemoveContainerChildren, useUpdateContainerChildren,
+} from '@src/library-authoring/data/apiHooks';
+import messages from '@src/library-authoring/components/messages';
+import { Container } from '@src/library-authoring/data/api';
 
 type ContainerRemoverProps = {
   close: () => void,
   containerKey: string,
   displayName: string,
+  index?: number,
 };
 
 const ContainerRemover = ({
   close,
   containerKey,
   displayName,
+  index,
 }: ContainerRemoverProps) => {
   const intl = useIntl();
   const {
     sidebarItemInfo,
     closeLibrarySidebar,
   } = useSidebarContext();
-  const { containerId } = useLibraryContext();
+  const { containerId, showOnlyPublished } = useLibraryContext();
   const { showToast } = useContext(ToastContext);
 
   const removeContainerMutation = useRemoveContainerChildren(containerId);
+  const updateContainerChildrenMutation = useUpdateContainerChildren(containerId);
   const { data: container, isPending } = useContainer(containerId);
+  // Use update api for children if duplicates are present to avoid removing all instances of the child
+  const { data: children } = useContainerChildren<Container>(containerId, showOnlyPublished);
+  const childrenUsageIds = children?.map((child) => child.id);
+  const hasDuplicates = (childrenUsageIds?.filter((child) => child === containerKey).length || 0) > 1;
   const itemType = getBlockType(containerKey);
 
   const removeWarningTitle = intl.formatMessage(messages.removeContainerWarningTitle, {
@@ -50,9 +60,19 @@ const ContainerRemover = ({
 
   const onRemove = useCallback(async () => {
     try {
-      await removeContainerMutation.mutateAsync([containerKey]);
-      if (sidebarItemInfo?.id === containerKey) {
-        closeLibrarySidebar();
+      if (hasDuplicates && childrenUsageIds && typeof index !== 'undefined') {
+        const updatedKeys = childrenUsageIds.filter((childId, idx) => childId !== containerKey || idx !== index);
+        await updateContainerChildrenMutation.mutateAsync(updatedKeys);
+        // istanbul ignore if
+        if (sidebarItemInfo?.id === containerKey && sidebarItemInfo?.index === index) {
+          closeLibrarySidebar();
+        }
+      } else {
+        await removeContainerMutation.mutateAsync([containerKey]);
+        // istanbul ignore if
+        if (sidebarItemInfo?.id === containerKey) {
+          closeLibrarySidebar();
+        }
       }
       showToast(removeSuccess);
     } catch (e) {
@@ -63,12 +83,16 @@ const ContainerRemover = ({
   }, [
     containerKey,
     removeContainerMutation,
+    updateContainerChildrenMutation,
     sidebarItemInfo,
     closeLibrarySidebar,
     showToast,
     removeSuccess,
     removeError,
     close,
+    hasDuplicates,
+    childrenUsageIds,
+    index,
   ]);
 
   // istanbul ignore if: loading state

src/library-authoring/data/api.mocks.ts

@@ -603,6 +603,7 @@ mockGetContainerMetadata.applyMock = () => {
 export async function mockGetContainerChildren(containerId: string): Promise<api.LibraryBlockMetadata[]> {
   let numChildren: number;
   let blockType = 'html';
+  let addDuplicate = false;
   switch (containerId) {
     case mockGetContainerMetadata.unitId:
     case mockGetContainerMetadata.sectionId:
@@ -615,6 +616,10 @@ export async function mockGetContainerChildren(containerId: string): Promise<api
     case mockGetContainerChildren.sixChildren:
       numChildren = 6;
       break;
+    case mockGetContainerChildren.unitIdWithDuplicate:
+      numChildren = 3;
+      addDuplicate = true;
+      break;
     default:
       numChildren = 0;
       break;
@@ -630,19 +635,22 @@ export async function mockGetContainerChildren(containerId: string): Promise<api
     name = blockType;
     typeNamespace = 'lct';
   }
-  return Promise.resolve(
-    Array(numChildren).fill(mockGetContainerChildren.childTemplate).map((child, idx) => (
-      {
-        ...child,
-        // Generate a unique ID for each child block to avoid "duplicate key" errors in tests
-        id: `${typeNamespace}:org1:Demo_course_generated:${blockType}:${name}-${idx}`,
-        displayName: `${name} block ${idx}`,
-        publishedDisplayName: `${name} block published ${idx}`,
-        blockType,
-      }
-    )),
-  );
+  let result = Array(numChildren).fill(mockGetContainerChildren.childTemplate).map((child, idx) => (
+    {
+      ...child,
+      // Generate a unique ID for each child block to avoid "duplicate key" errors in tests
+      id: `${typeNamespace}:org1:Demo_course_generated:${blockType}:${name}-${idx}`,
+      displayName: `${name} block ${idx}`,
+      publishedDisplayName: `${name} block published ${idx}`,
+      blockType,
+    }
+  ));
+  if (addDuplicate) {
+    result = [...result, result[0]];
+  }
+  return Promise.resolve(result);
 }
+mockGetContainerChildren.unitIdWithDuplicate = 'lct:org1:Demo_Course:unit:unit-duplicate';
 mockGetContainerChildren.fiveChildren = 'lct:org1:Demo_Course:unit:unit-5';
 mockGetContainerChildren.sixChildren = 'lct:org1:Demo_Course:unit:unit-6';
 mockGetContainerChildren.childTemplate = {

src/library-authoring/data/api.ts

@@ -702,10 +702,10 @@ export async function restoreContainer(containerId: string) {
 /**
  * Fetch a library container's children's metadata.
  */
-export async function getLibraryContainerChildren(
+export async function getLibraryContainerChildren<ChildType = LibraryBlockMetadata | Container>(
   containerId: string,
   published: boolean = false,
-): Promise<LibraryBlockMetadata[] | Container[]> {
+): Promise<ChildType[]> {
   const { data } = await getAuthenticatedHttpClient().get(
     getLibraryContainerChildrenApiUrl(containerId, published),
   );

src/library-authoring/data/apiHooks.test.tsx

@@ -329,10 +329,11 @@ describe('library api hooks', () => {
     // Keys should be invalidated:
     // 1. library
     // 2. containerChildren
-    // 3. containerHierarchy
-    // 4 & 5. subsections
-    // 6 all hierarchies
-    expect(spy).toHaveBeenCalledTimes(6);
+    // 3. container
+    // 4. containerHierarchy
+    // 5 & 6. subsections
+    // 7 all hierarchies
+    expect(spy).toHaveBeenCalledTimes(7);
   });
 
   describe('publishContainer', () => {

src/library-authoring/data/apiHooks.ts

@@ -736,32 +736,35 @@ export const useRestoreContainer = (containerId: string) => {
 /**
  * Get the metadata and children for a container in a library
  */
-export const useContainerChildren = (containerId?: string, published: boolean = false) => (
-  useQuery({
-    enabled: !!containerId,
-    queryKey: libraryAuthoringQueryKeys.containerChildren(containerId!),
-    queryFn: () => api.getLibraryContainerChildren(containerId!, published),
-    structuralSharing: (
-      oldData: api.LibraryBlockMetadata[] | api.Container[],
-      newData: api.LibraryBlockMetadata[] | api.Container[],
-    ) => {
+export const useContainerChildren = <ChildType extends {
+  id: string;
+  isNew?: boolean;
+} = api.LibraryBlockMetadata | api.Container>(
+    containerId?: string,
+    published: boolean = false,
+  ) => (
+    useQuery({
+      enabled: !!containerId,
+      queryKey: libraryAuthoringQueryKeys.containerChildren(containerId!),
+      queryFn: () => api.getLibraryContainerChildren<ChildType>(containerId!, published),
+      structuralSharing: (oldData: ChildType[], newData: ChildType[]) => {
       // This just sets `isNew` flag to new children components
-      if (oldData) {
-        const oldDataIds = oldData.map((obj) => obj.id);
-        // eslint-disable-next-line no-param-reassign
-        newData = newData.map((newObj) => {
-          if (!oldDataIds.includes(newObj.id)) {
+        if (oldData) {
+          const oldDataIds = oldData.map((obj) => obj.id);
+          // eslint-disable-next-line no-param-reassign
+          newData = newData.map((newObj) => {
+            if (!oldDataIds.includes(newObj.id)) {
             // Set isNew = true if we have new child on refetch
             // eslint-disable-next-line no-param-reassign
-            newObj.isNew = true;
-          }
-          return newObj;
-        });
-      }
-      return replaceEqualDeep(oldData, newData);
-    },
-  })
-);
+              newObj.isNew = true;
+            }
+            return newObj;
+          });
+        }
+        return replaceEqualDeep(oldData, newData);
+      },
+    })
+  );
 
 /**
  * If you work with `useContentFromSearchIndex`, you can use this
@@ -814,6 +817,8 @@ export const useAddItemsToContainer = (containerId?: string) => {
       // It would be complex to bring the entire hierarchy and only update the items within that hierarchy.
       queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.containerHierarchy(undefined) });
       queryClient.invalidateQueries({ queryKey: xblockQueryKeys.componentHierarchy(undefined) });
+      // Invalidate the container to update its publish status
+      queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.container(containerId) });
 
       const containerType = getBlockType(containerId);
       if (containerType === 'section') {

src/library-authoring/routes.ts

@@ -33,13 +33,13 @@ export const ROUTES = {
   COLLECTION: '/collection/:collectionId/:selectedItemId?',
   // LibrarySectionPage route:
   // * with a selected containerId and an optionally selected subsection.
-  SECTION: '/section/:containerId/:selectedItemId?',
+  SECTION: '/section/:containerId/:selectedItemId?/:index?',
   // LibrarySubsectionPage route:
   // * with a selected containerId and an optionally selected unit.
-  SUBSECTION: '/subsection/:containerId/:selectedItemId?',
+  SUBSECTION: '/subsection/:containerId/:selectedItemId?/:index?',
   // LibraryUnitPage route:
   // * with a selected containerId and/or an optionally selected componentId.
-  UNIT: '/unit/:containerId/:selectedItemId?',
+  UNIT: '/unit/:containerId/:selectedItemId?/:index?',
   // LibraryBackupPage route:
   BACKUP: '/backup',
 };
@@ -60,6 +60,7 @@ export type NavigateToData = {
   collectionId?: string,
   containerId?: string,
   contentType?: ContentType,
+  index?: number,
 };
 
 export type LibraryRoutesData = {
@@ -122,6 +123,7 @@ export const useLibraryRoutes = (): LibraryRoutesData => {
     collectionId,
     containerId,
     contentType,
+    index,
   }: NavigateToData = {}) => {
     const routeParams = {
       ...params,
@@ -129,6 +131,7 @@ export const useLibraryRoutes = (): LibraryRoutesData => {
       ...((selectedItemId !== undefined) && { selectedItemId }),
       ...((containerId !== undefined) && { containerId }),
       ...((collectionId !== undefined) && { collectionId }),
+      ...((index !== undefined) && { index }),
     };
     let route: string;
 
@@ -230,6 +233,12 @@ export const useLibraryRoutes = (): LibraryRoutesData => {
       route = ROUTES.HOME;
     }
 
+    // Since index is just the order number of the selectedItemId
+    // clear index if selectedItemId is undefined
+    if (routeParams.selectedItemId === undefined) {
+      routeParams.index = undefined;
+    }
+
     // Also remove the `sa` (sidebar action) search param if it exists.
     searchParams.delete('sa');
 

src/library-authoring/section-subsections/LibraryContainerChildren.tsx

@@ -39,9 +39,12 @@ interface LibraryContainerMetadataWithUniqueId extends Container {
 
 interface ContainerRowProps extends LibraryContainerChildrenProps {
   container: LibraryContainerMetadataWithUniqueId;
+  index?: number;
 }
 
-const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps) => {
+const ContainerRow = ({
+  containerKey, container, readOnly, index,
+}: ContainerRowProps) => {
   const intl = useIntl();
   const { showToast } = useContext(ToastContext);
   const updateMutation = useUpdateContainer(container.originalId, containerKey);
@@ -112,6 +115,7 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps)
           <ContainerMenu
             containerKey={container.originalId}
             displayName={container.displayName}
+            index={index}
           />
         )}
       </Stack>
@@ -148,7 +152,7 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont
     isLoading,
     isError,
     error,
-  } = useContainerChildren(containerKey, showOnlyPublished);
+  } = useContainerChildren<Container>(containerKey, showOnlyPublished);
 
   useEffect(() => {
     // Create new ids which are unique using index.
@@ -164,14 +168,18 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont
     return setOrderedChildren(newChildren || []);
   }, [children, setOrderedChildren]);
 
-  const handleChildClick = useCallback((child: LibraryContainerMetadataWithUniqueId, numberOfClicks: number) => {
+  const handleChildClick = useCallback((
+    child: LibraryContainerMetadataWithUniqueId,
+    numberOfClicks: number,
+    index: number,
+  ) => {
     if (readOnly) {
       // don't allow interaction if rendered as preview
       return;
     }
     const doubleClicked = numberOfClicks > 1;
     if (!doubleClicked) {
-      openItemSidebar(child.originalId, SidebarBodyItemId.ContainerInfo);
+      openItemSidebar(child.originalId, SidebarBodyItemId.ContainerInfo, index);
     } else {
       navigateTo({ containerId: child.originalId });
     }
@@ -215,7 +223,7 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont
         activeId={activeDraggingId}
         setActiveId={setActiveDraggingId}
       >
-        {orderedChildren?.map((child) => (
+        {orderedChildren?.map((child, index) => (
           // A container can have multiple instances of the same block
           // eslint-disable-next-line react/no-array-index-key
           <SortableItem
@@ -229,19 +237,20 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont
               borderLeft: '8px solid #E1DDDB',
             }}
             isClickable={!readOnly}
-            onClick={(e) => skipIfUnwantedTarget(e, (event) => handleChildClick(child, event.detail))}
+            onClick={(e) => skipIfUnwantedTarget(e, (event) => handleChildClick(child, event.detail, index))}
             onKeyDown={(e) => {
               if (e.key === 'Enter') {
-                handleChildClick(child, 1);
+                handleChildClick(child, 1, index);
               }
             }}
             disabled={readOnly || libReadOnly}
-            cardClassName={sidebarItemInfo?.id === child.originalId ? 'selected' : undefined}
+            cardClassName={sidebarItemInfo?.id === child.originalId && sidebarItemInfo?.index === index ? 'selected' : undefined}
             actions={(
               <ContainerRow
                 containerKey={containerKey}
                 container={child}
                 readOnly={readOnly || libReadOnly}
+                index={index}
               />
             )}
           />

src/library-authoring/units/LibraryUnitBlocks.tsx

@@ -46,13 +46,14 @@ interface LibraryBlockMetadataWithUniqueId extends LibraryBlockMetadata {
 }
 
 interface ComponentBlockProps {
+  index: number;
   block: LibraryBlockMetadataWithUniqueId;
   readOnly?: boolean;
   isDragging?: boolean;
 }
 
 /** Component header */
-const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => {
+const BlockHeader = ({ block, index, readOnly }: ComponentBlockProps) => {
   const intl = useIntl();
   const { showOnlyPublished } = useLibraryContext();
   const { showToast } = useContext(ToastContext);
@@ -118,17 +119,18 @@ const BlockHeader = ({ block, readOnly }: ComponentBlockProps) => {
           </Badge>
         )}
         <TagCount size="sm" count={block.tagsCount} onClick={readOnly ? undefined : jumpToManageTags} />
-        {!readOnly && <ComponentMenu usageKey={block.originalId} />}
+        {!readOnly && <ComponentMenu index={index} usageKey={block.originalId} />}
       </Stack>
     </>
   );
 };
 
 /** ComponentBlock to render preview of given component under Unit */
-const ComponentBlock = ({ block, readOnly, isDragging }: ComponentBlockProps) => {
-  const { showOnlyPublished } = useLibraryContext();
+const ComponentBlock = ({
+  block, readOnly, isDragging, index,
+}: ComponentBlockProps) => {
+  const { showOnlyPublished, openComponentEditor } = useLibraryContext();
 
-  const { openComponentEditor } = useLibraryContext();
   const { sidebarItemInfo, openItemSidebar } = useSidebarContext();
 
   const handleComponentSelection = useCallback((numberOfClicks: number) => {
@@ -136,7 +138,11 @@ const ComponentBlock = ({ block, readOnly, isDragging }: ComponentBlockProps) =>
       // don't allow interaction if rendered as preview
       return;
     }
-    openItemSidebar(block.originalId, SidebarBodyItemId.ComponentInfo);
+    openItemSidebar(
+      block.originalId,
+      SidebarBodyItemId.ComponentInfo,
+      index,
+    );
     const canEdit = canEditComponent(block.originalId);
     if (numberOfClicks > 1 && canEdit) {
       // Open editor on double click.
@@ -174,7 +180,7 @@ const ComponentBlock = ({ block, readOnly, isDragging }: ComponentBlockProps) =>
       <SortableItem
         id={block.id}
         componentStyle={getComponentStyle()}
-        actions={<BlockHeader block={block} readOnly={readOnly} />}
+        actions={<BlockHeader block={block} index={index} readOnly={readOnly} />}
         actionStyle={{
           borderRadius: '8px 8px 0px 0px',
           padding: '0.5rem 1rem',
@@ -189,7 +195,7 @@ const ComponentBlock = ({ block, readOnly, isDragging }: ComponentBlockProps) =>
           }
         }}
         disabled={readOnly}
-        cardClassName={sidebarItemInfo?.id === block.originalId ? 'selected' : undefined}
+        cardClassName={sidebarItemInfo?.id === block.originalId && sidebarItemInfo?.index === index ? 'selected' : undefined}
       >
         {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
         <div
@@ -236,7 +242,7 @@ export const LibraryUnitBlocks = ({ unitId, readOnly: componentReadOnly }: Libra
     isLoading,
     isError,
     error,
-  } = useContainerChildren(unitId, showOnlyPublished);
+  } = useContainerChildren<LibraryBlockMetadata>(unitId, showOnlyPublished);
 
   const handleReorder = useCallback(() => async (newOrder?: LibraryBlockMetadataWithUniqueId[]) => {
     if (!newOrder) {
@@ -294,6 +300,7 @@ export const LibraryUnitBlocks = ({ unitId, readOnly: componentReadOnly }: Libra
             // eslint-disable-next-line react/no-array-index-key
             key={`${block.originalId}-${idx}-${block.modified}`}
             block={block}
+            index={idx}
             isDragging={hidePreviewFor === block.id}
             readOnly={readOnly}
           />

src/library-authoring/units/LibraryUnitPage.test.tsx

@@ -381,15 +381,44 @@ describe('<LibraryUnitPage />', () => {
     const restoreFn = mockShowToast.mock.calls[0][1].onClick;
 
     const restoreUrl = getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.unitId);
-    axiosMock.onPost(restoreUrl).reply(200);
+    axiosMock.onPatch(restoreUrl).reply(200);
     // restore collection
     restoreFn();
     await waitFor(() => {
-      expect(axiosMock.history.post.length).toEqual(1);
+      expect(axiosMock.history.patch.length).toEqual(1);
     });
     expect(mockShowToast).toHaveBeenCalledWith('Undo successful');
   });
 
+  it('should remove only one instance of component even if it is present multiple times in this page', async () => {
+    const user = userEvent.setup();
+    const url = getLibraryContainerChildrenApiUrl(mockGetContainerChildren.unitIdWithDuplicate);
+    axiosMock.onPatch(url).reply(200);
+    renderLibraryUnitPage(mockGetContainerChildren.unitIdWithDuplicate);
+
+    expect((await screen.findAllByText('text block 0')).length).toEqual(2);
+    const menu = (await screen.findAllByRole('button', { name: /component actions menu/i }))[0];
+    await user.click(menu);
+
+    const removeButton = await screen.findByText('Remove from unit');
+    await user.click(removeButton);
+
+    const modal = await screen.findByRole('dialog', { name: 'Remove Component' });
+    expect(modal).toBeVisible();
+
+    const confirmButton = await within(modal).findByRole('button', { name: 'Remove' });
+    await user.click(confirmButton);
+    const result = await mockGetContainerChildren(mockGetContainerChildren.unitIdWithDuplicate);
+    const resultIds = result.map((obj) => obj.id);
+
+    await waitFor(() => {
+      expect(axiosMock.history.patch[0].url).toEqual(url);
+    });
+    // Only the first element is removed even though the last element has the same id.
+    expect(JSON.parse(axiosMock.history.patch[0].data).usage_keys).toEqual(resultIds.slice(1));
+    await waitFor(() => expect(mockShowToast).toHaveBeenCalled());
+  });
+
   it('should show error on remove a component', async () => {
     const user = userEvent.setup();
     const url = getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.unitId);
@@ -444,11 +473,11 @@ describe('<LibraryUnitPage />', () => {
     const restoreFn = mockShowToast.mock.calls[0][1].onClick;
 
     const restoreUrl = getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.unitId);
-    axiosMock.onPost(restoreUrl).reply(404);
+    axiosMock.onPatch(restoreUrl).reply(404);
     // restore collection
     restoreFn();
     await waitFor(() => {
-      expect(axiosMock.history.post.length).toEqual(1);
+      expect(axiosMock.history.patch.length).toEqual(1);
     });
     expect(mockShowToast).toHaveBeenCalledWith('Failed to undo remove component operation');
   });