Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 1 addition & 1 deletion src/editors/containers/EditorContainer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { useEditorContext } from '../../EditorContext';
import TitleHeader from './components/TitleHeader';
import * as hooks from './hooks';
import messages from './messages';
import { parseErrorMsg } from '../../../library-authoring/add-content/AddContentContainer';
import { parseErrorMsg } from '../../../library-authoring/add-content/AddContent';
import libraryMessages from '../../../library-authoring/add-content/messages';

import './index.scss';
Expand Down
14 changes: 14 additions & 0 deletions src/generic/key-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {
getLibraryId,
isLibraryKey,
isLibraryV1Key,
getContainerTypeFromId,
ContainerType,
} from './key-utils';

describe('component utils', () => {
Expand Down Expand Up @@ -97,4 +99,16 @@ describe('component utils', () => {
});
}
});

describe('getContainerTypeFromId', () => {
for (const [input, expected] of [
['lct:org:lib:unit:my-unit-9284e2', ContainerType.Unit],
['lct:OpenCraftX:ALPHA:my-unit-a3223f', undefined],
['', undefined],
]) {
it(`returns '${expected}' for container key '${input}'`, () => {
expect(getContainerTypeFromId(input!)).toStrictEqual(expected);
});
}
});
});
23 changes: 23 additions & 0 deletions src/generic/key-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,26 @@ export const buildCollectionUsageKey = (learningContextKey: string, collectionId
const orgLib = learningContextKey.replace('lib:', '');
return `lib-collection:${orgLib}:${collectionId}`;
};

export enum ContainerType {
Unit = 'unit',
}

/**
* Given a container key like `ltc:org:lib:unit:id`
* get the container type
*/
export function getContainerTypeFromId(containerId: string): ContainerType | undefined {
const parts = containerId.split(':');
if (parts.length < 2) {
return undefined;
}

const maybeType = parts[parts.length - 2];

if (Object.values(ContainerType).includes(maybeType as ContainerType)) {
return maybeType as ContainerType;
}

return undefined;
}
2 changes: 1 addition & 1 deletion src/library-authoring/LibraryLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const LibraryLayout = () => {
/** The component picker modal to use. We need to pass it as a reference instead of
* directly importing it to avoid the import cycle:
* ComponentPicker > LibraryAuthoringPage/LibraryCollectionPage >
* Sidebar > AddContentContainer > ComponentPicker */
* Sidebar > AddContent > ComponentPicker */
componentPicker={ComponentPicker}
>
<SidebarProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import {
} from '../data/api.mocks';
import {
getContentLibraryApiUrl, getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl,
getXBlockFieldsApiUrl,
getXBlockFieldsApiUrl, getLibraryContainerChildrenApiUrl,
} from '../data/api';
import { mockBroadcastChannel, mockClipboardEmpty, mockClipboardHtml } from '../../generic/data/api.mock';
import { LibraryProvider } from '../common/context/LibraryContext';
import AddContentContainer from './AddContentContainer';
import AddContent from './AddContent';
import { ComponentEditorModal } from '../components/ComponentEditorModal';
import editorCmsApi from '../../editors/data/services/cms/api';
import { ToastActionData } from '../../generic/toast-context';
Expand All @@ -32,7 +32,7 @@ jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCs
const { libraryId } = mockContentLibrary;
const render = (collectionId?: string) => {
const params: { libraryId: string, collectionId?: string } = { libraryId, collectionId };
return baseRender(<AddContentContainer />, {
return baseRender(<AddContent />, {
path: '/library/:libraryId/:collectionId?',
params,
extraWrapper: ({ children }) => (
Expand All @@ -45,10 +45,25 @@ const render = (collectionId?: string) => {
),
});
};
const renderWithUnit = (unitId: string) => {
const params: { libraryId: string, unitId?: string } = { libraryId, unitId };
return baseRender(<AddContent />, {
path: '/library/:libraryId/:unitId?',
params,
extraWrapper: ({ children }) => (
<LibraryProvider
libraryId={libraryId}
>
{ children }
<ComponentEditorModal />
</LibraryProvider>
),
});
};
let axiosMock: MockAdapter;
let mockShowToast: (message: string, action?: ToastActionData | undefined) => void;

describe('<AddContentContainer />', () => {
describe('<AddContent />', () => {
beforeEach(() => {
const mocks = initializeMocks();
axiosMock = mocks.axiosMock;
Expand Down Expand Up @@ -290,4 +305,71 @@ describe('<AddContentContainer />', () => {
expect(mockShowToast).toHaveBeenCalledWith(expectedError);
});
});

it('should not show collection/unit buttons when create component in container', async () => {
const unitId = 'lct:orf1:lib1:unit:test-1';
renderWithUnit(unitId);

expect(await screen.findByRole('button', { name: 'Text' })).toBeInTheDocument();

expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Unit' })).not.toBeInTheDocument();
});

it('should create a component in unit', async () => {
const unitId = 'lct:orf1:lib1:unit:test-1';
const usageKey = mockXBlockFields.usageKeyNewHtml;
const createUrl = getCreateLibraryBlockUrl(libraryId);
const updateBlockUrl = getXBlockFieldsApiUrl(usageKey);
const linkUrl = getLibraryContainerChildrenApiUrl(unitId);

axiosMock.onPost(createUrl).reply(200, {
id: usageKey,
});
axiosMock.onPost(updateBlockUrl).reply(200, mockXBlockFields.dataHtml);
axiosMock.onPost(linkUrl).reply(200);

renderWithUnit(unitId);

const textButton = screen.getByRole('button', { name: /text/i });
fireEvent.click(textButton);

// Component should be linked to Unit on saving the changes in the editor.
const saveButton = screen.getByLabelText('Save changes and return to learning context');
fireEvent.click(saveButton);

await waitFor(() => expect(axiosMock.history.post.length).toEqual(3));
expect(axiosMock.history.post[0].url).toEqual(createUrl);
expect(axiosMock.history.post[1].url).toEqual(updateBlockUrl);
expect(axiosMock.history.post[2].url).toEqual(linkUrl);
});

it('should show error on create a component in unit', async () => {
const unitId = 'lct:orf1:lib1:unit:test-1';
const usageKey = mockXBlockFields.usageKeyNewHtml;
const createUrl = getCreateLibraryBlockUrl(libraryId);
const updateBlockUrl = getXBlockFieldsApiUrl(usageKey);
const linkUrl = getLibraryContainerChildrenApiUrl(unitId);

axiosMock.onPost(createUrl).reply(200, {
id: usageKey,
});
axiosMock.onPost(updateBlockUrl).reply(200, mockXBlockFields.dataHtml);
axiosMock.onPost(linkUrl).reply(400);

renderWithUnit(unitId);

const textButton = screen.getByRole('button', { name: /text/i });
fireEvent.click(textButton);

const saveButton = screen.getByLabelText('Save changes and return to learning context');
fireEvent.click(saveButton);

await waitFor(() => expect(axiosMock.history.post.length).toEqual(3));
expect(axiosMock.history.post[0].url).toEqual(createUrl);
expect(axiosMock.history.post[1].url).toEqual(updateBlockUrl);
expect(axiosMock.history.post[2].url).toEqual(linkUrl);

expect(mockShowToast).toHaveBeenCalledWith('There was an error linking the content to this container.');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import {
useLibraryPasteClipboard,
useAddComponentsToCollection,
useBlockTypesMetadata,
useAddComponentsToContainer,
} from '../data/apiHooks';
import { useLibraryContext } from '../common/context/LibraryContext';
import { PickLibraryContentModal } from './PickLibraryContentModal';
import { blockTypes } from '../../editors/data/constants/app';

import messages from './messages';
import type { BlockTypeMetadata } from '../data/api';
import { getContainerTypeFromId, ContainerType } from '../../generic/key-utils';

type ContentType = {
name: string,
Expand Down Expand Up @@ -87,7 +89,12 @@ const AddContentView = ({
const {
collectionId,
componentPicker,
unitId,
} = useLibraryContext();
let upstreamContainerType: ContainerType | undefined;
if (unitId) {
upstreamContainerType = getContainerTypeFromId(unitId);
}

const collectionButtonData = {
name: intl.formatMessage(messages.collectionButton),
Expand All @@ -109,21 +116,25 @@ const AddContentView = ({

return (
<>
{collectionId ? (
componentPicker && (
<>
<AddContentButton contentType={libraryContentButtonData} onCreateContent={onCreateContent} />
<PickLibraryContentModal
isOpen={isAddLibraryContentModalOpen}
onClose={closeAddLibraryContentModal}
/>
</>
)
) : (
<AddContentButton contentType={collectionButtonData} onCreateContent={onCreateContent} />
{upstreamContainerType !== ContainerType.Unit && (
<>
{collectionId ? (
componentPicker && (
<>
<AddContentButton contentType={libraryContentButtonData} onCreateContent={onCreateContent} />
<PickLibraryContentModal
isOpen={isAddLibraryContentModalOpen}
onClose={closeAddLibraryContentModal}
/>
</>
)
) : (
<AddContentButton contentType={collectionButtonData} onCreateContent={onCreateContent} />
)}
<AddContentButton contentType={unitButtonData} onCreateContent={onCreateContent} />
<hr className="w-100 bg-gray-500" />
</>
)}
<AddContentButton contentType={unitButtonData} onCreateContent={onCreateContent} />
<hr className="w-100 bg-gray-500" />
{/* Note: for MVP we are hiding the unuspported types, not just disabling them. */}
{contentTypes.filter(ct => !ct.disabled).map((contentType) => (
<AddContentButton
Expand Down Expand Up @@ -186,16 +197,18 @@ export const parseErrorMsg = (
return intl.formatMessage(defaultMessage);
};

const AddContentContainer = () => {
const AddContent = () => {
const intl = useIntl();
const {
libraryId,
collectionId,
openCreateCollectionModal,
openCreateUnitModal,
openComponentEditor,
unitId,
} = useLibraryContext();
const updateComponentsMutation = useAddComponentsToCollection(libraryId, collectionId);
const addComponentsToCollectionMutation = useAddComponentsToCollection(libraryId, collectionId);
const addComponentsToContainerMutation = useAddComponentsToContainer(libraryId, unitId);
const createBlockMutation = useCreateLibraryBlock();
const pasteClipboardMutation = useLibraryPasteClipboard();
const { showToast } = useContext(ToastContext);
Expand Down Expand Up @@ -274,9 +287,15 @@ const AddContentContainer = () => {
}

const linkComponent = (usageKey: string) => {
updateComponentsMutation.mutateAsync([usageKey]).catch(() => {
showToast(intl.formatMessage(messages.errorAssociateComponentMessage));
});
if (collectionId) {
addComponentsToCollectionMutation.mutateAsync([usageKey]).catch(() => {
showToast(intl.formatMessage(messages.errorAssociateComponentToCollectionMessage));
});
} else if (unitId) {

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.

Suggested change
} else if (unitId) {
}
if (unitId) {

These don't necessarily have to be exclusive - you could perhaps create a component in a container in a collection.

addComponentsToContainerMutation.mutateAsync([usageKey]).catch(() => {
showToast(intl.formatMessage(messages.errorAssociateComponentToContainerMessage));
});
}
};

const onPaste = () => {
Expand Down Expand Up @@ -374,4 +393,4 @@ const AddContentContainer = () => {
);
};

export default AddContentContainer;
export default AddContent;
4 changes: 2 additions & 2 deletions src/library-authoring/add-content/PickLibraryContentModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const PickLibraryContentModal: React.FC<PickLibraryContentModalProps> = (
collectionId,
/** We need to get it as a reference instead of directly importing it to avoid the import cycle:
* ComponentPicker > LibraryAuthoringPage/LibraryCollectionPage >
* Sidebar > AddContentContainer > ComponentPicker */
* Sidebar > AddContent > ComponentPicker */
componentPicker: ComponentPicker,
} = useLibraryContext();

Expand All @@ -65,7 +65,7 @@ export const PickLibraryContentModal: React.FC<PickLibraryContentModalProps> = (
showToast(intl.formatMessage(messages.successAssociateComponentMessage));
})
.catch(() => {
showToast(intl.formatMessage(messages.errorAssociateComponentMessage));
showToast(intl.formatMessage(messages.errorAssociateComponentToCollectionMessage));
});
}, [selectedComponents]);

Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/add-content/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { default as AddContentContainer } from './AddContentContainer';
export { default as AddContent } from './AddContent';
export { default as AddContentHeader } from './AddContentHeader';
7 changes: 6 additions & 1 deletion src/library-authoring/add-content/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,16 @@ const messages = defineMessages({
defaultMessage: 'Content linked successfully.',
description: 'Message when linking of content to a collection in library is success',
},
errorAssociateComponentMessage: {
errorAssociateComponentToCollectionMessage: {
id: 'course-authoring.library-authoring.associate-collection-content.error.text',
defaultMessage: 'There was an error linking the content to this collection.',
description: 'Message when linking of content to a collection in library fails',
},
errorAssociateComponentToContainerMessage: {
id: 'course-authoring.library-authoring.associate-container-content.error.text',
defaultMessage: 'There was an error linking the content to this container.',
description: 'Message when linking of content to a container in library fails',
},
addContentTitle: {
id: 'course-authoring.library-authoring.sidebar.title.add-content',
defaultMessage: 'Add Content',
Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/common/context/LibraryContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type LibraryProviderProps = {
/** The component picker modal to use. We need to pass it as a reference instead of
* directly importing it to avoid the import cycle:
* ComponentPicker > LibraryAuthoringPage/LibraryCollectionPage >
* Sidebar > AddContentContainer > ComponentPicker */
* Sidebar > AddContent > ComponentPicker */
componentPicker?: typeof ComponentPicker;
};

Expand Down
3 changes: 2 additions & 1 deletion src/library-authoring/create-unit/CreateUnitModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import messages from './messages';
import { useCreateLibraryContainer } from '../data/apiHooks';
import { ToastContext } from '../../generic/toast-context';
import LoadingButton from '../../generic/loading-button';
import { ContainerType } from '../../generic/key-utils';

const CreateUnitModal = () => {
const intl = useIntl();
Expand All @@ -27,7 +28,7 @@ const CreateUnitModal = () => {
const handleCreate = React.useCallback(async (values) => {
try {
await create.mutateAsync({
containerType: 'unit',
containerType: ContainerType.Unit,
...values,
});
// TODO: Navigate to the new unit
Expand Down
11 changes: 11 additions & 0 deletions src/library-authoring/data/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ describe('library data API', () => {
axiosMock.onPost(url).reply(200);

await api.restoreContainer(containerId);
});

it('should add components to unit', async () => {
const { axiosMock } = initializeMocks();
const componentId = 'lb:org:lib:html:1';
const containerId = 'ltc:org:lib:unit:1';
const url = api.getLibraryContainerChildrenApiUrl(containerId);

axiosMock.onPost(url).reply(200);

await api.addComponentsToContainer(containerId, [componentId]);
expect(axiosMock.history.post[0].url).toEqual(url);
});
});
Loading