feat: handle duplicate children in container pages [FC-0112] (#2584)

If we have duplicate container or component in parent page in library, clicking on one of them selects both and removing any one from the parent blocks removes all instances.
This PR handles duplicates by including index/order_number of each child component in the url and using it to exclude a single instance and update parent structure.
This commit is contained in:
Navin Karkera
2025-11-03 20:29:37 +05:30
committed by GitHub
parent cec074e6d4
commit 75ae9d549c
15 changed files with 341 additions and 120 deletions

View File

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

View File

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

View File

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

View File

@@ -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}
/>
)}

View File

@@ -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"
/>

View File

@@ -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}
/>
)}
</>

View File

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

View File

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

View File

@@ -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 = {

View File

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

View File

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

View File

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

View File

@@ -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}
/>
)}
/>

View File

@@ -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}
/>

View File

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