From 747f7b613309a17bd74720c0f1f57fee6a655f13 Mon Sep 17 00:00:00 2001 From: Jillian Date: Tue, 24 Jun 2025 03:05:35 -0400 Subject: [PATCH] Add subsection/unit to section/subsection full-page view within library [FC-0090] (#2152) * Removes the "content type" tab bar from the "Add Existing Component/Unit/Subsection" modal, and in all cases where only one type of content is shown for selection. * Updates the section/subsection sidebar to show "Existing Library Content" + "New Subsection" / "New Unit" buttons. * Updates the "Add New Unit/Subsection" buttons to directly launch the new container modal, instead of going via the container sidebar. * Ensures that whenever a subsection/unit is created from within a section/subsection, that it is linked to the parent section/subsection after created. --- .../LibraryAuthoringPage.tsx | 28 ++- .../add-content/AddContent.test.tsx | 17 +- .../add-content/AddContent.tsx | 179 +++++++++++------- .../add-content/AddContentHeader.tsx | 2 +- .../PickLibraryContentModal.test.tsx | 47 +++-- .../add-content/PickLibraryContentModal.tsx | 60 +++--- src/library-authoring/add-content/messages.ts | 119 ++++++++++-- .../component-picker/ComponentPicker.test.tsx | 9 +- .../containers/ContainerInfo.test.tsx | 4 +- .../containers/FooterActions.tsx | 14 +- .../CreateContainerModal.test.tsx | 2 +- .../create-container/CreateContainerModal.tsx | 37 ++-- src/library-authoring/routes.ts | 59 +++--- .../LibrarySectionPage.tsx | 2 + .../LibrarySectionSubsectionPage.test.tsx | 105 ++++++++++ .../LibrarySubsectionPage.tsx | 2 + .../units/LibraryUnitPage.test.tsx | 10 + 17 files changed, 486 insertions(+), 210 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index a49224ad2..bfc13677c 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -158,7 +158,9 @@ const LibraryAuthoringPage = ({ insideCollections, insideComponents, insideUnits, + insideSection, insideSections, + insideSubsection, insideSubsections, navigateTo, } = useLibraryRoutes(); @@ -251,8 +253,12 @@ const LibraryAuthoringPage = ({ extraFilter.push(activeTypeFilters[activeKey]); } - // Disable filtering by block/problem type when viewing the Collections/Units/Sections/Subsections tab. - const onlyOneType = (insideCollections || insideUnits || insideSections || insideSubsections); + // Disable filtering by block/problem type when viewing the Collections/Units/Sections/Subsections tab, + // or when inside a specific Section or Subsection. + const onlyOneType = ( + insideCollections || insideUnits || insideSections || insideSubsections + || insideSection || insideSubsection + ); const overrideTypesFilter = onlyOneType ? new TypesFilterData() : undefined; @@ -298,14 +304,16 @@ const LibraryAuthoringPage = ({ headerActions={} hideBorder /> - - {visibleTabsToRender} - + {visibleTabs.length > 1 && ( + + {visibleTabsToRender} + + )} diff --git a/src/library-authoring/add-content/AddContent.test.tsx b/src/library-authoring/add-content/AddContent.test.tsx index 9bb24dbca..a8789d2b3 100644 --- a/src/library-authoring/add-content/AddContent.test.tsx +++ b/src/library-authoring/add-content/AddContent.test.tsx @@ -90,6 +90,7 @@ describe('', () => { expect(screen.queryByRole('button', { name: /video/i })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: /copy from clipboard/i })).not.toBeInTheDocument(); expect(await screen.findByRole('button', { name: /advanced \/ other/i })).toBeInTheDocument(); + expect(await screen.queryByRole('button', { name: /existing library content/i })).not.toBeInTheDocument(); }); it('should render advanced content buttons', async () => { @@ -331,6 +332,7 @@ describe('', () => { const unitId = 'lct:orf1:lib1:unit:test-1'; renderWithContainer(unitId); + expect(await screen.findByRole('button', { name: /existing library content/i })).toBeInTheDocument(); expect(await screen.findByRole('button', { name: 'Text' })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument(); @@ -396,12 +398,16 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalledWith('There was an error linking the content to this container.'); }); - it('should only show subsection button when inside a section', async () => { + it('should only show subsection buttons when inside a section', async () => { mockClipboardEmpty.applyMock(); const sectionId = 'lct:orf1:lib1:section:test-1'; renderWithContainer(sectionId, 'section'); - expect(await screen.findByRole('button', { name: 'Subsection' })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: /existing subsection/i })).toBeInTheDocument(); + + // Button is labeled "New Subsection" , not "Subsection" + expect(await screen.findByRole('button', { name: /new subsection/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Subsection' })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Unit' })).not.toBeInTheDocument(); @@ -409,12 +415,15 @@ describe('', () => { expect(screen.queryByRole('button', { name: 'Text' })).not.toBeInTheDocument(); }); - it('should only show unit button when inside a subsection', async () => { + it('should only show unit buttons when inside a subsection', async () => { mockClipboardEmpty.applyMock(); const subsectionId = 'lct:orf1:lib1:subsection:test-1'; renderWithContainer(subsectionId, 'subsection'); - expect(await screen.findByRole('button', { name: 'Unit' })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: /existing unit/i })).toBeInTheDocument(); + // Button is labeled "New Unit" in this context, not just "Unit" + expect(await screen.findByRole('button', { name: /new unit/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Unit' })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Collection' })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Subsection' })).not.toBeInTheDocument(); diff --git a/src/library-authoring/add-content/AddContent.tsx b/src/library-authoring/add-content/AddContent.tsx index 882aa12c2..0698b0457 100644 --- a/src/library-authoring/add-content/AddContent.tsx +++ b/src/library-authoring/add-content/AddContent.tsx @@ -31,13 +31,13 @@ import { blockTypes } from '../../editors/data/constants/app'; import { useLibraryRoutes } from '../routes'; import genericMessages from '../generic/messages'; -import messages from './messages'; +import { messages, getContentMessages } from './messages'; import type { BlockTypeMetadata } from '../data/api'; import { ContainerType } from '../../generic/key-utils'; type ContentType = { name: string, - disabled: boolean, + disabled?: boolean, icon?: React.ComponentType, blockType: string, }; @@ -64,7 +64,7 @@ type AddAdvancedContentViewProps = { const AddContentButton = ({ contentType, onCreateContent } : AddContentButtonProps) => { const { name, - disabled, + disabled = false, icon, blockType, } = contentType; @@ -96,97 +96,132 @@ const AddContentView = ({ insideSubsection, } = useLibraryRoutes(); - const collectionButtonData = { - name: intl.formatMessage(messages.collectionButton), - disabled: false, - blockType: 'collection', - }; + const contentMessages = useMemo(() => ( + getContentMessages(insideSection, insideSubsection, insideUnit) + ), [insideSection, insideSubsection, insideUnit]); - const unitButtonData = { - name: intl.formatMessage(messages.unitButton), - disabled: false, - blockType: 'vertical', - }; + const collectionButton = ( + + ); - const sectionButtonData = { - name: intl.formatMessage(messages.sectionButton), - disabled: false, - blockType: 'chapter', - }; + const unitButton = ( + + ); - const subsectionButtonData = { - name: intl.formatMessage(messages.subsectionButton), - disabled: false, - blockType: 'sequential', - }; + const sectionButton = ( + + ); - const libraryContentButtonData = { - name: intl.formatMessage(messages.libraryContentButton), - disabled: false, - blockType: 'libraryContent', - }; + const subsectionButton = ( + + ); - /** List container content types that should be displayed based on current path */ - const visibleContentTypes = useMemo(() => { + const existingContentButton = ( + + ); + + /* Note: for MVP we are hiding the unsupported types, not just disabling them. */ + const componentButtons = contentTypes.filter(ct => !ct.disabled).map((contentType) => ( + + )); + const separator = ( +
+ ); + + /** List buttons that should be displayed based on current path */ + const visibleButtons = useMemo(() => { if (insideCollection) { - // except for add collection button, show everthing. + // except for add collection button, show everything. return [ - libraryContentButtonData, - sectionButtonData, - subsectionButtonData, - unitButtonData, + existingContentButton, + sectionButton, + subsectionButton, + unitButton, + separator, + ...componentButtons, ]; } if (insideUnit) { - // Only show libraryContentButton - return [libraryContentButtonData]; + // Only show existing content button + component buttons + return [ + existingContentButton, + separator, + ...componentButtons, + ]; } if (insideSection) { - // Should only allow adding subsections - return [subsectionButtonData]; + // Only allow adding subsections + return [ + existingContentButton, + subsectionButton, + ]; } if (insideSubsection) { - // Should only allow adding units - return [unitButtonData]; + // Only allow adding units + return [ + existingContentButton, + unitButton, + ]; } - // except for libraryContentButton, show everthing. + // except for existing content, show everything. return [ - collectionButtonData, - sectionButtonData, - subsectionButtonData, - unitButtonData, + collectionButton, + sectionButton, + subsectionButton, + unitButton, + separator, + ...componentButtons, ]; - }, [insideCollection, insideUnit, insideSection, insideSubsection]); + }, [componentButtons, insideCollection, insideUnit, insideSection, insideSubsection]); return ( <> - {visibleContentTypes.map((contentType) => ( - - ))} - {componentPicker && visibleContentTypes.includes(libraryContentButtonData) && ( - /// Show the "Add Library Content" button for units and collections + {visibleButtons} + {componentPicker && visibleButtons.includes(existingContentButton) && ( )} - {(!insideSection && !insideSubsection) && ( - <> -
- {/* Note: for MVP we are hiding the unuspported types, not just disabling them. */} - {contentTypes.filter(ct => !ct.disabled).map((contentType) => ( - - ))} - - )} ); }; @@ -423,9 +458,9 @@ const AddContent = () => { } else if (blockType === 'advancedXBlock') { showAdvancedList(); } else if ([ - ContainerType.Vertical, - ContainerType.Chapter, - ContainerType.Sequential, + ContainerType.Unit, + ContainerType.Subsection, + ContainerType.Section, ].includes(blockType as ContainerType)) { setCreateContainerModalType(blockType as ContainerType); } else { diff --git a/src/library-authoring/add-content/AddContentHeader.tsx b/src/library-authoring/add-content/AddContentHeader.tsx index a73a41a03..07bf86ea6 100644 --- a/src/library-authoring/add-content/AddContentHeader.tsx +++ b/src/library-authoring/add-content/AddContentHeader.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; -import messages from './messages'; +import { messages } from './messages'; const AddContentHeader = () => ( diff --git a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx index c874a6fe6..03126f610 100644 --- a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx +++ b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx @@ -81,12 +81,24 @@ describe('', () => { }); [ - 'collection' as const, - 'unit' as const, - 'section' as const, - 'subsection' as const, - ].forEach((context) => { - it(`can pick components from the modal (${context})`, async () => { + { + context: 'collection' as const, + addType: 'component', + }, + { + context: 'unit' as const, + addType: 'component', + }, + { + context: 'subsection' as const, + addType: 'unit', + }, + { + context: 'section' as const, + addType: 'subsection', + }, + ].forEach(({ context, addType }) => { + it(`can pick content from the modal (${context})`, async () => { render(context); // Wait for the content library to load @@ -95,11 +107,16 @@ describe('', () => { expect(screen.queryAllByText('Introduction to Testing')[0]).toBeInTheDocument(); }); - // Select the first component - fireEvent.click(screen.queryAllByRole('button', { name: 'Select' })[0]); - expect(await screen.findByText('1 Selected Component')).toBeInTheDocument(); + expect(screen.getByText(`Select ${addType}s`)).toBeInTheDocument(); - fireEvent.click(screen.getByRole('button', { name: /add to .*/i })); + // Select and add the first item + fireEvent.click(screen.queryAllByRole('button', { name: 'Select' })[0]); + expect(await screen.findByText( + new RegExp(`1 selected ${addType}`, 'i'), + )).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { + name: new RegExp(`add to ${context}`, 'i'), + })); await waitFor(() => { switch (context) { @@ -143,11 +160,15 @@ describe('', () => { expect(screen.queryAllByText('Introduction to Testing')[0]).toBeInTheDocument(); }); - // Select the first component + // Select and add the first item fireEvent.click(screen.queryAllByRole('button', { name: 'Select' })[0]); - expect(await screen.findByText('1 Selected Component')).toBeInTheDocument(); + expect(await screen.findByText( + new RegExp(`1 selected ${addType}`, 'i'), + )).toBeInTheDocument(); - fireEvent.click(screen.getByRole('button', { name: /add to .*/i })); + fireEvent.click(screen.getByRole('button', { + name: new RegExp(`add to ${context}`, 'i'), + })); await waitFor(() => { switch (context) { diff --git a/src/library-authoring/add-content/PickLibraryContentModal.tsx b/src/library-authoring/add-content/PickLibraryContentModal.tsx index 3e9f396d7..43993d375 100644 --- a/src/library-authoring/add-content/PickLibraryContentModal.tsx +++ b/src/library-authoring/add-content/PickLibraryContentModal.tsx @@ -1,4 +1,9 @@ -import React, { useCallback, useContext, useState } from 'react'; +import React, { + useCallback, + useContext, + useMemo, + useState, +} from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, Button, StandardModal } from '@openedx/paragon'; @@ -8,27 +13,7 @@ import type { SelectedComponent } from '../common/context/ComponentPickerContext import { useAddItemsToCollection, useAddItemsToContainer } from '../data/apiHooks'; import genericMessages from '../generic/messages'; import { allLibraryPageTabs, ContentType, useLibraryRoutes } from '../routes'; -import messages from './messages'; - -interface PickLibraryContentModalFooterProps { - onSubmit: () => void; - selectedComponents: SelectedComponent[]; - buttonText: React.ReactNode; -} - -const PickLibraryContentModalFooter: React.FC = ({ - onSubmit, - selectedComponents, - buttonText, -}) => ( - - - - - -); +import { messages, getContentMessages } from './messages'; interface PickLibraryContentModalProps { isOpen: boolean; @@ -57,10 +42,14 @@ export const PickLibraryContentModal: React.FC = ( const { showToast } = useContext(ToastContext); - const [selectedComponents, setSelectedComponents] = useState([]); + const contentMessages = useMemo(() => ( + getContentMessages(insideSection, insideSubsection, insideUnit) + ), [insideSection, insideSubsection, insideUnit]); + + const [selectedContent, setSelectedComponents] = useState([]); const onSubmit = useCallback(() => { - const usageKeys = selectedComponents.map(({ usageKey }) => usageKey); + const usageKeys = selectedContent.map(({ usageKey }) => usageKey); onClose(); if (insideCollection && collectionId) { updateCollectionItemsMutation.mutateAsync(usageKeys) @@ -80,7 +69,7 @@ export const PickLibraryContentModal: React.FC = ( }); } }, [ - selectedComponents, + selectedContent, insideSection, insideSubsection, insideUnit, @@ -91,16 +80,13 @@ export const PickLibraryContentModal: React.FC = ( // determine filter an visibleTabs based on current location let extraFilter = ['NOT type = "collection"']; let visibleTabs = allLibraryPageTabs.filter((tab) => tab !== ContentType.collections); - let addBtnText = messages.addToCollectionButton; if (insideSection) { // show only subsections extraFilter = ['block_type = "subsection"']; - addBtnText = messages.addToSectionButton; visibleTabs = [ContentType.subsections]; } else if (insideSubsection) { // show only units extraFilter = ['block_type = "unit"']; - addBtnText = messages.addToSubsectionButton; visibleTabs = [ContentType.units]; } else if (insideUnit) { // show only components @@ -109,7 +95,6 @@ export const PickLibraryContentModal: React.FC = ( 'NOT block_type = "subsection"', 'NOT block_type = "section"', ]; - addBtnText = messages.addToUnitButton; visibleTabs = [ContentType.components]; } @@ -120,17 +105,22 @@ export const PickLibraryContentModal: React.FC = ( return ( + + + + + )} > { + if (insideSection) { + return { + ...messages, + ...sectionMessages, + }; + } + if (insideSubsection) { + return { + ...messages, + ...subsectionMessages, + }; + } + if (insideUnit) { + return { + ...messages, + ...unitMessages, + }; + } + return messages; +}; + export default messages; diff --git a/src/library-authoring/component-picker/ComponentPicker.test.tsx b/src/library-authoring/component-picker/ComponentPicker.test.tsx index 668b8e869..dc028f758 100644 --- a/src/library-authoring/component-picker/ComponentPicker.test.tsx +++ b/src/library-authoring/component-picker/ComponentPicker.test.tsx @@ -377,16 +377,15 @@ describe('', () => { expect(screen.getByRole('tab', { name: /units/i })).toBeInTheDocument(); }); - it('should display only unit tab', async () => { + it('should display only units', async () => { render(); expect(await screen.findByText('Test Library 1')).toBeInTheDocument(); fireEvent.click(screen.getByDisplayValue(/lib:sampletaxonomyorg1:tl1/i)); - expect(await screen.findByRole('tab', { name: /units/i })).toBeInTheDocument(); - expect(screen.queryByRole('tab', { name: /all content/i })).not.toBeInTheDocument(); - expect(screen.queryByRole('tab', { name: /collections/i })).not.toBeInTheDocument(); - expect(screen.queryByRole('tab', { name: /components/i })).not.toBeInTheDocument(); + expect(await screen.findByText('Published Test Unit')).toBeInTheDocument(); + // No tabs shown when only one tab is visible + expect(screen.queryByRole('tab')).not.toBeInTheDocument(); }); it('should not display never published filter', async () => { diff --git a/src/library-authoring/containers/ContainerInfo.test.tsx b/src/library-authoring/containers/ContainerInfo.test.tsx index 75c5cc91b..5370fb0b6 100644 --- a/src/library-authoring/containers/ContainerInfo.test.tsx +++ b/src/library-authoring/containers/ContainerInfo.test.tsx @@ -120,10 +120,10 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalledWith('Failed to publish changes'); }); - testIf(containerType === 'Unit')(`show only published ${containerType} content`, async () => { + it(`show only published ${containerType} content`, async () => { render(containerId, true); expect(await screen.findByTestId('container-info-menu-toggle')).toBeInTheDocument(); - expect(screen.getByText(/text block published 1/i)).toBeInTheDocument(); + expect(screen.getByText(/block published 1/i)).toBeInTheDocument(); }); it(`shows the ${containerType} Preview tab by default and the children are readonly`, async () => { diff --git a/src/library-authoring/containers/FooterActions.tsx b/src/library-authoring/containers/FooterActions.tsx index 7f6de80b1..2fd3767e0 100644 --- a/src/library-authoring/containers/FooterActions.tsx +++ b/src/library-authoring/containers/FooterActions.tsx @@ -1,21 +1,31 @@ import { Button, useToggle } from '@openedx/paragon'; import { Add } from '@openedx/paragon/icons'; +import { type ContainerType } from '../../generic/key-utils'; import { PickLibraryContentModal } from '../add-content'; import { useLibraryContext } from '../common/context/LibraryContext'; import { useSidebarContext } from '../common/context/SidebarContext'; interface FooterActionsProps { + addContentType?: ContainerType; addContentBtnText: string; addExistingContentBtnText: string; } export const FooterActions = ({ + addContentType, addContentBtnText, addExistingContentBtnText, }: FooterActionsProps) => { const [isAddLibraryContentModalOpen, showAddLibraryContentModal, closeAddLibraryContentModal] = useToggle(); const { openAddContentSidebar } = useSidebarContext(); - const { readOnly } = useLibraryContext(); + const { readOnly, setCreateContainerModalType } = useLibraryContext(); + const addContent = () => { + if (addContentType) { + setCreateContainerModalType(addContentType); + } else { + openAddContentSidebar(); + } + }; return (
@@ -23,7 +33,7 @@ export const FooterActions = ({ className="ml-2" iconBefore={Add} variant="outline-primary rounded-0" - onClick={openAddContentSidebar} + onClick={addContent} disabled={readOnly} block > diff --git a/src/library-authoring/create-container/CreateContainerModal.test.tsx b/src/library-authoring/create-container/CreateContainerModal.test.tsx index bff91e8dc..7c7fb96fa 100644 --- a/src/library-authoring/create-container/CreateContainerModal.test.tsx +++ b/src/library-authoring/create-container/CreateContainerModal.test.tsx @@ -105,7 +105,7 @@ describe('CreateContainerModal container linking', () => { { containerId: newSectionId, routeType: 'section' }, ); - const subsectionButton = await screen.findByRole('button', { name: /^Subsection$/ }); + const subsectionButton = await screen.findByRole('button', { name: /New subsection/i }); userEvent.click(subsectionButton); const nameInput = await screen.findByLabelText(/name your subsection/i); userEvent.type(nameInput, 'Test Subsection'); diff --git a/src/library-authoring/create-container/CreateContainerModal.tsx b/src/library-authoring/create-container/CreateContainerModal.tsx index 8773e8c14..8861b7736 100644 --- a/src/library-authoring/create-container/CreateContainerModal.tsx +++ b/src/library-authoring/create-container/CreateContainerModal.tsx @@ -39,7 +39,7 @@ const CreateContainerModal = () => { /** labels based on the type of modal open, i.e., section, subsection or unit */ const labels = React.useMemo(() => { - if (createContainerModalType === ContainerType.Chapter) { + if (createContainerModalType === ContainerType.Section) { return { modalTitle: intl.formatMessage(messages.createSectionModalTitle), validationError: intl.formatMessage(messages.createSectionModalNameInvalid), @@ -49,7 +49,7 @@ const CreateContainerModal = () => { errorMsg: intl.formatMessage(messages.createSectionError), }; } - if (createContainerModalType === ContainerType.Sequential) { + if (createContainerModalType === ContainerType.Subsection) { return { modalTitle: intl.formatMessage(messages.createSubsectionModalTitle), validationError: intl.formatMessage(messages.createSubsectionModalNameInvalid), @@ -72,40 +72,41 @@ const CreateContainerModal = () => { /** Call close for section, subsection and unit as the operation is idempotent */ const handleClose = () => setCreateContainerModalType(undefined); - /** Calculate containerType based on type of open modal */ - const containerType = React.useMemo(() => { - if (createContainerModalType === ContainerType.Chapter) { - return ContainerType.Section; - } - if (createContainerModalType === ContainerType.Sequential) { - return ContainerType.Subsection; - } - return ContainerType.Unit; - }, [createContainerModalType]); - const handleCreate = React.useCallback(async (values) => { try { const canStandAlone = !(insideCollection || insideSection || insideSubsection); const container = await create.mutateAsync({ canStandAlone, - containerType, + containerType: createContainerModalType, ...values, }); - // link container to parent + + // Navigate to the new container + navigateTo({ containerId: container.id }); + + // Link container to parent after navigating -- we may still show an + // error if this linking fails, but at least the user can see that + // the container was created. if (collectionId && insideCollection) { await updateCollectionItemsMutation.mutateAsync([container.id]); } else if (containerId && (insideSection || insideSubsection)) { await updateContainerItemsMutation.mutateAsync([container.id]); } - // Navigate to the new container - navigateTo({ containerId: container.id }); + showToast(labels.successMsg); } catch (error) { showToast(labels.errorMsg); } finally { handleClose(); } - }, [containerType, labels, handleClose, navigateTo]); + }, [ + createContainerModalType, + handleClose, + labels, + navigateTo, + updateCollectionItemsMutation, + updateContainerItemsMutation, + ]); return ( | null; - insideCollections: PathMatch | null; - insideComponents: PathMatch | null; - insideSections: PathMatch | null; - insideSection: PathMatch | null; - insideSubsections: PathMatch | null; - insideSubsection: PathMatch | null; - insideUnits: PathMatch | null; - insideUnit: PathMatch | null; + insideCollection: boolean; + insideCollections: boolean; + insideComponents: boolean; + insideSections: boolean; + insideSection: boolean; + insideSubsections: boolean; + insideSubsection: boolean; + insideUnits: boolean; + insideUnit: boolean; /** Navigate using the best route from the current location for the given parameters. * This function can be mutated if there are changes in the current route, so always include @@ -85,15 +84,17 @@ export const useLibraryRoutes = (): LibraryRoutesData => { const [searchParams] = useSearchParams(); const navigate = useNavigate(); - const insideCollection = matchPath(BASE_ROUTE + ROUTES.COLLECTION, pathname); - const insideCollections = matchPath(BASE_ROUTE + ROUTES.COLLECTIONS, pathname); - const insideComponents = matchPath(BASE_ROUTE + ROUTES.COMPONENTS, pathname); - const insideSections = matchPath(BASE_ROUTE + ROUTES.SECTIONS, pathname); - const insideSection = matchPath(BASE_ROUTE + ROUTES.SECTION, pathname); - const insideSubsections = matchPath(BASE_ROUTE + ROUTES.SUBSECTIONS, pathname); - const insideSubsection = matchPath(BASE_ROUTE + ROUTES.SUBSECTION, pathname); - const insideUnits = matchPath(BASE_ROUTE + ROUTES.UNITS, pathname); - const insideUnit = matchPath(BASE_ROUTE + ROUTES.UNIT, pathname); + // Convert the returned PathMatch | null values to PathMatch | false + // to make it easier to return them as booleans below. + const insideCollection = matchPath(BASE_ROUTE + ROUTES.COLLECTION, pathname) || false; + const insideCollections = matchPath(BASE_ROUTE + ROUTES.COLLECTIONS, pathname) || false; + const insideComponents = matchPath(BASE_ROUTE + ROUTES.COMPONENTS, pathname) || false; + const insideSections = matchPath(BASE_ROUTE + ROUTES.SECTIONS, pathname) || false; + const insideSection = matchPath(BASE_ROUTE + ROUTES.SECTION, pathname) || false; + const insideSubsections = matchPath(BASE_ROUTE + ROUTES.SUBSECTIONS, pathname) || false; + const insideSubsection = matchPath(BASE_ROUTE + ROUTES.SUBSECTION, pathname) || false; + const insideUnits = matchPath(BASE_ROUTE + ROUTES.UNITS, pathname) || false; + const insideUnit = matchPath(BASE_ROUTE + ROUTES.UNIT, pathname) || false; // Sanity check to ensure that we are not inside more than one route at the same time. // istanbul ignore if: this is a developer error, not a user error. @@ -108,7 +109,7 @@ export const useLibraryRoutes = (): LibraryRoutesData => { insideSubsection, insideUnits, insideUnit, - ].filter((match): match is PathMatch => match !== null).length > 1) { + ].filter((match) => match).length > 1) { throw new Error('Cannot be inside more than one route at the same time.'); } @@ -247,15 +248,15 @@ export const useLibraryRoutes = (): LibraryRoutesData => { return useMemo(() => ({ navigateTo, - insideCollection, - insideCollections, - insideComponents, - insideSections, - insideSection, - insideSubsections, - insideSubsection, - insideUnits, - insideUnit, + insideCollection: !!insideCollection, + insideCollections: !!insideCollections, + insideComponents: !!insideComponents, + insideSections: !!insideSections, + insideSection: !!insideSection, + insideSubsections: !!insideSubsections, + insideSubsection: !!insideSubsection, + insideUnits: !!insideUnits, + insideUnit: !!insideUnit, }), [ navigateTo, insideCollection, diff --git a/src/library-authoring/section-subsections/LibrarySectionPage.tsx b/src/library-authoring/section-subsections/LibrarySectionPage.tsx index 95da17add..63055a2c2 100644 --- a/src/library-authoring/section-subsections/LibrarySectionPage.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionPage.tsx @@ -8,6 +8,7 @@ import { useContainer, useContentLibrary } from '../data/apiHooks'; import Loading from '../../generic/Loading'; import NotFoundAlert from '../../generic/NotFoundAlert'; import ErrorAlert from '../../generic/alert-error'; +import { ContainerType } from '../../generic/key-utils'; import Header from '../../header'; import SubHeader from '../../generic/sub-header/SubHeader'; import { SubHeaderTitle } from '../LibraryAuthoringPage'; @@ -105,6 +106,7 @@ export const LibrarySectionPage = () => { diff --git a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx index a24ba9276..4e3b365c0 100644 --- a/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx +++ b/src/library-authoring/section-subsections/LibrarySectionSubsectionPage.test.tsx @@ -13,6 +13,7 @@ import { import { getLibraryContainerApiUrl, getLibraryContainerChildrenApiUrl, + getLibraryContainersApiUrl, } from '../data/api'; import { mockContentLibrary, @@ -390,6 +391,110 @@ describe('', () => { expect(await screen.findByRole('button', { name: new RegExp(`${childType} Info`, 'i') })).toBeInTheDocument(); }); + it(`${cType} sidebar should render "new ${childType}" and "existing ${childType}" buttons`, async () => { + renderLibrarySectionPage(undefined, undefined, cType); + const addChild = await screen.findByRole('button', { name: new RegExp(`add ${childType}`, 'i') }); + userEvent.click(addChild); + const addNew = await screen.findByRole('button', { name: new RegExp(`^new ${childType}$`, 'i') }); + const addExisting = await screen.findByRole('button', { name: new RegExp(`^existing ${childType}$`, 'i') }); + + // Clicking "add new" shows create container modal (tested below) + userEvent.click(addNew); + expect(await screen.findByLabelText(new RegExp(`name your ${childType}`, 'i'))).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: /cancel/i })); + + // Clicking "add existing" shows content picker modal + userEvent.click(addExisting); + expect(await screen.findByRole('dialog')).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: new RegExp(`add to ${cType}`, 'i') })).toBeInTheDocument(); + // No "Types" filter shown + expect(screen.queryByRole('button', { name: /type/i })).not.toBeInTheDocument(); + }); + + it(`"add new" button should add ${childType} to the ${cType}`, async () => { + const { libraryId } = mockContentLibrary; + const containerId = cType === ContainerType.Section + ? mockGetContainerMetadata.sectionId + : mockGetContainerMetadata.subsectionId; + const childId = cType === ContainerType.Section + ? mockGetContainerMetadata.subsectionId + : mockGetContainerMetadata.unitId; + + axiosMock + .onPost(getLibraryContainersApiUrl(libraryId)) + .reply(200, { id: childId }); + axiosMock + .onPost(getLibraryContainerChildrenApiUrl(containerId)) + .reply(200); + renderLibrarySectionPage(containerId, libraryId, cType); + + const addChild = await screen.findByRole('button', { name: new RegExp(`add new ${childType}`, 'i') }); + userEvent.click(addChild); + const textBox = await screen.findByLabelText(new RegExp(`name your ${childType}`, 'i')); + fireEvent.change(textBox, { target: { value: `New ${childType} Title` } }); + fireEvent.click(screen.getByRole('button', { name: /create/i })); + await waitFor(() => { + expect(axiosMock.history.post.length).toEqual(2); + }); + expect(axiosMock.history.post[0].data).toEqual(JSON.stringify({ + can_stand_alone: false, + container_type: childType, + display_name: `New ${childType} Title`, + })); + expect(axiosMock.history.post[1].data).toEqual(JSON.stringify({ usage_keys: [childId] })); + expect(textBox).not.toBeInTheDocument(); + const childTypeTitle = childType.charAt(0).toUpperCase() + childType.slice(1); + expect(mockShowToast).toHaveBeenCalledWith(`${childTypeTitle} created successfully`); + }); + + it(`"add new" button should show error when adding ${childType} to the ${cType}`, async () => { + const { libraryId } = mockContentLibrary; + const containerId = cType === ContainerType.Section + ? mockGetContainerMetadata.sectionId + : mockGetContainerMetadata.subsectionId; + const childId = cType === ContainerType.Section + ? mockGetContainerMetadata.subsectionId + : mockGetContainerMetadata.unitId; + + axiosMock + .onPost(getLibraryContainersApiUrl(libraryId)) + .reply(200, { id: childId }); + axiosMock + .onPost(getLibraryContainerChildrenApiUrl(containerId)) + .reply(500); + renderLibrarySectionPage(containerId, libraryId, cType); + + const addChild = await screen.findByRole('button', { name: new RegExp(`add new ${childType}`, 'i') }); + userEvent.click(addChild); + const textBox = await screen.findByLabelText(new RegExp(`name your ${childType}`, 'i')); + fireEvent.change(textBox, { target: { value: `New ${childType} Title` } }); + fireEvent.click(screen.getByRole('button', { name: /create/i })); + await waitFor(() => { + expect(axiosMock.history.post.length).toEqual(2); + }); + expect(axiosMock.history.post[0].data).toEqual(JSON.stringify({ + can_stand_alone: false, + container_type: childType, + display_name: `New ${childType} Title`, + })); + expect(axiosMock.history.post[1].data).toEqual(JSON.stringify({ usage_keys: [childId] })); + expect(textBox).not.toBeInTheDocument(); + expect(mockShowToast).toHaveBeenCalledWith(`There is an error when creating the library ${childType}`); + }); + + it(`"add existing ${childType}" button should load ${cType} content picker modal`, async () => { + renderLibrarySectionPage(undefined, undefined, cType); + + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + + const addChild = await screen.findByRole('button', { name: new RegExp(`add existing ${childType}`, 'i') }); + userEvent.click(addChild); + + // Content picker loaded (modal behavior is tested elsewhere) + expect(await screen.findByRole('dialog')).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: new RegExp(`add to ${cType}`, 'i') })).toBeInTheDocument(); + }); + it(`should open manage tags on click tag count in ${cType} page`, async () => { const cId = cType === ContainerType.Section ? mockGetContainerMetadata.sectionId diff --git a/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx b/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx index 0881f92b0..661477afe 100644 --- a/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx +++ b/src/library-authoring/section-subsections/LibrarySubsectionPage.tsx @@ -11,6 +11,7 @@ import { useContentFromSearchIndex, useContentLibrary } from '../data/apiHooks'; import Loading from '../../generic/Loading'; import NotFoundAlert from '../../generic/NotFoundAlert'; import ErrorAlert from '../../generic/alert-error'; +import { ContainerType } from '../../generic/key-utils'; import Header from '../../header'; import SubHeader from '../../generic/sub-header/SubHeader'; import { SubHeaderTitle } from '../LibraryAuthoringPage'; @@ -154,6 +155,7 @@ export const LibrarySubsectionPage = () => { diff --git a/src/library-authoring/units/LibraryUnitPage.test.tsx b/src/library-authoring/units/LibraryUnitPage.test.tsx index 1009147fb..dc331d266 100644 --- a/src/library-authoring/units/LibraryUnitPage.test.tsx +++ b/src/library-authoring/units/LibraryUnitPage.test.tsx @@ -442,4 +442,14 @@ describe('', () => { userEvent.click(component.parentElement!.parentElement!.parentElement!, undefined, { clickCount: 2 }); expect(await screen.findByRole('dialog', { name: 'Editor Dialog' })).toBeInTheDocument(); }); + + it('"Add New Content" button should open "Add Content" sidebar', async () => { + renderLibraryUnitPage(); + const addContent = await screen.findByRole('button', { name: /add new content/i }); + userEvent.click(addContent); + + expect(await screen.findByRole('button', { name: /existing library content/i })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: /text/i })).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: /problem/i })).toBeInTheDocument(); + }); });