From 604d19fba9b8815bbd8f5097a38865958e2d9233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 10 Apr 2025 12:55:27 -0300 Subject: [PATCH 1/6] feat: add container collections support --- .../add-component/AddComponent.jsx | 1 + src/index.jsx | 10 ++- .../LibraryAuthoringPage.tsx | 5 ++ .../add-content/AddContentContainer.test.tsx | 8 +- .../add-content/AddContentContainer.tsx | 8 +- .../PickLibraryContentModal.test.tsx | 12 +-- .../add-content/PickLibraryContentModal.tsx | 4 +- .../LibraryCollectionPage.test.tsx | 6 +- .../collections/LibraryCollectionPage.tsx | 8 +- .../common/context/ComponentPickerContext.tsx | 2 +- .../common/context/LibraryContext.tsx | 6 ++ .../component-info/ComponentManagement.tsx | 12 ++- .../component-info/messages.ts | 45 ---------- .../component-picker/ComponentPicker.tsx | 4 +- .../components/AddComponentWidget.tsx | 82 ++++++++++++++++++ .../components/ComponentCard.tsx | 85 ++----------------- .../components/ContainerCard.tsx | 57 +++++++++++-- src/library-authoring/components/messages.ts | 12 +-- .../containers/ContainerOrganize.test.tsx | 23 +++-- .../containers/ContainerOrganize.tsx | 58 +++++++++++-- src/library-authoring/containers/UnitInfo.tsx | 17 +++- src/library-authoring/containers/messages.ts | 5 ++ .../create-unit/CreateUnitModal.tsx | 11 ++- src/library-authoring/data/api.mocks.ts | 8 ++ src/library-authoring/data/api.ts | 39 ++++++--- src/library-authoring/data/apiHooks.test.tsx | 8 +- src/library-authoring/data/apiHooks.ts | 35 ++++++-- .../ManageCollections.test.tsx | 71 +++++++++++----- .../manage-collections}/ManageCollections.tsx | 49 ++++++----- .../generic/manage-collections/index.tsx | 1 + .../generic/manage-collections/messages.tsx | 51 +++++++++++ 31 files changed, 497 insertions(+), 246 deletions(-) create mode 100644 src/library-authoring/components/AddComponentWidget.tsx rename src/library-authoring/{component-info => generic/manage-collections}/ManageCollections.test.tsx (61%) rename src/library-authoring/{component-info => generic/manage-collections}/ManageCollections.tsx (81%) create mode 100644 src/library-authoring/generic/manage-collections/index.tsx create mode 100644 src/library-authoring/generic/manage-collections/messages.tsx diff --git a/src/course-unit/add-component/AddComponent.jsx b/src/course-unit/add-component/AddComponent.jsx index be9481bad7..72938bcbf9 100644 --- a/src/course-unit/add-component/AddComponent.jsx +++ b/src/course-unit/add-component/AddComponent.jsx @@ -195,6 +195,7 @@ const AddComponent = ({ > { } /> } /> } /> - } /> - } /> + } + /> + } + /> } /> } /> } /> diff --git a/src/library-authoring/LibraryAuthoringPage.tsx b/src/library-authoring/LibraryAuthoringPage.tsx index 7871fd99b0..33ef83d6dc 100644 --- a/src/library-authoring/LibraryAuthoringPage.tsx +++ b/src/library-authoring/LibraryAuthoringPage.tsx @@ -141,6 +141,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage libraryData, isLoadingLibraryData, showOnlyPublished, + extraFilter: contextExtraFilter, componentId, collectionId, unitId, @@ -223,6 +224,10 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage extraFilter.push('last_published IS NOT NULL'); } + if (contextExtraFilter) { + extraFilter.push(...contextExtraFilter); + } + const activeTypeFilters = { components: 'type = "library_block"', collections: 'type = "collection"', diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx index 1f1807a15e..00fe2d835b 100644 --- a/src/library-authoring/add-content/AddContentContainer.test.tsx +++ b/src/library-authoring/add-content/AddContentContainer.test.tsx @@ -12,7 +12,7 @@ import { mockXBlockFields, } from '../data/api.mocks'; import { - getContentLibraryApiUrl, getCreateLibraryBlockUrl, getLibraryCollectionComponentApiUrl, getLibraryPasteClipboardUrl, + getContentLibraryApiUrl, getCreateLibraryBlockUrl, getLibraryCollectionItemsApiUrl, getLibraryPasteClipboardUrl, getXBlockFieldsApiUrl, } from '../data/api'; import { mockBroadcastChannel, mockClipboardEmpty, mockClipboardHtml } from '../../generic/data/api.mock'; @@ -137,7 +137,7 @@ describe('', () => { const url = getCreateLibraryBlockUrl(libraryId); const usageKey = mockXBlockFields.usageKeyNewHtml; const updateBlockUrl = getXBlockFieldsApiUrl(usageKey); - const collectionComponentUrl = getLibraryCollectionComponentApiUrl( + const collectionComponentUrl = getLibraryCollectionItemsApiUrl( libraryId, collectionId, ); @@ -195,7 +195,7 @@ describe('', () => { const pasteUrl = getLibraryPasteClipboardUrl(libraryId); const collectionId = 'some-collection-id'; - const collectionComponentUrl = getLibraryCollectionComponentApiUrl( + const collectionComponentUrl = getLibraryCollectionItemsApiUrl( libraryId, collectionId, ); @@ -220,7 +220,7 @@ describe('', () => { const pasteUrl = getLibraryPasteClipboardUrl(libraryId); const collectionId = 'some-collection-id'; - const collectionComponentUrl = getLibraryCollectionComponentApiUrl( + const collectionComponentUrl = getLibraryCollectionItemsApiUrl( libraryId, collectionId, ); diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index 1899318d56..1f0c72f6b5 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -21,7 +21,7 @@ import { getCanEdit } from '../../course-unit/data/selectors'; import { useCreateLibraryBlock, useLibraryPasteClipboard, - useAddComponentsToCollection, + useAddItemsToCollection, useBlockTypesMetadata, } from '../data/apiHooks'; import { useLibraryContext } from '../common/context/LibraryContext'; @@ -195,7 +195,7 @@ const AddContentContainer = () => { openCreateUnitModal, openComponentEditor, } = useLibraryContext(); - const updateComponentsMutation = useAddComponentsToCollection(libraryId, collectionId); + const updateItemsMutation = useAddItemsToCollection(libraryId, collectionId); const createBlockMutation = useCreateLibraryBlock(); const pasteClipboardMutation = useLibraryPasteClipboard(); const { showToast } = useContext(ToastContext); @@ -273,8 +273,8 @@ const AddContentContainer = () => { contentTypes.push(pasteButton); } - const linkComponent = (usageKey: string) => { - updateComponentsMutation.mutateAsync([usageKey]).catch(() => { + const linkComponent = (opaque_key: string) => { + updateItemsMutation.mutateAsync([opaque_key]).catch(() => { showToast(intl.formatMessage(messages.errorAssociateComponentMessage)); }); }; diff --git a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx index 80efc8cb3c..46b65ed47c 100644 --- a/src/library-authoring/add-content/PickLibraryContentModal.test.tsx +++ b/src/library-authoring/add-content/PickLibraryContentModal.test.tsx @@ -49,8 +49,8 @@ describe('', () => { }); it('can pick components from the modal', async () => { - const mockAddComponentsToCollection = jest.fn(); - jest.spyOn(api, 'addComponentsToCollection').mockImplementation(mockAddComponentsToCollection); + const mockAddItemsToCollection = jest.fn(); + jest.spyOn(api, 'addItemsToCollection').mockImplementation(mockAddItemsToCollection); render(); @@ -67,7 +67,7 @@ describe('', () => { fireEvent.click(screen.queryAllByRole('button', { name: 'Add to Collection' })[0]); await waitFor(() => { - expect(mockAddComponentsToCollection).toHaveBeenCalledWith( + expect(mockAddItemsToCollection).toHaveBeenCalledWith( libraryId, 'collectionId', ['lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd'], @@ -78,8 +78,8 @@ describe('', () => { }); it('show error when api call fails', async () => { - const mockAddComponentsToCollection = jest.fn().mockRejectedValue(new Error('Failed to add components')); - jest.spyOn(api, 'addComponentsToCollection').mockImplementation(mockAddComponentsToCollection); + const mockAddItemsToCollection = jest.fn().mockRejectedValue(new Error('Failed to add components')); + jest.spyOn(api, 'addItemsToCollection').mockImplementation(mockAddItemsToCollection); render(); // Wait for the content library to load @@ -95,7 +95,7 @@ describe('', () => { fireEvent.click(screen.queryAllByRole('button', { name: 'Add to Collection' })[0]); await waitFor(() => { - expect(mockAddComponentsToCollection).toHaveBeenCalledWith( + expect(mockAddItemsToCollection).toHaveBeenCalledWith( libraryId, 'collectionId', ['lb:Axim:TEST:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd'], diff --git a/src/library-authoring/add-content/PickLibraryContentModal.tsx b/src/library-authoring/add-content/PickLibraryContentModal.tsx index e5977d2407..dd38b95c3b 100644 --- a/src/library-authoring/add-content/PickLibraryContentModal.tsx +++ b/src/library-authoring/add-content/PickLibraryContentModal.tsx @@ -5,7 +5,7 @@ import { ActionRow, Button, StandardModal } from '@openedx/paragon'; import { ToastContext } from '../../generic/toast-context'; import { useLibraryContext } from '../common/context/LibraryContext'; import type { SelectedComponent } from '../common/context/ComponentPickerContext'; -import { useAddComponentsToCollection } from '../data/apiHooks'; +import { useAddItemsToCollection } from '../data/apiHooks'; import messages from './messages'; interface PickLibraryContentModalFooterProps { @@ -51,7 +51,7 @@ export const PickLibraryContentModal: React.FC = ( throw new Error('libraryId and componentPicker are required'); } - const updateComponentsMutation = useAddComponentsToCollection(libraryId, collectionId); + const updateComponentsMutation = useAddItemsToCollection(libraryId, collectionId); const { showToast } = useContext(ToastContext); diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index 2397cbb756..79ab0d8063 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -20,7 +20,7 @@ import { mockContentSearchConfig, mockGetBlockTypes } from '../../search-manager import { mockBroadcastChannel, mockClipboardEmpty } from '../../generic/data/api.mock'; import { LibraryLayout } from '..'; import { ContentTagsDrawer } from '../../content-tags-drawer'; -import { getLibraryCollectionComponentApiUrl } from '../data/api'; +import { getLibraryCollectionItemsApiUrl } from '../data/api'; let axiosMock: MockAdapter; let mockShowToast; @@ -351,7 +351,7 @@ describe('', () => { }); it('should remove component from collection and hides sidebar', async () => { - const url = getLibraryCollectionComponentApiUrl( + const url = getLibraryCollectionItemsApiUrl( mockContentLibrary.libraryId, mockCollection.collectionId, ); @@ -370,8 +370,8 @@ describe('', () => { fireEvent.click(await screen.findByText('Remove from collection')); await waitFor(() => { expect(axiosMock.history.delete.length).toEqual(1); - expect(mockShowToast).toHaveBeenCalledWith('Component successfully removed'); }); + expect(mockShowToast).toHaveBeenCalledWith('Item successfully removed'); // Should close sidebar as component was removed await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); diff --git a/src/library-authoring/collections/LibraryCollectionPage.tsx b/src/library-authoring/collections/LibraryCollectionPage.tsx index 306a64ab5b..81986c062c 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.tsx @@ -109,7 +109,9 @@ const LibraryCollectionPage = () => { } const { componentPickerMode } = useComponentPickerContext(); - const { showOnlyPublished, setCollectionId, componentId } = useLibraryContext(); + const { + showOnlyPublished, extraFilter: contextExtraFilter, setCollectionId, componentId, + } = useLibraryContext(); const { sidebarComponentInfo, openInfoSidebar } = useSidebarContext(); const { @@ -182,6 +184,10 @@ const LibraryCollectionPage = () => { extraFilter.push('last_published IS NOT NULL'); } + if (contextExtraFilter) { + extraFilter.push(...contextExtraFilter); + } + return (
diff --git a/src/library-authoring/common/context/ComponentPickerContext.tsx b/src/library-authoring/common/context/ComponentPickerContext.tsx index 83c258ec1d..51cc521374 100644 --- a/src/library-authoring/common/context/ComponentPickerContext.tsx +++ b/src/library-authoring/common/context/ComponentPickerContext.tsx @@ -8,7 +8,7 @@ import { export interface SelectedComponent { usageKey: string; - blockType: string; + blockType?: string; } export type ComponentSelectedEvent = (selectedComponent: SelectedComponent) => void; diff --git a/src/library-authoring/common/context/LibraryContext.tsx b/src/library-authoring/common/context/LibraryContext.tsx index 9b467e19a7..1a92b2f7a5 100644 --- a/src/library-authoring/common/context/LibraryContext.tsx +++ b/src/library-authoring/common/context/LibraryContext.tsx @@ -34,6 +34,8 @@ export type LibraryContextData = { setUnitId: (unitId?: string) => void; // Only show published components showOnlyPublished: boolean; + // Additional filtering + extraFilter?: string[]; // "Create New Collection" modal isCreateCollectionModalOpen: boolean; openCreateCollectionModal: () => void; @@ -66,6 +68,7 @@ type LibraryProviderProps = { children?: React.ReactNode; libraryId: string; showOnlyPublished?: boolean; + extraFilter?: string[] // If set, will initialize the current collection and/or component from the current URL skipUrlUpdate?: boolean; @@ -83,6 +86,7 @@ export const LibraryProvider = ({ children, libraryId, showOnlyPublished = false, + extraFilter = [], skipUrlUpdate = false, componentPicker, }: LibraryProviderProps) => { @@ -139,6 +143,7 @@ export const LibraryProvider = ({ readOnly, isLoadingLibraryData, showOnlyPublished, + extraFilter, isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal, @@ -164,6 +169,7 @@ export const LibraryProvider = ({ readOnly, isLoadingLibraryData, showOnlyPublished, + extraFilter, isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal, diff --git a/src/library-authoring/component-info/ComponentManagement.tsx b/src/library-authoring/component-info/ComponentManagement.tsx index 2d740d7f1a..c0eccdc5e4 100644 --- a/src/library-authoring/component-info/ComponentManagement.tsx +++ b/src/library-authoring/component-info/ComponentManagement.tsx @@ -8,11 +8,11 @@ import { import { useLibraryContext } from '../common/context/LibraryContext'; import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; -import { useLibraryBlockMetadata } from '../data/apiHooks'; +import { ContentTagsDrawer, useContentTaxonomyTagsData } from '../../content-tags-drawer'; +import { useLibraryBlockMetadata, useUpdateComponentCollections } from '../data/apiHooks'; import StatusWidget from '../generic/status-widget'; +import { ManageCollections } from '../generic/manage-collections'; import messages from './messages'; -import { ContentTagsDrawer, useContentTaxonomyTagsData } from '../../content-tags-drawer'; -import ManageCollections from './ManageCollections'; const ComponentManagement = () => { const intl = useIntl(); @@ -130,7 +130,11 @@ const ComponentManagement = () => { - + diff --git a/src/library-authoring/component-info/messages.ts b/src/library-authoring/component-info/messages.ts index 7e4068d53e..9674447339 100644 --- a/src/library-authoring/component-info/messages.ts +++ b/src/library-authoring/component-info/messages.ts @@ -136,51 +136,6 @@ const messages = defineMessages({ defaultMessage: 'Component Preview', description: 'Title for preview modal', }, - manageCollectionsText: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.text', - defaultMessage: 'Manage Collections', - description: 'Header and button text for collection section in manage tab', - }, - manageCollectionsAddBtnText: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.btn-text', - defaultMessage: 'Add to Collection', - description: 'Button text for collection section in manage tab', - }, - manageCollectionsSearchPlaceholder: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.search-placeholder', - defaultMessage: 'Search', - description: 'Placeholder text for collection search in manage tab', - }, - manageCollectionsSelectionLabel: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.selection-aria-label', - defaultMessage: 'Collection selection', - description: 'Aria label text for collection selection box', - }, - manageCollectionsToComponentSuccess: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.add-success', - defaultMessage: 'Component collections updated', - description: 'Message to display on updating component collections', - }, - manageCollectionsToComponentFailed: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.add-failed', - defaultMessage: 'Failed to update Component collections', - description: 'Message to display on failure of updating component collections', - }, - manageCollectionsToComponentConfirmBtn: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.add-confirm-btn', - defaultMessage: 'Confirm', - description: 'Button text to confirm collections for a component', - }, - manageCollectionsToComponentCancelBtn: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.add-cancel-btn', - defaultMessage: 'Cancel', - description: 'Button text to cancel collections selection for a component', - }, - componentNotOrganizedIntoCollection: { - id: 'course-authoring.library-authoring.component.manage-tab.collections.no-collections', - defaultMessage: 'This component is not organized into any collection.', - description: 'Message to display in manage collections section when component is not part of any collection.', - }, componentPickerSingleSelect: { id: 'course-authoring.library-authoring.component-picker.single-select', defaultMessage: 'Add to Course', // TODO: Change this message to a generic one? diff --git a/src/library-authoring/component-picker/ComponentPicker.tsx b/src/library-authoring/component-picker/ComponentPicker.tsx index 75bf178ac3..ec99b5fe42 100644 --- a/src/library-authoring/component-picker/ComponentPicker.tsx +++ b/src/library-authoring/component-picker/ComponentPicker.tsx @@ -38,7 +38,7 @@ const defaultSelectionChangedCallback: ComponentSelectionChangedEvent = (selecti window.parent.postMessage({ type: 'pickerSelectionChanged', selections }, '*'); }; -type ComponentPickerProps = { libraryId?: string, showOnlyPublished?: boolean } & ( +type ComponentPickerProps = { libraryId?: string, showOnlyPublished?: boolean, extraFilter?: string[] } & ( { componentPickerMode?: 'single', onComponentSelected?: ComponentSelectedEvent, @@ -54,6 +54,7 @@ export const ComponentPicker: React.FC = ({ /** Restrict the component picker to a specific library */ libraryId, showOnlyPublished, + extraFilter, componentPickerMode = 'single', /** This default callback is used to send the selected component back to the parent window, * when the component picker is used in an iframe. @@ -105,6 +106,7 @@ export const ComponentPicker: React.FC = ({ diff --git a/src/library-authoring/components/AddComponentWidget.tsx b/src/library-authoring/components/AddComponentWidget.tsx new file mode 100644 index 0000000000..a6e1c0f9ba --- /dev/null +++ b/src/library-authoring/components/AddComponentWidget.tsx @@ -0,0 +1,82 @@ +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { Button } from '@openedx/paragon'; +import { + AddCircleOutline, + CheckBoxIcon, + CheckBoxOutlineBlank, +} from '@openedx/paragon/icons'; + +import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; +import messages from './messages'; + +interface AddComponentWidgetProps { + usageKey: string; + blockType: string; +} + +const AddComponentWidget = ({ usageKey, blockType }: AddComponentWidgetProps) => { + const intl = useIntl(); + + const { + componentPickerMode, + onComponentSelected, + addComponentToSelectedComponents, + removeComponentFromSelectedComponents, + selectedComponents, + } = useComponentPickerContext(); + + // istanbul ignore if: this should never happen + if (!usageKey) { + throw new Error('usageKey is required'); + } + + // istanbul ignore if: this should never happen + if (!componentPickerMode) { + return null; + } + + if (componentPickerMode === 'single') { + return ( + + ); + } + + if (componentPickerMode === 'multiple') { + const isChecked = selectedComponents.some((component) => component.usageKey === usageKey); + + const handleChange = () => { + const selectedComponent = { + usageKey, + blockType, + }; + if (!isChecked) { + addComponentToSelectedComponents(selectedComponent); + } else { + removeComponentFromSelectedComponents(selectedComponent); + } + }; + + return ( + + ); + } + + // istanbul ignore next: this should never happen + return null; +}; + +export default AddComponentWidget; diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index 672a48b8bb..54380c9b3a 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -2,18 +2,12 @@ import { useCallback, useContext } from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, - Button, Dropdown, Icon, IconButton, useToggle, } from '@openedx/paragon'; -import { - AddCircleOutline, - CheckBoxIcon, - CheckBoxOutlineBlank, - MoreVert, -} from '@openedx/paragon/icons'; +import { MoreVert } from '@openedx/paragon/icons'; import { useClipboard } from '../../generic/clipboard'; import { ToastContext } from '../../generic/toast-context'; @@ -21,13 +15,14 @@ import { type ContentHit, PublishStatus } from '../../search-manager'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; -import { useRemoveComponentsFromCollection } from '../data/apiHooks'; +import { useRemoveItemsFromCollection } from '../data/apiHooks'; import { useLibraryRoutes } from '../routes'; +import AddComponentWidget from './AddComponentWidget'; import BaseCard from './BaseCard'; import { canEditComponent } from './ComponentEditorModal'; -import messages from './messages'; import ComponentDeleter from './ComponentDeleter'; +import messages from './messages'; type ComponentCardProps = { hit: ContentHit, @@ -50,7 +45,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { const canEdit = usageKey && canEditComponent(usageKey); const { showToast } = useContext(ToastContext); - const removeComponentsMutation = useRemoveComponentsFromCollection(libraryId, collectionId); + const removeComponentsMutation = useRemoveItemsFromCollection(libraryId, collectionId); const [isConfirmingDelete, confirmDelete, cancelDelete] = useToggle(false); const { copyToClipboard } = useClipboard(); @@ -110,76 +105,6 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { ); }; -interface AddComponentWidgetProps { - usageKey: string; - blockType: string; -} - -const AddComponentWidget = ({ usageKey, blockType }: AddComponentWidgetProps) => { - const intl = useIntl(); - - const { - componentPickerMode, - onComponentSelected, - addComponentToSelectedComponents, - removeComponentFromSelectedComponents, - selectedComponents, - } = useComponentPickerContext(); - - // istanbul ignore if: this should never happen - if (!usageKey) { - throw new Error('usageKey is required'); - } - - // istanbul ignore if: this should never happen - if (!componentPickerMode) { - return null; - } - - if (componentPickerMode === 'single') { - return ( - - ); - } - - if (componentPickerMode === 'multiple') { - const isChecked = selectedComponents.some((component) => component.usageKey === usageKey); - - const handleChange = () => { - const selectedComponent = { - usageKey, - blockType, - }; - if (!isChecked) { - addComponentToSelectedComponents(selectedComponent); - } else { - removeComponentFromSelectedComponents(selectedComponent); - } - }; - - return ( - - ); - } - - // istanbul ignore next: this should never happen - return null; -}; - const ComponentCard = ({ hit }: ComponentCardProps) => { const { showOnlyPublished } = useLibraryContext(); const { openComponentInfoSidebar } = useSidebarContext(); diff --git a/src/library-authoring/components/ContainerCard.tsx b/src/library-authoring/components/ContainerCard.tsx index d508c2082c..7b0e10c41f 100644 --- a/src/library-authoring/components/ContainerCard.tsx +++ b/src/library-authoring/components/ContainerCard.tsx @@ -1,4 +1,4 @@ -import { ReactNode, useCallback } from 'react'; +import { ReactNode, useCallback, useContext } from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, @@ -11,13 +11,15 @@ import { MoreVert } from '@openedx/paragon/icons'; import { Link } from 'react-router-dom'; import { getItemIcon, getComponentStyleColor } from '../../generic/block-type-utils'; +import { ToastContext } from '../../generic/toast-context'; import { type ContainerHit, PublishStatus } from '../../search-manager'; import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { useSidebarContext } from '../common/context/SidebarContext'; -import BaseCard from './BaseCard'; -import { useContainerChildren } from '../data/apiHooks'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; +import { useContainerChildren, useRemoveItemsFromCollection } from '../data/apiHooks'; import { useLibraryRoutes } from '../routes'; +import AddComponentWidget from './AddComponentWidget'; +import BaseCard from './BaseCard'; import messages from './messages'; type ContainerMenuProps = { @@ -26,7 +28,34 @@ type ContainerMenuProps = { const ContainerMenu = ({ hit } : ContainerMenuProps) => { const intl = useIntl(); - const { contextKey, blockId } = hit; + const { contextKey, usageKey: containerId } = hit; + const { libraryId, collectionId } = useLibraryContext(); + const { + sidebarComponentInfo, + openUnitInfoSidebar, + closeLibrarySidebar, + setSidebarAction, + } = useSidebarContext(); + const { showToast } = useContext(ToastContext); + + const removeComponentsMutation = useRemoveItemsFromCollection(libraryId, collectionId); + + const removeFromCollection = () => { + removeComponentsMutation.mutateAsync([containerId]).then(() => { + if (sidebarComponentInfo?.id === containerId) { + // Close sidebar if current component is open + closeLibrarySidebar(); + } + showToast(intl.formatMessage(messages.removeComponentSucess)); + }).catch(() => { + showToast(intl.formatMessage(messages.removeComponentFailure)); + }); + }; + + const showManageCollections = useCallback(() => { + setSidebarAction(SidebarActions.JumpToAddCollections); + openUnitInfoSidebar(containerId); + }, [setSidebarAction, openUnitInfoSidebar, containerId]); return ( @@ -42,11 +71,19 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => { + {collectionId && ( + + + + )} + + + ); @@ -149,9 +186,13 @@ const ContainerCard = ({ hit } : ContainerCardProps) => { preview={} tags={tags} numChildren={numChildrenCount} - actions={!componentPickerMode && ( + actions={( - + {componentPickerMode ? ( + + ) : ( + + )} )} hasUnpublishedChanges={publishStatus !== PublishStatus.Published} diff --git a/src/library-authoring/components/messages.ts b/src/library-authoring/components/messages.ts index 831855abc4..144fb066f2 100644 --- a/src/library-authoring/components/messages.ts +++ b/src/library-authoring/components/messages.ts @@ -44,17 +44,17 @@ const messages = defineMessages({ menuRemoveFromCollection: { id: 'course-authoring.library-authoring.component.menu.remove', defaultMessage: 'Remove from collection', - description: 'Menu item for remove a component from collection.', + description: 'Menu item for remove an item from collection.', }, removeComponentSucess: { id: 'course-authoring.library-authoring.component.remove-from-collection-success', - defaultMessage: 'Component successfully removed', - description: 'Message for successful removal of component from collection.', + defaultMessage: 'Item successfully removed', + description: 'Message for successful removal of an item from collection.', }, removeComponentFailure: { id: 'course-authoring.library-authoring.component.remove-from-collection-failure', - defaultMessage: 'Failed to remove Component', - description: 'Message for failure of removal of component from collection.', + defaultMessage: 'Failed to remove item', + description: 'Message for failure of removal of an item from collection.', }, deleteComponentWarningTitle: { id: 'course-authoring.library-authoring.component.delete-confirmation-title', @@ -137,7 +137,7 @@ const messages = defineMessages({ description: 'Message to display on failure to undo delete collection', }, componentPickerSingleSelectTitle: { - id: 'course-authoring.library-authoring.component-picker.single..title', + id: 'course-authoring.library-authoring.component-picker.single.title', defaultMessage: 'Add', description: 'Button title for picking a component', }, diff --git a/src/library-authoring/containers/ContainerOrganize.test.tsx b/src/library-authoring/containers/ContainerOrganize.test.tsx index 079aab0237..8fb366fa12 100644 --- a/src/library-authoring/containers/ContainerOrganize.test.tsx +++ b/src/library-authoring/containers/ContainerOrganize.test.tsx @@ -23,14 +23,18 @@ mockGetContainerMetadata.applyMock(); mockContentLibrary.applyMock(); mockContentTaxonomyTagsData.applyMock(); -const { containerIdForTags } = mockGetContainerMetadata; - -const render = (libraryId?: string) => baseRender(, { +const render = ({ + libraryId = mockContentLibrary.libraryId, + containerId = mockGetContainerMetadata.containerId, +}: { + libraryId?: string; + containerId?: string; +}) => baseRender(, { extraWrapper: ({ children }) => ( - + @@ -61,7 +65,7 @@ describe('', () => { ...getConfig(), ENABLE_TAGGING_TAXONOMY_PAGES: 'true', }); - render(libraryId); + render({ libraryId }); await waitFor(() => { expect(screen.getByText(`Mocked ${expected} ContentTagsDrawer`)).toBeInTheDocument(); }); @@ -73,7 +77,12 @@ describe('', () => { ...getConfig(), ENABLE_TAGGING_TAXONOMY_PAGES: 'true', }); - render(); + render({ containerId: mockGetContainerMetadata.containerIdForTags }); expect(await screen.findByText('Tags (6)')).toBeInTheDocument(); }); + + it('should render collection count in collection info section', async () => { + render({ containerId: mockGetContainerMetadata.containerIdWithCollections }); + expect(await screen.findByText('Collections (1)')).toBeInTheDocument(); + }); }); diff --git a/src/library-authoring/containers/ContainerOrganize.tsx b/src/library-authoring/containers/ContainerOrganize.tsx index 4a4e2706ed..e6585785e2 100644 --- a/src/library-authoring/containers/ContainerOrganize.tsx +++ b/src/library-authoring/containers/ContainerOrganize.tsx @@ -1,4 +1,4 @@ -import { useMemo } from 'react'; +import { useMemo, useEffect } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { getConfig } from '@edx/frontend-platform'; import { @@ -8,20 +8,27 @@ import { useToggle, } from '@openedx/paragon'; import { - ExpandLess, ExpandMore, Tag, + BookOpen, + ExpandLess, + ExpandMore, + Tag, } from '@openedx/paragon/icons'; import { ContentTagsDrawer, useContentTaxonomyTagsData } from '../../content-tags-drawer'; +import { ManageCollections } from '../generic/manage-collections'; +import { useContainer, useUpdateContainerCollections } from '../data/apiHooks'; import { useLibraryContext } from '../common/context/LibraryContext'; -import { useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; import messages from './messages'; const ContainerOrganize = () => { const intl = useIntl(); - const [tagsCollapseIsOpen, , , toggleTags] = useToggle(true); + const [tagsCollapseIsOpen, ,setTagsCollapseClose, toggleTags] = useToggle(true); + const [collectionsCollapseIsOpen, setCollectionsCollapseOpen, , toggleCollections] = useToggle(true); const { readOnly } = useLibraryContext(); - const { sidebarComponentInfo } = useSidebarContext(); + const { sidebarComponentInfo, sidebarAction } = useSidebarContext(); + const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; const containerId = sidebarComponentInfo?.id; // istanbul ignore if: this should never happen @@ -29,8 +36,17 @@ const ContainerOrganize = () => { throw new Error('containerId is required'); } + const { data: containerMetadata } = useContainer(containerId); const { data: componentTags } = useContentTaxonomyTagsData(containerId); + useEffect(() => { + if (jumpToCollections) { + setTagsCollapseClose(); + setCollectionsCollapseOpen(); + } + }, [jumpToCollections, tagsCollapseIsOpen, collectionsCollapseIsOpen]); + + const collectionsCount = useMemo(() => containerMetadata?.collections?.length || 0, [containerMetadata]); const tagsCount = useMemo(() => { if (!componentTags) { return 0; @@ -50,6 +66,11 @@ const ContainerOrganize = () => { return result; }, [componentTags]); + // istanbul ignore if: this should never happen + if (!containerMetadata) { + return null; + } + return ( {[true, 'true'].includes(getConfig().ENABLE_TAGGING_TAXONOMY_PAGES) @@ -82,6 +103,33 @@ const ContainerOrganize = () => { )} + + + + + {intl.formatMessage(messages.organizeTabCollectionsTitle, { count: collectionsCount })} + + + + + + + + + + + + ); }; diff --git a/src/library-authoring/containers/UnitInfo.tsx b/src/library-authoring/containers/UnitInfo.tsx index 90ad23008d..6fd4734049 100644 --- a/src/library-authoring/containers/UnitInfo.tsx +++ b/src/library-authoring/containers/UnitInfo.tsx @@ -1,3 +1,4 @@ +import { useEffect } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Button, @@ -8,6 +9,7 @@ import { import { useComponentPickerContext } from '../common/context/ComponentPickerContext'; import { type UnitInfoTab, + SidebarActions, UNIT_INFO_TABS, isUnitInfoTab, useSidebarContext, @@ -19,7 +21,13 @@ const UnitInfo = () => { const intl = useIntl(); const { componentPickerMode } = useComponentPickerContext(); - const { sidebarComponentInfo, sidebarTab, setSidebarTab } = useSidebarContext(); + const { + sidebarTab, + setSidebarTab, + sidebarComponentInfo, + sidebarAction, + } = useSidebarContext(); + const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; const tab: UnitInfoTab = ( sidebarTab && isUnitInfoTab(sidebarTab) @@ -31,6 +39,13 @@ const UnitInfo = () => { throw new Error('unitId is required'); } + useEffect(() => { + // Show Organize tab if JumpToAddCollections action is set in sidebarComponentInfo + if (jumpToCollections) { + setSidebarTab(UNIT_INFO_TABS.Organize); + } + }, [jumpToCollections, setSidebarTab]); + const showOpenCollectionButton = !componentPickerMode; return ( diff --git a/src/library-authoring/containers/messages.ts b/src/library-authoring/containers/messages.ts index be3158d126..b47bdebffb 100644 --- a/src/library-authoring/containers/messages.ts +++ b/src/library-authoring/containers/messages.ts @@ -21,6 +21,11 @@ const messages = defineMessages({ defaultMessage: 'Tags ({count})', description: 'Title for tags section in organize tab', }, + organizeTabCollectionsTitle: { + id: 'course-authoring.library-authoring.container-sidebar.organize-tab.collections.title', + defaultMessage: 'Collections ({count})', + description: 'Title for collections section in organize tab', + }, settingsTabTitle: { id: 'course-authoring.library-authoring.container-sidebar.settings-tab.title', defaultMessage: 'Settings', diff --git a/src/library-authoring/create-unit/CreateUnitModal.tsx b/src/library-authoring/create-unit/CreateUnitModal.tsx index 30ae485465..1be6db4e09 100644 --- a/src/library-authoring/create-unit/CreateUnitModal.tsx +++ b/src/library-authoring/create-unit/CreateUnitModal.tsx @@ -10,28 +10,33 @@ import * as Yup from 'yup'; import FormikControl from '../../generic/FormikControl'; import { useLibraryContext } from '../common/context/LibraryContext'; import messages from './messages'; -import { useCreateLibraryContainer } from '../data/apiHooks'; +import { useAddItemsToCollection, useCreateLibraryContainer } from '../data/apiHooks'; import { ToastContext } from '../../generic/toast-context'; import LoadingButton from '../../generic/loading-button'; const CreateUnitModal = () => { const intl = useIntl(); const { + collectionId, libraryId, isCreateUnitModalOpen, closeCreateUnitModal, } = useLibraryContext(); const create = useCreateLibraryContainer(libraryId); + const updateItemsMutation = useAddItemsToCollection(libraryId, collectionId); const { showToast } = React.useContext(ToastContext); const handleCreate = React.useCallback(async (values) => { try { - await create.mutateAsync({ + const container = await create.mutateAsync({ containerType: 'unit', ...values, }); + if (collectionId) { + await updateItemsMutation.mutateAsync([container.containerKey]); + } // TODO: Navigate to the new unit - // navigate(`/library/${libraryId}/units/${data.key}`); + // navigate(`/library/${libraryId}/units/${container.containerKey}`); showToast(intl.formatMessage(messages.createUnitSuccess)); } catch (error) { showToast(intl.formatMessage(messages.createUnitError)); diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 185169b904..761e968de1 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -472,6 +472,8 @@ export async function mockGetContainerMetadata(containerId: string): Promise { }); + case mockGetContainerMetadata.containerIdWithCollections: + return Promise.resolve(mockGetContainerMetadata.containerDataWithCollections); default: return Promise.resolve(mockGetContainerMetadata.containerData); } @@ -480,6 +482,7 @@ mockGetContainerMetadata.containerId = 'lct:org:lib:unit:test-unit-9a207'; mockGetContainerMetadata.containerIdError = 'lct:org:lib:unit:container_error'; mockGetContainerMetadata.containerIdLoading = 'lct:org:lib:unit:container_loading'; mockGetContainerMetadata.containerIdForTags = mockContentTaxonomyTagsData.largeTagsId; +mockGetContainerMetadata.containerIdWithCollections = 'lct:org:lib:unit:container_collections'; mockGetContainerMetadata.containerData = { containerKey: 'lct:org:lib:unit:test-unit-9a2072', containerType: 'unit', @@ -494,6 +497,11 @@ mockGetContainerMetadata.containerData = { hasUnpublishedChanges: true, collections: [], } satisfies api.Container; +mockGetContainerMetadata.containerDataWithCollections = { + ...mockGetContainerMetadata.containerData, + containerKey: mockGetContainerMetadata.containerIdWithCollections, + collections: [{ title: 'My first collection', key: 'my-first-collection' }], +} satisfies api.Container; /** Apply this mock. Returns a spy object that can tell you if it's been called. */ mockGetContainerMetadata.applyMock = () => { jest.spyOn(api, 'getContainerMetadata').mockImplementation(mockGetContainerMetadata); diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 7348eedc9a..8032f6dcc2 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -40,7 +40,7 @@ export const getLibraryBlockMetadataUrl = (usageKey: string) => `${getApiBaseUrl export const getLibraryBlockRestoreUrl = (usageKey: string) => `${getLibraryBlockMetadataUrl(usageKey)}restore/`; /** - * Get the URL for library block metadata. + * Get the URL for library block collections. */ export const getLibraryBlockCollectionsUrl = (usageKey: string) => `${getLibraryBlockMetadataUrl(usageKey)}collections/`; @@ -88,9 +88,9 @@ export const getLibraryCollectionsApiUrl = (libraryId: string) => `${getApiBaseU */ export const getLibraryCollectionApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionsApiUrl(libraryId)}${collectionId}/`; /** - * Get the URL for the collection components API. + * Get the URL for the collection items API. */ -export const getLibraryCollectionComponentApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionApiUrl(libraryId, collectionId)}components/`; +export const getLibraryCollectionItemsApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionApiUrl(libraryId, collectionId)}items/`; /** * Get the API URL for restoring deleted collection. */ @@ -115,6 +115,10 @@ export const getLibraryContainerApiUrl = (containerId: string) => `${getApiBaseU * Get the URL for a single container children api. */ export const getLibraryContainerChildrenApiUrl = (containerId: string) => `${getLibraryContainerApiUrl(containerId)}children/`; +/** + * Get the URL for library container collections. + */ +export const getLibraryContainerCollectionsUrl = (containerId: string) => `${getLibraryContainerApiUrl(containerId)}collections/`; export interface ContentLibrary { id: string; @@ -528,19 +532,19 @@ export async function updateCollectionMetadata( } /** - * Add components to collection. + * Add items to collection. */ -export async function addComponentsToCollection(libraryId: string, collectionId: string, usageKeys: string[]) { - await getAuthenticatedHttpClient().patch(getLibraryCollectionComponentApiUrl(libraryId, collectionId), { +export async function addItemsToCollection(libraryId: string, collectionId: string, usageKeys: string[]) { + await getAuthenticatedHttpClient().patch(getLibraryCollectionItemsApiUrl(libraryId, collectionId), { usage_keys: usageKeys, }); } /** - * Remove components from collection. + * Remove items from collection. */ -export async function removeComponentsFromCollection(libraryId: string, collectionId: string, usageKeys: string[]) { - await getAuthenticatedHttpClient().delete(getLibraryCollectionComponentApiUrl(libraryId, collectionId), { +export async function removeItemsFromCollection(libraryId: string, collectionId: string, usageKeys: string[]) { + await getAuthenticatedHttpClient().delete(getLibraryCollectionItemsApiUrl(libraryId, collectionId), { data: { usage_keys: usageKeys }, }); } @@ -578,9 +582,13 @@ export interface CreateLibraryContainerDataRequest { /** * Create a library container */ -export async function createLibraryContainer(libraryId: string, containerData: CreateLibraryContainerDataRequest) { +export async function createLibraryContainer( + libraryId: string, + containerData: CreateLibraryContainerDataRequest, +): Promise { const client = getAuthenticatedHttpClient(); - await client.post(getLibraryContainersApiUrl(libraryId), snakeCaseObject(containerData)); + const { data } = await client.post(getLibraryContainersApiUrl(libraryId), snakeCaseObject(containerData)); + return camelCaseObject(data); } export interface Container { @@ -629,3 +637,12 @@ export async function getContainerChildren(containerId: string): Promise { it('should add components to collection', async () => { const libraryId = 'lib:org:1'; const collectionId = 'my-first-collection'; - const url = getLibraryCollectionComponentApiUrl(libraryId, collectionId); + const url = getLibraryCollectionItemsApiUrl(libraryId, collectionId); axiosMock.onPatch(url).reply(200); - const { result } = renderHook(() => useAddComponentsToCollection(libraryId, collectionId), { wrapper }); + const { result } = renderHook(() => useAddItemsToCollection(libraryId, collectionId), { wrapper }); await result.current.mutateAsync(['some-usage-key']); expect(axiosMock.history.patch[0].url).toEqual(url); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 8394927481..54dd904223 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -33,7 +33,7 @@ import { getXBlockOLX, updateCollectionMetadata, type UpdateCollectionComponentsRequest, - addComponentsToCollection, + addItemsToCollection, type CreateLibraryCollectionDataRequest, getCollectionMetadata, deleteCollection, @@ -41,7 +41,7 @@ import { setXBlockOLX, getXBlockAssets, updateComponentCollections, - removeComponentsFromCollection, + removeItemsFromCollection, publishXBlock, deleteXBlockAsset, restoreLibraryBlock, @@ -52,6 +52,7 @@ import { updateContainerMetadata, type UpdateContainerDataRequest, getContainerChildren, + updateContainerCollections, } from './api'; import { VersionSpec } from '../LibraryBlock'; @@ -500,14 +501,14 @@ export const useUpdateCollection = (libraryId: string, collectionId: string) => }; /** - * Use this mutation to add components to a collection in a library + * Use this mutation to add items to a collection in a library */ -export const useAddComponentsToCollection = (libraryId?: string, collectionId?: string) => { +export const useAddItemsToCollection = (libraryId?: string, collectionId?: string) => { const queryClient = useQueryClient(); return useMutation({ mutationFn: async (usageKeys: string[]) => { if (libraryId !== undefined && collectionId !== undefined) { - return addComponentsToCollection(libraryId, collectionId, usageKeys); + return addItemsToCollection(libraryId, collectionId, usageKeys); } return undefined; }, @@ -520,14 +521,14 @@ export const useAddComponentsToCollection = (libraryId?: string, collectionId?: }; /** - * Use this mutation to remove components from a collection in a library + * Use this mutation to remove items from a collection in a library */ -export const useRemoveComponentsFromCollection = (libraryId?: string, collectionId?: string) => { +export const useRemoveItemsFromCollection = (libraryId?: string, collectionId?: string) => { const queryClient = useQueryClient(); return useMutation({ mutationFn: async (usageKeys: string[]) => { if (libraryId !== undefined && collectionId !== undefined) { - return removeComponentsFromCollection(libraryId, collectionId, usageKeys); + return removeItemsFromCollection(libraryId, collectionId, usageKeys); } return undefined; }, @@ -570,8 +571,9 @@ export const useRestoreCollection = (libraryId: string, collectionId: string) => /** * Use this mutation to update collections related a component in a library */ -export const useUpdateComponentCollections = (libraryId: string, usageKey: string) => { +export const useUpdateComponentCollections = (usageKey: string) => { const queryClient = useQueryClient(); + const libraryId = getLibraryId(usageKey); return useMutation({ mutationFn: async (collectionKeys: string[]) => updateComponentCollections(usageKey, collectionKeys), onSettled: () => { @@ -631,3 +633,18 @@ export const useContainerChildren = (containerId: string) => ( queryFn: () => getContainerChildren(containerId!), }) ); + +/** + * Use this mutation to update collections related a container in a library + */ +export const useUpdateContainerCollections = (containerId: string) => { + const queryClient = useQueryClient(); + const libraryId = getLibraryId(containerId); + return useMutation({ + mutationFn: async (collectionKeys: string[]) => updateContainerCollections(containerId, collectionKeys), + onSettled: () => { + queryClient.invalidateQueries({ queryKey: containerQueryKeys.container(containerId) }); + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); + }, + }); +}; diff --git a/src/library-authoring/component-info/ManageCollections.test.tsx b/src/library-authoring/generic/manage-collections/ManageCollections.test.tsx similarity index 61% rename from src/library-authoring/component-info/ManageCollections.test.tsx rename to src/library-authoring/generic/manage-collections/ManageCollections.test.tsx index 790fb0a993..b73dd0d837 100644 --- a/src/library-authoring/component-info/ManageCollections.test.tsx +++ b/src/library-authoring/generic/manage-collections/ManageCollections.test.tsx @@ -2,25 +2,27 @@ import fetchMock from 'fetch-mock-jest'; import userEvent from '@testing-library/user-event'; import MockAdapter from 'axios-mock-adapter/types'; +import { mockContentSearchConfig } from '../../../search-manager/data/api.mock'; import { initializeMocks, render as baseRender, screen, waitFor, -} from '../../testUtils'; -import mockCollectionsResults from '../__mocks__/collection-search.json'; -import { mockContentSearchConfig } from '../../search-manager/data/api.mock'; -import { mockContentLibrary, mockLibraryBlockMetadata } from '../data/api.mocks'; +} from '../../../testUtils'; +import mockCollectionsResults from '../../__mocks__/collection-search.json'; +import { LibraryProvider } from '../../common/context/LibraryContext'; +import { SidebarProvider } from '../../common/context/SidebarContext'; +import { getLibraryBlockCollectionsUrl, getLibraryContainerCollectionsUrl } from '../../data/api'; +import { useUpdateComponentCollections, useUpdateContainerCollections } from '../../data/apiHooks'; +import { mockContentLibrary, mockLibraryBlockMetadata, mockGetContainerMetadata } from '../../data/api.mocks'; import ManageCollections from './ManageCollections'; -import { LibraryProvider } from '../common/context/LibraryContext'; -import { SidebarProvider } from '../common/context/SidebarContext'; -import { getLibraryBlockCollectionsUrl } from '../data/api'; let axiosMock: MockAdapter; let mockShowToast; mockContentLibrary.applyMock(); mockLibraryBlockMetadata.applyMock(); +mockGetContainerMetadata.applyMock(); mockContentSearchConfig.applyMock(); const render = (ui: React.ReactElement) => baseRender(ui, { @@ -56,12 +58,13 @@ describe('', () => { }); }); - it('should show all collections in library and allow users to select for the current component ', async () => { + it('should show all collections in library and allow users to select for the current component', async () => { const url = getLibraryBlockCollectionsUrl(mockLibraryBlockMetadata.usageKeyWithCollections); axiosMock.onPatch(url).reply(200); render(); const manageBtn = await screen.findByRole('button', { name: 'Manage Collections' }); userEvent.click(manageBtn); @@ -73,10 +76,36 @@ describe('', () => { userEvent.click(confirmBtn); await waitFor(() => { expect(axiosMock.history.patch.length).toEqual(1); - expect(mockShowToast).toHaveBeenCalledWith('Component collections updated'); - expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ - collection_keys: ['my-first-collection', 'my-second-collection'], - }); + }); + expect(mockShowToast).toHaveBeenCalledWith('Item collections updated'); + expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ + collection_keys: ['my-first-collection', 'my-second-collection'], + }); + expect(screen.queryByRole('search')).not.toBeInTheDocument(); + }); + + it('should show all collections in library and allow users to select for the current container', async () => { + const url = getLibraryContainerCollectionsUrl(mockGetContainerMetadata.containerIdWithCollections); + axiosMock.onPatch(url).reply(200); + render(); + const manageBtn = await screen.findByRole('button', { name: 'Manage Collections' }); + userEvent.click(manageBtn); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); + expect(screen.queryByRole('search')).toBeInTheDocument(); + const secondCollection = await screen.findByRole('button', { name: 'My second collection' }); + userEvent.click(secondCollection); + const confirmBtn = await screen.findByRole('button', { name: 'Confirm' }); + userEvent.click(confirmBtn); + await waitFor(() => { + expect(axiosMock.history.patch.length).toEqual(1); + }); + expect(mockShowToast).toHaveBeenCalledWith('Item collections updated'); + expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ + collection_keys: ['my-first-collection', 'my-second-collection'], }); expect(screen.queryByRole('search')).not.toBeInTheDocument(); }); @@ -85,8 +114,9 @@ describe('', () => { const url = getLibraryBlockCollectionsUrl(mockLibraryBlockMetadata.usageKeyWithCollections); axiosMock.onPatch(url).reply(400); render(); screen.logTestingPlaygroundURL(); const manageBtn = await screen.findByRole('button', { name: 'Add to Collection' }); @@ -99,11 +129,11 @@ describe('', () => { userEvent.click(confirmBtn); await waitFor(() => { expect(axiosMock.history.patch.length).toEqual(1); - expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ - collection_keys: ['my-second-collection'], - }); - expect(mockShowToast).toHaveBeenCalledWith('Failed to update Component collections'); }); + expect(JSON.parse(axiosMock.history.patch[0].data)).toEqual({ + collection_keys: ['my-second-collection'], + }); + expect(mockShowToast).toHaveBeenCalledWith('Failed to update item collections'); expect(screen.queryByRole('search')).not.toBeInTheDocument(); }); @@ -111,8 +141,9 @@ describe('', () => { const url = getLibraryBlockCollectionsUrl(mockLibraryBlockMetadata.usageKeyWithCollections); axiosMock.onPatch(url).reply(400); render(); const manageBtn = await screen.findByRole('button', { name: 'Add to Collection' }); userEvent.click(manageBtn); @@ -124,8 +155,8 @@ describe('', () => { userEvent.click(cancelBtn); await waitFor(() => { expect(axiosMock.history.patch.length).toEqual(0); - expect(mockShowToast).not.toHaveBeenCalled(); }); + expect(mockShowToast).not.toHaveBeenCalled(); expect(screen.queryByRole('search')).not.toBeInTheDocument(); }); }); diff --git a/src/library-authoring/component-info/ManageCollections.tsx b/src/library-authoring/generic/manage-collections/ManageCollections.tsx similarity index 81% rename from src/library-authoring/component-info/ManageCollections.tsx rename to src/library-authoring/generic/manage-collections/ManageCollections.tsx index 1ab32c8b93..41bc36ce7c 100644 --- a/src/library-authoring/component-info/ManageCollections.tsx +++ b/src/library-authoring/generic/manage-collections/ManageCollections.tsx @@ -1,5 +1,6 @@ import { useContext, useState } from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import type { UseMutationResult } from '@tanstack/react-query'; import { Button, Icon, Scrollable, SelectableBox, Stack, StatefulButton, useCheckboxSetValues, } from '@openedx/paragon'; @@ -10,24 +11,29 @@ import { SearchKeywordsField, SearchSortWidget, useSearchContext, -} from '../../search-manager'; +} from '../../../search-manager'; +import { ToastContext } from '../../../generic/toast-context'; +import { CollectionMetadata } from '../../data/api'; +import { useLibraryContext } from '../../common/context/LibraryContext'; +import { SidebarActions, useSidebarContext } from '../../common/context/SidebarContext'; import messages from './messages'; -import { useUpdateComponentCollections } from '../data/apiHooks'; -import { ToastContext } from '../../generic/toast-context'; -import { CollectionMetadata } from '../data/api'; -import { useLibraryContext } from '../common/context/LibraryContext'; -import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; interface ManageCollectionsProps { - usageKey: string; + opaqueKey: string; collections: CollectionMetadata[], + useUpdateCollectionsHook: (opaqueKey: string) => UseMutationResult; } interface CollectionsDrawerProps extends ManageCollectionsProps { onClose: () => void; } -const CollectionsSelectableBox = ({ usageKey, collections, onClose }: CollectionsDrawerProps) => { +const CollectionsSelectableBox = ({ + opaqueKey, + collections, + useUpdateCollectionsHook, + onClose, +}: CollectionsDrawerProps) => { const type = 'checkbox'; const intl = useIntl(); const { hits } = useSearchContext(); @@ -39,9 +45,7 @@ const CollectionsSelectableBox = ({ usageKey, collections, onClose }: Collection }] = useCheckboxSetValues(collectionKeys); const [btnState, setBtnState] = useState('default'); - const { libraryId } = useLibraryContext(); - - const updateCollectionsMutation = useUpdateComponentCollections(libraryId, usageKey); + const updateCollectionsMutation = useUpdateCollectionsHook(opaqueKey); const handleConfirmation = () => { setBtnState('pending'); @@ -107,7 +111,12 @@ const CollectionsSelectableBox = ({ usageKey, collections, onClose }: Collection ); }; -const AddToCollectionsDrawer = ({ usageKey, collections, onClose }: CollectionsDrawerProps) => { +const AddToCollectionsDrawer = ({ + opaqueKey, + collections, + useUpdateCollectionsHook, + onClose, +}: CollectionsDrawerProps) => { const intl = useIntl(); const { libraryId } = useLibraryContext(); @@ -128,19 +137,20 @@ const AddToCollectionsDrawer = ({ usageKey, collections, onClose }: CollectionsD /> - {/* Set key to update selection when component usageKey changes */} + {/* Set key to update selection when entity opaqueKey changes */} ); }; -const ComponentCollections = ({ collections, onManageClick }: { +const EntityCollections = ({ collections, onManageClick }: { collections?: string[]; onManageClick: () => void; }) => { @@ -190,7 +200,7 @@ const ComponentCollections = ({ collections, onManageClick }: { ); }; -const ManageCollections = ({ usageKey, collections }: ManageCollectionsProps) => { +const ManageCollections = ({ opaqueKey, collections, useUpdateCollectionsHook }: ManageCollectionsProps) => { const { sidebarAction, resetSidebarAction, setSidebarAction } = useSidebarContext(); const collectionNames = collections.map((collection) => collection.title); @@ -198,12 +208,13 @@ const ManageCollections = ({ usageKey, collections }: ManageCollectionsProps) => sidebarAction === SidebarActions.JumpToAddCollections ? ( resetSidebarAction()} /> ) : ( - setSidebarAction(SidebarActions.JumpToAddCollections)} /> diff --git a/src/library-authoring/generic/manage-collections/index.tsx b/src/library-authoring/generic/manage-collections/index.tsx new file mode 100644 index 0000000000..ee943733e1 --- /dev/null +++ b/src/library-authoring/generic/manage-collections/index.tsx @@ -0,0 +1 @@ +export { default as ManageCollections } from './ManageCollections'; diff --git a/src/library-authoring/generic/manage-collections/messages.tsx b/src/library-authoring/generic/manage-collections/messages.tsx new file mode 100644 index 0000000000..c9b998be47 --- /dev/null +++ b/src/library-authoring/generic/manage-collections/messages.tsx @@ -0,0 +1,51 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + manageCollectionsText: { + id: 'course-authoring.library-authoring.manage-collections.title', + defaultMessage: 'Manage Collections', + description: 'Header and button text for the manage collection widget', + }, + manageCollectionsAddBtnText: { + id: 'course-authoring.library-authoring.manage-collections.btn-text', + defaultMessage: 'Add to Collection', + description: 'Button text for collection section in the manage collections widget', + }, + manageCollectionsSearchPlaceholder: { + id: 'course-authoring.library-authoring.manage-collections.search-placeholder', + defaultMessage: 'Search', + description: 'Placeholder text for collection search in the manage collections widget', + }, + manageCollectionsSelectionLabel: { + id: 'course-authoring.library-authoring.manage-collections.selection-aria-label', + defaultMessage: 'Collection selection', + description: 'Aria label text for collection selection box', + }, + manageCollectionsToComponentSuccess: { + id: 'course-authoring.library-authoring.manage-collections.add-success', + defaultMessage: 'Item collections updated', + description: 'Message to display on updating item collections', + }, + manageCollectionsToComponentFailed: { + id: 'course-authoring.library-authoring.manage-collections.add-failed', + defaultMessage: 'Failed to update item collections', + description: 'Message to display on failure of updating item collections', + }, + manageCollectionsToComponentConfirmBtn: { + id: 'course-authoring.library-authoring.manage-collections.add-confirm-btn', + defaultMessage: 'Confirm', + description: 'Button text to confirm adding collections for an item', + }, + manageCollectionsToComponentCancelBtn: { + id: 'course-authoring.library-authoring.manage-collections.add-cancel-btn', + defaultMessage: 'Cancel', + description: 'Button text to cancel collections selection for am item', + }, + componentNotOrganizedIntoCollection: { + id: 'course-authoring.library-authoring.manage-collections.no-collections', + defaultMessage: 'This item is not organized into any collection.', + description: 'Message to display in the manage collections widget when an item is not part of any collection.', + }, +}); + +export default messages; From 8ecb57578d4fb4b4afeca29174532f7c2d1d2d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 11 Apr 2025 23:01:51 -0300 Subject: [PATCH 2/6] fix: fixes from merge --- .../components/ContainerCard.tsx | 3 +- .../containers/ContainerInfoHeader.tsx | 4 +-- src/library-authoring/containers/UnitInfo.tsx | 4 +-- src/library-authoring/data/apiHooks.test.tsx | 6 ++-- src/library-authoring/data/apiHooks.ts | 29 ++++++------------- .../units/LibraryUnitBlocks.tsx | 7 ++--- .../units/LibraryUnitPage.tsx | 2 +- 7 files changed, 20 insertions(+), 35 deletions(-) diff --git a/src/library-authoring/components/ContainerCard.tsx b/src/library-authoring/components/ContainerCard.tsx index e39871215f..ecc098a419 100644 --- a/src/library-authoring/components/ContainerCard.tsx +++ b/src/library-authoring/components/ContainerCard.tsx @@ -114,8 +114,7 @@ type ContainerCardPreviewProps = { }; const ContainerCardPreview = ({ containerId, showMaxChildren = 5 }: ContainerCardPreviewProps) => { - const { libraryId } = useLibraryContext(); - const { data, isLoading, isError } = useContainerChildren(libraryId, containerId); + const { data, isLoading, isError } = useContainerChildren(containerId); if (isLoading || isError) { return null; } diff --git a/src/library-authoring/containers/ContainerInfoHeader.tsx b/src/library-authoring/containers/ContainerInfoHeader.tsx index cec30a8c45..3ac06045a2 100644 --- a/src/library-authoring/containers/ContainerInfoHeader.tsx +++ b/src/library-authoring/containers/ContainerInfoHeader.tsx @@ -18,7 +18,7 @@ const ContainerInfoHeader = () => { const intl = useIntl(); const [inputIsActive, setIsActive] = useState(false); - const { libraryId, readOnly } = useLibraryContext(); + const { readOnly } = useLibraryContext(); const { sidebarComponentInfo } = useSidebarContext(); const containerId = sidebarComponentInfo?.id; @@ -27,7 +27,7 @@ const ContainerInfoHeader = () => { throw new Error('containerId is required'); } - const { data: container } = useContainer(libraryId, containerId); + const { data: container } = useContainer(containerId); const updateMutation = useUpdateContainer(containerId); const { showToast } = useContext(ToastContext); diff --git a/src/library-authoring/containers/UnitInfo.tsx b/src/library-authoring/containers/UnitInfo.tsx index b7ca3430aa..ab07baf9a0 100644 --- a/src/library-authoring/containers/UnitInfo.tsx +++ b/src/library-authoring/containers/UnitInfo.tsx @@ -70,7 +70,7 @@ const UnitMenu = ({ containerId, displayName }: ContainerMenuProps) => { const UnitInfo = () => { const intl = useIntl(); - const { libraryId, setUnitId } = useLibraryContext(); + const { setUnitId } = useLibraryContext(); const { componentPickerMode } = useComponentPickerContext(); const { defaultTab, @@ -88,7 +88,7 @@ const UnitInfo = () => { ) ? sidebarTab : defaultTab.unit; const unitId = sidebarComponentInfo?.id; - const { data: container } = useContainer(libraryId, unitId); + const { data: container } = useContainer(unitId); const handleOpenUnit = useCallback(() => { if (componentPickerMode) { diff --git a/src/library-authoring/data/apiHooks.test.tsx b/src/library-authoring/data/apiHooks.test.tsx index 217f5822cb..27f14bb2a3 100644 --- a/src/library-authoring/data/apiHooks.test.tsx +++ b/src/library-authoring/data/apiHooks.test.tsx @@ -146,12 +146,11 @@ describe('library api hooks', () => { }); it('should get container metadata', async () => { - const libraryId = 'lib:org:1'; const containerId = 'lct:lib:org:unit:unit1'; const url = getLibraryContainerApiUrl(containerId); axiosMock.onGet(url).reply(200, { 'test-data': 'test-value' }); - const { result } = renderHook(() => useContainer(libraryId, containerId), { wrapper }); + const { result } = renderHook(() => useContainer(containerId), { wrapper }); await waitFor(() => { expect(result.current.isLoading).toBeFalsy(); }); @@ -184,7 +183,6 @@ describe('library api hooks', () => { }); it('should get container children', async () => { - const libraryId = 'lib:org:1'; const containerId = 'lct:lib:org:unit:unit1'; const url = getLibraryContainerChildrenApiUrl(containerId); @@ -220,7 +218,7 @@ describe('library api hooks', () => { collections: ['col2'], }, ]); - const { result } = renderHook(() => useContainerChildren(libraryId, containerId), { wrapper }); + const { result } = renderHook(() => useContainerChildren(containerId), { wrapper }); await waitFor(() => { expect(result.current.isLoading).toBeFalsy(); }); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 3b92382900..c22e631d4e 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -94,17 +94,6 @@ export const libraryAuthoringQueryKeys = { libraryId, collectionId, ], - container: (libraryId?: string, containerId?: string) => [ - ...libraryAuthoringQueryKeys.all, - libraryId, - containerId, - ], - containerChildren: (libraryId?: string, containerId?: string) => [ - ...libraryAuthoringQueryKeys.all, - libraryId, - containerId, - 'children', - ], blockTypes: (libraryId?: string) => [ ...libraryAuthoringQueryKeys.all, 'blockTypes', @@ -133,8 +122,8 @@ export const containerQueryKeys = { /** * Base key for data specific to a container */ - container: (usageKey?: string) => [...containerQueryKeys.all, usageKey], - children: (usageKey?: string) => [...containerQueryKeys.all, usageKey, 'children'], + container: (containerId?: string) => [...containerQueryKeys.all, containerId], + children: (containerId?: string) => [...containerQueryKeys.all, containerId, 'children'], }; /** @@ -607,10 +596,10 @@ export const useCreateLibraryContainer = (libraryId: string) => { /** * Get the metadata for a container in a library */ -export const useContainer = (libraryId?: string, containerId?: string) => ( +export const useContainer = (containerId?: string) => ( useQuery({ - enabled: !!libraryId && !!containerId, - queryKey: libraryAuthoringQueryKeys.container(libraryId, containerId), + enabled: !!containerId, + queryKey: containerQueryKeys.container(containerId), queryFn: () => getContainerMetadata(containerId!), }) ); @@ -627,7 +616,7 @@ export const useUpdateContainer = (containerId: string) => { // NOTE: We invalidate the library query here because we need to update the library's // container list. queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); - queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.container(libraryId, containerId) }); + queryClient.invalidateQueries({ queryKey: containerQueryKeys.container(containerId) }); }, }); }; @@ -664,10 +653,10 @@ export const useRestoreContainer = (containerId: string) => { /** * Get the metadata and children for a container in a library */ -export const useContainerChildren = (libraryId?: string, containerId?: string) => ( +export const useContainerChildren = (containerId?: string) => ( useQuery({ - enabled: !!libraryId && !!containerId, - queryKey: libraryAuthoringQueryKeys.containerChildren(libraryId, containerId), + enabled: !!containerId, + queryKey: containerQueryKeys.children(containerId), queryFn: () => getLibraryContainerChildren(containerId!), }) ); diff --git a/src/library-authoring/units/LibraryUnitBlocks.tsx b/src/library-authoring/units/LibraryUnitBlocks.tsx index 6bb1f06ace..34169cebc9 100644 --- a/src/library-authoring/units/LibraryUnitBlocks.tsx +++ b/src/library-authoring/units/LibraryUnitBlocks.tsx @@ -18,7 +18,7 @@ import TagCount from '../../generic/tag-count'; import { useLibraryContext } from '../common/context/LibraryContext'; import ComponentMenu from '../components'; import { LibraryBlockMetadata } from '../data/api'; -import { libraryAuthoringQueryKeys, useContainerChildren } from '../data/apiHooks'; +import { containerQueryKeys, useContainerChildren } from '../data/apiHooks'; import { LibraryBlock } from '../LibraryBlock'; import { useLibraryRoutes } from '../routes'; import messages from './messages'; @@ -29,7 +29,6 @@ export const LibraryUnitBlocks = () => { const { navigateTo } = useLibraryRoutes(); const { - libraryId, unitId, showOnlyPublished, componentId, @@ -42,7 +41,7 @@ export const LibraryUnitBlocks = () => { isLoading, isError, error, - } = useContainerChildren(libraryId, unitId); + } = useContainerChildren(unitId); useEffect(() => setOrderedBlocks(blocks || []), [blocks]); @@ -63,7 +62,7 @@ export const LibraryUnitBlocks = () => { }; const onTagSidebarClose = () => { - queryClient.invalidateQueries(libraryAuthoringQueryKeys.containerChildren(libraryId, unitId)); + queryClient.invalidateQueries(containerQueryKeys.children(unitId)); closeManageTagsDrawer(); }; diff --git a/src/library-authoring/units/LibraryUnitPage.tsx b/src/library-authoring/units/LibraryUnitPage.tsx index 2bd5e42a05..502b38920c 100644 --- a/src/library-authoring/units/LibraryUnitPage.tsx +++ b/src/library-authoring/units/LibraryUnitPage.tsx @@ -119,7 +119,7 @@ export const LibraryUnitPage = () => { isLoading, isError, error, - } = useContainer(libraryId, unitId); + } = useContainer(unitId); // Only show loading if unit or library data is not fetched from index yet if (isLibLoading || isLoading) { From 0962eabcf8fbbbb841cd0db3bb179f954ab8ea4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 11 Apr 2025 23:08:20 -0300 Subject: [PATCH 3/6] fix: fixes from review --- src/library-authoring/common/context/ComponentPickerContext.tsx | 2 +- .../generic/manage-collections/{messages.tsx => messages.ts} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/library-authoring/generic/manage-collections/{messages.tsx => messages.ts} (100%) diff --git a/src/library-authoring/common/context/ComponentPickerContext.tsx b/src/library-authoring/common/context/ComponentPickerContext.tsx index 51cc521374..83c258ec1d 100644 --- a/src/library-authoring/common/context/ComponentPickerContext.tsx +++ b/src/library-authoring/common/context/ComponentPickerContext.tsx @@ -8,7 +8,7 @@ import { export interface SelectedComponent { usageKey: string; - blockType?: string; + blockType: string; } export type ComponentSelectedEvent = (selectedComponent: SelectedComponent) => void; diff --git a/src/library-authoring/generic/manage-collections/messages.tsx b/src/library-authoring/generic/manage-collections/messages.ts similarity index 100% rename from src/library-authoring/generic/manage-collections/messages.tsx rename to src/library-authoring/generic/manage-collections/messages.ts From 073d1f4624262e5d71a355a4b59c4c766dcc4cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 11 Apr 2025 23:45:37 -0300 Subject: [PATCH 4/6] fix: nits and improving coverage --- .../LibraryAuthoringPage.test.tsx | 29 ++++++++++++-- .../__mocks__/collection-search.json | 39 ++++++++++++++++++- .../LibraryCollectionPage.test.tsx | 32 +++++++++++++++ .../components/ContainerCard.tsx | 6 +-- 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 89c4812264..e1d6f58a01 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -394,7 +394,7 @@ describe('', () => { await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); - it('should open component sidebar, showing manage tab on clicking add to collection menu item', async () => { + it('should open component sidebar, showing manage tab on clicking add to collection menu item (component)', async () => { const mockResult0 = { ...mockResult }.results[0].hits[0]; const displayName = 'Introduction to Testing'; expect(mockResult0.display_name).toStrictEqual(displayName); @@ -419,6 +419,29 @@ describe('', () => { await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); + it('should open component sidebar, showing manage tab on clicking add to collection menu item (unit)', async () => { + const displayName = 'Test Unit'; + await renderLibraryPage(); + + waitFor(() => expect(screen.getAllByTestId('container-card-menu-toggle').length).toBeGreaterThan(0)); + + // Open menu + fireEvent.click((await screen.findAllByTestId('container-card-menu-toggle'))[0]); + // Click add to collection + fireEvent.click(screen.getByRole('button', { name: 'Add to collection' })); + + const sidebar = screen.getByTestId('library-sidebar'); + + const { getByRole, queryByText } = within(sidebar); + + await waitFor(() => expect(queryByText(displayName)).toBeInTheDocument()); + expect(getByRole('tab', { selected: true })).toHaveTextContent('Organize'); + const closeButton = getByRole('button', { name: /close/i }); + fireEvent.click(closeButton); + + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); + }); + it('should open and close the collection sidebar', async () => { await renderLibraryPage(); @@ -734,7 +757,7 @@ describe('', () => { fireEvent.click(cancelButton); expect(unitModalHeading).not.toBeInTheDocument(); - // Open new unit modal again and create a collection + // Open new unit modal again and create a unit fireEvent.click(newUnitButton); const createButton = screen.getByRole('button', { name: /create/i }); const nameField = screen.getByRole('textbox', { name: /name your unit/i }); @@ -802,7 +825,7 @@ describe('', () => { fireEvent.click(newButton); expect(screen.getByText(/add content/i)).toBeInTheDocument(); - // Open New collection Modal + // Open New Unit Modal const sidebar = screen.getByTestId('library-sidebar'); const newUnitButton = within(sidebar).getAllByRole('button', { name: /unit/i })[0]; fireEvent.click(newUnitButton); diff --git a/src/library-authoring/__mocks__/collection-search.json b/src/library-authoring/__mocks__/collection-search.json index 81e8afcb58..58d4b3e199 100644 --- a/src/library-authoring/__mocks__/collection-search.json +++ b/src/library-authoring/__mocks__/collection-search.json @@ -218,8 +218,45 @@ "org": "OpenedX", "access_id": 16, "num_children": 1 + }, + { + "display_name": "Test Unit", + "block_id": "test-unit-9284e2", + "id": "lctAximTESTunittest-unit-9284e2-a9a4386e", + "type": "library_container", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": 1742221203.895054, + "modified": 1742221203.895054, + "usage_key": "lct:Axim:TEST:unit:test-unit-9284e2", + "block_type": "unit", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": 15, + "num_children": 0, + "_formatted": { + "display_name": "Test Unit", + "block_id": "test-unit-9284e2", + "id": "lctAximTESTunittest-unit-9284e2-a9a4386e", + "type": "library_container", + "breadcrumbs": [ + { + "display_name": "Test Library" + } + ], + "created": "1742221203.895054", + "modified": "1742221203.895054", + "usage_key": "lct:Axim:TEST:unit:test-unit-9284e2", + "block_type": "unit", + "context_key": "lib:Axim:TEST", + "org": "Axim", + "access_id": "15", + "num_children": "0" + } } - ], "query": "", "processingTimeMs": 1, diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index 79ab0d8063..5ac94efa89 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -15,6 +15,7 @@ import { mockContentLibrary, mockXBlockFields, mockGetCollectionMetadata, + mockGetContainerMetadata, } from '../data/api.mocks'; import { mockContentSearchConfig, mockGetBlockTypes } from '../../search-manager/data/api.mock'; import { mockBroadcastChannel, mockClipboardEmpty } from '../../generic/data/api.mock'; @@ -31,6 +32,7 @@ mockContentSearchConfig.applyMock(); mockGetBlockTypes.applyMock(); mockContentLibrary.applyMock(); mockXBlockFields.applyMock(); +mockGetContainerMetadata.applyMock(); mockBroadcastChannel(); const searchEndpoint = 'http://mock.meilisearch.local/multi-search'; @@ -375,4 +377,34 @@ describe('', () => { // Should close sidebar as component was removed await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); + + it('should remove unit from collection and hides sidebar', async () => { + const url = getLibraryCollectionItemsApiUrl( + mockContentLibrary.libraryId, + mockCollection.collectionId, + ); + axiosMock.onDelete(url).reply(204); + const displayName = 'Test Unit'; + await renderLibraryCollectionPage(); + + // Wait for the unit cards to load + waitFor(() => expect(screen.getAllByTestId('container-card-menu-toggle').length).toBeGreaterThan(0)); + + // open sidebar + fireEvent.click(await screen.findByText(displayName)); + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).toBeInTheDocument()); + + // Open menu + fireEvent.click((await screen.findAllByTestId('container-card-menu-toggle'))[0]); + + // Click remove to collection + fireEvent.click(screen.getByRole('button', { name: 'Remove from collection' })); + + await waitFor(() => { + expect(axiosMock.history.delete.length).toEqual(1); + }); + expect(mockShowToast).toHaveBeenCalledWith('Item successfully removed'); + // Should close sidebar as component was removed + await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); + }); }); diff --git a/src/library-authoring/components/ContainerCard.tsx b/src/library-authoring/components/ContainerCard.tsx index ecc098a419..d25c1c5601 100644 --- a/src/library-authoring/components/ContainerCard.tsx +++ b/src/library-authoring/components/ContainerCard.tsx @@ -32,7 +32,6 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => { const intl = useIntl(); const { contextKey, - blockId, usageKey: containerId, displayName, } = hit; @@ -74,14 +73,13 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => { src={MoreVert} iconAs={Icon} variant="primary" - alt={intl.formatMessage(messages.collectionCardMenuAlt)} + alt={intl.formatMessage(messages.containerCardMenuAlt)} data-testid="container-card-menu-toggle" /> From a1bc214590130f35f1b061b293c7e043e966287c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Sat, 12 Apr 2025 02:06:49 -0300 Subject: [PATCH 5/6] fix: navigate to new unit on creation --- src/library-authoring/create-unit/CreateUnitModal.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/library-authoring/create-unit/CreateUnitModal.tsx b/src/library-authoring/create-unit/CreateUnitModal.tsx index 1be6db4e09..46dc07d27f 100644 --- a/src/library-authoring/create-unit/CreateUnitModal.tsx +++ b/src/library-authoring/create-unit/CreateUnitModal.tsx @@ -6,6 +6,7 @@ import { } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Formik } from 'formik'; +import { useNavigate } from 'react-router'; import * as Yup from 'yup'; import FormikControl from '../../generic/FormikControl'; import { useLibraryContext } from '../common/context/LibraryContext'; @@ -16,6 +17,7 @@ import LoadingButton from '../../generic/loading-button'; const CreateUnitModal = () => { const intl = useIntl(); + const navigate = useNavigate(); const { collectionId, libraryId, @@ -35,8 +37,8 @@ const CreateUnitModal = () => { if (collectionId) { await updateItemsMutation.mutateAsync([container.containerKey]); } - // TODO: Navigate to the new unit - // navigate(`/library/${libraryId}/units/${container.containerKey}`); + // Navigate to the new unit + navigate(`/library/${libraryId}/unit/${container.containerKey}`); showToast(intl.formatMessage(messages.createUnitSuccess)); } catch (error) { showToast(intl.formatMessage(messages.createUnitError)); From bbdf3d4d3ef5ed1f5f5a62dda65e4de2769c3e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 15 Apr 2025 11:35:22 -0300 Subject: [PATCH 6/6] fix: fixes from merge --- .../add-content/AddContent.tsx | 2 +- src/library-authoring/containers/UnitInfo.tsx | 7 +--- src/library-authoring/data/apiHooks.test.tsx | 3 +- src/library-authoring/data/apiHooks.ts | 35 +++++++------------ .../units/LibraryUnitBlocks.tsx | 10 ++---- 5 files changed, 17 insertions(+), 40 deletions(-) diff --git a/src/library-authoring/add-content/AddContent.tsx b/src/library-authoring/add-content/AddContent.tsx index 38574c6117..d29a805aaf 100644 --- a/src/library-authoring/add-content/AddContent.tsx +++ b/src/library-authoring/add-content/AddContent.tsx @@ -208,7 +208,7 @@ const AddContent = () => { unitId, } = useLibraryContext(); const addComponentsToCollectionMutation = useAddItemsToCollection(libraryId, collectionId); - const addComponentsToContainerMutation = useAddComponentsToContainer(libraryId, unitId); + const addComponentsToContainerMutation = useAddComponentsToContainer(unitId); const createBlockMutation = useCreateLibraryBlock(); const pasteClipboardMutation = useLibraryPasteClipboard(); const { showToast } = useContext(ToastContext); diff --git a/src/library-authoring/containers/UnitInfo.tsx b/src/library-authoring/containers/UnitInfo.tsx index ab07baf9a0..a6385009a2 100644 --- a/src/library-authoring/containers/UnitInfo.tsx +++ b/src/library-authoring/containers/UnitInfo.tsx @@ -112,11 +112,6 @@ const UnitInfo = () => { ); }, [hiddenTabs, defaultTab.unit, unitId]); - // istanbul ignore if: this should never happen - if (!unitId) { - throw new Error('unitId is required'); - } - useEffect(() => { // Show Organize tab if JumpToAddCollections action is set in sidebarComponentInfo if (jumpToCollections) { @@ -124,7 +119,7 @@ const UnitInfo = () => { } }, [jumpToCollections, setSidebarTab]); - if (!container) { + if (!container || !unitId) { return null; } diff --git a/src/library-authoring/data/apiHooks.test.tsx b/src/library-authoring/data/apiHooks.test.tsx index 75262e9537..e4c6a7dbae 100644 --- a/src/library-authoring/data/apiHooks.test.tsx +++ b/src/library-authoring/data/apiHooks.test.tsx @@ -259,14 +259,13 @@ describe('library api hooks', () => { }); it('should add components to container', async () => { - const libraryId = 'lib:org:1'; const componentId = 'lb:org:lib:html:1'; const containerId = 'ltc:org:lib:unit:1'; const url = getLibraryContainerChildrenApiUrl(containerId); axiosMock.onPost(url).reply(200); - const { result } = renderHook(() => useAddComponentsToContainer(libraryId, containerId), { wrapper }); + const { result } = renderHook(() => useAddComponentsToContainer(containerId), { wrapper }); await result.current.mutateAsync([componentId]); expect(axiosMock.history.post[0].url).toEqual(url); diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index afcc308d8f..e20671a2b2 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -100,16 +100,14 @@ export const libraryAuthoringQueryKeys = { 'blockTypes', libraryId, ], - container: (libraryId?: string, containerId?: string) => [ + container: (containerId?: string) => [ ...libraryAuthoringQueryKeys.all, 'container', - libraryId, containerId, ], - containerChildren: (libraryId?: string, containerId?: string) => [ + containerChildren: (containerId?: string) => [ ...libraryAuthoringQueryKeys.all, 'container', - libraryId, containerId, 'children', ], @@ -131,14 +129,6 @@ export const xblockQueryKeys = { componentDownstreamLinks: (usageKey: string) => [...xblockQueryKeys.xblock(usageKey), 'downstreamLinks'], }; -export const containerQueryKeys = { - all: ['container', 'children'], - /** - * Base key for data specific to a container - */ - container: (containerId?: string) => [...containerQueryKeys.all, containerId], -}; - /** * Tell react-query to refresh its cache of any data related to the given * component (XBlock). @@ -276,7 +266,7 @@ export const useRevertLibraryChanges = () => { /** * Hook to fetch a content library's team members */ -export const useLibraryTeam = (libraryId: string | undefined) => ( +export const useLibraryTeam = (libraryId?: string) => ( useQuery({ queryKey: libraryAuthoringQueryKeys.libraryTeam(libraryId), queryFn: () => getLibraryTeam(libraryId!), @@ -287,7 +277,7 @@ export const useLibraryTeam = (libraryId: string | undefined) => ( /** * Hook to fetch the list of XBlock types that can be added to this library. */ -export const useBlockTypesMetadata = (libraryId: string | undefined) => ( +export const useBlockTypesMetadata = (libraryId?: string) => ( useQuery({ queryKey: libraryAuthoringQueryKeys.blockTypes(libraryId), queryFn: () => getBlockTypes(libraryId!), @@ -612,7 +602,7 @@ export const useCreateLibraryContainer = (libraryId: string) => { export const useContainer = (containerId?: string) => ( useQuery({ enabled: !!containerId, - queryKey: containerQueryKeys.container(containerId), + queryKey: libraryAuthoringQueryKeys.container(containerId!), queryFn: () => getContainerMetadata(containerId!), }) ); @@ -629,7 +619,7 @@ export const useUpdateContainer = (containerId: string) => { // NOTE: We invalidate the library query here because we need to update the library's // container list. queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); - queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.containerChildren(libraryId, containerId) }); + queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.container(containerId) }); }, }); }; @@ -666,19 +656,18 @@ export const useRestoreContainer = (containerId: string) => { /** * Get the metadata and children for a container in a library */ -export const useContainerChildren = (containerId: string) => { - const libraryId = getLibraryId(containerId); - return useQuery({ +export const useContainerChildren = (containerId?: string) => ( + useQuery({ enabled: !!containerId, - queryKey: libraryAuthoringQueryKeys.containerChildren(libraryId, containerId), + queryKey: libraryAuthoringQueryKeys.containerChildren(containerId), queryFn: () => getLibraryContainerChildren(containerId!), - }); + }) ); /** * Use this mutation to add components to a container */ -export const useAddComponentsToContainer = (libraryId?: string, containerId?: string) => { +export const useAddComponentsToContainer = (containerId?: string) => { const queryClient = useQueryClient(); return useMutation({ mutationFn: async (componentIds: string[]) => { @@ -688,7 +677,7 @@ export const useAddComponentsToContainer = (libraryId?: string, containerId?: st return undefined; }, onSettled: () => { - queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.containerChildren(libraryId, containerId) }); + queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.containerChildren(containerId!) }); }, }); }; diff --git a/src/library-authoring/units/LibraryUnitBlocks.tsx b/src/library-authoring/units/LibraryUnitBlocks.tsx index b6f363fe39..30252a36b0 100644 --- a/src/library-authoring/units/LibraryUnitBlocks.tsx +++ b/src/library-authoring/units/LibraryUnitBlocks.tsx @@ -18,7 +18,7 @@ import TagCount from '../../generic/tag-count'; import { useLibraryContext } from '../common/context/LibraryContext'; import ComponentMenu from '../components'; import { LibraryBlockMetadata } from '../data/api'; -import { containerQueryKeys, useContainerChildren } from '../data/apiHooks'; +import { libraryAuthoringQueryKeys, useContainerChildren } from '../data/apiHooks'; import { LibraryBlock } from '../LibraryBlock'; import { useLibraryRoutes } from '../routes'; import messages from './messages'; @@ -42,11 +42,6 @@ export const LibraryUnitBlocks = () => { openAddContentSidebar, } = useSidebarContext(); - // istanbul ignore if: this should never happen - if (!unitId) { - throw new Error('unitId is required'); - } - const queryClient = useQueryClient(); const { data: blocks, @@ -55,7 +50,6 @@ export const LibraryUnitBlocks = () => { error, } = useContainerChildren(unitId); - useEffect(() => setOrderedBlocks(blocks || []), [blocks]); if (isLoading) { @@ -75,7 +69,7 @@ export const LibraryUnitBlocks = () => { }; const onTagSidebarClose = () => { - queryClient.invalidateQueries(containerQueryKeys.children(unitId)); + queryClient.invalidateQueries(libraryAuthoringQueryKeys.containerChildren(unitId)); closeManageTagsDrawer(); };