diff --git a/src/container-comparison/CompareContainersWidget.tsx b/src/container-comparison/CompareContainersWidget.tsx index cb1b318bf..06968a503 100644 --- a/src/container-comparison/CompareContainersWidget.tsx +++ b/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(state === 'removed' ? undefined : upstreamBlockId, true); const { data: containerData, isError: isContainerTitleError, diff --git a/src/library-authoring/common/context/SidebarContext.tsx b/src/library-authoring/common/context/SidebarContext.tsx index fea6d9c35..d1323adf6 100644 --- a/src/library-authoring/common/context/SidebarContext.tsx +++ b/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); diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx index 967a4050a..8e8d8adaa 100644 --- a/src/library-authoring/component-info/ComponentInfo.tsx +++ b/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 (
- +
); diff --git a/src/library-authoring/components/ComponentMenu.tsx b/src/library-authoring/components/ComponentMenu.tsx index 1a46ce50f..33b67f1e6 100644 --- a/src/library-authoring/components/ComponentMenu.tsx +++ b/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 && ( )} diff --git a/src/library-authoring/components/ComponentRemover.tsx b/src/library-authoring/components/ComponentRemover.tsx index 1f3a1ca6d..b776978b5 100644 --- a/src/library-authoring/components/ComponentRemover.tsx +++ b/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(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" /> diff --git a/src/library-authoring/containers/ContainerCard.tsx b/src/library-authoring/containers/ContainerCard.tsx index fb1132af7..229c22b7f 100644 --- a/src/library-authoring/containers/ContainerCard.tsx +++ b/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} /> )} diff --git a/src/library-authoring/containers/ContainerRemover.test.tsx b/src/library-authoring/containers/ContainerRemover.test.tsx new file mode 100644 index 000000000..7a4945238 --- /dev/null +++ b/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( + + + {element} + + , + ); +}; + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: jest.fn().mockImplementation(() => ({ + containerId: mockGetContainerChildren.unitIdWithDuplicate, + })), +})); + +describe('', () => { + 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(); + 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(); + }); +}); diff --git a/src/library-authoring/containers/ContainerRemover.tsx b/src/library-authoring/containers/ContainerRemover.tsx index 2261edb59..b60ca6240 100644 --- a/src/library-authoring/containers/ContainerRemover.tsx +++ b/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(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 diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 44151f34b..760642bb5 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -603,6 +603,7 @@ mockGetContainerMetadata.applyMock = () => { export async function mockGetContainerChildren(containerId: string): Promise { 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 ( - { - ...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 = { diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index f1d29d4c2..211b1614b 100644 --- a/src/library-authoring/data/api.ts +++ b/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( containerId: string, published: boolean = false, -): Promise { +): Promise { const { data } = await getAuthenticatedHttpClient().get( getLibraryContainerChildrenApiUrl(containerId, published), ); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 64e571d94..dca9de9d6 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/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 = ( + containerId?: string, + published: boolean = false, + ) => ( + useQuery({ + enabled: !!containerId, + queryKey: libraryAuthoringQueryKeys.containerChildren(containerId!), + queryFn: () => api.getLibraryContainerChildren(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 diff --git a/src/library-authoring/routes.ts b/src/library-authoring/routes.ts index 82c181329..c0b4d35da 100644 --- a/src/library-authoring/routes.ts +++ b/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'); diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 9a315a4aa..cf1d731b3 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/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) )} @@ -148,7 +152,7 @@ export const LibraryContainerChildren = ({ containerKey, readOnly }: LibraryCont isLoading, isError, error, - } = useContainerChildren(containerKey, showOnlyPublished); + } = useContainerChildren(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 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={( )} /> diff --git a/src/library-authoring/units/LibraryUnitBlocks.tsx b/src/library-authoring/units/LibraryUnitBlocks.tsx index 7bd5985fb..a96636def 100644 --- a/src/library-authoring/units/LibraryUnitBlocks.tsx +++ b/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) => { )} - {!readOnly && } + {!readOnly && } ); }; /** 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) => } + actions={} 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 */}
(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} /> diff --git a/src/library-authoring/units/LibraryUnitPage.test.tsx b/src/library-authoring/units/LibraryUnitPage.test.tsx index 1f9fe10ea..991436d2f 100644 --- a/src/library-authoring/units/LibraryUnitPage.test.tsx +++ b/src/library-authoring/units/LibraryUnitPage.test.tsx @@ -381,15 +381,44 @@ describe('', () => { 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('', () => { 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'); });