Skip to content

Commit e5da40f

Browse files
navinkarkeranavinkarkera
committed
feat: handle duplicate children in container pages
Adds index number to url and allow selecting one instance and removing it without affecting other instances in the same page
1 parent 2dc087f commit e5da40f

14 files changed

Lines changed: 194 additions & 70 deletions

File tree

‎src/container-comparison/CompareContainersWidget.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
} from './types';
2222
import { diffPreviewContainerChildren, isRowClickable } from './utils';
2323
import messages from './messages';
24+
import { Container, LibraryBlockMetadata } from '@src/library-authoring/data/api';
2425

2526
interface ContainerInfoProps {
2627
upstreamBlockId: string;
@@ -60,7 +61,7 @@ const CompareContainersWidgetInner = ({
6061
data: libData,
6162
isError: isLibError,
6263
error: libError,
63-
} = useContainerChildren(state === 'removed' ? undefined : upstreamBlockId, true);
64+
} = useContainerChildren<Container | LibraryBlockMetadata>(state === 'removed' ? undefined : upstreamBlockId, true);
6465
const {
6566
data: containerData,
6667
isError: isContainerTitleError,

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export interface DefaultTabs {
7272
export interface SidebarItemInfo {
7373
type: SidebarBodyItemId;
7474
id: string;
75+
index?: number;
7576
}
7677

7778
export enum SidebarActions {
@@ -88,7 +89,7 @@ export type SidebarContextData = {
8889
openCollectionInfoSidebar: (collectionId: string) => void;
8990
openComponentInfoSidebar: (usageKey: string) => void;
9091
openContainerInfoSidebar: (usageKey: string) => void;
91-
openItemSidebar: (selectedItemId: string, type: SidebarBodyItemId) => void;
92+
openItemSidebar: (selectedItemId: string, type: SidebarBodyItemId, index?: number) => void;
9293
sidebarItemInfo?: SidebarItemInfo;
9394
sidebarAction: SidebarActions;
9495
setSidebarAction: (action: SidebarActions) => void;
@@ -154,35 +155,38 @@ export const SidebarProvider = ({
154155
setSidebarItemInfo({ id: '', type: SidebarBodyItemId.Info });
155156
}, []);
156157

157-
const openComponentInfoSidebar = useCallback((usageKey: string) => {
158+
const openComponentInfoSidebar = useCallback((usageKey: string, index?: number) => {
158159
setSidebarItemInfo({
159160
id: usageKey,
160161
type: SidebarBodyItemId.ComponentInfo,
162+
index,
161163
});
162164
}, []);
163165

164-
const openCollectionInfoSidebar = useCallback((newCollectionId: string) => {
166+
const openCollectionInfoSidebar = useCallback((newCollectionId: string, index?: number) => {
165167
setSidebarItemInfo({
166168
id: newCollectionId,
167169
type: SidebarBodyItemId.CollectionInfo,
170+
index,
168171
});
169172
}, []);
170173

171-
const openContainerInfoSidebar = useCallback((usageKey: string) => {
174+
const openContainerInfoSidebar = useCallback((usageKey: string, index?: number) => {
172175
setSidebarItemInfo({
173176
id: usageKey,
174177
type: SidebarBodyItemId.ContainerInfo,
178+
index,
175179
});
176180
}, []);
177181

178182
const { navigateTo } = useLibraryRoutes();
179-
const openItemSidebar = useCallback((selectedItemId: string, type: SidebarBodyItemId) => {
180-
navigateTo({ selectedItemId });
181-
setSidebarItemInfo({ id: selectedItemId, type });
183+
const openItemSidebar = useCallback((selectedItemId: string, type: SidebarBodyItemId, index?: number) => {
184+
navigateTo({ selectedItemId, index });
185+
setSidebarItemInfo({ id: selectedItemId, type, index });
182186
}, [navigateTo, setSidebarItemInfo]);
183187

184188
// Set the initial sidebar state based on the URL parameters and context.
185-
const { selectedItemId } = useParams();
189+
const { selectedItemId, index: indexParam } = useParams();
186190
const { collectionId, containerId } = useLibraryContext();
187191
const { componentPickerMode } = useComponentPickerContext();
188192

@@ -198,12 +202,15 @@ export const SidebarProvider = ({
198202

199203
// Handle selected item id changes
200204
if (selectedItemId) {
205+
// if a item is selected that means we have list of items displayed
206+
// which means we can get the index from url and set it.
207+
const indexNumber = indexParam ? Number(indexParam): undefined;
201208
if (selectedItemId.startsWith('lct:')) {
202-
openContainerInfoSidebar(selectedItemId);
209+
openContainerInfoSidebar(selectedItemId, indexNumber);
203210
} else if (selectedItemId.startsWith('lb:')) {
204-
openComponentInfoSidebar(selectedItemId);
211+
openComponentInfoSidebar(selectedItemId, indexNumber);
205212
} else {
206-
openCollectionInfoSidebar(selectedItemId);
213+
openCollectionInfoSidebar(selectedItemId, indexNumber);
207214
}
208215
} else if (collectionId) {
209216
openCollectionInfoSidebar(collectionId);

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ const ComponentActions = ({
111111
const [isPublisherOpen, openPublisher, closePublisher] = useToggle(false);
112112
const canEdit = canEditComponent(componentId);
113113

114+
const { sidebarItemInfo } = useSidebarContext();
115+
114116
if (isPublisherOpen) {
115117
return (
116118
<ComponentPublisher
@@ -141,7 +143,7 @@ const ComponentActions = ({
141143
)}
142144
</div>
143145
<div className="mt-2">
144-
<ComponentMenu usageKey={componentId} />
146+
<ComponentMenu usageKey={componentId} index={sidebarItemInfo?.index} />
145147
</div>
146148
</div>
147149
);

‎src/library-authoring/components/ComponentMenu.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,12 @@ import containerMessages from '../containers/messages';
2323
import { useLibraryRoutes } from '../routes';
2424
import { useRunOnNextRender } from '../../utils';
2525

26-
export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
26+
interface Props {
27+
usageKey: string;
28+
index?: number;
29+
}
30+
31+
export const ComponentMenu = ({ usageKey, index }: Props) => {
2732
const intl = useIntl();
2833
const {
2934
libraryId,
@@ -135,6 +140,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
135140
{isRemoveModalOpen && (
136141
<ComponentRemover
137142
usageKey={usageKey}
143+
index={index}
138144
close={closeRemoveModal}
139145
/>
140146
)}

‎src/library-authoring/components/ComponentRemover.tsx‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,33 @@ import { useSidebarContext } from '../common/context/SidebarContext';
99
import {
1010
useContainer,
1111
useRemoveContainerChildren,
12-
useAddItemsToContainer,
1312
useLibraryBlockMetadata,
13+
useContainerChildren,
14+
useUpdateContainerChildren,
1415
} from '../data/apiHooks';
1516
import messages from './messages';
17+
import { LibraryBlockMetadata } from '../data/api';
1618

1719
interface Props {
1820
usageKey: string;
21+
index?: number;
1922
close: () => void;
2023
}
2124

22-
const ComponentRemover = ({ usageKey, close }: Props) => {
25+
const ComponentRemover = ({ usageKey, index, close }: Props) => {
2326
const intl = useIntl();
2427
const { sidebarItemInfo, closeLibrarySidebar } = useSidebarContext();
25-
const { containerId } = useLibraryContext();
28+
const { containerId, showOnlyPublished } = useLibraryContext();
2629
const { showToast } = useContext(ToastContext);
2730

2831
const removeContainerItemMutation = useRemoveContainerChildren(containerId);
29-
const addItemToContainerMutation = useAddItemsToContainer(containerId);
32+
const updateContainerChildrenMutation = useUpdateContainerChildren(containerId);
3033
const { data: container, isPending: isPendingParentContainer } = useContainer(containerId);
3134
const { data: component, isPending } = useLibraryBlockMetadata(usageKey);
35+
// Use update api for children if duplicates are present to avoid removing all instances of the child
36+
const { data: children } = useContainerChildren<LibraryBlockMetadata>(containerId, showOnlyPublished);
37+
const childrenUsageIds = children?.map((child) => child.id);
38+
const hasDuplicates = (childrenUsageIds?.filter((child) => child === usageKey).length || 0) > 1;
3239

3340
// istanbul ignore if: loading state
3441
if (isPending || isPendingParentContainer) {
@@ -37,8 +44,11 @@ const ComponentRemover = ({ usageKey, close }: Props) => {
3744
}
3845

3946
const removeFromContainer = () => {
47+
if (!childrenUsageIds) {
48+
return;
49+
}
4050
const restoreComponent = () => {
41-
addItemToContainerMutation.mutateAsync([usageKey]).then(() => {
51+
updateContainerChildrenMutation.mutateAsync(childrenUsageIds).then(() => {
4252
showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastSuccess));
4353
}).catch(() => {
4454
showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastFailed));
@@ -63,6 +73,37 @@ const ComponentRemover = ({ usageKey, close }: Props) => {
6373
close();
6474
};
6575

76+
const excludeOneInstance = () => {
77+
if (!childrenUsageIds || typeof index === 'undefined') {
78+
return;
79+
}
80+
const updatedKeys = childrenUsageIds.filter((childId, idx) => childId !== usageKey || idx !== index);
81+
const restoreComponent = () => {
82+
updateContainerChildrenMutation.mutateAsync(childrenUsageIds).then(() => {
83+
showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastSuccess));
84+
}).catch(() => {
85+
showToast(intl.formatMessage(messages.undoRemoveComponentFromContainerToastFailed));
86+
});
87+
};
88+
updateContainerChildrenMutation.mutateAsync(updatedKeys).then(() => {
89+
if (sidebarItemInfo?.id === usageKey && sidebarItemInfo?.index === index) {
90+
// Close sidebar if current component is open
91+
closeLibrarySidebar();
92+
}
93+
showToast(
94+
intl.formatMessage(messages.removeComponentFromContainerSuccess),
95+
{
96+
label: intl.formatMessage(messages.undoRemoveComponentFromContainerToastAction),
97+
onClick: restoreComponent,
98+
},
99+
);
100+
}).catch(() => {
101+
showToast(intl.formatMessage(messages.removeComponentFromContainerFailure));
102+
});
103+
104+
close();
105+
}
106+
66107
const removeText = intl.formatMessage(messages.removeComponentConfirm, {
67108
componentName: <b>{component?.displayName}</b>,
68109
parentContainerName: <b>{container?.displayName}</b>,
@@ -76,7 +117,7 @@ const ComponentRemover = ({ usageKey, close }: Props) => {
76117
title={intl.formatMessage(messages.removeComponentWarningTitle)}
77118
icon={Warning}
78119
description={removeText}
79-
onDeleteSubmit={removeFromContainer}
120+
onDeleteSubmit={hasDuplicates ? excludeOneInstance: removeFromContainer}
80121
btnLabel={intl.formatMessage(messages.componentRemoveButtonLabel)}
81122
buttonVariant="primary"
82123
/>

‎src/library-authoring/containers/ContainerCard.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ import AddComponentWidget from '../components/AddComponentWidget';
3131
type ContainerMenuProps = {
3232
containerKey: string;
3333
displayName: string;
34+
index?: number;
3435
};
3536

36-
export const ContainerMenu = ({ containerKey, displayName } : ContainerMenuProps) => {
37+
export const ContainerMenu = ({ containerKey, displayName, index } : ContainerMenuProps) => {
3738
const intl = useIntl();
3839
const { libraryId, collectionId, containerId } = useLibraryContext();
3940
const {
@@ -144,6 +145,7 @@ export const ContainerMenu = ({ containerKey, displayName } : ContainerMenuProps
144145
close={cancelRemove}
145146
containerKey={containerKey}
146147
displayName={displayName}
148+
index={index}
147149
/>
148150
)}
149151
</>

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,38 @@ import { getBlockType } from '@src/generic/key-utils';
99

1010
import { useSidebarContext } from '../common/context/SidebarContext';
1111
import { useLibraryContext } from '../common/context/LibraryContext';
12-
import { useContainer, useRemoveContainerChildren } from '../data/apiHooks';
12+
import { useContainer, useContainerChildren, useRemoveContainerChildren, useUpdateContainerChildren } from '../data/apiHooks';
1313
import messages from '../components/messages';
14+
import { Container } from '../data/api';
1415

1516
type ContainerRemoverProps = {
1617
close: () => void,
1718
containerKey: string,
1819
displayName: string,
20+
index?: number,
1921
};
2022

2123
const ContainerRemover = ({
2224
close,
2325
containerKey,
2426
displayName,
27+
index,
2528
}: ContainerRemoverProps) => {
2629
const intl = useIntl();
2730
const {
2831
sidebarItemInfo,
2932
closeLibrarySidebar,
3033
} = useSidebarContext();
31-
const { containerId } = useLibraryContext();
34+
const { containerId, showOnlyPublished } = useLibraryContext();
3235
const { showToast } = useContext(ToastContext);
3336

3437
const removeContainerMutation = useRemoveContainerChildren(containerId);
38+
const updateContainerChildrenMutation = useUpdateContainerChildren(containerId);
3539
const { data: container, isPending } = useContainer(containerId);
40+
// Use update api for children if duplicates are present to avoid removing all instances of the child
41+
const { data: children } = useContainerChildren<Container>(containerId, showOnlyPublished);
42+
const childrenUsageIds = children?.map((child) => child.id);
43+
const hasDuplicates = (childrenUsageIds?.filter((child) => child === containerKey).length || 0) > 1;
3644
const itemType = getBlockType(containerKey);
3745

3846
const removeWarningTitle = intl.formatMessage(messages.removeContainerWarningTitle, {
@@ -50,9 +58,17 @@ const ContainerRemover = ({
5058

5159
const onRemove = useCallback(async () => {
5260
try {
53-
await removeContainerMutation.mutateAsync([containerKey]);
54-
if (sidebarItemInfo?.id === containerKey) {
55-
closeLibrarySidebar();
61+
if (hasDuplicates && childrenUsageIds && typeof index !== 'undefined') {
62+
const updatedKeys = childrenUsageIds.filter((childId, idx) => childId !== containerKey || idx !== index);
63+
await updateContainerChildrenMutation.mutateAsync(updatedKeys);
64+
if (sidebarItemInfo?.id === containerKey && sidebarItemInfo?.index === index) {
65+
closeLibrarySidebar();
66+
}
67+
} else {
68+
await removeContainerMutation.mutateAsync([containerKey]);
69+
if (sidebarItemInfo?.id === containerKey) {
70+
closeLibrarySidebar();
71+
}
5672
}
5773
showToast(removeSuccess);
5874
} catch (e) {

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ mockGetContainerMetadata.applyMock = () => {
603603
export async function mockGetContainerChildren(containerId: string): Promise<api.LibraryBlockMetadata[]> {
604604
let numChildren: number;
605605
let blockType = 'html';
606+
let addDuplicate = false;
606607
switch (containerId) {
607608
case mockGetContainerMetadata.unitId:
608609
case mockGetContainerMetadata.sectionId:
@@ -615,6 +616,10 @@ export async function mockGetContainerChildren(containerId: string): Promise<api
615616
case mockGetContainerChildren.sixChildren:
616617
numChildren = 6;
617618
break;
619+
case mockGetContainerChildren.unitIdWithDuplicate:
620+
numChildren = 3;
621+
addDuplicate = true;
622+
break;
618623
default:
619624
numChildren = 0;
620625
break;
@@ -630,19 +635,22 @@ export async function mockGetContainerChildren(containerId: string): Promise<api
630635
name = blockType;
631636
typeNamespace = 'lct';
632637
}
633-
return Promise.resolve(
634-
Array(numChildren).fill(mockGetContainerChildren.childTemplate).map((child, idx) => (
635-
{
636-
...child,
637-
// Generate a unique ID for each child block to avoid "duplicate key" errors in tests
638-
id: `${typeNamespace}:org1:Demo_course_generated:${blockType}:${name}-${idx}`,
639-
displayName: `${name} block ${idx}`,
640-
publishedDisplayName: `${name} block published ${idx}`,
641-
blockType,
642-
}
643-
)),
644-
);
638+
let result =Array(numChildren).fill(mockGetContainerChildren.childTemplate).map((child, idx) => (
639+
{
640+
...child,
641+
// Generate a unique ID for each child block to avoid "duplicate key" errors in tests
642+
id: `${typeNamespace}:org1:Demo_course_generated:${blockType}:${name}-${idx}`,
643+
displayName: `${name} block ${idx}`,
644+
publishedDisplayName: `${name} block published ${idx}`,
645+
blockType,
646+
}
647+
));
648+
if (addDuplicate) {
649+
result = [...result, result[0]];
650+
}
651+
return Promise.resolve(result);
645652
}
653+
mockGetContainerChildren.unitIdWithDuplicate = 'lct:org1:Demo_Course:unit:unit-duplicate';
646654
mockGetContainerChildren.fiveChildren = 'lct:org1:Demo_Course:unit:unit-5';
647655
mockGetContainerChildren.sixChildren = 'lct:org1:Demo_Course:unit:unit-6';
648656
mockGetContainerChildren.childTemplate = {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,10 +702,10 @@ export async function restoreContainer(containerId: string) {
702702
/**
703703
* Fetch a library container's children's metadata.
704704
*/
705-
export async function getLibraryContainerChildren(
705+
export async function getLibraryContainerChildren<T>(
706706
containerId: string,
707707
published: boolean = false,
708-
): Promise<LibraryBlockMetadata[] | Container[]> {
708+
): Promise<T[]> {
709709
const { data } = await getAuthenticatedHttpClient().get(
710710
getLibraryContainerChildrenApiUrl(containerId, published),
711711
);

0 commit comments

Comments
 (0)