Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/library-authoring/data/api.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<p>This is a text component which uses <strong>HTML</strong>.</p>',
Expand Down Expand Up @@ -493,6 +493,17 @@ export async function mockGetContainerMetadata(containerId: string): Promise<api
case mockGetContainerMetadata.subsectionIdEmpty:
return Promise.resolve(mockGetContainerMetadata.subsectionData);
default:
if (containerId.startsWith('lct:org1:Demo_course_generated:')) {
const lastPart = containerId.split(':').pop();
if (lastPart) {
const [name, idx] = lastPart.split('-');
return Promise.resolve({
...mockGetContainerMetadata.containerData,
displayName: `${name} block ${idx}`,
publishedDisplayName: `${name} block published ${idx}`,
});
}
}
return Promise.resolve(mockGetContainerMetadata.containerData);
}
}
Expand Down Expand Up @@ -575,19 +586,22 @@ export async function mockGetContainerChildren(containerId: string): Promise<api
}
let blockType = 'html';
let name = 'text';
let typeNamespace = 'lb';
if (containerId.includes('subsection')) {
blockType = 'unit';
name = blockType;
typeNamespace = 'lct';
} else if (containerId.includes('section')) {
blockType = 'subsection';
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: `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}`,
}
Expand Down
40 changes: 32 additions & 8 deletions src/library-authoring/data/apiHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -611,25 +611,49 @@ 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) => {
export const useUpdateContainer = (containerId: string, affectedParentContainerId?: string) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please just update the JSDoc comment about to explain what the new affectedParentContainerId parameter is doing exactly?

const libraryId = getLibraryId(containerId);
const queryClient = useQueryClient();
const containerQueryKey = libraryAuthoringQueryKeys.container(containerId);
return useMutation({
mutationFn: (data: api.UpdateContainerDataRequest) => api.updateContainerMetadata(containerId, data),
onMutate: (data) => {
const previousData = queryClient.getQueryData(containerQueryKey) as api.Container;
queryClient.setQueryData(containerQueryKey, {
...previousData,
...data,
});

return { previousData };
if (previousData) {
queryClient.setQueryData(containerQueryKey, {
...previousData,
...data,
});
}

let childrenPreviousData;
if (affectedParentContainerId) {
const childrenQueryKey = libraryAuthoringQueryKeys.containerChildren(affectedParentContainerId);
childrenPreviousData = queryClient.getQueryData(childrenQueryKey) as api.Container[];
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 && context?.childrenPreviousData) {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -19,7 +20,6 @@ import {
import { messages, subsectionMessages, sectionMessages } from './messages';
import containerMessages from '../containers/messages';
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';
Expand All @@ -40,10 +40,10 @@ interface ContainerRowProps extends LibraryContainerChildrenProps {
container: LibraryContainerMetadataWithUniqueId;
}

const ContainerRow = ({ container, readOnly }: ContainerRowProps) => {
const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps) => {
const intl = useIntl();
const { showToast } = useContext(ToastContext);
const updateMutation = useUpdateContainer(container.originalId);
const updateMutation = useUpdateContainer(container.originalId, containerKey);
const { showOnlyPublished } = useLibraryContext();

const handleSaveDisplayName = async (newDisplayName: string) => {
Expand All @@ -59,12 +59,18 @@ const ContainerRow = ({ container, readOnly }: ContainerRowProps) => {

return (
<>
<InplaceTextEditor
onSave={handleSaveDisplayName}
text={showOnlyPublished ? (container.publishedDisplayName ?? container.displayName) : container.displayName}
textClassName="font-weight-bold small"
readOnly={readOnly || showOnlyPublished}
/>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div
// Prevent parent card from being clicked.
onClick={(e) => e.stopPropagation()}
>
<InplaceTextEditor
onSave={handleSaveDisplayName}
text={showOnlyPublished ? (container.publishedDisplayName ?? container.displayName) : container.displayName}
textClassName="font-weight-bold small"
readOnly={readOnly || showOnlyPublished}
/>
</div>
<ActionRow.Spacer />
<Stack
direction="horizontal"
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -67,7 +69,7 @@ jest.mock('../../generic/DraggableList/verticalSortableList', () => ({

describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
beforeEach(() => {
({ axiosMock, mockShowToast } = initializeMocks());
({ axiosMock, mockShowToast, queryClient } = initializeMocks());
});

afterEach(() => {
Expand Down Expand Up @@ -104,6 +106,10 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
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
Expand Down Expand Up @@ -253,7 +259,8 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
});

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

Expand Down Expand Up @@ -282,10 +289,11 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
}));
expect(textBox).not.toBeInTheDocument();
expect(mockShowToast).toHaveBeenCalledWith('Container updated successfully.');
expect(mockSetQueryData).toHaveBeenCalledTimes(1);
});

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

Expand Down Expand Up @@ -326,7 +334,7 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
.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' });
Expand All @@ -344,7 +352,9 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
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' });
});
Expand All @@ -361,7 +371,9 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
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' });
});
Expand All @@ -372,9 +384,9 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
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 });
expect((await screen.findAllByText(new RegExp(`Test ${childType}`, 'i')))[0]).toBeInTheDocument();
// 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();
});
});
Expand Down
16 changes: 11 additions & 5 deletions src/library-authoring/units/LibraryUnitPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('<LibraryUnitPage />', () => {
});

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

Expand Down Expand Up @@ -251,7 +251,7 @@ describe('<LibraryUnitPage />', () => {
});

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

Expand Down Expand Up @@ -290,7 +290,9 @@ describe('<LibraryUnitPage />', () => {
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' });
});
Expand All @@ -304,7 +306,9 @@ describe('<LibraryUnitPage />', () => {
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' });
});
Expand All @@ -318,7 +322,9 @@ describe('<LibraryUnitPage />', () => {
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' });
});
Expand Down