From db9524e1bdc883c30da152f823974fb8486ae1e6 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 5 Jun 2025 20:21:22 -0500 Subject: [PATCH 01/12] fix: Rename optimistic update in children containers --- .../containers/ContainerEditableTitle.tsx | 32 +++++++++++++++---- .../LibraryContainerChildren.tsx | 26 +++------------ 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/library-authoring/containers/ContainerEditableTitle.tsx b/src/library-authoring/containers/ContainerEditableTitle.tsx index 5a1ea0f6df..4d18c39810 100644 --- a/src/library-authoring/containers/ContainerEditableTitle.tsx +++ b/src/library-authoring/containers/ContainerEditableTitle.tsx @@ -8,15 +8,29 @@ import messages from './messages'; interface EditableTitleProps { containerId: string; + readOnly?: boolean; textClassName?: string; + // In some cases, the title is already available, but it's retrieved in a list of containers. + // In these cases, it's necessary to use this `ContainerEditableTitle` for the optimistic update to work. + // By using `placeHolderText`, we can give the illusion that the data has already been loaded before using the real data. + placeHolderText?: string; } -export const ContainerEditableTitle = ({ containerId, textClassName }: EditableTitleProps) => { +export const ContainerEditableTitle = ({ + containerId, + readOnly, + textClassName, + placeHolderText, +}: EditableTitleProps) => { const intl = useIntl(); - const { readOnly, showOnlyPublished } = useLibraryContext(); + const { readOnly: libReadOnly, showOnlyPublished } = useLibraryContext(); - const { data: container } = useContainer(containerId); + if (!readOnly) { + readOnly = libReadOnly; + } + + const { data: container, isLoading } = useContainer(containerId); const updateMutation = useUpdateContainer(containerId); const { showToast } = useContext(ToastContext); @@ -32,15 +46,19 @@ export const ContainerEditableTitle = ({ containerId, textClassName }: EditableT } }; - // istanbul ignore if: this should never happen - if (!container) { - return null; + let textTitle; + if (isLoading && placeHolderText) { + textTitle = placeHolderText; + } else if (isLoading || !container) { + textTitle = ''; + } else { + textTitle = showOnlyPublished ? (container.publishedDisplayName ?? container.displayName) : container.displayName } return ( diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 70913afd68..ad9e7b7130 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -12,13 +12,11 @@ import ErrorAlert from '../../generic/alert-error'; import { useLibraryContext } from '../common/context/LibraryContext'; import { useContainerChildren, - useUpdateContainer, useUpdateContainerChildren, } from '../data/apiHooks'; import { messages, subsectionMessages, sectionMessages } from './messages'; -import containerMessages from '../containers/messages'; +import { ContainerEditableTitle } from '../containers'; import { Container } from '../data/api'; -import { InplaceTextEditor } from '../../generic/inplace-text-editor'; import { ToastContext } from '../../generic/toast-context'; import TagCount from '../../generic/tag-count'; import { ContainerMenu } from '../components/ContainerCard'; @@ -40,29 +38,15 @@ interface ContainerRowProps extends LibraryContainerChildrenProps { } const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { - const intl = useIntl(); - const { showToast } = useContext(ToastContext); - const updateMutation = useUpdateContainer(container.originalId); const { showOnlyPublished } = useLibraryContext(); - const handleSaveDisplayName = async (newDisplayName: string) => { - try { - await updateMutation.mutateAsync({ - displayName: newDisplayName, - }); - showToast(intl.formatMessage(containerMessages.updateContainerSuccessMsg)); - } catch (err) { - showToast(intl.formatMessage(containerMessages.updateContainerErrorMsg)); - } - }; - return ( <> - Date: Thu, 5 Jun 2025 20:53:59 -0500 Subject: [PATCH 02/12] style: Fix broken lint --- .../containers/ContainerEditableTitle.tsx | 11 ++++------- .../section-subsections/LibraryContainerChildren.tsx | 6 ++++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/library-authoring/containers/ContainerEditableTitle.tsx b/src/library-authoring/containers/ContainerEditableTitle.tsx index 4d18c39810..a20fd1fbe6 100644 --- a/src/library-authoring/containers/ContainerEditableTitle.tsx +++ b/src/library-authoring/containers/ContainerEditableTitle.tsx @@ -12,7 +12,8 @@ interface EditableTitleProps { textClassName?: string; // In some cases, the title is already available, but it's retrieved in a list of containers. // In these cases, it's necessary to use this `ContainerEditableTitle` for the optimistic update to work. - // By using `placeHolderText`, we can give the illusion that the data has already been loaded before using the real data. + // By using `placeHolderText`, we can give the illusion that the data + // has already been loaded before using the real data. placeHolderText?: string; } @@ -26,10 +27,6 @@ export const ContainerEditableTitle = ({ const { readOnly: libReadOnly, showOnlyPublished } = useLibraryContext(); - if (!readOnly) { - readOnly = libReadOnly; - } - const { data: container, isLoading } = useContainer(containerId); const updateMutation = useUpdateContainer(containerId); @@ -52,14 +49,14 @@ export const ContainerEditableTitle = ({ } else if (isLoading || !container) { textTitle = ''; } else { - textTitle = showOnlyPublished ? (container.publishedDisplayName ?? container.displayName) : container.displayName + textTitle = showOnlyPublished ? (container.publishedDisplayName ?? container.displayName) : container.displayName; } return ( ); diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index ad9e7b7130..6f547dd0b3 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -42,11 +42,13 @@ const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { return ( <> - Date: Tue, 10 Jun 2025 22:10:12 -0500 Subject: [PATCH 03/12] test: Fix test for optimistic rename --- src/library-authoring/data/api.mocks.ts | 16 ++++++++++++++- .../LibrarySectionSubsectionPage.test.tsx | 20 +++++++++++++------ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index d4f678d798..262de0c6c6 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -493,6 +493,17 @@ export async function mockGetContainerMetadata(containerId: string): Promise ( { ...child, // Generate a unique ID for each child block to avoid "duplicate key" errors in tests - id: `lb:org1:Demo_course:${blockType}:${name}-${idx}`, + id: `${typeNamespace}:org1:Demo_course_generated:${blockType}:${name}-${idx}`, displayName: `${name} block ${idx}`, publishedDisplayName: `${name} block published ${idx}`, } diff --git a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx index 727773c914..e4da6eb903 100644 --- a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx @@ -104,6 +104,10 @@ describe('', () => { const childType = cType === ContainerType.Section ? ContainerType.Subsection : ContainerType.Unit; + let typeNamespace = 'lct'; + if (cType === ContainerType.Unit) { + typeNamespace = 'lb'; + } it(`shows the spinner before the query is complete in ${cType} page`, async () => { // This mock will never return data about the collection (it loads forever): const cId = cType === ContainerType.Section @@ -253,7 +257,7 @@ describe('', () => { }); it(`should rename child by clicking edit icon besides name in ${cType} page`, async () => { - const url = getLibraryContainerApiUrl(`lb:org1:Demo_course:${childType}:${childType}-0`); + const url = getLibraryContainerApiUrl(`${typeNamespace}:org1:Demo_course_generated:${childType}:${childType}-0`); axiosMock.onPatch(url).reply(200); renderLibrarySectionPage(undefined, undefined, cType); @@ -285,7 +289,7 @@ describe('', () => { }); it(`should show error while updating child name in ${cType} page`, async () => { - const url = getLibraryContainerApiUrl(`lb:org1:Demo_course:${childType}:${childType}-0`); + const url = getLibraryContainerApiUrl(`${typeNamespace}:org1:Demo_course_generated:${childType}:${childType}-0`); axiosMock.onPatch(url).reply(400); renderLibrarySectionPage(undefined, undefined, cType); @@ -326,7 +330,7 @@ describe('', () => { .onPatch(getLibraryContainerChildrenApiUrl(cId)) .reply(200); verticalSortableListCollisionDetection.mockReturnValue([{ - id: `lb:org1:Demo_course:${childType}:${childType}-1----1`, + id: `${typeNamespace}:org1:Demo_course_generated:${childType}:${childType}-1----1`, }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); @@ -344,7 +348,9 @@ describe('', () => { axiosMock .onPatch(getLibraryContainerChildrenApiUrl(cId)) .reply(200); - verticalSortableListCollisionDetection.mockReturnValue([{ id: `lb:org1:Demo_course:${childType}:${childType}-1----1` }]); + verticalSortableListCollisionDetection.mockReturnValue([{ + id: `${typeNamespace}:org1:Demo_course_generated:${childType}:${childType}-1----1`, + }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); @@ -361,7 +367,9 @@ describe('', () => { axiosMock .onPatch(getLibraryContainerChildrenApiUrl(cId)) .reply(500); - verticalSortableListCollisionDetection.mockReturnValue([{ id: `lb:org1:Demo_course:${childType}:${childType}-1----1` }]); + verticalSortableListCollisionDetection.mockReturnValue([{ + id: `${typeNamespace}:org1:Demo_course_generated:${childType}:${childType}-1----1`, + }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); @@ -374,7 +382,7 @@ describe('', () => { const child = await screen.findByText(`${childType} block 0`); // trigger double click userEvent.click(child.parentElement!, undefined, { clickCount: 2 }); - expect((await screen.findAllByText(new RegExp(`Test ${childType}`, 'i')))[0]).toBeInTheDocument(); + expect((await screen.findAllByText(new RegExp(`${childType} block 0`, 'i')))[0]).toBeInTheDocument(); expect(await screen.findByRole('button', { name: new RegExp(`${childType} Info`, 'i') })).toBeInTheDocument(); }); }); From c5b1dd333b3a5237800b19826dc3203ea61c2b80 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 10 Jun 2025 22:31:37 -0500 Subject: [PATCH 04/12] fix: broken tests --- src/library-authoring/data/api.mocks.ts | 2 +- .../units/LibraryUnitPage.test.tsx | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 262de0c6c6..39ba3d67ab 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -283,7 +283,7 @@ mockXBlockFields.dataHtml = { metadata: { displayName: 'Introduction to Testing' }, } satisfies api.XBlockFields; // Mock of another "regular" HTML (Text) block: -mockXBlockFields.usageKey0 = 'lb:org1:Demo_course:html:text-0'; +mockXBlockFields.usageKey0 = 'lb:org1:Demo_course_generated:html:text-0'; mockXBlockFields.dataHtml0 = { displayName: 'text block 0', data: '

This is a text component which uses HTML.

', diff --git a/src/library-authoring/units/LibraryUnitPage.test.tsx b/src/library-authoring/units/LibraryUnitPage.test.tsx index e9584a8491..399311064b 100644 --- a/src/library-authoring/units/LibraryUnitPage.test.tsx +++ b/src/library-authoring/units/LibraryUnitPage.test.tsx @@ -211,7 +211,7 @@ describe('', () => { }); it('should rename component while clicking on name', async () => { - const url = getXBlockFieldsApiUrl('lb:org1:Demo_course:html:text-0'); + const url = getXBlockFieldsApiUrl('lb:org1:Demo_course_generated:html:text-0'); axiosMock.onPost(url).reply(200); renderLibraryUnitPage(); @@ -245,7 +245,7 @@ describe('', () => { }); it('should show error while updating component name', async () => { - const url = getXBlockFieldsApiUrl('lb:org1:Demo_course:html:text-0'); + const url = getXBlockFieldsApiUrl('lb:org1:Demo_course_generated:html:text-0'); axiosMock.onPost(url).reply(400); renderLibraryUnitPage(); @@ -284,7 +284,9 @@ describe('', () => { axiosMock .onPatch(getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.unitId)) .reply(200); - verticalSortableListCollisionDetection.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1----1' }]); + verticalSortableListCollisionDetection.mockReturnValue([{ + id: 'lb:org1:Demo_course_generated:html:text-1----1', + }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); @@ -298,7 +300,9 @@ describe('', () => { axiosMock .onPatch(getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.unitId)) .reply(200); - verticalSortableListCollisionDetection.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1----1' }]); + verticalSortableListCollisionDetection.mockReturnValue([{ + id: 'lb:org1:Demo_course_generated:html:text-1----1', + }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); @@ -312,7 +316,9 @@ describe('', () => { axiosMock .onPatch(getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.unitId)) .reply(500); - verticalSortableListCollisionDetection.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1----1' }]); + verticalSortableListCollisionDetection.mockReturnValue([{ + id: 'lb:org1:Demo_course_generated:html:text-1----1', + }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); From cb33e4e2981fa9df81deaf0fc75dc42628b9bc1d Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 12 Jun 2025 18:31:25 -0500 Subject: [PATCH 05/12] fix: Cycle import --- .../section-subsections/LibraryContainerChildren.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 6f547dd0b3..a98e513a6d 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -15,7 +15,7 @@ import { useUpdateContainerChildren, } from '../data/apiHooks'; import { messages, subsectionMessages, sectionMessages } from './messages'; -import { ContainerEditableTitle } from '../containers'; +import { ContainerEditableTitle } from '../containers/ContainerEditableTitle'; import { Container } from '../data/api'; import { ToastContext } from '../../generic/toast-context'; import TagCount from '../../generic/tag-count'; From 78dc044481e4346b4ea396befb50f580e454cdf6 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 18 Jun 2025 14:55:25 -0500 Subject: [PATCH 06/12] fix: Avoid open the container page when click renam text --- .../LibraryContainerChildren.tsx | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index ecd396c6c0..8f4fa4a232 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -43,14 +43,19 @@ const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { return ( <> - +
e.stopPropagation()} + > + +
Date: Wed, 18 Jun 2025 15:09:41 -0500 Subject: [PATCH 07/12] fix: Broken lint --- .../section-subsections/LibraryContainerChildren.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 8f4fa4a232..5769635d7d 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -43,6 +43,7 @@ const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { return ( <> + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
e.stopPropagation()} From ca2ed2c6b59c1c13da66666cab26e32c73b3fe71 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 18 Jun 2025 15:42:49 -0500 Subject: [PATCH 08/12] fix: Broken test --- .../section-subsections/LibrarySectionSubsectionPage.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx index e4da6eb903..62b0cd5321 100644 --- a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx @@ -380,8 +380,8 @@ describe('', () => { it(`should open ${childType} page on double click`, async () => { renderLibrarySectionPage(undefined, undefined, cType); const child = await screen.findByText(`${childType} block 0`); - // trigger double click - userEvent.click(child.parentElement!, undefined, { clickCount: 2 }); + // Trigger double click. Find the chidl card as the parent element + userEvent.click(child.parentElement!.parentElement!.parentElement!, undefined, { clickCount: 2 }); expect((await screen.findAllByText(new RegExp(`${childType} block 0`, 'i')))[0]).toBeInTheDocument(); expect(await screen.findByRole('button', { name: new RegExp(`${childType} Info`, 'i') })).toBeInTheDocument(); }); From 5cc7e96f4bdd6da34bd0699bfcd138645b0cd777 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 18 Jun 2025 19:48:48 -0500 Subject: [PATCH 09/12] refactor: Optimistic update of container children --- .../containers/ContainerEditableTitle.tsx | 31 +++++-------------- src/library-authoring/data/apiHooks.ts | 18 +++++++++-- .../LibraryContainerChildren.tsx | 30 +++++++++++++----- .../LibrarySectionSubsectionPage.test.tsx | 8 +++-- 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/library-authoring/containers/ContainerEditableTitle.tsx b/src/library-authoring/containers/ContainerEditableTitle.tsx index a20fd1fbe6..5a1ea0f6df 100644 --- a/src/library-authoring/containers/ContainerEditableTitle.tsx +++ b/src/library-authoring/containers/ContainerEditableTitle.tsx @@ -8,26 +8,15 @@ import messages from './messages'; interface EditableTitleProps { containerId: string; - readOnly?: boolean; textClassName?: string; - // In some cases, the title is already available, but it's retrieved in a list of containers. - // In these cases, it's necessary to use this `ContainerEditableTitle` for the optimistic update to work. - // By using `placeHolderText`, we can give the illusion that the data - // has already been loaded before using the real data. - placeHolderText?: string; } -export const ContainerEditableTitle = ({ - containerId, - readOnly, - textClassName, - placeHolderText, -}: EditableTitleProps) => { +export const ContainerEditableTitle = ({ containerId, textClassName }: EditableTitleProps) => { const intl = useIntl(); - const { readOnly: libReadOnly, showOnlyPublished } = useLibraryContext(); + const { readOnly, showOnlyPublished } = useLibraryContext(); - const { data: container, isLoading } = useContainer(containerId); + const { data: container } = useContainer(containerId); const updateMutation = useUpdateContainer(containerId); const { showToast } = useContext(ToastContext); @@ -43,20 +32,16 @@ export const ContainerEditableTitle = ({ } }; - let textTitle; - if (isLoading && placeHolderText) { - textTitle = placeHolderText; - } else if (isLoading || !container) { - textTitle = ''; - } else { - textTitle = showOnlyPublished ? (container.publishedDisplayName ?? container.displayName) : container.displayName; + // istanbul ignore if: this should never happen + if (!container) { + return null; } return ( ); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 6d583fa3c9..9febb96277 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -613,7 +613,7 @@ export const useContainer = (containerId?: string) => ( /** * Use this mutation to update the fields of a container in a library */ -export const useUpdateContainer = (containerId: string) => { +export const useUpdateContainer = (containerId: string, affectedParentContainerId?: string) => { const libraryId = getLibraryId(containerId); const queryClient = useQueryClient(); const containerQueryKey = libraryAuthoringQueryKeys.container(containerId); @@ -626,10 +626,24 @@ export const useUpdateContainer = (containerId: string) => { ...data, }); - return { previousData }; + let childrenPreviousData; + if (affectedParentContainerId) { + const childrenQueryKey = libraryAuthoringQueryKeys.containerChildren(affectedParentContainerId); + childrenPreviousData = queryClient.getQueryData(childrenQueryKey) as api.Container[]; + queryClient.setQueryData(childrenQueryKey, childrenPreviousData.map(item => ( + item.id === containerId ? { ...item, ...data } : item + ))); + } + + return { previousData, childrenPreviousData }; }, onError: (_err, _data, context) => { queryClient.setQueryData(containerQueryKey, context?.previousData); + + if (affectedParentContainerId) { + const childrenQueryKey = libraryAuthoringQueryKeys.containerChildren(affectedParentContainerId); + queryClient.setQueryData(childrenQueryKey, context?.childrenPreviousData); + } }, onSettled: () => { // NOTE: We invalidate the library query here because we need to update the library's diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 5769635d7d..561d8f23b3 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -6,6 +6,7 @@ import { ActionRow, Badge, Icon, Stack, } from '@openedx/paragon'; import { Description } from '@openedx/paragon/icons'; +import { InplaceTextEditor } from '@src/generic/inplace-text-editor'; import DraggableList, { SortableItem } from '../../generic/DraggableList'; import Loading from '../../generic/Loading'; import ErrorAlert from '../../generic/alert-error'; @@ -13,10 +14,11 @@ import { ContainerType, getBlockType } from '../../generic/key-utils'; import { useLibraryContext } from '../common/context/LibraryContext'; import { useContainerChildren, + useUpdateContainer, useUpdateContainerChildren, } from '../data/apiHooks'; +import containerMessages from '../containers/messages'; import { messages, subsectionMessages, sectionMessages } from './messages'; -import { ContainerEditableTitle } from '../containers/ContainerEditableTitle'; import { Container } from '../data/api'; import { ToastContext } from '../../generic/toast-context'; import TagCount from '../../generic/tag-count'; @@ -38,8 +40,22 @@ interface ContainerRowProps extends LibraryContainerChildrenProps { container: LibraryContainerMetadataWithUniqueId; } -const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { +const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps) => { const { showOnlyPublished } = useLibraryContext(); + const intl = useIntl(); + const { showToast } = useContext(ToastContext); + const updateMutation = useUpdateContainer(container.originalId, containerKey); + + const handleSaveDisplayName = async (newDisplayName: string) => { + try { + await updateMutation.mutateAsync({ + displayName: newDisplayName, + }); + showToast(intl.formatMessage(containerMessages.updateContainerSuccessMsg)); + } catch (err) { + showToast(intl.formatMessage(containerMessages.updateContainerErrorMsg)); + } + }; return ( <> @@ -48,13 +64,11 @@ const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { // Prevent parent card from being clicked. onClick={(e) => e.stopPropagation()} > -
diff --git a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx index 62b0cd5321..029cab724a 100644 --- a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx @@ -1,5 +1,6 @@ import userEvent from '@testing-library/user-event'; import type MockAdapter from 'axios-mock-adapter'; +import { QueryClient } from '@tanstack/react-query'; import { act } from 'react'; import { @@ -31,6 +32,7 @@ const path = '/library/:libraryId/*'; const libraryTitle = mockContentLibrary.libraryData.title; let axiosMock: MockAdapter; +let queryClient: QueryClient; let mockShowToast: (message: string, action?: ToastActionData | undefined) => void; mockClipboardEmpty.applyMock(); @@ -67,7 +69,7 @@ jest.mock('../../generic/DraggableList/verticalSortableList', () => ({ describe('', () => { beforeEach(() => { - ({ axiosMock, mockShowToast } = initializeMocks()); + ({ axiosMock, mockShowToast, queryClient } = initializeMocks()); }); afterEach(() => { @@ -257,6 +259,7 @@ describe('', () => { }); it(`should rename child by clicking edit icon besides name in ${cType} page`, async () => { + const mockSetQueryData = jest.spyOn(queryClient, 'setQueryData'); const url = getLibraryContainerApiUrl(`${typeNamespace}:org1:Demo_course_generated:${childType}:${childType}-0`); axiosMock.onPatch(url).reply(200); renderLibrarySectionPage(undefined, undefined, cType); @@ -286,6 +289,7 @@ describe('', () => { })); expect(textBox).not.toBeInTheDocument(); expect(mockShowToast).toHaveBeenCalledWith('Container updated successfully.'); + expect(mockSetQueryData).toHaveBeenCalledTimes(2); }); it(`should show error while updating child name in ${cType} page`, async () => { @@ -380,7 +384,7 @@ describe('', () => { it(`should open ${childType} page on double click`, async () => { renderLibrarySectionPage(undefined, undefined, cType); const child = await screen.findByText(`${childType} block 0`); - // Trigger double click. Find the chidl card as the parent element + // Trigger double click. Find the child card as the parent element userEvent.click(child.parentElement!.parentElement!.parentElement!, undefined, { clickCount: 2 }); expect((await screen.findAllByText(new RegExp(`${childType} block 0`, 'i')))[0]).toBeInTheDocument(); expect(await screen.findByRole('button', { name: new RegExp(`${childType} Info`, 'i') })).toBeInTheDocument(); From d5353dba07f9e5bd26158b8147664d2181551782 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 18 Jun 2025 19:56:30 -0500 Subject: [PATCH 10/12] style: nits on the code --- .../section-subsections/LibraryContainerChildren.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 561d8f23b3..44c25ad201 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -17,8 +17,8 @@ import { useUpdateContainer, useUpdateContainerChildren, } from '../data/apiHooks'; -import containerMessages from '../containers/messages'; import { messages, subsectionMessages, sectionMessages } from './messages'; +import containerMessages from '../containers/messages'; import { Container } from '../data/api'; import { ToastContext } from '../../generic/toast-context'; import TagCount from '../../generic/tag-count'; From e941ede65515a2054fc2e2be6f666aabc5d2d819 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 18 Jun 2025 20:01:52 -0500 Subject: [PATCH 11/12] style: nits on the code --- .../section-subsections/LibraryContainerChildren.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index 44c25ad201..cf5a62c888 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -41,10 +41,10 @@ interface ContainerRowProps extends LibraryContainerChildrenProps { } const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps) => { - const { showOnlyPublished } = useLibraryContext(); const intl = useIntl(); const { showToast } = useContext(ToastContext); const updateMutation = useUpdateContainer(container.originalId, containerKey); + const { showOnlyPublished } = useLibraryContext(); const handleSaveDisplayName = async (newDisplayName: string) => { try { From 4b51703452a78f76eda097edf20dd42a51aedb14 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 19 Jun 2025 15:49:47 -0500 Subject: [PATCH 12/12] fix: Unnecessary setQuery if is not called yet --- src/library-authoring/data/apiHooks.ts | 30 ++++++++++++------- .../LibrarySectionSubsectionPage.test.tsx | 2 +- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 9febb96277..72db994e64 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -611,7 +611,10 @@ export const useContainer = (containerId?: string) => ( ); /** - * Use this mutation to update the fields of a container in a library + * Use this mutation to update the fields of a container in a library. + * + * Use `affectedParentContainerId` to enable the optimistic update when the container + * is updated from a children list of a container */ export const useUpdateContainer = (containerId: string, affectedParentContainerId?: string) => { const libraryId = getLibraryId(containerId); @@ -621,26 +624,33 @@ export const useUpdateContainer = (containerId: string, affectedParentContainerI mutationFn: (data: api.UpdateContainerDataRequest) => api.updateContainerMetadata(containerId, data), onMutate: (data) => { const previousData = queryClient.getQueryData(containerQueryKey) as api.Container; - queryClient.setQueryData(containerQueryKey, { - ...previousData, - ...data, - }); + + if (previousData) { + queryClient.setQueryData(containerQueryKey, { + ...previousData, + ...data, + }); + } let childrenPreviousData; if (affectedParentContainerId) { const childrenQueryKey = libraryAuthoringQueryKeys.containerChildren(affectedParentContainerId); childrenPreviousData = queryClient.getQueryData(childrenQueryKey) as api.Container[]; - queryClient.setQueryData(childrenQueryKey, childrenPreviousData.map(item => ( - item.id === containerId ? { ...item, ...data } : item - ))); + if (childrenPreviousData) { + queryClient.setQueryData(childrenQueryKey, childrenPreviousData.map(item => ( + item.id === containerId ? { ...item, ...data } : item + ))); + } } return { previousData, childrenPreviousData }; }, onError: (_err, _data, context) => { - queryClient.setQueryData(containerQueryKey, context?.previousData); + if (context?.previousData) { + queryClient.setQueryData(containerQueryKey, context?.previousData); + } - if (affectedParentContainerId) { + if (affectedParentContainerId && context?.childrenPreviousData) { const childrenQueryKey = libraryAuthoringQueryKeys.containerChildren(affectedParentContainerId); queryClient.setQueryData(childrenQueryKey, context?.childrenPreviousData); } diff --git a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx index 029cab724a..3dc0f48968 100644 --- a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx @@ -289,7 +289,7 @@ describe('', () => { })); expect(textBox).not.toBeInTheDocument(); expect(mockShowToast).toHaveBeenCalledWith('Container updated successfully.'); - expect(mockSetQueryData).toHaveBeenCalledTimes(2); + expect(mockSetQueryData).toHaveBeenCalledTimes(1); }); it(`should show error while updating child name in ${cType} page`, async () => {