Skip to content

Commit

Permalink
feat: Menu option to delete a component + small fixes (#1408)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
bradenmacdonald authored Oct 21, 2024
1 parent d49fc85 commit 6ae68bd
Show file tree
Hide file tree
Showing 13 changed files with 261 additions and 18 deletions.
14 changes: 7 additions & 7 deletions src/library-authoring/add-content/AddContentContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ describe('<AddContentContainer />', () => {
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();
});

Expand Down
3 changes: 2 additions & 1 deletion src/library-authoring/add-content/AddContentContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ const AddContentContainer = () => {
<Stack direction="vertical">
{!collectionId && <AddContentButton contentType={collectionButtonData} onCreateContent={onCreateContent} />}
<hr className="w-100 bg-gray-500" />
{contentTypes.map((contentType) => (
{/* Note: for MVP we are hiding the unuspported types, not just disabling them. */}
{contentTypes.filter(ct => !ct.disabled).map((contentType) => (
<AddContentButton
key={`add-content-${contentType.blockType}`}
contentType={contentType}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('<ComponentDetails />', () => {
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 () => {
Expand Down
14 changes: 5 additions & 9 deletions src/library-authoring/component-info/ComponentDetails.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand All @@ -38,18 +36,16 @@ const ComponentDetails = () => {
<Stack gap={3}>
<div>
<h3 className="h5">
{intl.formatMessage(messages.detailsTabUsageTitle)}
<FormattedMessage {...messages.detailsTabUsageTitle} />
</h3>
<small>This will show the courses that use this component.</small>
<small><FormattedMessage {...messages.detailsTabUsagePlaceholder} /></small>
</div>
<hr className="w-100" />
<div>
<h3 className="h5">
{intl.formatMessage(messages.detailsTabHistoryTitle)}
<FormattedMessage {...messages.detailsTabHistoryTitle} />
</h3>
<HistoryWidget
{...componentMetadata}
/>
<HistoryWidget {...componentMetadata} />
</div>
<ComponentAdvancedInfo />
</Stack>
Expand Down
5 changes: 5 additions & 0 deletions src/library-authoring/component-info/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 8 additions & 0 deletions src/library-authoring/components/ComponentCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Dropdown,
Icon,
IconButton,
useToggle,
} from '@openedx/paragon';
import { AddCircleOutline, MoreVert } from '@openedx/paragon/icons';

Expand All @@ -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,
Expand All @@ -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) => {
Expand Down Expand Up @@ -76,6 +80,9 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
<Dropdown.Item onClick={updateClipboardClick}>
<FormattedMessage {...messages.menuCopyToClipboard} />
</Dropdown.Item>
<Dropdown.Item onClick={confirmDelete}>
<FormattedMessage {...messages.menuDelete} />
</Dropdown.Item>
{collectionId && (
<Dropdown.Item onClick={removeFromCollection}>
<FormattedMessage {...messages.menuRemoveFromCollection} />
Expand All @@ -85,6 +92,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => {
<FormattedMessage {...messages.menuAddToCollection} />
</Dropdown.Item>
</Dropdown.Menu>
<ComponentDeleter usageKey={usageKey} isConfirmingDelete={isConfirmingDelete} cancelDelete={cancelDelete} />
</Dropdown>
);
};
Expand Down
68 changes: 68 additions & 0 deletions src/library-authoring/components/ComponentDeleter.test.tsx
Original file line number Diff line number Diff line change
@@ -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 <LibraryProvider> loads data
mockLibraryBlockMetadata.applyMock();
const mockDelete = mockDeleteLibraryBlock.applyMock();

const usageKey = mockLibraryBlockMetadata.usageKeyPublished;

const renderArgs = {
extraWrapper: ({ children }: { children: React.ReactNode }) => (
<LibraryProvider libraryId={getLibraryId(usageKey)}>{children}</LibraryProvider>
),
};

describe('<ComponentDeleter />', () => {
beforeEach(() => {
initializeMocks();
});

it('is invisible when isConfirmingDelete is false', async () => {
const mockCancel = jest.fn();
render(<ComponentDeleter usageKey={usageKey} isConfirmingDelete={false} cancelDelete={mockCancel} />, 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(<ComponentDeleter usageKey={usageKey} isConfirmingDelete cancelDelete={mockCancel} />, 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(<ComponentDeleter usageKey={usageKey} isConfirmingDelete cancelDelete={mockCancel} />, 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.
});
});
84 changes: 84 additions & 0 deletions src/library-authoring/components/ComponentDeleter.tsx
Original file line number Diff line number Diff line change
@@ -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}</> ?? <FormattedMessage {...messages.deleteComponentNamePlaceholder} />;
};

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 (
<AlertModal
title={intl.formatMessage(messages.deleteComponentWarningTitle)}
isOpen
onClose={props.cancelDelete}
variant="warning"
icon={Warning}
footerNode={(
<ActionRow>
<Button variant="tertiary" onClick={props.cancelDelete}><FormattedMessage {...messages.deleteComponentCancelButton} /></Button>
<Button variant="danger" onClick={doDelete}><FormattedMessage {...messages.deleteComponentButton} /></Button>
</ActionRow>
)}
>
<p>
<FormattedMessage
{...messages.deleteComponentConfirm}
values={{
componentName: (
<strong><BlockName usageKey={usageKey} /></strong>
),
}}
/>
</p>
</AlertModal>
);
};

export default ComponentDeleter;
30 changes: 30 additions & 0 deletions src/library-authoring/components/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
14 changes: 14 additions & 0 deletions src/library-authoring/data/api.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export async function mockContentLibrary(libraryId: string): Promise<api.Content
throw createAxiosError({ code: 500, message: 'Internal Error.', path: api.getContentLibraryApiUrl(libraryId) });
case mockContentLibrary.libraryId:
return mockContentLibrary.libraryData;
case mockContentLibrary.libraryId2:
return { ...mockContentLibrary.libraryData, id: mockContentLibrary.libraryId2, slug: 'TEST2' };
case mockContentLibrary.libraryIdReadOnly:
return {
...mockContentLibrary.libraryData,
Expand Down Expand Up @@ -148,6 +150,7 @@ mockContentLibrary.libraryData = {
created: '2024-06-26T14:19:59Z',
updated: '2024-07-20T17:36:51Z',
} satisfies api.ContentLibrary;
mockContentLibrary.libraryId2 = 'lib:Axim:TEST2';
mockContentLibrary.libraryIdReadOnly = 'lib:Axim:readOnly';
mockContentLibrary.libraryIdThatNeverLoads = 'lib:Axim:infiniteLoading';
mockContentLibrary.library404 = 'lib:Axim:error404';
Expand Down Expand Up @@ -229,6 +232,17 @@ mockCreateLibraryBlock.applyMock = () => (
jest.spyOn(api, 'createLibraryBlock').mockImplementation(mockCreateLibraryBlock)
);

/**
* Mock for `deleteLibraryBlock()`
*/
export async function mockDeleteLibraryBlock(): ReturnType<typeof api.deleteLibraryBlock> {
// 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()`
*
Expand Down
11 changes: 11 additions & 0 deletions src/library-authoring/data/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 6ae68bd

Please sign in to comment.