From 6ae68bd1229235537c4e6d87589a481a56d6bd45 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 21 Oct 2024 14:04:45 -0700 Subject: [PATCH] feat: Menu option to delete a component + small fixes (#1408) * feat: menu option to delete a component * feat: close component sidebar if it's open when that component id deleted * feat: hide unsupported block types from the "Add Content" menu * fix: expand and internationalize the "component usage" text --- .../add-content/AddContentContainer.test.tsx | 14 ++-- .../add-content/AddContentContainer.tsx | 3 +- .../component-info/ComponentDetails.test.tsx | 2 +- .../component-info/ComponentDetails.tsx | 14 ++-- .../component-info/messages.ts | 5 ++ .../components/ComponentCard.tsx | 8 ++ .../components/ComponentDeleter.test.tsx | 68 +++++++++++++++ .../components/ComponentDeleter.tsx | 84 +++++++++++++++++++ src/library-authoring/components/messages.ts | 30 +++++++ src/library-authoring/data/api.mocks.ts | 14 ++++ src/library-authoring/data/api.test.ts | 11 +++ src/library-authoring/data/api.ts | 9 ++ src/library-authoring/data/apiHooks.ts | 17 ++++ 13 files changed, 261 insertions(+), 18 deletions(-) create mode 100644 src/library-authoring/components/ComponentDeleter.test.tsx create mode 100644 src/library-authoring/components/ComponentDeleter.tsx diff --git a/src/library-authoring/add-content/AddContentContainer.test.tsx b/src/library-authoring/add-content/AddContentContainer.test.tsx index 64f5784aa6..e8f53c3fd4 100644 --- a/src/library-authoring/add-content/AddContentContainer.test.tsx +++ b/src/library-authoring/add-content/AddContentContainer.test.tsx @@ -25,13 +25,13 @@ describe('', () => { initializeMocks(); mockClipboardEmpty.applyMock(); render(); - expect(screen.getByRole('button', { name: /collection/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /text/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /problem/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /open reponse/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /drag drop/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /video/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /advanced \/ other/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /collection/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /text/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /problem/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /open reponse/i })).not.toBeInTheDocument(); // Excluded from MVP + expect(screen.queryByRole('button', { name: /drag drop/i })).not.toBeInTheDocument(); // Excluded from MVP + expect(screen.queryByRole('button', { name: /video/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /advanced \/ other/i })).not.toBeInTheDocument(); // Excluded from MVP expect(screen.queryByRole('button', { name: /copy from clipboard/i })).not.toBeInTheDocument(); }); diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index 989c44d099..92a43b2b9e 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -199,7 +199,8 @@ const AddContentContainer = () => { {!collectionId && }
- {contentTypes.map((contentType) => ( + {/* Note: for MVP we are hiding the unuspported types, not just disabling them. */} + {contentTypes.filter(ct => !ct.disabled).map((contentType) => ( ', () => { render(mockLibraryBlockMetadata.usageKeyNeverPublished); expect(await screen.findByText('Component Usage')).toBeInTheDocument(); // TODO: replace with actual data when implement course list - expect(screen.queryByText('This will show the courses that use this component.')).toBeInTheDocument(); + expect(screen.queryByText(/This will show the courses that use this component./)).toBeInTheDocument(); }); it('should render the component history', async () => { diff --git a/src/library-authoring/component-info/ComponentDetails.tsx b/src/library-authoring/component-info/ComponentDetails.tsx index 0ecf4f0a64..52834d1a7f 100644 --- a/src/library-authoring/component-info/ComponentDetails.tsx +++ b/src/library-authoring/component-info/ComponentDetails.tsx @@ -1,4 +1,4 @@ -import { useIntl } from '@edx/frontend-platform/i18n'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; import { Stack } from '@openedx/paragon'; import AlertError from '../../generic/alert-error'; @@ -10,8 +10,6 @@ import { ComponentAdvancedInfo } from './ComponentAdvancedInfo'; import messages from './messages'; const ComponentDetails = () => { - const intl = useIntl(); - const { sidebarComponentUsageKey: usageKey } = useLibraryContext(); // istanbul ignore if: this should never happen @@ -38,18 +36,16 @@ const ComponentDetails = () => {

- {intl.formatMessage(messages.detailsTabUsageTitle)} +

- This will show the courses that use this component. +

- {intl.formatMessage(messages.detailsTabHistoryTitle)} +

- +
diff --git a/src/library-authoring/component-info/messages.ts b/src/library-authoring/component-info/messages.ts index f7d7d0dab6..5be6ba1346 100644 --- a/src/library-authoring/component-info/messages.ts +++ b/src/library-authoring/component-info/messages.ts @@ -106,6 +106,11 @@ const messages = defineMessages({ defaultMessage: 'Component Usage', description: 'Title for the Component Usage container in the details tab', }, + detailsTabUsagePlaceholder: { + id: 'course-authoring.library-authoring.component.details-tab.usage-placeholder', + defaultMessage: 'This will show the courses that use this component. Feature coming soon.', + description: 'Explanation/placeholder for the future "Component Usage" feature', + }, detailsTabHistoryTitle: { id: 'course-authoring.library-authoring.component.details-tab.history-title', defaultMessage: 'Component History', diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index 5dd56381a4..03b6b3aaa9 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -6,6 +6,7 @@ import { Dropdown, Icon, IconButton, + useToggle, } from '@openedx/paragon'; import { AddCircleOutline, MoreVert } from '@openedx/paragon/icons'; @@ -18,6 +19,7 @@ import { useRemoveComponentsFromCollection } from '../data/apiHooks'; import BaseComponentCard from './BaseComponentCard'; import { canEditComponent } from './ComponentEditorModal'; import messages from './messages'; +import ComponentDeleter from './ComponentDeleter'; type ComponentCardProps = { contentHit: ContentHit, @@ -37,6 +39,8 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { const { showToast } = useContext(ToastContext); const [clipboardBroadcastChannel] = useState(() => new BroadcastChannel(STUDIO_CLIPBOARD_CHANNEL)); const removeComponentsMutation = useRemoveComponentsFromCollection(libraryId, collectionId); + const [isConfirmingDelete, confirmDelete, cancelDelete] = useToggle(false); + const updateClipboardClick = () => { updateClipboard(usageKey) .then((clipboardData) => { @@ -76,6 +80,9 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { + + + {collectionId && ( @@ -85,6 +92,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { + ); }; diff --git a/src/library-authoring/components/ComponentDeleter.test.tsx b/src/library-authoring/components/ComponentDeleter.test.tsx new file mode 100644 index 0000000000..5db365efe1 --- /dev/null +++ b/src/library-authoring/components/ComponentDeleter.test.tsx @@ -0,0 +1,68 @@ +import { getLibraryId } from '../../generic/key-utils'; +import { + fireEvent, + render, + screen, + initializeMocks, + waitFor, +} from '../../testUtils'; +import { LibraryProvider } from '../common/context'; +import { mockContentLibrary, mockDeleteLibraryBlock, mockLibraryBlockMetadata } from '../data/api.mocks'; +import ComponentDeleter from './ComponentDeleter'; + +mockContentLibrary.applyMock(); // Not required, but avoids 404 errors in the logs when loads data +mockLibraryBlockMetadata.applyMock(); +const mockDelete = mockDeleteLibraryBlock.applyMock(); + +const usageKey = mockLibraryBlockMetadata.usageKeyPublished; + +const renderArgs = { + extraWrapper: ({ children }: { children: React.ReactNode }) => ( + {children} + ), +}; + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('is invisible when isConfirmingDelete is false', async () => { + const mockCancel = jest.fn(); + render(, renderArgs); + + const modal = screen.queryByRole('dialog', { name: 'Delete Component' }); + expect(modal).not.toBeInTheDocument(); + }); + + it('should shows a confirmation prompt the card with title and description', async () => { + const mockCancel = jest.fn(); + render(, renderArgs); + + const modal = screen.getByRole('dialog', { name: 'Delete Component' }); + expect(modal).toBeVisible(); + + // It should mention the component's name in the confirm dialog: + await screen.findByText('Introduction to Testing 2'); + + // Clicking cancel will cancel: + const cancelButton = screen.getByRole('button', { name: 'Cancel' }); + fireEvent.click(cancelButton); + expect(mockCancel).toHaveBeenCalled(); + }); + + it('deletes the block when confirmed', async () => { + const mockCancel = jest.fn(); + render(, renderArgs); + + const modal = screen.getByRole('dialog', { name: 'Delete Component' }); + expect(modal).toBeVisible(); + + const deleteButton = screen.getByRole('button', { name: 'Delete' }); + fireEvent.click(deleteButton); + await waitFor(() => { + expect(mockDelete).toHaveBeenCalled(); + }); + expect(mockCancel).toHaveBeenCalled(); // In order to close the modal, this also gets called. + }); +}); diff --git a/src/library-authoring/components/ComponentDeleter.tsx b/src/library-authoring/components/ComponentDeleter.tsx new file mode 100644 index 0000000000..53d52e8a4e --- /dev/null +++ b/src/library-authoring/components/ComponentDeleter.tsx @@ -0,0 +1,84 @@ +import React from 'react'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { + ActionRow, + AlertModal, + Button, +} from '@openedx/paragon'; +import { Warning } from '@openedx/paragon/icons'; + +import { useLibraryContext } from '../common/context'; +import { useDeleteLibraryBlock, useLibraryBlockMetadata } from '../data/apiHooks'; +import messages from './messages'; + +/** + * Helper component to load and display the name of the block. + * + * This needs to be a separate component so that we only query the metadata of + * the block when needed (when this is displayed), not on every card shown in + * the search results. + */ +const BlockName = (props: { usageKey: string }) => { + const { data: blockMetadata } = useLibraryBlockMetadata(props.usageKey); + + // eslint-disable-next-line react/jsx-no-useless-fragment + return <>{blockMetadata?.displayName} ?? ; +}; + +interface Props { + usageKey: string; + /** If true, show a confirmation modal that asks the user if they want to delete this component. */ + isConfirmingDelete: boolean; + cancelDelete: () => void; +} + +const ComponentDeleter = ({ usageKey, ...props }: Props) => { + const intl = useIntl(); + const { + sidebarComponentUsageKey, + closeLibrarySidebar, + } = useLibraryContext(); + + const deleteComponentMutation = useDeleteLibraryBlock(); + const doDelete = React.useCallback(() => { + deleteComponentMutation.mutateAsync({ usageKey }); + props.cancelDelete(); + // Close the sidebar if it's still open showing the deleted component: + if (usageKey === sidebarComponentUsageKey) { + closeLibrarySidebar(); + } + }, [usageKey, sidebarComponentUsageKey, closeLibrarySidebar]); + + if (!props.isConfirmingDelete) { + return null; + } + + return ( + + + + + )} + > +

+ + ), + }} + /> +

+ + ); +}; + +export default ComponentDeleter; diff --git a/src/library-authoring/components/messages.ts b/src/library-authoring/components/messages.ts index 047a74bd0c..c8ee584a2c 100644 --- a/src/library-authoring/components/messages.ts +++ b/src/library-authoring/components/messages.ts @@ -26,6 +26,11 @@ const messages = defineMessages({ defaultMessage: 'Copy to clipboard', description: 'Menu item for copy a component.', }, + menuDelete: { + id: 'course-authoring.library-authoring.component.menu.delete', + defaultMessage: 'Delete', + description: 'Menu item for deleting a component.', + }, menuAddToCollection: { id: 'course-authoring.library-authoring.component.menu.add', defaultMessage: 'Add to collection', @@ -56,6 +61,31 @@ const messages = defineMessages({ defaultMessage: 'Failed to copy component to clipboard', description: 'Message for failed to copy component to clipboard.', }, + deleteComponentWarningTitle: { + id: 'course-authoring.library-authoring.component.delete-confirmation-title', + defaultMessage: 'Delete Component', + description: 'Title text for the warning displayed before deleting a component', + }, + deleteComponentNamePlaceholder: { + id: 'course-authoring.library-authoring.component.delete-confirmation-placeholder', + defaultMessage: 'this component', + description: 'Text shown in place of the component\'s title while we\'re loading the title', + }, + deleteComponentConfirm: { + id: 'course-authoring.library-authoring.component.delete-confirmation-text', + defaultMessage: 'Delete {componentName} permanently? If this component has been used in a course, those copies won\'t be deleted, but they will no longer receive updates from the library.', + description: 'Confirmation text to display before deleting a component', + }, + deleteComponentCancelButton: { + id: 'course-authoring.library-authoring.component.cancel-delete-button', + defaultMessage: 'Cancel', + description: 'Button to cancel deletion of a component', + }, + deleteComponentButton: { + id: 'course-authoring.library-authoring.component.confirm-delete-button', + defaultMessage: 'Delete', + description: 'Button to confirm deletion of a component', + }, deleteCollection: { id: 'course-authoring.library-authoring.collection.delete-menu-text', defaultMessage: 'Delete', diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 7376dd977a..995ccb569c 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -48,6 +48,8 @@ export async function mockContentLibrary(libraryId: string): Promise ( jest.spyOn(api, 'createLibraryBlock').mockImplementation(mockCreateLibraryBlock) ); +/** + * Mock for `deleteLibraryBlock()` + */ +export async function mockDeleteLibraryBlock(): ReturnType { + // no-op +} +/** Apply this mock. Returns a spy object that can tell you if it's been called. */ +mockDeleteLibraryBlock.applyMock = () => ( + jest.spyOn(api, 'deleteLibraryBlock').mockImplementation(mockDeleteLibraryBlock) +); + /** * Mock for `getXBlockFields()` * diff --git a/src/library-authoring/data/api.test.ts b/src/library-authoring/data/api.test.ts index c3e7831869..9cace9b3ae 100644 --- a/src/library-authoring/data/api.test.ts +++ b/src/library-authoring/data/api.test.ts @@ -18,6 +18,17 @@ describe('library data API', () => { }); }); + describe('deleteLibraryBlock', () => { + it('should delete a library block', async () => { + const { axiosMock } = initializeMocks(); + const usageKey = 'lib:org:1'; + const url = api.getLibraryBlockMetadataUrl(usageKey); + axiosMock.onDelete(url).reply(200); + await api.deleteLibraryBlock({ usageKey }); + expect(axiosMock.history.delete[0].url).toEqual(url); + }); + }); + describe('commitLibraryChanges', () => { it('should commit library changes', async () => { const { axiosMock } = initializeMocks(); diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index d8f22c741d..6d77417ce5 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -183,6 +183,10 @@ export interface CreateBlockDataRequest { definitionId: string; } +export interface DeleteBlockDataRequest { + usageKey: string; +} + export interface CollectionMetadata { key: string; title: string; @@ -257,6 +261,11 @@ export async function createLibraryBlock({ return camelCaseObject(data); } +export async function deleteLibraryBlock({ usageKey }: DeleteBlockDataRequest): Promise { + const client = getAuthenticatedHttpClient(); + await client.delete(getLibraryBlockMetadataUrl(usageKey)); +} + /** * Update library metadata. */ diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index de43624855..14c1fcf681 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -15,6 +15,7 @@ import { type UpdateXBlockFieldsRequest, getContentLibrary, createLibraryBlock, + deleteLibraryBlock, getContentLibraryV2List, commitLibraryChanges, revertLibraryChanges, @@ -137,6 +138,22 @@ export const useCreateLibraryBlock = () => { }); }; +/** + * Use this mutation to delete a block in a library + */ +export const useDeleteLibraryBlock = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: deleteLibraryBlock, + onSettled: (_data, _error, variables) => { + const libraryId = getLibraryId(variables.usageKey); + queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); + invalidateComponentData(queryClient, libraryId, variables.usageKey); + }, + }); +}; + export const useUpdateLibraryMetadata = () => { const queryClient = useQueryClient(); return useMutation({