diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 007c8df572..c32c68e483 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -30,7 +30,7 @@ jobs: runs-on: ubuntu-latest needs: tests steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download code coverage results uses: actions/download-artifact@v4 with: diff --git a/package-lock.json b/package-lock.json index e0083c84be..a194d2f2fa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6626,9 +6626,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001664", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001664.tgz", - "integrity": "sha512-AmE7k4dXiNKQipgn7a2xg558IRqPN3jMQY/rOsbxDhrd0tyChwbITBfiwtnqz8bi2M5mIWbxAYBvk7W7QBUS2g==", + "version": "1.0.30001667", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001667.tgz", + "integrity": "sha512-7LTwJjcRkzKFmtqGsibMeuXmvFDfZq/nzIjnmgCGzKKRVzjD72selLDK1oPF/Oxzmt4fNcPvTDvGqSDG4tCALw==", "funding": [ { "type": "opencollective", diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index 8abd78e1fb..75ffe35b79 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -55,10 +55,6 @@ describe('', () => { initializeMocks(); }); - afterEach(() => { - jest.clearAllMocks(); - }); - it('should render page and page title correctly', () => { renderDrawer(stagedTagsId); expect(screen.getByText('Manage tags')).toBeInTheDocument(); diff --git a/src/course-outline/CourseOutline.test.jsx b/src/course-outline/CourseOutline.test.jsx index 062711d41a..b7f8332eeb 100644 --- a/src/course-outline/CourseOutline.test.jsx +++ b/src/course-outline/CourseOutline.test.jsx @@ -226,7 +226,7 @@ describe('', () => { }); it('check video sharing option shows error on failure', async () => { - const { findByLabelText, queryByRole } = render(); + render(); axiosMock .onPost(getCourseBlockApiUrl(courseId), { @@ -235,7 +235,7 @@ describe('', () => { }, }) .reply(500); - const optionDropdown = await findByLabelText(statusBarMessages.videoSharingTitle.defaultMessage); + const optionDropdown = await screen.findByLabelText(statusBarMessages.videoSharingTitle.defaultMessage); await act( async () => fireEvent.change(optionDropdown, { target: { value: VIDEO_SHARING_OPTIONS.allOff } }), ); @@ -247,8 +247,10 @@ describe('', () => { }, })); - const alertElement = queryByRole('alert'); - expect(alertElement).toHaveTextContent( + const alertElements = screen.queryAllByRole('alert'); + expect(alertElements.find( + (el) => el.classList.contains('alert-content'), + )).toHaveTextContent( pageAlertMessages.alertFailedGeneric.defaultMessage, ); }); @@ -511,9 +513,10 @@ describe('', () => { notificationDismissUrl: '/some/url', }); - const { findByRole } = render(); - expect(await findByRole('alert')).toBeInTheDocument(); - const dismissBtn = await findByRole('button', { name: 'Dismiss' }); + render(); + const alert = await screen.findByText(pageAlertMessages.configurationErrorTitle.defaultMessage); + expect(alert).toBeInTheDocument(); + const dismissBtn = await screen.findByRole('button', { name: 'Dismiss' }); axiosMock .onDelete('/some/url') .reply(204); @@ -2160,10 +2163,10 @@ describe('', () => { }); it('check whether unit copy & paste option works correctly', async () => { - const { findAllByTestId, queryByTestId, findAllByRole } = render(); + render(); // get first section -> first subsection -> first unit element const [section] = courseOutlineIndexMock.courseStructure.childInfo.children; - const [sectionElement] = await findAllByTestId('section-card'); + const [sectionElement] = await screen.findAllByTestId('section-card'); const [subsection] = section.childInfo.children; axiosMock .onGet(getXBlockApiUrl(section.id)) @@ -2202,7 +2205,7 @@ describe('', () => { await act(async () => fireEvent.mouseOver(clipboardLabel)); // find clipboard content popover link - const popoverContent = queryByTestId('popover-content'); + const popoverContent = screen.queryByTestId('popover-content'); expect(popoverContent.tagName).toBe('A'); expect(popoverContent).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}${unit.studioUrl}`); @@ -2233,8 +2236,10 @@ describe('', () => { errorFiles: ['error.css'], }); + let alerts = await screen.findAllByRole('alert'); + // Exclude processing notification toast + alerts = alerts.filter((el) => !el.classList.contains('toast-container')); // 3 alerts should be present - const alerts = await findAllByRole('alert'); expect(alerts.length).toEqual(3); // check alerts for errorFiles diff --git a/src/course-unit/CourseUnit.test.jsx b/src/course-unit/CourseUnit.test.jsx index 91e8a2f51a..d6b00a385d 100644 --- a/src/course-unit/CourseUnit.test.jsx +++ b/src/course-unit/CourseUnit.test.jsx @@ -1,6 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; import { - act, render, waitFor, fireEvent, within, + act, render, waitFor, fireEvent, within, screen, } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { IntlProvider } from '@edx/frontend-platform/i18n'; @@ -525,17 +525,19 @@ describe('', () => { }); it('should display a warning alert for unpublished course unit version', async () => { - const { getByRole } = render(); + render(); await waitFor(() => { - const unpublishedAlert = getByRole('alert', { class: 'course-unit-unpublished-alert' }); + const unpublishedAlert = screen.getAllByRole('alert').find( + (el) => el.classList.contains('alert-content'), + ); expect(unpublishedAlert).toHaveTextContent(messages.alertUnpublishedVersion.defaultMessage); expect(unpublishedAlert).toHaveClass('alert-warning'); }); }); it('should not display an unpublished alert for a course unit with explicit staff lock and unpublished status', async () => { - const { queryByRole } = render(); + render(); axiosMock .onGet(getCourseUnitApiUrl(courseId)) @@ -547,8 +549,10 @@ describe('', () => { await executeThunk(fetchCourseUnitQuery(courseId), store.dispatch); await waitFor(() => { - const unpublishedAlert = queryByRole('alert', { class: 'course-unit-unpublished-alert' }); - expect(unpublishedAlert).toBeNull(); + const alert = screen.queryAllByRole('alert').find( + (el) => el.classList.contains('alert-content'), + ); + expect(alert).toBeUndefined(); }); }); diff --git a/src/editors/Editor.test.jsx b/src/editors/Editor.test.jsx deleted file mode 100644 index fa19e60689..0000000000 --- a/src/editors/Editor.test.jsx +++ /dev/null @@ -1,55 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import React from 'react'; -import { useDispatch } from 'react-redux'; -import { shallow } from '@edx/react-unit-test-utils'; -import Editor from './Editor'; -import supportedEditors from './supportedEditors'; -import * as hooks from './hooks'; -import { blockTypes } from './data/constants/app'; - -jest.mock('./hooks', () => ({ - initializeApp: jest.fn(), -})); - -jest.mock('./containers/TextEditor', () => 'TextEditor'); -jest.mock('./containers/VideoEditor', () => 'VideoEditor'); -jest.mock('./containers/ProblemEditor', () => 'ProblemEditor'); - -const initData = { - blockId: 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', - blockType: blockTypes.html, - learningContextId: 'course-v1:edX+DemoX+Demo_Course', - lmsEndpointUrl: 'evenfakerurl.com', - studioEndpointUrl: 'fakeurl.com', -}; -const props = { - initialize: jest.fn(), - onClose: jest.fn().mockName('props.onClose'), - courseId: 'course-v1:edX+DemoX+Demo_Course', - ...initData, -}; - -let el; -describe('Editor', () => { - describe('render', () => { - test('snapshot: renders correct editor given blockType (html -> TextEditor)', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - test('presents error message if no relevant editor found and ref ready', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - test.each(Object.values(blockTypes))('renders %p editor when ref is ready', (blockType) => { - el = shallow(); - expect(el.shallowWrapper.props.children.props.children.type).toBe(supportedEditors[blockType]); - }); - }); - describe('behavior', () => { - it('calls initializeApp hook with dispatch, and passed data', () => { - el = shallow(); - expect(hooks.initializeApp).toHaveBeenCalledWith({ - dispatch: useDispatch(), - data: initData, - }); - }); - }); -}); diff --git a/src/editors/Editor.tsx b/src/editors/Editor.tsx index cc42647f56..8e52448242 100644 --- a/src/editors/Editor.tsx +++ b/src/editors/Editor.tsx @@ -1,3 +1,5 @@ +// Note: there is no Editor.test.tsx. This component only works together with +// as its parent, so they are tested together in EditorPage.test.tsx import React from 'react'; import { useDispatch } from 'react-redux'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; @@ -7,6 +9,7 @@ import * as hooks from './hooks'; import supportedEditors from './supportedEditors'; import type { EditorComponent } from './EditorComponent'; +import { useEditorContext } from './EditorContext'; export interface Props extends EditorComponent { blockType: string; @@ -14,6 +17,7 @@ export interface Props extends EditorComponent { learningContextId: string | null; lmsEndpointUrl: string | null; studioEndpointUrl: string | null; + fullScreen?: boolean; } const Editor: React.FC = ({ @@ -36,23 +40,29 @@ const Editor: React.FC = ({ studioEndpointUrl, }, }); + const { fullScreen } = useEditorContext(); const EditorComponent = supportedEditors[blockType]; - return ( -
+ const innerEditor = (EditorComponent !== undefined) + ? + : ; + + if (fullScreen) { + return (
- {(EditorComponent !== undefined) - ? - : } +
+ {innerEditor} +
-
- ); + ); + } + return innerEditor; }; export default Editor; diff --git a/src/editors/EditorContext.tsx b/src/editors/EditorContext.tsx new file mode 100644 index 0000000000..e43b60a815 --- /dev/null +++ b/src/editors/EditorContext.tsx @@ -0,0 +1,39 @@ +import React from 'react'; + +/** + * Shared context that's used by all our editors. + * + * Note: we're in the process of moving things from redux into this. + */ +export interface EditorContext { + learningContextId: string; + /** + * When editing components in the libraries part of the Authoring MFE, we show + * the editors in a modal (fullScreen = false). This is the preferred approach + * so that authors can see context behind the modal. + * However, when making edits from the legacy course view, we display the + * editors in a fullscreen view. This approach is deprecated. + */ + fullScreen: boolean; +} + +const context = React.createContext(undefined); + +/** Hook to get the editor context (shared context) */ +export function useEditorContext() { + const ctx = React.useContext(context); + if (ctx === undefined) { + /* istanbul ignore next */ + throw new Error('This component needs to be wrapped in '); + } + return ctx; +} + +export const EditorContextProvider: React.FC<{ + children: React.ReactNode, + learningContextId: string; + fullScreen: boolean; +}> = ({ children, ...contextData }) => { + const ctx: EditorContext = React.useMemo(() => ({ ...contextData }), []); + return {children}; +}; diff --git a/src/editors/EditorPage.jsx b/src/editors/EditorPage.jsx deleted file mode 100644 index 60de6e1cc6..0000000000 --- a/src/editors/EditorPage.jsx +++ /dev/null @@ -1,58 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { Provider } from 'react-redux'; - -import store from './data/store'; -import Editor from './Editor'; -import ErrorBoundary from './sharedComponents/ErrorBoundary'; - -const EditorPage = ({ - courseId, - blockType, - blockId, - lmsEndpointUrl, - studioEndpointUrl, - onClose, - returnFunction, -}) => ( - - - - - -); -EditorPage.defaultProps = { - blockId: null, - courseId: null, - lmsEndpointUrl: null, - onClose: null, - returnFunction: null, - studioEndpointUrl: null, -}; - -EditorPage.propTypes = { - blockId: PropTypes.string, - blockType: PropTypes.string.isRequired, - courseId: PropTypes.string, - lmsEndpointUrl: PropTypes.string, - onClose: PropTypes.func, - returnFunction: PropTypes.func, - studioEndpointUrl: PropTypes.string, -}; - -export default EditorPage; diff --git a/src/editors/EditorPage.test.jsx b/src/editors/EditorPage.test.jsx deleted file mode 100644 index 24dfffe293..0000000000 --- a/src/editors/EditorPage.test.jsx +++ /dev/null @@ -1,32 +0,0 @@ -import React from 'react'; -import { shallow } from '@edx/react-unit-test-utils'; -import EditorPage from './EditorPage'; - -const props = { - courseId: 'course-v1:edX+DemoX+Demo_Course', - blockType: 'html', - blockId: 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', - lmsEndpointUrl: 'evenfakerurl.com', - studioEndpointUrl: 'fakeurl.com', - onClose: jest.fn().mockName('props.onClose'), -}; -jest.mock('react-redux', () => ({ - Provider: 'Provider', - connect: (mapStateToProps, mapDispatchToProps) => (component) => ({ - mapStateToProps, - mapDispatchToProps, - component, - }), -})); -jest.mock('./Editor', () => 'Editor'); - -describe('Editor Page', () => { - describe('snapshots', () => { - test('rendering correctly with expected Input', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - test('props besides blockType default to null', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - }); -}); diff --git a/src/editors/EditorPage.test.tsx b/src/editors/EditorPage.test.tsx new file mode 100644 index 0000000000..e4d0de4091 --- /dev/null +++ b/src/editors/EditorPage.test.tsx @@ -0,0 +1,98 @@ +import { snakeCaseObject } from '@edx/frontend-platform'; +import { + render, + screen, + initializeMocks, +} from '../testUtils'; +import editorCmsApi from './data/services/cms/api'; + +import EditorPage from './EditorPage'; + +// Mock this plugins component: +jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); +// Always mock out the "fetch course images" endpoint: +jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line + { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } +)); +// Mock out the 'get ancestors' API: +jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ + status: 200, + data: { + ancestors: [{ + id: 'block-v1:Org+TS100+24+type@vertical+block@parent', + display_name: 'You-Knit? The Test Unit', + category: 'vertical', + has_children: true, + }], + }, +})); + +const defaultPropsHtml = { + blockId: 'block-v1:Org+TS100+24+type@html+block@123456html', + blockType: 'html', + courseId: 'course-v1:Org+TS100+24', + lmsEndpointUrl: 'http://lms.test.none/', + studioEndpointUrl: 'http://cms.test.none/', + onClose: jest.fn(), + fullScreen: false, +}; +const fieldsHtml = { + displayName: 'Introduction to Testing', + data: '

This is a text component which uses HTML.

', + metadata: { displayName: 'Introduction to Testing' }, +}; + +describe('EditorPage', () => { + beforeEach(() => { + initializeMocks(); + }); + + test('it can display the Text (html) editor in a modal', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + const modalElement = screen.getByRole('dialog'); + expect(modalElement.classList).toContain('pgn__modal'); + expect(modalElement.classList).toContain('pgn__modal-xl'); + expect(modalElement.classList).not.toContain('pgn__modal-fullscreen'); + }); + + test('it can display the Text (html) editor as a full page (when coming from the legacy UI)', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + const modalElement = screen.getByRole('dialog'); + expect(modalElement.classList).toContain('pgn__modal-fullscreen'); + expect(modalElement.classList).not.toContain('pgn__modal'); + expect(modalElement.classList).not.toContain('pgn__modal-xl'); + }); + + test('it shows an error message if there is no corresponding editor', async () => { + // We can edit 'html', 'problem', and 'video' blocks. + // But if we try to edit some other type, say 'fake', we should get an error: + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( // eslint-disable-next-line + { status: 200, data: { display_name: 'Fake Un-editable Block', category: 'fake', metadata: {}, data: '' } } + )); + + const defaultPropsFake = { + ...defaultPropsHtml, + blockId: 'block-v1:Org+TS100+24+type@fake+block@123456fake', + blockType: 'fake', + }; + render(); + + expect(await screen.findByText('Error: Could Not find Editor')).toBeInTheDocument(); + }); +}); diff --git a/src/editors/EditorPage.tsx b/src/editors/EditorPage.tsx new file mode 100644 index 0000000000..a0dd365954 --- /dev/null +++ b/src/editors/EditorPage.tsx @@ -0,0 +1,58 @@ +import React from 'react'; +import { Provider } from 'react-redux'; + +import store from './data/store'; +import Editor from './Editor'; +import ErrorBoundary from './sharedComponents/ErrorBoundary'; +import { EditorComponent } from './EditorComponent'; +import { EditorContextProvider } from './EditorContext'; + +interface Props extends EditorComponent { + blockId?: string; + blockType: string; + courseId: string; + lmsEndpointUrl?: string; + studioEndpointUrl?: string; + fullScreen?: boolean; + children?: never; +} + +/** + * Wraps the editors with the redux state provider. + * TODO: refactor some of this to be React Context and React Query + */ +const EditorPage: React.FC = ({ + courseId, + blockType, + blockId = null, + lmsEndpointUrl = null, + studioEndpointUrl = null, + onClose = null, + returnFunction = null, + fullScreen = true, +}) => ( + + + + + + + +); + +export default EditorPage; diff --git a/src/editors/__snapshots__/Editor.test.jsx.snap b/src/editors/__snapshots__/Editor.test.jsx.snap deleted file mode 100644 index 5c9cb23a39..0000000000 --- a/src/editors/__snapshots__/Editor.test.jsx.snap +++ /dev/null @@ -1,36 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Editor render presents error message if no relevant editor found and ref ready 1`] = ` -
-
- -
-
-`; - -exports[`Editor render snapshot: renders correct editor given blockType (html -> TextEditor) 1`] = ` -
-
- -
-
-`; diff --git a/src/editors/__snapshots__/EditorPage.test.jsx.snap b/src/editors/__snapshots__/EditorPage.test.jsx.snap deleted file mode 100644 index 7e15005764..0000000000 --- a/src/editors/__snapshots__/EditorPage.test.jsx.snap +++ /dev/null @@ -1,59 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Editor Page snapshots props besides blockType default to null 1`] = ` - - - - - -`; - -exports[`Editor Page snapshots rendering correctly with expected Input 1`] = ` - - - - - -`; diff --git a/src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap b/src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap deleted file mode 100644 index 02c89e55d7..0000000000 --- a/src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap +++ /dev/null @@ -1,161 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`EditorContainer component render snapshot: initialized. enable save and pass to header 1`] = ` -
- - - - } - footerAction={null} - headerComponent={null} - isFullscreenScroll={true} - isOpen={false} - size="md" - title="Exit the editor?" - > - - - -
-

- -

- -
-
- -

- My test content -

-
- -
-`; - -exports[`EditorContainer component render snapshot: not initialized. disable save and pass to header 1`] = ` -
- - - - } - footerAction={null} - headerComponent={null} - isFullscreenScroll={true} - isOpen={false} - size="md" - title="Exit the editor?" - > - - - -
-

- -

- -
-
- - -
-`; diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap b/src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap deleted file mode 100644 index dbfca713db..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap +++ /dev/null @@ -1,114 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`EditorFooter render snapshot: default args (disableSave: false, saveFailed: false) 1`] = ` -
- - - - - - -
-`; - -exports[`EditorFooter render snapshot: save disabled. Show button spinner 1`] = ` -
- - - - - - -
-`; - -exports[`EditorFooter render snapshot: save failed. Show error message 1`] = ` -
- - - - - - - - - -
-`; diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/index.jsx b/src/editors/containers/EditorContainer/components/EditorFooter/index.jsx deleted file mode 100644 index 20c2f2d560..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/index.jsx +++ /dev/null @@ -1,64 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -import { - Spinner, - ActionRow, - Button, - ModalDialog, - Toast, -} from '@openedx/paragon'; -import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; - -import messages from './messages'; - -const EditorFooter = ({ - clearSaveFailed, - disableSave, - onCancel, - onSave, - saveFailed, - // injected - intl, -}) => ( -
- {saveFailed && ( - - - - )} - - - - - - -
-); - -EditorFooter.propTypes = { - clearSaveFailed: PropTypes.func.isRequired, - disableSave: PropTypes.bool.isRequired, - onCancel: PropTypes.func.isRequired, - onSave: PropTypes.func.isRequired, - saveFailed: PropTypes.bool.isRequired, - // injected - intl: intlShape.isRequired, -}; - -export const EditorFooterInternal = EditorFooter; // For testing only -export default injectIntl(EditorFooter); diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx b/src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx deleted file mode 100644 index aaa78980a2..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx +++ /dev/null @@ -1,33 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import React from 'react'; -import { shallow } from '@edx/react-unit-test-utils'; - -import { formatMessage } from '../../../../testUtils'; -import { EditorFooterInternal as EditorFooter } from '.'; - -jest.mock('../../hooks', () => ({ - nullMethod: jest.fn().mockName('hooks.nullMethod'), -})); - -describe('EditorFooter', () => { - const props = { - intl: { formatMessage }, - disableSave: false, - onCancel: jest.fn().mockName('args.onCancel'), - onSave: jest.fn().mockName('args.onSave'), - saveFailed: false, - }; - describe('render', () => { - test('snapshot: default args (disableSave: false, saveFailed: false)', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - - test('snapshot: save disabled. Show button spinner', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - - test('snapshot: save failed. Show error message', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - }); -}); diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/messages.js b/src/editors/containers/EditorContainer/components/EditorFooter/messages.js deleted file mode 100644 index ce7503ff14..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/messages.js +++ /dev/null @@ -1,32 +0,0 @@ -import { defineMessages } from '@edx/frontend-platform/i18n'; - -const messages = defineMessages({ - - contentSaveFailed: { - id: 'authoring.editorfooter.save.error', - defaultMessage: 'Error: Content save failed. Please check recent changes and try again later.', - description: 'Error message displayed when content fails to save.', - }, - cancelButtonAriaLabel: { - id: 'authoring.editorfooter.cancelButton.ariaLabel', - defaultMessage: 'Discard changes and return to learning context', - description: 'Screen reader label for cancel button', - }, - cancelButtonLabel: { - id: 'authoring.editorfooter.cancelButton.label', - defaultMessage: 'Cancel', - description: 'Label for cancel button', - }, - saveButtonAriaLabel: { - id: 'authoring.editorfooter.savebutton.ariaLabel', - defaultMessage: 'Save changes and return to learning context', - description: 'Screen reader label for save button', - }, - saveButtonLabel: { - id: 'authoring.editorfooter.savebutton.label', - defaultMessage: 'Save', - description: 'Label for Save button', - }, -}); - -export default messages; diff --git a/src/editors/containers/EditorContainer/hooks.test.jsx b/src/editors/containers/EditorContainer/hooks.test.jsx index e73534f5b3..815a4cc12a 100644 --- a/src/editors/containers/EditorContainer/hooks.test.jsx +++ b/src/editors/containers/EditorContainer/hooks.test.jsx @@ -118,6 +118,7 @@ describe('EditorContainer hooks', () => { destination: reactRedux.useSelector(selectors.app.returnUrl), analyticsEvent: analyticsEvt.editorCancelClick, analytics: reactRedux.useSelector(selectors.app.analytics), + returnFunction: null, }), ); }); diff --git a/src/editors/containers/EditorContainer/hooks.js b/src/editors/containers/EditorContainer/hooks.ts similarity index 81% rename from src/editors/containers/EditorContainer/hooks.js rename to src/editors/containers/EditorContainer/hooks.ts index 72b8a0e20f..935e3ad89d 100644 --- a/src/editors/containers/EditorContainer/hooks.js +++ b/src/editors/containers/EditorContainer/hooks.ts @@ -6,11 +6,6 @@ import { RequestKeys } from '../../data/constants/requests'; import { selectors } from '../../data/redux'; import { StrictDict } from '../../utils'; import * as appHooks from '../../hooks'; -// This 'module' self-import hack enables mocking during tests. -// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested -// should be re-thought and cleaned up to avoid this pattern. -// eslint-disable-next-line import/no-self-import -import * as module from './hooks'; export const { clearSaveError, @@ -47,7 +42,7 @@ export const handleSaveClicked = ({ }; export const cancelConfirmModalToggle = () => { - const [isCancelConfirmOpen, setIsOpen] = module.state.isCancelConfirmModalOpen(false); + const [isCancelConfirmOpen, setIsOpen] = state.isCancelConfirmModalOpen(false); return { isCancelConfirmOpen, openCancelConfirmModal: () => setIsOpen(true), @@ -55,7 +50,13 @@ export const cancelConfirmModalToggle = () => { }; }; -export const handleCancel = ({ onClose, returnFunction }) => { +export const handleCancel = ({ + onClose = null, + returnFunction = null, +}: { + onClose?: (() => void) | null; + returnFunction?: (() => (result: any) => void) | null; +}): ((result?: any) => void) => { if (onClose) { return onClose; } diff --git a/src/editors/containers/EditorContainer/index.jsx b/src/editors/containers/EditorContainer/index.jsx deleted file mode 100644 index 9cfd96888b..0000000000 --- a/src/editors/containers/EditorContainer/index.jsx +++ /dev/null @@ -1,104 +0,0 @@ -import React from 'react'; -import { useDispatch } from 'react-redux'; -import PropTypes from 'prop-types'; - -import { - Icon, ModalDialog, IconButton, Button, -} from '@openedx/paragon'; -import { Close } from '@openedx/paragon/icons'; -import { injectIntl, intlShape, FormattedMessage } from '@edx/frontend-platform/i18n'; - -import BaseModal from '../../sharedComponents/BaseModal'; -import EditorFooter from './components/EditorFooter'; -import TitleHeader from './components/TitleHeader'; -import * as hooks from './hooks'; -import messages from './messages'; -import './index.scss'; - -const EditorContainer = ({ - children, - getContent, - onClose, - validateEntry, - returnFunction, - // injected - intl, -}) => { - const dispatch = useDispatch(); - const isInitialized = hooks.isInitialized(); - const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle(); - const handleCancel = hooks.handleCancel({ onClose, returnFunction }); - return ( -
- { - handleCancel(); - if (returnFunction) { - closeCancelConfirmModal(); - } - }} - > - - - )} - isOpen={isCancelConfirmOpen} - close={closeCancelConfirmModal} - title={intl.formatMessage(messages.cancelConfirmTitle)} - > - - - -
-

- -

- -
-
- - {isInitialized && children} - - -
- ); -}; -EditorContainer.defaultProps = { - onClose: null, - returnFunction: null, - validateEntry: null, -}; -EditorContainer.propTypes = { - children: PropTypes.node.isRequired, - getContent: PropTypes.func.isRequired, - onClose: PropTypes.func, - returnFunction: PropTypes.func, - validateEntry: PropTypes.func, - // injected - intl: intlShape.isRequired, -}; - -export const EditorContainerInternal = EditorContainer; // For testing only -export default injectIntl(EditorContainer); diff --git a/src/editors/containers/EditorContainer/index.test.jsx b/src/editors/containers/EditorContainer/index.test.jsx deleted file mode 100644 index 5360fdb7de..0000000000 --- a/src/editors/containers/EditorContainer/index.test.jsx +++ /dev/null @@ -1,68 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import { shallow } from '@edx/react-unit-test-utils'; -import { useDispatch } from 'react-redux'; - -import { EditorContainerInternal as EditorContainer } from '.'; -import * as hooks from './hooks'; -import { formatMessage } from '../../testUtils'; - -const props = { - getContent: jest.fn().mockName('props.getContent'), - onClose: jest.fn().mockName('props.onClose'), - validateEntry: jest.fn().mockName('props.validateEntry'), - returnFunction: jest.fn().mockName('props.returnFunction'), - // inject - intl: { formatMessage }, -}; - -jest.mock('./hooks', () => ({ - clearSaveError: jest.fn().mockName('hooks.clearSaveError'), - isInitialized: jest.fn().mockReturnValue(true), - handleCancel: (args) => ({ handleCancel: args }), - handleSaveClicked: (args) => ({ handleSaveClicked: args }), - saveFailed: jest.fn().mockName('hooks.saveFailed'), - cancelConfirmModalToggle: jest.fn(() => ({ - isCancelConfirmOpen: false, - openCancelConfirmModal: jest.fn().mockName('openCancelConfirmModal'), - closeCancelConfirmModal: jest.fn().mockName('closeCancelConfirmModal'), - })), -})); - -let el; - -describe('EditorContainer component', () => { - describe('render', () => { - const testContent = (

My test content

); - test('snapshot: not initialized. disable save and pass to header', () => { - hooks.isInitialized.mockReturnValueOnce(false); - expect( - shallow({testContent}).snapshot, - ).toMatchSnapshot(); - }); - test('snapshot: initialized. enable save and pass to header', () => { - expect( - shallow({testContent}).snapshot, - ).toMatchSnapshot(); - }); - describe('behavior inspection', () => { - beforeEach(() => { - el = shallow({testContent}); - }); - test('save behavior is linked to footer onSave', () => { - const expected = hooks.handleSaveClicked({ - dispatch: useDispatch(), - getContent: props.getContent, - validateEntry: props.validateEntry, - returnFunction: props.returnFunction, - }); - expect(el.shallowWrapper.props.children[3] - .props.onSave).toEqual(expected); - }); - test('behavior is linked to clearSaveError', () => { - const expected = hooks.clearSaveError({ dispatch: useDispatch() }); - expect(el.shallowWrapper.props.children[3] - .props.clearSaveFailed).toEqual(expected); - }); - }); - }); -}); diff --git a/src/editors/containers/EditorContainer/index.test.tsx b/src/editors/containers/EditorContainer/index.test.tsx new file mode 100644 index 0000000000..2d63041341 --- /dev/null +++ b/src/editors/containers/EditorContainer/index.test.tsx @@ -0,0 +1,97 @@ +import { snakeCaseObject } from '@edx/frontend-platform'; +import { + render, + screen, + initializeMocks, + fireEvent, + waitFor, +} from '../../../testUtils'; +import editorCmsApi from '../../data/services/cms/api'; + +import EditorPage from '../../EditorPage'; + +// Mock this plugins component: +jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); +// Always mock out the "fetch course images" endpoint: +jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line + { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } +)); +// Mock out the 'get ancestors' API: +jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ + status: 200, + data: { + ancestors: [{ + id: 'block-v1:Org+TS100+24+type@vertical+block@parent', + display_name: 'You-Knit? The Test Unit', + category: 'vertical', + has_children: true, + }], + }, +})); + +const defaultPropsHtml = { + blockId: 'block-v1:Org+TS100+24+type@html+block@123456html', + blockType: 'html', + courseId: 'course-v1:Org+TS100+24', + lmsEndpointUrl: 'http://lms.test.none/', + studioEndpointUrl: 'http://cms.test.none/', + onClose: jest.fn(), + fullScreen: false, +}; +const fieldsHtml = { + displayName: 'Introduction to Testing', + data: '

This is a text component which uses HTML.

', + metadata: { displayName: 'Introduction to Testing' }, +}; + +describe('EditorContainer', () => { + beforeEach(() => { + initializeMocks(); + }); + + test('it displays a confirmation dialog when closing the editor modal', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + // Assert the "are you sure?" message isn't visible yet + const confirmMessage = /Are you sure you want to exit the editor/; + expect(screen.queryByText(confirmMessage)).not.toBeInTheDocument(); + + // Find and click the close button + const closeButton = await screen.findByRole('button', { name: 'Exit the editor' }); + fireEvent.click(closeButton); + // Now we should see the confirmation message: + expect(await screen.findByText(confirmMessage)).toBeInTheDocument(); + + expect(defaultPropsHtml.onClose).not.toHaveBeenCalled(); + // And can confirm the cancelation: + const confirmButton = await screen.findByRole('button', { name: 'OK' }); + fireEvent.click(confirmButton); + expect(defaultPropsHtml.onClose).toHaveBeenCalled(); + }); + + test('it disables the save button until the fields have been loaded', async () => { + // Mock that loading the block data has begun but not completed yet: + let resolver: (result: { data: any }) => void; + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(() => new Promise((r) => { resolver = r; })); + + render(); + + // Then the editor should open. The "Save" button should be disabled + const saveButton = await screen.findByRole('button', { name: /Save changes and return/ }); + expect(saveButton).toBeDisabled(); + + // Now complete the loading of the data: + await waitFor(() => expect(resolver).not.toBeUndefined()); + resolver!({ data: snakeCaseObject(fieldsHtml) }); + + // Now the save button should be active: + await waitFor(() => expect(saveButton).not.toBeDisabled()); + }); +}); diff --git a/src/editors/containers/EditorContainer/index.tsx b/src/editors/containers/EditorContainer/index.tsx new file mode 100644 index 0000000000..3414680b58 --- /dev/null +++ b/src/editors/containers/EditorContainer/index.tsx @@ -0,0 +1,158 @@ +import React from 'react'; +import { useDispatch } from 'react-redux'; + +import { + ActionRow, + Button, + Icon, + IconButton, + ModalDialog, + Spinner, + Toast, +} from '@openedx/paragon'; +import { Close } from '@openedx/paragon/icons'; +import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; + +import { EditorComponent } from '../../EditorComponent'; +import { useEditorContext } from '../../EditorContext'; +import BaseModal from '../../sharedComponents/BaseModal'; +import TitleHeader from './components/TitleHeader'; +import * as hooks from './hooks'; +import messages from './messages'; +import './index.scss'; + +interface WrapperProps { + children: React.ReactNode; +} + +export const EditorModalWrapper: React.FC void }> = ({ children, onClose }) => { + const { fullScreen } = useEditorContext(); + const intl = useIntl(); + if (fullScreen) { + return ( +
+ {children} +
+ ); + } + const title = intl.formatMessage(messages.modalTitle); + return ( + {children} + ); +}; + +export const EditorModalBody: React.FC = ({ children }) => { + const { fullScreen } = useEditorContext(); + return { children }; +}; + +export const FooterWrapper: React.FC = ({ children }) => { + const { fullScreen } = useEditorContext(); + if (fullScreen) { + return
{children}
; + } + // eslint-disable-next-line react/jsx-no-useless-fragment + return <>{ children }; +}; + +interface Props extends EditorComponent { + children: React.ReactNode; + getContent: Function; + validateEntry?: Function | null; +} + +const EditorContainer: React.FC = ({ + children, + getContent, + onClose = null, + validateEntry = null, + returnFunction = null, +}) => { + const intl = useIntl(); + const dispatch = useDispatch(); + const isInitialized = hooks.isInitialized(); + const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle(); + const handleCancel = hooks.handleCancel({ onClose, returnFunction }); + const disableSave = !isInitialized; + const saveFailed = hooks.saveFailed(); + const clearSaveFailed = hooks.clearSaveError({ dispatch }); + const onSave = hooks.handleSaveClicked({ + dispatch, + getContent, + validateEntry, + returnFunction, + }); + return ( + + {saveFailed && ( + + + + )} + { + handleCancel(); + if (returnFunction) { + closeCancelConfirmModal(); + } + }} + > + + + )} + isOpen={isCancelConfirmOpen} + close={closeCancelConfirmModal} + title={intl.formatMessage(messages.cancelConfirmTitle)} + > + + + +
+

+ +

+ +
+
+ + {isInitialized && children} + + + + + + + + + +
+ ); +}; + +export default EditorContainer; diff --git a/src/editors/containers/EditorContainer/messages.ts b/src/editors/containers/EditorContainer/messages.ts index a6f1754fb2..55b4259ca9 100644 --- a/src/editors/containers/EditorContainer/messages.ts +++ b/src/editors/containers/EditorContainer/messages.ts @@ -22,6 +22,36 @@ const messages = defineMessages({ defaultMessage: 'OK', description: 'Label for OK button', }, + modalTitle: { + id: 'authoring.editorContainer.accessibleTitle', + defaultMessage: 'Editor Dialog', + description: 'Text that labels the the editor modal dialog for non-visual users', + }, + contentSaveFailed: { + id: 'authoring.editorfooter.save.error', + defaultMessage: 'Error: Content save failed. Please check recent changes and try again later.', + description: 'Error message displayed when content fails to save.', + }, + cancelButtonAriaLabel: { + id: 'authoring.editorfooter.cancelButton.ariaLabel', + defaultMessage: 'Discard changes and return to learning context', + description: 'Screen reader label for cancel button', + }, + cancelButtonLabel: { + id: 'authoring.editorfooter.cancelButton.label', + defaultMessage: 'Cancel', + description: 'Label for cancel button', + }, + saveButtonAriaLabel: { + id: 'authoring.editorfooter.savebutton.ariaLabel', + defaultMessage: 'Save changes and return to learning context', + description: 'Screen reader label for save button', + }, + saveButtonLabel: { + id: 'authoring.editorfooter.savebutton.label', + defaultMessage: 'Save', + description: 'Label for Save button', + }, }); export default messages; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap index 24c6543af9..428feabce6 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EditorProblemView component renders raw editor 1`] = ` - @@ -66,11 +66,11 @@ exports[`EditorProblemView component renders raw editor 1`] = ` /> - + `; exports[`EditorProblemView component renders simple view 1`] = ` - @@ -139,5 +139,5 @@ exports[`EditorProblemView component renders simple view 1`] = ` /> - + `; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx deleted file mode 100644 index cd41940bb3..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx +++ /dev/null @@ -1,63 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { useDispatch, useSelector } from 'react-redux'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; -import { - ActionRow, - Button, - ModalDialog, -} from '@openedx/paragon'; -import messages from './messages'; -import * as hooks from '../hooks'; - -import { actions, selectors } from '../../../../../data/redux'; - -const SelectTypeFooter = ({ - onCancel, - selected, -}) => { - const intl = useIntl(); - const defaultSettings = useSelector(selectors.problem.defaultSettings); - const dispatch = useDispatch(); - const updateField = React.useCallback((data) => dispatch(actions.problem.updateField(data)), [dispatch]); - const setBlockTitle = React.useCallback((title) => dispatch(actions.app.setBlockTitle(title)), [dispatch]); - return ( -
- - - - - - - -
- ); -}; - -SelectTypeFooter.defaultProps = { - selected: null, -}; - -SelectTypeFooter.propTypes = { - onCancel: PropTypes.func.isRequired, - selected: PropTypes.string, -}; - -export default SelectTypeFooter; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx deleted file mode 100644 index d0795c36ba..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx +++ /dev/null @@ -1,45 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import { shallow } from '@edx/react-unit-test-utils'; - -import { Button } from '@openedx/paragon'; -import { formatMessage } from '../../../../../testUtils'; -import SelectTypeFooter from './SelectTypeFooter'; -import * as hooks from '../hooks'; - -jest.mock('../hooks', () => ({ - onSelect: jest.fn().mockName('onSelect'), -})); - -describe('SelectTypeFooter', () => { - const props = { - onCancel: jest.fn().mockName('onCancel'), - selected: null, - // redux - defaultSettings: {}, - updateField: jest.fn().mockName('UpdateField'), - // inject - intl: { formatMessage }, - }; - - test('snapshot', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - - describe('behavior', () => { - let el; - beforeEach(() => { - el = shallow(); - }); - test('close behavior is linked to modal onCancel', () => { - const expected = props.onCancel; - expect(el.instance.findByType(Button)[0].props.onClick) - .toEqual(expected); - }); - test('select behavior is linked to modal onSelect', () => { - const expected = hooks.onSelect(props.selected, props.updateField); - const button = el.instance.findByType(Button); - expect(button[button.length - 1].props.onClick) - .toEqual(expected); - }); - }); -}); diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap deleted file mode 100644 index 5d136ff50b..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap +++ /dev/null @@ -1,36 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`SelectTypeFooter snapshot 1`] = ` -
- - - - - - - -
-`; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap deleted file mode 100644 index fb228f3205..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap +++ /dev/null @@ -1,38 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`SelectTypeWrapper snapshot 1`] = ` -
- - - -
- -
-
-
- -

- test child -

-
- -
-`; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap new file mode 100644 index 0000000000..44c6994873 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap @@ -0,0 +1,61 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SelectTypeWrapper snapshot 1`] = ` + + + + +
+ +
+
+
+ +

+ test child +

+
+ + + + + + + + + +
+`; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx deleted file mode 100644 index e452967172..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx +++ /dev/null @@ -1,57 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; -import { Icon, ModalDialog, IconButton } from '@openedx/paragon'; -import { Close } from '@openedx/paragon/icons'; -import SelectTypeFooter from './SelectTypeFooter'; - -import * as hooks from '../../../../EditorContainer/hooks'; -import ecMessages from '../../../../EditorContainer/messages'; -import messages from './messages'; - -const SelectTypeWrapper = ({ - children, - onClose, - selected, -}) => { - const handleCancel = hooks.handleCancel({ onClose }); - const intl = useIntl(); - - return ( -
- - - -
- -
-
-
- - {children} - - -
- ); -}; - -SelectTypeWrapper.defaultProps = { - onClose: null, -}; -SelectTypeWrapper.propTypes = { - selected: PropTypes.string.isRequired, - children: PropTypes.node.isRequired, - onClose: PropTypes.func, -}; - -export default SelectTypeWrapper; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.tsx similarity index 100% rename from src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.jsx rename to src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.tsx diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx new file mode 100644 index 0000000000..5df0c0be09 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx @@ -0,0 +1,85 @@ +import React from 'react'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { useDispatch, useSelector } from 'react-redux'; +import { + ActionRow, + Button, + Icon, + ModalDialog, + IconButton, +} from '@openedx/paragon'; +import { Close } from '@openedx/paragon/icons'; + +import { EditorModalBody, EditorModalWrapper, FooterWrapper } from '../../../../EditorContainer'; +import { actions, selectors } from '../../../../../data/redux'; +import * as containerHooks from '../../../../EditorContainer/hooks'; +import * as hooks from '../hooks'; +import ecMessages from '../../../../EditorContainer/messages'; +import messages from './messages'; + +interface Props { + selected: string; + onClose: (() => void) | null; +} + +const SelectTypeWrapper: React.FC = ({ + children, + onClose = null, + selected, +}) => { + const handleCancel = containerHooks.handleCancel({ onClose }); + const intl = useIntl(); + const defaultSettings = useSelector(selectors.problem.defaultSettings); + const dispatch = useDispatch(); + const updateField = React.useCallback((data) => dispatch(actions.problem.updateField(data)), [dispatch]); + const setBlockTitle = React.useCallback((title) => dispatch(actions.app.setBlockTitle(title)), [dispatch]); + + return ( + + + + +
+ +
+
+
+ + {children} + + + + + + + + + + +
+ ); +}; + +export default SelectTypeWrapper; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx index 9be7578c5f..ce2fbd7037 100644 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx @@ -6,6 +6,7 @@ import { initializeMocks, } from '../../../../../testUtils'; import editorStore from '../../../../data/store'; +import { EditorContextProvider } from '../../../../EditorContext'; import * as hooks from './hooks'; import SelectTypeModal from '.'; @@ -18,7 +19,13 @@ describe('SelectTypeModal', () => { const mockSelect = jest.fn(); jest.spyOn(hooks, 'onSelect').mockImplementation(mockSelect); // This is a new-style test, unlike most of the old snapshot-based editor tests. - render(); + render( + + + + + , + ); // First we see the menu of problem types: expect(await screen.findByRole('button', { name: 'Numerical input' })).toBeInTheDocument(); diff --git a/src/editors/data/services/cms/api.ts b/src/editors/data/services/cms/api.ts index 673309bb00..d40c9d5f36 100644 --- a/src/editors/data/services/cms/api.ts +++ b/src/editors/data/services/cms/api.ts @@ -19,6 +19,21 @@ interface AssetResponse { assets: Record[]; // In the raw response here, these are NOT camel-cased yet. } +type FieldsResponse = { + display_name: string; // In the raw response here, these are NOT camel-cased yet. + data: any; + metadata: Record; +} & Record; // In courses (but not in libraries), there are many other fields returned here. + +interface AncestorsResponse { + ancestors: { + id: string; + display_name: string; // In the raw response here, these are NOT camel-cased yet. + category: string; + has_children: boolean; + }[]; +} + export const loadImage = (imageData) => ({ ...imageData, dateAdded: new Date(imageData.dateAdded.replace(' at', '')).getTime(), @@ -89,10 +104,11 @@ export const processLicense = (licenseType, licenseDetails) => { }; export const apiMethods = { - fetchBlockById: ({ blockId, studioEndpointUrl }) => get( + fetchBlockById: ({ blockId, studioEndpointUrl }): Promise<{ data: FieldsResponse }> => get( urls.block({ blockId, studioEndpointUrl }), ), - fetchByUnitId: ({ blockId, studioEndpointUrl }) => get( + /** A better name for this would be 'get ancestors of block' */ + fetchByUnitId: ({ blockId, studioEndpointUrl }): Promise<{ data: AncestorsResponse }> => get( urls.blockAncestor({ studioEndpointUrl, blockId }), fetchByUnitIdOptions, ), diff --git a/src/editors/hooks.js b/src/editors/hooks.ts similarity index 77% rename from src/editors/hooks.js rename to src/editors/hooks.ts index c0d1b70a87..d2a7403d87 100644 --- a/src/editors/hooks.js +++ b/src/editors/hooks.ts @@ -4,11 +4,6 @@ import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import analyticsEvt from './data/constants/analyticsEvt'; import { actions, thunkActions } from './data/redux'; -// This 'module' self-import hack enables mocking during tests. -// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested -// should be re-thought and cleaned up to avoid this pattern. -// eslint-disable-next-line import/no-self-import -import * as module from './hooks'; import { RequestKeys } from './data/constants/requests'; // eslint-disable-next-line react-hooks/rules-of-hooks @@ -17,7 +12,7 @@ export const initializeApp = ({ dispatch, data }) => useEffect( [data], ); -export const navigateTo = (destination) => { +export const navigateTo = (destination: string | URL) => { window.location.assign(destination); }; @@ -34,7 +29,7 @@ export const navigateCallback = ({ returnFunction()(response); return; } - module.navigateTo(destination); + navigateTo(destination); }; export const nullMethod = () => ({}); @@ -61,7 +56,7 @@ export const saveBlock = ({ if (attemptSave) { dispatch(thunkActions.app.saveBlock( content, - module.navigateCallback({ + navigateCallback({ destination, analyticsEvent: analyticsEvt.editorSaveClick, analytics, diff --git a/src/editors/messages.js b/src/editors/messages.ts similarity index 100% rename from src/editors/messages.js rename to src/editors/messages.ts diff --git a/src/generic/CodeEditor.tsx b/src/generic/CodeEditor.tsx new file mode 100644 index 0000000000..fececa4213 --- /dev/null +++ b/src/generic/CodeEditor.tsx @@ -0,0 +1,54 @@ +import React from 'react'; +import { basicSetup, EditorView } from 'codemirror'; +import { EditorState, Compartment } from '@codemirror/state'; +import { xml } from '@codemirror/lang-xml'; + +export type EditorAccessor = EditorView; + +interface Props { + readOnly?: boolean; + children?: string; + editorRef?: React.MutableRefObject; +} + +export const CodeEditor: React.FC = ({ + readOnly = false, + children = '', + editorRef, +}) => { + const divRef = React.useRef(null); + const language = React.useMemo(() => new Compartment(), []); + const tabSize = React.useMemo(() => new Compartment(), []); + + React.useEffect(() => { + if (!divRef.current) { return; } + const state = EditorState.create({ + doc: children, + extensions: [ + basicSetup, + language.of(xml()), + tabSize.of(EditorState.tabSize.of(2)), + EditorState.readOnly.of(readOnly), + ], + }); + + const view = new EditorView({ + state, + parent: divRef.current, + }); + if (editorRef) { + // eslint-disable-next-line no-param-reassign + editorRef.current = view; + } + // eslint-disable-next-line consistent-return + return () => { + if (editorRef) { + // eslint-disable-next-line no-param-reassign + editorRef.current = undefined; + } + view.destroy(); // Clean up + }; + }, [divRef.current, readOnly, editorRef]); + + return
; +}; diff --git a/src/generic/delete-modal/DeleteModal.jsx b/src/generic/delete-modal/DeleteModal.jsx index 97768f6808..82dfd0b139 100644 --- a/src/generic/delete-modal/DeleteModal.jsx +++ b/src/generic/delete-modal/DeleteModal.jsx @@ -3,6 +3,7 @@ import { ActionRow, Button, AlertModal, + StatefulButton, } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -15,6 +16,8 @@ const DeleteModal = ({ onDeleteSubmit, title, description, + variant, + btnState, }) => { const intl = useIntl(); @@ -26,20 +29,32 @@ const DeleteModal = ({ title={modalTitle} isOpen={isOpen} onClose={close} + variant={variant} footerNode={( - - + labels={{ + default: intl.formatMessage(messages.deleteButton), + pending: intl.formatMessage(messages.pendingDeleteButton), + }} + /> )} > @@ -52,6 +67,8 @@ DeleteModal.defaultProps = { category: '', title: '', description: '', + variant: 'default', + btnState: 'default', }; DeleteModal.propTypes = { @@ -61,6 +78,8 @@ DeleteModal.propTypes = { onDeleteSubmit: PropTypes.func.isRequired, title: PropTypes.string, description: PropTypes.string, + variant: PropTypes.string, + btnState: PropTypes.string, }; export default DeleteModal; diff --git a/src/generic/delete-modal/messages.js b/src/generic/delete-modal/messages.js index 748120551e..c586a05991 100644 --- a/src/generic/delete-modal/messages.js +++ b/src/generic/delete-modal/messages.js @@ -13,6 +13,10 @@ const messages = defineMessages({ id: 'course-authoring.course-outline.delete-modal.button.delete', defaultMessage: 'Delete', }, + pendingDeleteButton: { + id: 'course-authoring.course-outline.delete-modal.button.pending-delete', + defaultMessage: 'Deleting', + }, cancelButton: { id: 'course-authoring.course-outline.delete-modal.button.cancel', defaultMessage: 'Cancel', diff --git a/src/generic/processing-notification/ProccessingNotification.scss b/src/generic/processing-notification/ProccessingNotification.scss index 257cbd2afc..455c16a407 100644 --- a/src/generic/processing-notification/ProccessingNotification.scss +++ b/src/generic/processing-notification/ProccessingNotification.scss @@ -1,25 +1,15 @@ -.processing-notification { - display: flex; - position: fixed; - bottom: -13rem; - transition: bottom 1s; - right: 1.25rem; - padding: .625rem 1.25rem; - z-index: $zindex-popover; - - &.is-show { - bottom: .625rem; - } +.processing-notification-icon { + animation: rotate 1s linear infinite; +} - .processing-notification-icon { - margin-right: .625rem; - animation: rotate 1s linear infinite; +.processing-notification-hide-close-button { + .btn-icon { + display: none; } +} - .processing-notification-title { - font-size: 1rem; - line-height: 1.5rem; - color: $white; - margin-bottom: 0; - } +.toast-container { + right: 1.25rem; + left: unset; + z-index: $zindex-popover; } diff --git a/src/generic/processing-notification/ProcessingNotification.test.jsx b/src/generic/processing-notification/ProcessingNotification.test.jsx index 16b86401ea..97f57429bf 100644 --- a/src/generic/processing-notification/ProcessingNotification.test.jsx +++ b/src/generic/processing-notification/ProcessingNotification.test.jsx @@ -1,17 +1,37 @@ -import React from 'react'; -import { render } from '@testing-library/react'; import { capitalize } from 'lodash'; +import userEvent from '@testing-library/user-event'; +import { initializeMocks, render, screen } from '../../testUtils'; import { NOTIFICATION_MESSAGES } from '../../constants'; import ProcessingNotification from '.'; +const mockUndo = jest.fn(); + const props = { title: NOTIFICATION_MESSAGES.saving, isShow: true, + action: { + label: 'Undo', + onClick: mockUndo, + }, }; describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + it('renders successfully', () => { - const { getByText } = render(); - expect(getByText(capitalize(props.title))).toBeInTheDocument(); + render( {}} />); + expect(screen.getByText(capitalize(props.title))).toBeInTheDocument(); + expect(screen.getByText('Undo')).toBeInTheDocument(); + expect(screen.getByRole('alert').querySelector('.processing-notification-hide-close-button')).not.toBeInTheDocument(); + userEvent.click(screen.getByText('Undo')); + expect(mockUndo).toBeCalled(); + }); + + it('add hide-close-button class if no close action is passed', () => { + render(); + expect(screen.getByText(capitalize(props.title))).toBeInTheDocument(); + expect(screen.getByRole('alert').querySelector('.processing-notification-hide-close-button')).toBeInTheDocument(); }); }); diff --git a/src/generic/processing-notification/index.jsx b/src/generic/processing-notification/index.jsx index 75c718c830..b31150a957 100644 --- a/src/generic/processing-notification/index.jsx +++ b/src/generic/processing-notification/index.jsx @@ -1,28 +1,40 @@ -import React from 'react'; import PropTypes from 'prop-types'; -import classNames from 'classnames'; -import { Badge, Icon } from '@openedx/paragon'; +import { + Icon, Toast, +} from '@openedx/paragon'; import { Settings as IconSettings } from '@openedx/paragon/icons'; import { capitalize } from 'lodash'; +import classNames from 'classnames'; -const ProcessingNotification = ({ isShow, title }) => ( - ( + {})} > - -

- {capitalize(title)} -

-
+ + + {capitalize(title)} + + ); +ProcessingNotification.defaultProps = { + close: null, +}; + ProcessingNotification.propTypes = { isShow: PropTypes.bool.isRequired, title: PropTypes.string.isRequired, + action: PropTypes.shape({ + label: PropTypes.string.isRequired, + onClick: PropTypes.func, + }), + close: PropTypes.func, }; export default ProcessingNotification; diff --git a/src/generic/toast-context/index.tsx b/src/generic/toast-context/index.tsx index 40145068fa..ea47dce8d2 100644 --- a/src/generic/toast-context/index.tsx +++ b/src/generic/toast-context/index.tsx @@ -2,9 +2,15 @@ import React from 'react'; import ProcessingNotification from '../processing-notification'; +export interface ToastActionData { + label: string; + onClick: () => void; +} + export interface ToastContextData { toastMessage: string | null; - showToast: (message: string) => void; + toastAction?: ToastActionData; + showToast: (message: string, action?: ToastActionData) => void; closeToast: () => void; } @@ -18,6 +24,7 @@ export interface ToastProviderProps { */ export const ToastContext = React.createContext({ toastMessage: null, + toastAction: undefined, showToast: () => {}, closeToast: () => {}, }); @@ -30,32 +37,41 @@ export const ToastProvider = (props: ToastProviderProps) => { // see: https://github.com/open-craft/frontend-app-course-authoring/pull/38#discussion_r1638990647 const [toastMessage, setToastMessage] = React.useState(null); + const [toastAction, setToastAction] = React.useState(undefined); + + const resetState = React.useCallback(() => { + setToastMessage(null); + setToastAction(undefined); + }, []); React.useEffect(() => () => { // Cleanup function to avoid updating state on unmounted component - setToastMessage(null); + resetState(); }, []); - const showToast = React.useCallback((message) => { + const showToast = React.useCallback((message, action?: ToastActionData) => { setToastMessage(message); - // Close the toast after 5 seconds - setTimeout(() => { - setToastMessage(null); - }, 5000); - }, [setToastMessage]); - const closeToast = React.useCallback(() => setToastMessage(null), [setToastMessage]); + setToastAction(action); + }, [setToastMessage, setToastAction]); + const closeToast = React.useCallback(() => resetState(), [setToastMessage, setToastAction]); const context = React.useMemo(() => ({ toastMessage, + toastAction, showToast, closeToast, - }), [toastMessage, showToast, closeToast]); + }), [toastMessage, toastAction, showToast, closeToast]); return ( {props.children} { toastMessage && ( - + )} ); diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 0fa83a8e73..0e88ea61cb 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -14,7 +14,6 @@ import mockEmptyResult from '../search-modal/__mocks__/empty-search-result.json' import { mockContentLibrary, mockGetCollectionMetadata, - mockLibraryBlockTypes, mockXBlockFields, } from './data/api.mocks'; import { mockContentSearchConfig } from '../search-manager/data/api.mock'; @@ -25,7 +24,6 @@ import { getLibraryCollectionsApiUrl } from './data/api'; mockGetCollectionMetadata.applyMock(); mockContentSearchConfig.applyMock(); mockContentLibrary.applyMock(); -mockLibraryBlockTypes.applyMock(); mockXBlockFields.applyMock(); mockBroadcastChannel(); @@ -82,6 +80,7 @@ describe('', () => { initializeMocks(); // The Meilisearch client-side API uses fetch, not Axios. + fetchMock.mockReset(); fetchMock.post(searchEndpoint, (_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); const query = requestData?.queries[0]?.q ?? ''; @@ -96,11 +95,6 @@ describe('', () => { }); }); - afterEach(() => { - jest.clearAllMocks(); - fetchMock.mockReset(); - }); - const renderLibraryPage = async () => { render(, { path, params: { libraryId: mockContentLibrary.libraryId } }); diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 56718cc5ef..78d60674ae 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -1,70 +1,26 @@ -import React from 'react'; import { Route, Routes, - useNavigate, useParams, } from 'react-router-dom'; -import { PageWrap } from '@edx/frontend-platform/react'; -import { useQueryClient } from '@tanstack/react-query'; -import EditorContainer from '../editors/EditorContainer'; import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context'; import { CreateCollectionModal } from './create-collection'; -import { invalidateComponentData } from './data/apiHooks'; import LibraryCollectionPage from './collections/LibraryCollectionPage'; +import { ComponentEditorModal } from './components/ComponentEditorModal'; const LibraryLayout = () => { const { libraryId } = useParams(); - const queryClient = useQueryClient(); if (libraryId === undefined) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. throw new Error('Error: route is missing libraryId.'); } - const navigate = useNavigate(); - const goBack = React.useCallback((prevPath?: string) => { - if (prevPath) { - // Redirects back to the previous route like collection page or library page - navigate(prevPath); - } else { - // Go back to the library - navigate(`/library/${libraryId}`); - } - }, []); - - const returnFunction = React.useCallback((prevPath?: string) => { - // When changes are cancelled, either onClose (goBack) or this returnFunction will be called. - // When changes are saved, this returnFunction is called. - goBack(prevPath); - return (args) => { - if (args === undefined) { - return; // Do nothing - the user cancelled the changes - } - const { id: usageKey } = args; - // invalidate any queries that involve this XBlock: - invalidateComponentData(queryClient, libraryId, usageKey); - }; - }, [goBack]); - return ( - {/* - TODO: we should be opening this editor as a modal, not making it a separate page/URL. - That will be a much nicer UX because users can just close the modal and be on the same page they were already - on, instead of always getting sent back to the library home. - */} - - - - )} - /> } @@ -75,6 +31,7 @@ const LibraryLayout = () => { /> + ); }; diff --git a/src/library-authoring/LibraryRecentlyModified.tsx b/src/library-authoring/LibraryRecentlyModified.tsx index 4d66cb8801..5577d45877 100644 --- a/src/library-authoring/LibraryRecentlyModified.tsx +++ b/src/library-authoring/LibraryRecentlyModified.tsx @@ -1,4 +1,3 @@ -import React, { useMemo } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { orderBy } from 'lodash'; @@ -7,7 +6,6 @@ import { type CollectionHit, type ContentHit, SearchSortOption } from '../search import LibrarySection, { LIBRARY_SECTION_PREVIEW_LIMIT } from './components/LibrarySection'; import messages from './messages'; import ComponentCard from './components/ComponentCard'; -import { useLibraryBlockTypes } from './data/apiHooks'; import CollectionCard from './components/CollectionCard'; import { useLibraryContext } from './common/context'; @@ -19,7 +17,6 @@ const RecentlyModified: React.FC> = () => { totalHits, totalCollectionHits, } = useSearchContext(); - const { libraryId } = useLibraryContext(); const componentCount = totalHits + totalCollectionHits; // Since we only display a fixed number of items in preview, @@ -32,17 +29,6 @@ const RecentlyModified: React.FC> = () => { ...collectionList, ], ['modified'], ['desc']).slice(0, LIBRARY_SECTION_PREVIEW_LIMIT); - const { data: blockTypesData } = useLibraryBlockTypes(libraryId); - const blockTypes = useMemo(() => { - const result = {}; - if (blockTypesData) { - blockTypesData.forEach(blockType => { - result[blockType.blockType] = blockType; - }); - } - return result; - }, [blockTypesData]); - return componentCount > 0 ? ( > = () => { ) ))} diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index fe88ba0cdc..d37fd75f05 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -16,14 +16,14 @@ import { ContentPaste, } from '@openedx/paragon/icons'; import { v4 as uuid4 } from 'uuid'; -import { useLocation, useNavigate, useParams } from 'react-router-dom'; +import { useParams } from 'react-router-dom'; import { ToastContext } from '../../generic/toast-context'; import { useCopyToClipboard } from '../../generic/clipboard'; import { getCanEdit } from '../../course-unit/data/selectors'; import { useCreateLibraryBlock, useLibraryPasteClipboard, useUpdateCollectionComponents } from '../data/apiHooks'; -import { getEditUrl } from '../components/utils'; import { useLibraryContext } from '../common/context'; +import { canEditComponent } from '../components/ComponentEditorModal'; import messages from './messages'; @@ -61,9 +61,6 @@ const AddContentButton = ({ contentType, onCreateContent } : AddContentButtonPro const AddContentContainer = () => { const intl = useIntl(); - const navigate = useNavigate(); - const location = useLocation(); - const currentPath = location.pathname; const { libraryId, collectionId } = useParams(); const createBlockMutation = useCreateLibraryBlock(); const updateComponentsMutation = useUpdateCollectionComponents(libraryId, collectionId); @@ -73,6 +70,7 @@ const AddContentContainer = () => { const { showPasteXBlock } = useCopyToClipboard(canEdit); const { openCreateCollectionModal, + openComponentEditor, } = useLibraryContext(); const collectionButtonData = { @@ -151,14 +149,12 @@ const AddContentContainer = () => { blockType, definitionId: `${uuid4()}`, }).then((data) => { - const editUrl = getEditUrl(data.id); + const hasEditor = canEditComponent(data.id); updateComponentsMutation.mutateAsync([data.id]).catch(() => { showToast(intl.formatMessage(messages.errorAssociateComponentMessage)); }); - if (editUrl) { - // Pass currentPath in state so that we can come back to - // current page on save or cancel - navigate(editUrl, { state: { from: currentPath } }); + if (hasEditor) { + openComponentEditor(data.id); } else { // We can't start editing this right away so just show a toast message: showToast(intl.formatMessage(messages.successCreateMessage)); diff --git a/src/library-authoring/add-content/AddContentWorkflow.test.tsx b/src/library-authoring/add-content/AddContentWorkflow.test.tsx index d16f8a644c..bd464f39b3 100644 --- a/src/library-authoring/add-content/AddContentWorkflow.test.tsx +++ b/src/library-authoring/add-content/AddContentWorkflow.test.tsx @@ -15,7 +15,6 @@ import * as textEditorHooks from '../../editors/containers/TextEditor/hooks'; import { mockContentLibrary, mockCreateLibraryBlock, - mockLibraryBlockTypes, mockXBlockFields, } from '../data/api.mocks'; import { mockBroadcastChannel, mockClipboardEmpty } from '../../generic/data/api.mock'; @@ -23,7 +22,6 @@ import { mockContentSearchConfig, mockSearchResult } from '../../search-manager/ import LibraryLayout from '../LibraryLayout'; mockContentSearchConfig.applyMock(); -mockLibraryBlockTypes.applyMock(); mockClipboardEmpty.applyMock(); mockBroadcastChannel(); mockContentLibrary.applyMock(); diff --git a/src/library-authoring/collections/CollectionDetails.test.tsx b/src/library-authoring/collections/CollectionDetails.test.tsx index ba48eb6e68..f0a0991680 100644 --- a/src/library-authoring/collections/CollectionDetails.test.tsx +++ b/src/library-authoring/collections/CollectionDetails.test.tsx @@ -44,7 +44,6 @@ describe('', () => { }); afterEach(() => { - jest.clearAllMocks(); fetchMock.mockReset(); }); @@ -133,6 +132,7 @@ describe('', () => { { blockType: 'Total', count: 3 }, { blockType: 'Text', count: 2 }, { blockType: 'Problem', count: 1 }, + { blockType: 'Video', count: 0 }, ].forEach(({ blockType, count }) => { const blockCount = screen.getByText(blockType).closest('div') as HTMLDivElement; expect(within(blockCount).getByText(count.toString())).toBeInTheDocument(); @@ -158,10 +158,10 @@ describe('', () => { [ { blockType: 'Total', count: 36 }, - { blockType: 'Video', count: 8 }, - { blockType: 'Problem', count: 7 }, - { blockType: 'Text', count: 6 }, - { blockType: 'Other', count: 15 }, + { blockType: 'Problem', count: 2 }, + { blockType: 'Text', count: 3 }, + { blockType: 'Video', count: 1 }, + { blockType: 'Other', count: 30 }, ].forEach(({ blockType, count }) => { const blockCount = screen.getByText(blockType).closest('div') as HTMLDivElement; expect(within(blockCount).getByText(count.toString())).toBeInTheDocument(); diff --git a/src/library-authoring/collections/CollectionDetails.tsx b/src/library-authoring/collections/CollectionDetails.tsx index fb0cb3b2fd..7a63447ddb 100644 --- a/src/library-authoring/collections/CollectionDetails.tsx +++ b/src/library-authoring/collections/CollectionDetails.tsx @@ -48,15 +48,9 @@ const CollectionStatsWidget = () => { return null; } - const blockTypesArray = Object.entries(blockTypes) - .map(([blockType, count]) => ({ blockType, count })) - .sort((a, b) => b.count - a.count); + const blockSlots = ['problem', 'html', 'video']; - const totalBlocksCount = blockTypesArray.reduce((acc, { count }) => acc + count, 0); - // Show the top 3 block type counts individually, and splice the remaining block types together under "Other". - const numBlockTypesShown = 3; - const otherBlocks = blockTypesArray.splice(numBlockTypesShown); - const otherBlocksCount = otherBlocks.reduce((acc, { count }) => acc + count, 0); + const totalBlocksCount = Object.values(blockTypes).reduce((acc, count) => acc + count, 0); if (totalBlocksCount === 0) { return ( @@ -71,6 +65,9 @@ const CollectionStatsWidget = () => { ); } + const otherBlocksCount = Object.entries(blockTypes).filter(([blockType]) => !blockSlots.includes(blockType)) + .reduce((acc, [, count]) => acc + count, 0); + return ( { count={totalBlocksCount} className="border-right" /> - {blockTypesArray.map(({ blockType, count }) => ( + {blockSlots.map((blockType) => ( } + label={} blockType={blockType} - count={count} + count={blockTypes[blockType] || 0} /> ))} - {otherBlocks.length > 0 && ( + {!!otherBlocksCount && ( } count={otherBlocksCount} diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index 67e4eea084..b242601474 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -11,7 +11,6 @@ import { import mockResult from '../__mocks__/collection-search.json'; import { mockContentLibrary, - mockLibraryBlockTypes, mockXBlockFields, mockGetCollectionMetadata, } from '../data/api.mocks'; @@ -24,7 +23,6 @@ mockGetCollectionMetadata.applyMock(); mockContentSearchConfig.applyMock(); mockGetBlockTypes.applyMock(); mockContentLibrary.applyMock(); -mockLibraryBlockTypes.applyMock(); mockXBlockFields.applyMock(); mockBroadcastChannel(); @@ -79,10 +77,6 @@ describe('', () => { }); }); - afterEach(() => { - jest.clearAllMocks(); - }); - const renderLibraryCollectionPage = async (collectionId?: string, libraryId?: string) => { const libId = libraryId || mockContentLibrary.libraryId; const colId = collectionId || mockCollection.collectionId; diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index f1fa20a743..2282d6b82d 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -33,6 +33,11 @@ export interface LibraryContextData { // Current collection openCollectionInfoSidebar: (collectionId: string) => void; currentCollectionId?: string; + // Editor modal - for editing some component + /** If the editor is open and the user is editing some component, this is its usageKey */ + componentBeingEdited: string | undefined; + openComponentEditor: (usageKey: string) => void; + closeComponentEditor: () => void; } /** @@ -67,6 +72,8 @@ export const LibraryProvider = ({ const [currentComponentUsageKey, setCurrentComponentUsageKey] = React.useState(componentUsageKey); const [currentCollectionId, setcurrentCollectionId] = React.useState(collectionId); const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false); + const [componentBeingEdited, openComponentEditor] = React.useState(); + const closeComponentEditor = React.useCallback(() => openComponentEditor(undefined), []); const resetSidebar = React.useCallback(() => { setCurrentComponentUsageKey(undefined); @@ -121,6 +128,9 @@ export const LibraryProvider = ({ closeCreateCollectionModal, openCollectionInfoSidebar, currentCollectionId, + componentBeingEdited, + openComponentEditor, + closeComponentEditor, }), [ libraryId, libraryData, @@ -138,6 +148,9 @@ export const LibraryProvider = ({ closeCreateCollectionModal, openCollectionInfoSidebar, currentCollectionId, + componentBeingEdited, + openComponentEditor, + closeComponentEditor, ]); return ( diff --git a/src/library-authoring/component-info/ComponentAdvancedInfo.test.tsx b/src/library-authoring/component-info/ComponentAdvancedInfo.test.tsx new file mode 100644 index 0000000000..228a8434fb --- /dev/null +++ b/src/library-authoring/component-info/ComponentAdvancedInfo.test.tsx @@ -0,0 +1,113 @@ +import { + fireEvent, + initializeMocks, + render, + screen, + waitFor, +} from '../../testUtils'; +import { + mockContentLibrary, + mockLibraryBlockMetadata, + mockSetXBlockOLX, + mockXBlockAssets, + mockXBlockOLX, +} from '../data/api.mocks'; +import { LibraryProvider } from '../common/context'; +import { ComponentAdvancedInfo } from './ComponentAdvancedInfo'; + +mockContentLibrary.applyMock(); +mockLibraryBlockMetadata.applyMock(); +mockXBlockAssets.applyMock(); +mockXBlockOLX.applyMock(); +const setOLXspy = mockSetXBlockOLX.applyMock(); + +const withLibraryId = (libraryId: string = mockContentLibrary.libraryId) => ({ + extraWrapper: ({ children }: { children: React.ReactNode }) => ( + {children} + ), +}); + +describe('', () => { + it('should display nothing when collapsed', async () => { + initializeMocks(); + render(, withLibraryId()); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + expect(expandButton).toBeInTheDocument(); + expect(screen.queryByText(mockLibraryBlockMetadata.usageKeyPublished)).not.toBeInTheDocument(); + }); + + it('should display the usage key of the block (when expanded)', async () => { + initializeMocks(); + render(, withLibraryId()); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + fireEvent.click(expandButton); + expect(await screen.findByText(mockLibraryBlockMetadata.usageKeyPublished)).toBeInTheDocument(); + }); + + it('should display the static assets of the block (when expanded)', async () => { + initializeMocks(); + render(, withLibraryId()); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + fireEvent.click(expandButton); + expect(await screen.findByText(/static\/image1\.png/)).toBeInTheDocument(); + expect(await screen.findByText(/\(12M\)/)).toBeInTheDocument(); // size of the above file + expect(await screen.findByText(/static\/data\.csv/)).toBeInTheDocument(); + expect(await screen.findByText(/\(8K\)/)).toBeInTheDocument(); // size of the above file + }); + + it('should display the OLX source of the block (when expanded)', async () => { + initializeMocks(); + render(, withLibraryId()); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + fireEvent.click(expandButton); + // Because of syntax highlighting, the OLX will be borken up by many different tags so we need to search for + // just a substring: + const olxPart = /This is a text component which uses/; + expect(await screen.findByText(olxPart)).toBeInTheDocument(); + }); + + it('does not display "Edit OLX" button when the library is read-only', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryIdReadOnly), + ); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + fireEvent.click(expandButton); + expect(screen.queryByRole('button', { name: /Edit OLX/ })).not.toBeInTheDocument(); + }); + + it('can edit the OLX', async () => { + initializeMocks(); + render(, withLibraryId()); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + fireEvent.click(expandButton); + const editButton = await screen.findByRole('button', { name: /Edit OLX/ }); + fireEvent.click(editButton); + + expect(setOLXspy).not.toHaveBeenCalled(); + + const saveButton = await screen.findByRole('button', { name: /Save/ }); + fireEvent.click(saveButton); + + await waitFor(() => expect(setOLXspy).toHaveBeenCalled()); + }); + + it('displays an error if editing the OLX failed', async () => { + initializeMocks(); + + setOLXspy.mockImplementation(async () => { + throw new Error('Example error - setting OLX failed'); + }); + + render(, withLibraryId()); + const expandButton = await screen.findByRole('button', { name: /Advanced details/ }); + fireEvent.click(expandButton); + const editButton = await screen.findByRole('button', { name: /Edit OLX/ }); + fireEvent.click(editButton); + const saveButton = await screen.findByRole('button', { name: /Save/ }); + fireEvent.click(saveButton); + + expect(await screen.findByText(/An error occurred and the OLX could not be saved./)).toBeInTheDocument(); + }); +}); diff --git a/src/library-authoring/component-info/ComponentAdvancedInfo.tsx b/src/library-authoring/component-info/ComponentAdvancedInfo.tsx new file mode 100644 index 0000000000..2b221bea5f --- /dev/null +++ b/src/library-authoring/component-info/ComponentAdvancedInfo.tsx @@ -0,0 +1,119 @@ +/* eslint-disable no-nested-ternary */ +/* eslint-disable import/prefer-default-export */ +import React from 'react'; +import { + Alert, + Button, + Collapsible, + OverlayTrigger, + Tooltip, +} from '@openedx/paragon'; +import { FormattedMessage, FormattedNumber, useIntl } from '@edx/frontend-platform/i18n'; + +import { LoadingSpinner } from '../../generic/Loading'; +import { CodeEditor, EditorAccessor } from '../../generic/CodeEditor'; +import { useLibraryContext } from '../common/context'; +import { + useContentLibrary, + useUpdateXBlockOLX, + useXBlockAssets, + useXBlockOLX, +} from '../data/apiHooks'; +import messages from './messages'; + +interface Props { + usageKey: string; +} + +export const ComponentAdvancedInfo: React.FC = ({ usageKey }) => { + const intl = useIntl(); + const { libraryId } = useLibraryContext(); + const { data: library } = useContentLibrary(libraryId); + const canEditLibrary = library?.canEditLibrary ?? false; + const { data: olx, isLoading: isOLXLoading } = useXBlockOLX(usageKey); + const { data: assets, isLoading: areAssetsLoading } = useXBlockAssets(usageKey); + const editorRef = React.useRef(undefined); + const [isEditingOLX, setEditingOLX] = React.useState(false); + const olxUpdater = useUpdateXBlockOLX(usageKey); + const updateOlx = React.useCallback(() => { + const newOLX = editorRef.current?.state.doc.toString(); + if (!newOLX) { + /* istanbul ignore next */ + throw new Error('Unable to get OLX string from codemirror.'); // Shouldn't happen. + } + olxUpdater.mutateAsync(newOLX).then(() => { + // Only if we succeeded: + setEditingOLX(false); + }).catch(() => { + // On error, an is shown below. We catch here to avoid the error propagating up. + }); + }, [editorRef, olxUpdater, intl]); + return ( + +
+

+

{usageKey}

+

+ {(() => { + if (isOLXLoading) { return ; } + if (!olx) { return ; } + return ( +
+ {olxUpdater.error && ( + +

+ {/* + TODO: fix the API so it returns 400 errors in a JSON object, not HTML 500 errors. Then display + a useful error message here like "parsing the XML failed on line 3". + (olxUpdater.error as Record)?.customAttributes?.httpErrorResponseData.errorMessage + */} +
+ )} + {olx} + { + isEditingOLX ? ( + <> + + + + ) : canEditLibrary ? ( + + + + )} + > + + + ) : ( + null + ) + } +
+ ); + })()} +

+
    + { areAssetsLoading ?
  • : null } + { assets?.map(a => ( +
  • + {a.path}{' '} + () +
  • + )) } +
+
+
+ ); +}; diff --git a/src/library-authoring/component-info/ComponentDetails.test.tsx b/src/library-authoring/component-info/ComponentDetails.test.tsx index 78751281f0..d25f4ee0cf 100644 --- a/src/library-authoring/component-info/ComponentDetails.test.tsx +++ b/src/library-authoring/component-info/ComponentDetails.test.tsx @@ -3,12 +3,19 @@ import { render as baseRender, screen, } from '../../testUtils'; +import { + mockContentLibrary, + mockLibraryBlockMetadata, + mockXBlockAssets, + mockXBlockOLX, +} from '../data/api.mocks'; import { LibraryProvider } from '../common/context'; -import { mockContentLibrary, mockLibraryBlockMetadata } from '../data/api.mocks'; import ComponentDetails from './ComponentDetails'; mockContentLibrary.applyMock(); mockLibraryBlockMetadata.applyMock(); +mockXBlockAssets.applyMock(); +mockXBlockOLX.applyMock(); const { libraryId: mockLibraryId } = mockContentLibrary; diff --git a/src/library-authoring/component-info/ComponentDetails.tsx b/src/library-authoring/component-info/ComponentDetails.tsx index aa56ce516c..709bb2a6a0 100644 --- a/src/library-authoring/component-info/ComponentDetails.tsx +++ b/src/library-authoring/component-info/ComponentDetails.tsx @@ -6,7 +6,7 @@ import Loading from '../../generic/Loading'; import { useLibraryContext } from '../common/context'; import { useLibraryBlockMetadata } from '../data/apiHooks'; import HistoryWidget from '../generic/history-widget'; -import { ComponentDeveloperInfo } from './ComponentDeveloperInfo'; +import { ComponentAdvancedInfo } from './ComponentAdvancedInfo'; import messages from './messages'; const ComponentDetails = () => { @@ -51,10 +51,7 @@ const ComponentDetails = () => { {...componentMetadata} />
- { - // istanbul ignore next: this is only shown in development - (process.env.NODE_ENV === 'development' ? : null) - } + ); }; diff --git a/src/library-authoring/component-info/ComponentDeveloperInfo.tsx b/src/library-authoring/component-info/ComponentDeveloperInfo.tsx deleted file mode 100644 index 8e73d1fdfb..0000000000 --- a/src/library-authoring/component-info/ComponentDeveloperInfo.tsx +++ /dev/null @@ -1,34 +0,0 @@ -/* istanbul ignore file */ -/* eslint-disable import/prefer-default-export */ -// This file doesn't need test coverage nor i18n because it's only seen by devs -import React from 'react'; -import { LoadingSpinner } from '../../generic/Loading'; -import { useXBlockOLX } from '../data/apiHooks'; - -interface Props { - usageKey: string; -} - -/* istanbul ignore next */ -export const ComponentDeveloperInfo: React.FC = ({ usageKey }) => { - const { data: olx, isLoading: isOLXLoading } = useXBlockOLX(usageKey); - return ( - <> -
-

Developer Component Details

-

(This panel is only visible in development builds.)

-
-
Usage key
-
{usageKey}
-
OLX
-
- { - olx ? {olx} : // eslint-disable-line - isOLXLoading ? : // eslint-disable-line - Error - } -
-
- - ); -}; diff --git a/src/library-authoring/component-info/ComponentInfo.test.tsx b/src/library-authoring/component-info/ComponentInfo.test.tsx new file mode 100644 index 0000000000..f200777bc3 --- /dev/null +++ b/src/library-authoring/component-info/ComponentInfo.test.tsx @@ -0,0 +1,63 @@ +import { + initializeMocks, + render, + screen, + waitFor, +} from '../../testUtils'; +import { mockContentLibrary, mockLibraryBlockMetadata } from '../data/api.mocks'; +import { mockBroadcastChannel } from '../../generic/data/api.mock'; +import { LibraryProvider } from '../common/context'; +import ComponentInfo from './ComponentInfo'; + +mockBroadcastChannel(); +mockContentLibrary.applyMock(); +mockLibraryBlockMetadata.applyMock(); +jest.mock('./ComponentPreview', () => ({ + __esModule: true, // Required when mocking 'default' export + default: () =>
Mocked preview
, +})); +jest.mock('./ComponentManagement', () => ({ + __esModule: true, // Required when mocking 'default' export + default: () =>
Mocked management tab
, +})); + +const withLibraryId = (libraryId: string) => ({ + extraWrapper: ({ children }: { children: React.ReactNode }) => ( + {children} + ), +}); + +describe(' Sidebar', () => { + it('should show a disabled "Edit" button when the component type is not editable', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryId), + ); + + const editButton = await screen.findByRole('button', { name: /Edit component/ }); + expect(editButton).toBeDisabled(); + }); + + it('should show a disabled "Edit" button when the library is read-only', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryIdReadOnly), + ); + + const editButton = await screen.findByRole('button', { name: /Edit component/ }); + expect(editButton).toBeDisabled(); + }); + + it('should show a working "Edit" button for a normal component', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryId), + ); + + const editButton = await screen.findByRole('button', { name: /Edit component/ }); + await waitFor(() => expect(editButton).not.toBeDisabled()); + }); +}); diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx index 9637eeeffc..ec19bba148 100644 --- a/src/library-authoring/component-info/ComponentInfo.tsx +++ b/src/library-authoring/component-info/ComponentInfo.tsx @@ -5,7 +5,6 @@ import { Tabs, Stack, } from '@openedx/paragon'; -import { Link } from 'react-router-dom'; import { useLibraryContext } from '../common/context'; import { getEditUrl } from '../components/utils'; @@ -14,24 +13,32 @@ import ComponentDetails from './ComponentDetails'; import ComponentManagement from './ComponentManagement'; import ComponentPreview from './ComponentPreview'; import messages from './messages'; +import { canEditComponent } from '../components/ComponentEditorModal'; +import { useLibraryContext } from '../common/context'; +import { useContentLibrary } from '../data/apiHooks'; const ComponentInfo = () => { const intl = useIntl(); - const { currentComponentUsageKey: usageKey, readOnly } = useLibraryContext(); + const { + currentComponentUsageKey: usageKey, + readOnly, + openComponentEditor, + } = useLibraryContext(); if (!usageKey) { return null; } const editUrl = getEditUrl(usageKey); + const canEdit = canEditComponent(usageKey); return ( {!readOnly && (
- + { + // key=modified below is used to auto-refresh the preview when changes are made, e.g. via OLX editor + componentMetadata + ? + : null + }
diff --git a/src/library-authoring/component-info/messages.ts b/src/library-authoring/component-info/messages.ts index 2847ab9137..8f52cdfefc 100644 --- a/src/library-authoring/component-info/messages.ts +++ b/src/library-authoring/component-info/messages.ts @@ -1,6 +1,56 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ + advancedDetailsTitle: { + id: 'course-authoring.library-authoring.component.advanced.title', + defaultMessage: 'Advanced details', + description: 'Heading for the advanced technical details of a component', + }, + advancedDetailsAssets: { + id: 'course-authoring.library-authoring.component.advanced.assets', + defaultMessage: 'Assets (Files)', + description: 'Heading for files attached to the component', + }, + advancedDetailsOLX: { + id: 'course-authoring.library-authoring.component.advanced.olx', + defaultMessage: 'OLX Source', + description: 'Heading for the component\'s OLX source code', + }, + advancedDetailsOLXEditButton: { + id: 'course-authoring.library-authoring.component.advanced.olx-edit', + defaultMessage: 'Edit OLX', + description: 'Label for button to enable editing the OLX', + }, + advancedDetailsOLXSaveButton: { + id: 'course-authoring.library-authoring.component.advanced.olx-save', + defaultMessage: 'Save', + description: 'Button to save changes to the OLX', + }, + advancedDetailsOLXCancelButton: { + id: 'course-authoring.library-authoring.component.advanced.olx-save', + defaultMessage: 'Cancel', + description: 'Button to cancel changes to the OLX', + }, + advancedDetailsOLXEditWarning: { + id: 'course-authoring.library-authoring.component.advanced.olx-warning', + defaultMessage: 'Be careful! This is an advanced feature and errors may break the component.', + description: 'Warning for users about editing OLX directly.', + }, + advancedDetailsOLXEditFailed: { + id: 'course-authoring.library-authoring.component.advanced.olx-failed', + defaultMessage: 'An error occurred and the OLX could not be saved.', + description: 'Error message shown when saving the OLX fails.', + }, + advancedDetailsOLXError: { + id: 'course-authoring.library-authoring.component.advanced.olx-error', + defaultMessage: 'Unable to load OLX', + description: 'Error message if OLX is unavailable', + }, + advancedDetailsUsageKey: { + id: 'course-authoring.library-authoring.component.advanced.usage-key', + defaultMessage: 'ID (Usage key)', + description: 'Heading for the component\'s ID', + }, editNameButtonAlt: { id: 'course-authoring.library-authoring.component.edit-name.alt', defaultMessage: 'Edit component name', diff --git a/src/library-authoring/components/BaseComponentCard.tsx b/src/library-authoring/components/BaseComponentCard.tsx index 4d98662bca..f614d0b5e3 100644 --- a/src/library-authoring/components/BaseComponentCard.tsx +++ b/src/library-authoring/components/BaseComponentCard.tsx @@ -8,26 +8,26 @@ import { import { getItemIcon, getComponentStyleColor } from '../../generic/block-type-utils'; import TagCount from '../../generic/tag-count'; -import { ContentHitTags, Highlight } from '../../search-manager'; +import { BlockTypeLabel, type ContentHitTags, Highlight } from '../../search-manager'; import { useLibraryContext } from '../common/context'; type BaseComponentCardProps = { - type: string, + componentType: string, displayName: string, description: string, + numChildren?: number, tags: ContentHitTags, actions: React.ReactNode, - blockTypeDisplayName: string, openInfoSidebar: () => void }; const BaseComponentCard = ({ - type, + componentType, displayName, description, + numChildren, tags, actions, - blockTypeDisplayName, openInfoSidebar, } : BaseComponentCardProps) => { const tagCount = useMemo(() => { @@ -53,7 +53,7 @@ const BaseComponentCard = ({ }} > } @@ -64,7 +64,9 @@ const BaseComponentCard = ({ - {blockTypeDisplayName} + + + diff --git a/src/library-authoring/components/CollectionCard.test.tsx b/src/library-authoring/components/CollectionCard.test.tsx index b2c7242f22..6aa55cdfdf 100644 --- a/src/library-authoring/components/CollectionCard.test.tsx +++ b/src/library-authoring/components/CollectionCard.test.tsx @@ -1,22 +1,22 @@ -import React from 'react'; +import userEvent from '@testing-library/user-event'; +import type MockAdapter from 'axios-mock-adapter'; + import { - initializeMocks, - fireEvent, - render as baseRender, - screen, + initializeMocks, render as baseRender, screen, waitFor, waitForElementToBeRemoved, within, } from '../../testUtils'; - import { LibraryProvider } from '../common/context'; import { type CollectionHit } from '../../search-manager'; import CollectionCard from './CollectionCard'; +import messages from './messages'; +import { getLibraryCollectionApiUrl, getLibraryCollectionRestoreApiUrl } from '../data/api'; const CollectionHitSample: CollectionHit = { - id: '1', + id: 'lib-collectionorg1democourse-collection-display-name', type: 'collection', contextKey: 'lb:org1:Demo_Course', - usageKey: 'lb:org1:Demo_Course:collection1', - blockId: 'collection1', + usageKey: 'lib-collection:org1:Demo_Course:collection-display-name', org: 'org1', + blockId: 'collection-display-name', breadcrumbs: [{ displayName: 'Demo Lib' }], displayName: 'Collection Display Name', description: 'Collection description', @@ -30,17 +30,18 @@ const CollectionHitSample: CollectionHit = { tags: {}, }; +let axiosMock: MockAdapter; +let mockShowToast; + const render = (ui: React.ReactElement) => baseRender(ui, { extraWrapper: ({ children }) => { children }, }); describe('', () => { beforeEach(() => { - initializeMocks(); - }); - - afterEach(() => { - jest.clearAllMocks(); + const mocks = initializeMocks(); + axiosMock = mocks.axiosMock; + mockShowToast = mocks.mockShowToast; }); it('should render the card with title and description', () => { @@ -56,12 +57,84 @@ describe('', () => { // Open menu expect(screen.getByTestId('collection-card-menu-toggle')).toBeInTheDocument(); - fireEvent.click(screen.getByTestId('collection-card-menu-toggle')); + userEvent.click(screen.getByTestId('collection-card-menu-toggle')); // Open menu item const openMenuItem = screen.getByRole('link', { name: 'Open' }); expect(openMenuItem).toBeInTheDocument(); - expect(openMenuItem).toHaveAttribute('href', '/library/lb:org1:Demo_Course/collection/collection1/'); + expect(openMenuItem).toHaveAttribute('href', '/library/lb:org1:Demo_Course/collection/collection-display-name/'); + }); + + it('should show confirmation box, delete collection and show toast to undo deletion', async () => { + const url = getLibraryCollectionApiUrl(CollectionHitSample.contextKey, CollectionHitSample.blockId); + axiosMock.onDelete(url).reply(204); + render(); + + expect(screen.queryByText('Collection Display Formated Name')).toBeInTheDocument(); + // Open menu + let menuBtn = await screen.findByRole('button', { name: messages.collectionCardMenuAlt.defaultMessage }); + userEvent.click(menuBtn); + // find and click delete menu option. + expect(screen.queryByText('Delete')).toBeInTheDocument(); + let deleteBtn = await screen.findByRole('button', { name: 'Delete' }); + userEvent.click(deleteBtn); + // verify confirmation dialog and click on cancel button + let dialog = await screen.findByRole('dialog', { name: 'Delete this collection?' }); + expect(dialog).toBeInTheDocument(); + const cancelBtn = await screen.findByRole('button', { name: 'Cancel' }); + userEvent.click(cancelBtn); + expect(axiosMock.history.delete.length).toEqual(0); + expect(cancelBtn).not.toBeInTheDocument(); + + // Open menu + menuBtn = await screen.findByRole('button', { name: messages.collectionCardMenuAlt.defaultMessage }); + userEvent.click(menuBtn); + // click on confirm button to delete + deleteBtn = await screen.findByRole('button', { name: 'Delete' }); + userEvent.click(deleteBtn); + dialog = await screen.findByRole('dialog', { name: 'Delete this collection?' }); + const confirmBtn = await within(dialog).findByRole('button', { name: 'Delete' }); + userEvent.click(confirmBtn); + await waitForElementToBeRemoved(() => screen.queryByRole('dialog', { name: 'Delete this collection?' })); + + await waitFor(() => { + expect(axiosMock.history.delete.length).toEqual(1); + expect(mockShowToast).toHaveBeenCalled(); + }); + // Get restore / undo func from the toast + const restoreFn = mockShowToast.mock.calls[0][1].onClick; + + const restoreUrl = getLibraryCollectionRestoreApiUrl(CollectionHitSample.contextKey, CollectionHitSample.blockId); + axiosMock.onPost(restoreUrl).reply(200); + // restore collection + restoreFn(); + await waitFor(() => { + expect(axiosMock.history.post.length).toEqual(1); + expect(mockShowToast).toHaveBeenCalledWith('Undo successful'); + }); + }); + + it('should show failed toast on delete collection failure', async () => { + const url = getLibraryCollectionApiUrl(CollectionHitSample.contextKey, CollectionHitSample.blockId); + axiosMock.onDelete(url).reply(404); + render(); + + expect(screen.queryByText('Collection Display Formated Name')).toBeInTheDocument(); + // Open menu + const menuBtn = await screen.findByRole('button', { name: messages.collectionCardMenuAlt.defaultMessage }); + userEvent.click(menuBtn); + // find and click delete menu option. + const deleteBtn = await screen.findByRole('button', { name: 'Delete' }); + userEvent.click(deleteBtn); + const dialog = await screen.findByRole('dialog', { name: 'Delete this collection?' }); + const confirmBtn = await within(dialog).findByRole('button', { name: 'Delete' }); + userEvent.click(confirmBtn); + await waitForElementToBeRemoved(() => screen.queryByRole('dialog', { name: 'Delete this collection?' })); + + await waitFor(() => { + expect(axiosMock.history.delete.length).toEqual(1); + expect(mockShowToast).toHaveBeenCalledWith('Failed to delete collection'); + }); }); }); diff --git a/src/library-authoring/components/CollectionCard.tsx b/src/library-authoring/components/CollectionCard.tsx index c10661230d..ad3a6c1628 100644 --- a/src/library-authoring/components/CollectionCard.tsx +++ b/src/library-authoring/components/CollectionCard.tsx @@ -1,9 +1,11 @@ -import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; +import { useCallback, useContext, useState } from 'react'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, Dropdown, Icon, IconButton, + useToggle, } from '@openedx/paragon'; import { MoreVert } from '@openedx/paragon/icons'; import { Link } from 'react-router-dom'; @@ -11,31 +13,93 @@ import { Link } from 'react-router-dom'; import { type CollectionHit } from '../../search-manager'; import { useLibraryContext } from '../common/context'; import BaseComponentCard from './BaseComponentCard'; +import { ToastContext } from '../../generic/toast-context'; +import { useDeleteCollection, useRestoreCollection } from '../data/apiHooks'; +import DeleteModal from '../../generic/delete-modal/DeleteModal'; import messages from './messages'; -export const CollectionMenu = ({ collectionHit }: { collectionHit: CollectionHit }) => { +type CollectionMenuProps = { + collectionHit: CollectionHit, +}; + +const CollectionMenu = ({ collectionHit } : CollectionMenuProps) => { const intl = useIntl(); + const { showToast } = useContext(ToastContext); + const [isDeleteModalOpen, openDeleteModal, closeDeleteModal] = useToggle(false); + const [confirmBtnState, setConfirmBtnState] = useState('default'); + const { closeLibrarySidebar, currentCollectionId } = useLibraryContext(); + + const restoreCollectionMutation = useRestoreCollection(collectionHit.contextKey, collectionHit.blockId); + const restoreCollection = useCallback(() => { + restoreCollectionMutation.mutateAsync() + .then(() => { + showToast(intl.formatMessage(messages.undoDeleteCollectionToastMessage)); + }).catch(() => { + showToast(intl.formatMessage(messages.undoDeleteCollectionToastFailed)); + }); + }, []); + + const deleteCollectionMutation = useDeleteCollection(collectionHit.contextKey, collectionHit.blockId); + const deleteCollection = useCallback(() => { + setConfirmBtnState('pending'); + if (currentCollectionId === collectionHit.blockId) { + // Close sidebar if current collection is open to avoid displaying + // deleted collection in sidebar + closeLibrarySidebar(); + } + deleteCollectionMutation.mutateAsync() + .then(() => { + showToast( + intl.formatMessage(messages.deleteCollectionSuccess), + { + label: intl.formatMessage(messages.undoDeleteCollectionToastAction), + onClick: restoreCollection, + }, + ); + }).catch(() => { + showToast(intl.formatMessage(messages.deleteCollectionFailed)); + }).finally(() => { + setConfirmBtnState('default'); + closeDeleteModal(); + }); + }, [currentCollectionId]); return ( - e.stopPropagation()}> - + e.stopPropagation()}> + + + + + + + + + + + - - - - - - + ); }; @@ -43,36 +107,31 @@ type CollectionCardProps = { collectionHit: CollectionHit, }; -const CollectionCard = ({ collectionHit }: CollectionCardProps) => { - const intl = useIntl(); +const CollectionCard = ({ collectionHit } : CollectionCardProps) => { const { openCollectionInfoSidebar, } = useLibraryContext(); const { - type, + type: componentType, formatted, tags, numChildren, } = collectionHit; const { displayName = '', description = '' } = formatted; - const blockTypeDisplayName = numChildren ? intl.formatMessage( - messages.collectionTypeWithCount, - { numChildren }, - ) : intl.formatMessage(messages.collectionType); return ( )} - blockTypeDisplayName={blockTypeDisplayName} openInfoSidebar={() => openCollectionInfoSidebar(collectionHit.blockId)} /> ); diff --git a/src/library-authoring/components/ComponentCard.test.tsx b/src/library-authoring/components/ComponentCard.test.tsx index 1196c18207..b22c6b0283 100644 --- a/src/library-authoring/components/ComponentCard.test.tsx +++ b/src/library-authoring/components/ComponentCard.test.tsx @@ -42,7 +42,7 @@ const clipboardBroadcastChannelMock = { (global as any).BroadcastChannel = jest.fn(() => clipboardBroadcastChannelMock); const libraryId = 'lib:org1:Demo_Course'; -const render = () => baseRender(, { +const render = () => baseRender(, { extraWrapper: ({ children }) => { children }, }); diff --git a/src/library-authoring/components/ComponentCard.tsx b/src/library-authoring/components/ComponentCard.tsx index a1314544d7..095512d2d8 100644 --- a/src/library-authoring/components/ComponentCard.tsx +++ b/src/library-authoring/components/ComponentCard.tsx @@ -7,7 +7,6 @@ import { Dropdown, } from '@openedx/paragon'; import { MoreVert } from '@openedx/paragon/icons'; -import { Link } from 'react-router-dom'; import { updateClipboard } from '../../generic/data/api'; import { ToastContext } from '../../generic/toast-context'; @@ -15,17 +14,17 @@ import { type ContentHit } from '../../search-manager'; import { useLibraryContext } from '../common/context'; import messages from './messages'; import { STUDIO_CLIPBOARD_CHANNEL } from '../../constants'; -import { getEditUrl } from './utils'; import BaseComponentCard from './BaseComponentCard'; +import { canEditComponent } from './ComponentEditorModal'; type ComponentCardProps = { contentHit: ContentHit, - blockTypeDisplayName: string, }; export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { const intl = useIntl(); - const editUrl = usageKey && getEditUrl(usageKey); + const { openComponentEditor } = useLibraryContext(); + const canEdit = usageKey && canEditComponent(usageKey); const { showToast } = useContext(ToastContext); const [clipboardBroadcastChannel] = useState(() => new BroadcastChannel(STUDIO_CLIPBOARD_CHANNEL)); const updateClipboardClick = () => { @@ -49,7 +48,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { data-testid="component-card-menu-toggle" /> - + openComponentEditor(usageKey) } : { disabled: true })}> @@ -63,7 +62,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { ); }; -const ComponentCard = ({ contentHit, blockTypeDisplayName } : ComponentCardProps) => { +const ComponentCard = ({ contentHit } : ComponentCardProps) => { const { openComponentInfoSidebar, } = useLibraryContext(); @@ -83,7 +82,7 @@ const ComponentCard = ({ contentHit, blockTypeDisplayName } : ComponentCardProps return ( )} - blockTypeDisplayName={blockTypeDisplayName} openInfoSidebar={() => openComponentInfoSidebar(usageKey)} /> ); diff --git a/src/library-authoring/components/ComponentEditorModal.tsx b/src/library-authoring/components/ComponentEditorModal.tsx new file mode 100644 index 0000000000..a2b6cc3fe3 --- /dev/null +++ b/src/library-authoring/components/ComponentEditorModal.tsx @@ -0,0 +1,42 @@ +import { getConfig } from '@edx/frontend-platform'; +import React from 'react'; + +import { useLibraryContext } from '../common/context'; +import { getBlockType } from '../../generic/key-utils'; +import EditorPage from '../../editors/EditorPage'; + +/* eslint-disable import/prefer-default-export */ +export function canEditComponent(usageKey: string): boolean { + let blockType: string; + try { + blockType = getBlockType(usageKey); + } catch { + return false; + } + + // Which XBlock/component types are supported by the 'editors' built in to this repo? + const mfeEditorTypes = ['html', 'problem', 'video']; + return mfeEditorTypes.includes(blockType); +} + +export const ComponentEditorModal: React.FC> = () => { + const { componentBeingEdited, closeComponentEditor, libraryId } = useLibraryContext(); + + if (componentBeingEdited === undefined) { + return null; + } + const blockType = getBlockType(componentBeingEdited); + + return ( + { closeComponentEditor(); return () => {}; }} + fullScreen={false} + /> + ); +}; diff --git a/src/library-authoring/components/LibraryComponents.test.tsx b/src/library-authoring/components/LibraryComponents.test.tsx index 0bf198063f..0e85cd7fe4 100644 --- a/src/library-authoring/components/LibraryComponents.test.tsx +++ b/src/library-authoring/components/LibraryComponents.test.tsx @@ -7,7 +7,7 @@ import { initializeMocks, } from '../../testUtils'; import { getContentSearchConfigUrl } from '../../search-manager/data/api'; -import { mockLibraryBlockTypes, mockContentLibrary } from '../data/api.mocks'; +import { mockContentLibrary } from '../data/api.mocks'; import mockEmptyResult from '../../search-modal/__mocks__/empty-search-result.json'; import { LibraryProvider } from '../common/context'; import { libraryComponentsMock } from '../__mocks__'; @@ -15,7 +15,6 @@ import LibraryComponents from './LibraryComponents'; const searchEndpoint = 'http://mock.meilisearch.local/multi-search'; -mockLibraryBlockTypes.applyMock(); mockContentLibrary.applyMock(); const mockFetchNextPage = jest.fn(); const mockUseSearchContext = jest.fn(); @@ -77,7 +76,6 @@ describe('', () => { afterEach(() => { fetchMock.reset(); - mockFetchNextPage.mockReset(); }); it('should render empty state', async () => { diff --git a/src/library-authoring/components/LibraryComponents.tsx b/src/library-authoring/components/LibraryComponents.tsx index 8f44e7762f..c260897b64 100644 --- a/src/library-authoring/components/LibraryComponents.tsx +++ b/src/library-authoring/components/LibraryComponents.tsx @@ -1,10 +1,7 @@ -import React, { useMemo } from 'react'; - import { LoadingSpinner } from '../../generic/Loading'; import { useLoadOnScroll } from '../../hooks'; import { useSearchContext } from '../../search-manager'; import { NoComponents, NoSearchResults } from '../EmptyStates'; -import { useLibraryBlockTypes } from '../data/apiHooks'; import ComponentCard from './ComponentCard'; import { LIBRARY_SECTION_PREVIEW_LIMIT } from './LibrarySection'; import { useLibraryContext } from '../common/context'; @@ -30,22 +27,10 @@ const LibraryComponents = ({ variant }: LibraryComponentsProps) => { isLoading, isFiltered, } = useSearchContext(); - const { libraryId, openAddContentSidebar } = useLibraryContext(); + const { openAddContentSidebar } = useLibraryContext(); const componentList = variant === 'preview' ? hits.slice(0, LIBRARY_SECTION_PREVIEW_LIMIT) : hits; - // TODO get rid of "useLibraryBlockTypes". Use instead. - const { data: blockTypesData } = useLibraryBlockTypes(libraryId); - const blockTypes = useMemo(() => { - const result = {}; - if (blockTypesData) { - blockTypesData.forEach(blockType => { - result[blockType.blockType] = blockType; - }); - } - return result; - }, [blockTypesData]); - useLoadOnScroll( hasNextPage, isFetchingNextPage, @@ -67,7 +52,6 @@ const LibraryComponents = ({ variant }: LibraryComponentsProps) => { )) } diff --git a/src/library-authoring/components/messages.ts b/src/library-authoring/components/messages.ts index c826bae09d..3bac3ad177 100644 --- a/src/library-authoring/components/messages.ts +++ b/src/library-authoring/components/messages.ts @@ -11,16 +11,6 @@ const messages = defineMessages({ defaultMessage: 'Collection actions menu', description: 'Alt/title text for the collection card menu button.', }, - collectionType: { - id: 'course-authoring.library-authoring.collection.type', - defaultMessage: 'Collection', - description: 'Collection type text', - }, - collectionTypeWithCount: { - id: 'course-authoring.library-authoring.collection.type-with-count', - defaultMessage: 'Collection ({numChildren})', - description: 'Collection type text with children count', - }, menuOpen: { id: 'course-authoring.library-authoring.collection.menu.open', defaultMessage: 'Open', @@ -51,6 +41,41 @@ const messages = defineMessages({ defaultMessage: 'Failed to copy component to clipboard', description: 'Message for failed to copy component to clipboard.', }, + deleteCollection: { + id: 'course-authoring.library-authoring.collection.delete-menu-text', + defaultMessage: 'Delete', + description: 'Menu item to delete a collection.', + }, + deleteCollectionConfirm: { + id: 'course-authoring.library-authoring.collection.delete-confirmation-text', + defaultMessage: 'Are you sure you want to delete collection: {collectionTitle}?', + description: 'Confirmation text to display before deleting collection', + }, + deleteCollectionFailed: { + id: 'course-authoring.library-authoring.collection.delete-failed-error', + defaultMessage: 'Failed to delete collection', + description: 'Message to display on failure to delete collection', + }, + deleteCollectionSuccess: { + id: 'course-authoring.library-authoring.collection.delete-error-success', + defaultMessage: 'Collection deleted', + description: 'Message to display on delete collection success', + }, + undoDeleteCollectionToastAction: { + id: 'course-authoring.library-authoring.collection.undo-delete-collection-toast-button', + defaultMessage: 'Undo', + description: 'Toast message to undo deletion of collection', + }, + undoDeleteCollectionToastMessage: { + id: 'course-authoring.library-authoring.collection.undo-delete-collection-toast-text', + defaultMessage: 'Undo successful', + description: 'Message to display on undo delete collection success', + }, + undoDeleteCollectionToastFailed: { + id: 'course-authoring.library-authoring.collection.undo-delete-collection-failed', + defaultMessage: 'Failed to undo delete collection operation', + description: 'Message to display on failure to undo delete collection', + }, }); export default messages; diff --git a/src/library-authoring/components/utils.test.ts b/src/library-authoring/components/utils.test.ts deleted file mode 100644 index 07d1265c5c..0000000000 --- a/src/library-authoring/components/utils.test.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { getEditUrl } from './utils'; - -describe('component utils', () => { - describe('getEditUrl', () => { - it('returns the right URL for an HTML (Text) block', () => { - const usageKey = 'lb:org:ALPHA:html:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; - expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:ALPHA/editor/html/${usageKey}`); - }); - it('returns the right URL for editing a Problem block', () => { - const usageKey = 'lb:org:beta:problem:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; - expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:beta/editor/problem/${usageKey}`); - }); - it('returns the right URL for editing a Video block', () => { - const usageKey = 'lb:org:beta:video:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; - expect(getEditUrl(usageKey)).toStrictEqual(`/library/lib:org:beta/editor/video/${usageKey}`); - }); - it('doesn\'t yet allow editing a drag-and-drop-v2 block', () => { - const usageKey = 'lb:org:beta:drag-and-drop-v2:571fe018-f3ce-45c9-8f53-5dafcb422fdd'; - expect(getEditUrl(usageKey)).toBeUndefined(); - }); - it('returns undefined for an invalid key', () => { - expect(getEditUrl('foobar')).toBeUndefined(); - expect(getEditUrl('')).toBeUndefined(); - expect(getEditUrl('lb:unknown:unknown:unknown')).toBeUndefined(); - }); - }); -}); diff --git a/src/library-authoring/components/utils.ts b/src/library-authoring/components/utils.ts deleted file mode 100644 index addaab73dc..0000000000 --- a/src/library-authoring/components/utils.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { getBlockType, getLibraryId } from '../../generic/key-utils'; - -/* eslint-disable import/prefer-default-export */ -export function getEditUrl(usageKey: string): string | undefined { - let blockType: string; - let libraryId: string; - try { - blockType = getBlockType(usageKey); - libraryId = getLibraryId(usageKey); - } catch { - return undefined; - } - - // Which XBlock/component types are supported by the 'editors' built in to this repo? - const mfeEditorTypes = ['html', 'problem', 'video']; - if (mfeEditorTypes.includes(blockType)) { - return `/library/${libraryId}/editor/${blockType}/${usageKey}`; - } - return undefined; -} diff --git a/src/library-authoring/data/api.mocks.ts b/src/library-authoring/data/api.mocks.ts index 7906760ed1..91a1b52d9b 100644 --- a/src/library-authoring/data/api.mocks.ts +++ b/src/library-authoring/data/api.mocks.ts @@ -1,51 +1,9 @@ /* istanbul ignore file */ import { mockContentTaxonomyTagsData } from '../../content-tags-drawer/data/api.mocks'; +import { getBlockType } from '../../generic/key-utils'; import { createAxiosError } from '../../testUtils'; import * as api from './api'; -/** - * Mock for `getLibraryBlockTypes()` - */ -export async function mockLibraryBlockTypes(): Promise { - return [ - { blockType: 'about', displayName: 'overview' }, - { blockType: 'annotatable', displayName: 'Annotation' }, - { blockType: 'chapter', displayName: 'Section' }, - { blockType: 'conditional', displayName: 'Conditional' }, - { blockType: 'course', displayName: 'Empty' }, - { blockType: 'course_info', displayName: 'Text' }, - { blockType: 'discussion', displayName: 'Discussion' }, - { blockType: 'done', displayName: 'Completion' }, - { blockType: 'drag-and-drop-v2', displayName: 'Drag and Drop' }, - { blockType: 'edx_sga', displayName: 'Staff Graded Assignment' }, - { blockType: 'google-calendar', displayName: 'Google Calendar' }, - { blockType: 'google-document', displayName: 'Google Document' }, - { blockType: 'html', displayName: 'Text' }, - { blockType: 'library', displayName: 'Library' }, - { blockType: 'library_content', displayName: 'Randomized Content Block' }, - { blockType: 'lti', displayName: 'LTI' }, - { blockType: 'lti_consumer', displayName: 'LTI Consumer' }, - { blockType: 'openassessment', displayName: 'Open Response Assessment' }, - { blockType: 'poll', displayName: 'Poll' }, - { blockType: 'problem', displayName: 'Problem' }, - { blockType: 'scorm', displayName: 'Scorm module' }, - { blockType: 'sequential', displayName: 'Subsection' }, - { blockType: 'split_test', displayName: 'Content Experiment' }, - { blockType: 'staffgradedxblock', displayName: 'Staff Graded Points' }, - { blockType: 'static_tab', displayName: 'Empty' }, - { blockType: 'survey', displayName: 'Survey' }, - { blockType: 'thumbs', displayName: 'Thumbs' }, - { blockType: 'unit', displayName: 'Unit' }, - { blockType: 'vertical', displayName: 'Unit' }, - { blockType: 'video', displayName: 'Video' }, - { blockType: 'videoalpha', displayName: 'Video' }, - { blockType: 'word_cloud', displayName: 'Word cloud' }, - ]; -} -mockLibraryBlockTypes.applyMock = () => { - jest.spyOn(api, 'getLibraryBlockTypes').mockImplementation(mockLibraryBlockTypes); -}; - /** * Mock for `getContentLibrary()` * @@ -247,6 +205,7 @@ export async function mockXBlockFields(usageKey: string): Promise jest.spyOn(api, 'getXBlockFields').mockImplementation(mockXBlockFields); @@ -297,6 +262,7 @@ export async function mockLibraryBlockMetadata(usageKey: string): Promise jest.spyOn(api, 'getLibraryBlockMetadata').mockImplementation(mockLibraryBlockMetadata); @@ -374,3 +346,53 @@ mockGetCollectionMetadata.collectionData = { mockGetCollectionMetadata.applyMock = () => { jest.spyOn(api, 'getCollectionMetadata').mockImplementation(mockGetCollectionMetadata); }; + +/** + * Mock for `getXBlockOLX()` + * + * This mock returns different data/responses depending on the ID of the block + * that you request. Use `mockXBlockOLX.applyMock()` to apply it to the whole + * test suite. + */ +export async function mockXBlockOLX(usageKey: string): Promise { + const thisMock = mockXBlockOLX; + switch (usageKey) { + case thisMock.usageKeyHtml: return thisMock.olxHtml; + default: { + const blockType = getBlockType(usageKey); + return `<${blockType}>This is mock OLX for usageKey "${usageKey}"`; + } + } +} +// Mock of a "regular" HTML (Text) block: +mockXBlockOLX.usageKeyHtml = mockXBlockFields.usageKeyHtml; +mockXBlockOLX.olxHtml = ` + + ${mockXBlockFields.dataHtml.data} + +`; +/** Apply this mock. Returns a spy object that can tell you if it's been called. */ +mockXBlockOLX.applyMock = () => jest.spyOn(api, 'getXBlockOLX').mockImplementation(mockXBlockOLX); + +/** + * Mock for `setXBlockOLX()` + */ +export async function mockSetXBlockOLX(_usageKey: string, newOLX: string): Promise { + return newOLX; +} +/** Apply this mock. Returns a spy object that can tell you if it's been called. */ +mockSetXBlockOLX.applyMock = () => jest.spyOn(api, 'setXBlockOLX').mockImplementation(mockSetXBlockOLX); + +/** + * Mock for `getXBlockAssets()` + * + * Use `getXBlockAssets.applyMock()` to apply it to the whole test suite. + */ +export async function mockXBlockAssets(): ReturnType { + return [ + { path: 'static/image1.png', url: 'https://cdn.test.none/image1.png', size: 12_345_000 }, + { path: 'static/data.csv', url: 'https://cdn.test.none/data.csv', size: 8_000 }, + ]; +} +/** Apply this mock. Returns a spy object that can tell you if it's been called. */ +mockXBlockAssets.applyMock = () => jest.spyOn(api, 'getXBlockAssets').mockImplementation(mockXBlockAssets); diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index c4a680b341..e08d4af727 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -8,11 +8,6 @@ const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; */ export const getContentLibraryApiUrl = (libraryId: string) => `${getApiBaseUrl()}/api/libraries/v2/${libraryId}/`; -/** - * Get the URL for getting block types of a library (what types can be created). - */ -export const getLibraryBlockTypesUrl = (libraryId: string) => `${getApiBaseUrl()}/api/libraries/v2/${libraryId}/block_types/`; - /** * Get the URL for create content in library. */ @@ -46,18 +41,26 @@ export const getXBlockFieldsApiUrl = (usageKey: string) => `${getApiBaseUrl()}/a * Get the URL for the xblock OLX API */ export const getXBlockOLXApiUrl = (usageKey: string) => `${getApiBaseUrl()}/api/libraries/v2/blocks/${usageKey}/olx/`; +/** + * Get the URL for the xblock Assets List API + */ +export const getXBlockAssetsApiUrl = (usageKey: string) => `${getApiBaseUrl()}/api/libraries/v2/blocks/${usageKey}/assets/`; /** * Get the URL for the Library Collections API. */ export const getLibraryCollectionsApiUrl = (libraryId: string) => `${getApiBaseUrl()}/api/libraries/v2/${libraryId}/collections/`; /** - * Get the URL for the collection API. + * Get the URL for the collection detail API. */ export const getLibraryCollectionApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionsApiUrl(libraryId)}${collectionId}/`; /** * Get the URL for the collection API. */ export const getLibraryCollectionComponentApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionApiUrl(libraryId, collectionId)}components/`; +/** + * Get the API URL for restoring deleted collection. + */ +export const getLibraryCollectionRestoreApiUrl = (libraryId: string, collectionId: string) => `${getLibraryCollectionApiUrl(libraryId, collectionId)}restore/`; export interface ContentLibrary { id: string; @@ -182,14 +185,6 @@ export interface CreateLibraryCollectionDataRequest { export type UpdateCollectionComponentsRequest = Partial; -/** - * Fetch the list of XBlock types that can be added to this library - */ -export async function getLibraryBlockTypes(libraryId: string): Promise { - const { data } = await getAuthenticatedHttpClient().get(getLibraryBlockTypesUrl(libraryId)); - return camelCaseObject(data); -} - /** * Fetch a content library by its ID. */ @@ -314,11 +309,31 @@ export async function createCollection(libraryId: string, collectionData: Create /** * Fetch the OLX for the given XBlock. */ +// istanbul ignore next export async function getXBlockOLX(usageKey: string): Promise { const { data } = await getAuthenticatedHttpClient().get(getXBlockOLXApiUrl(usageKey)); return data.olx; } +/** + * Set the OLX for the given XBlock. + * Returns the OLX as it was actually saved. + */ +// istanbul ignore next +export async function setXBlockOLX(usageKey: string, newOLX: string): Promise { + const { data } = await getAuthenticatedHttpClient().post(getXBlockOLXApiUrl(usageKey), { olx: newOLX }); + return data.olx; +} + +/** + * Fetch the asset (static file) list for the given XBlock. + */ +// istanbul ignore next +export async function getXBlockAssets(usageKey: string): Promise<{ path: string; url: string; size: number }[]> { + const { data } = await getAuthenticatedHttpClient().get(getXBlockAssetsApiUrl(usageKey)); + return data.files; +} + /** * Get the collection metadata. */ @@ -347,3 +362,19 @@ export async function updateCollectionComponents(libraryId: string, collectionId usage_keys: usageKeys, }); } + +/** + * Soft-Delete collection. + */ +export async function deleteCollection(libraryId: string, collectionId: string) { + const client = getAuthenticatedHttpClient(); + await client.delete(getLibraryCollectionApiUrl(libraryId, collectionId)); +} + +/** + * Restore soft-deleted collection + */ +export async function restoreCollection(libraryId: string, collectionId: string) { + const client = getAuthenticatedHttpClient(); + await client.post(getLibraryCollectionRestoreApiUrl(libraryId, collectionId)); +} diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 40601356b4..42a1f53a34 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -14,7 +14,6 @@ import { type XBlockFields, type UpdateXBlockFieldsRequest, getContentLibrary, - getLibraryBlockTypes, createLibraryBlock, getContentLibraryV2List, commitLibraryChanges, @@ -31,6 +30,10 @@ import { updateCollectionComponents, type CreateLibraryCollectionDataRequest, getCollectionMetadata, + deleteCollection, + restoreCollection, + setXBlockOLX, + getXBlockAssets, } from './api'; export const libraryQueryPredicate = (query: Query, libraryId: string): boolean => { @@ -59,12 +62,6 @@ export const libraryAuthoringQueryKeys = { 'list', ...(customParams ? [customParams] : []), ], - contentLibraryBlockTypes: (contentLibraryId?: string) => [ - ...libraryAuthoringQueryKeys.all, - ...libraryAuthoringQueryKeys.contentLibrary(contentLibraryId), - 'content', - 'libraryBlockTypes', - ], collection: (libraryId?: string, collectionId?: string) => [ ...libraryAuthoringQueryKeys.all, libraryId, @@ -82,6 +79,8 @@ export const xblockQueryKeys = { xblockFields: (usageKey: string) => [...xblockQueryKeys.xblock(usageKey), 'fields'], /** OLX (XML representation of the fields/content) */ xblockOLX: (usageKey: string) => [...xblockQueryKeys.xblock(usageKey), 'OLX'], + /** assets (static files) */ + xblockAssets: (usageKey: string) => [...xblockQueryKeys.xblock(usageKey), 'assets'], componentMetadata: (usageKey: string) => [...xblockQueryKeys.xblock(usageKey), 'componentMetadata'], }; @@ -113,16 +112,6 @@ export const useContentLibrary = (libraryId: string | undefined) => ( }) ); -/** - * Hook to fetch block types of a library. - */ -export const useLibraryBlockTypes = (libraryId: string) => ( - useQuery({ - queryKey: libraryAuthoringQueryKeys.contentLibraryBlockTypes(libraryId), - queryFn: () => getLibraryBlockTypes(libraryId), - }) -); - /** * Use this mutation to create a block in a library */ @@ -272,7 +261,7 @@ export const useCreateLibraryCollection = (libraryId: string) => { }); }; -/* istanbul ignore next */ // This is only used in developer builds, and the associated UI doesn't work in test or prod +/** Get the OLX source of a library component */ export const useXBlockOLX = (usageKey: string) => ( useQuery({ queryKey: xblockQueryKeys.xblockOLX(usageKey), @@ -281,6 +270,34 @@ export const useXBlockOLX = (usageKey: string) => ( }) ); +/** + * Update the OLX of a library component (advanced feature) + */ +export const useUpdateXBlockOLX = (usageKey: string) => { + const contentLibraryId = getLibraryId(usageKey); + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (newOLX: string) => setXBlockOLX(usageKey, newOLX), + onSuccess: (olxFromServer) => { + queryClient.setQueryData(xblockQueryKeys.xblockOLX(usageKey), olxFromServer); + // Reload the other data for this component: + invalidateComponentData(queryClient, contentLibraryId, usageKey); + // And the description and display name etc. may have changed, so refresh everything in the library too: + queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(contentLibraryId) }); + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, contentLibraryId) }); + }, + }); +}; + +/** Get the list of assets (static files) attached to a library component */ +export const useXBlockAssets = (usageKey: string) => ( + useQuery({ + queryKey: xblockQueryKeys.xblockAssets(usageKey), + queryFn: () => getXBlockAssets(usageKey), + enabled: !!usageKey, + }) +); + /** * Get the metadata for a collection in a library */ @@ -320,11 +337,38 @@ export const useUpdateCollectionComponents = (libraryId?: string, collectionId?: } return undefined; }, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - onSettled: (_data, _error, _variables) => { + onSettled: () => { if (libraryId !== undefined && collectionId !== undefined) { queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); } }, }); }; + +/** + * Use this mutation to soft delete collections in a library + */ +export const useDeleteCollection = (libraryId: string, collectionId: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async () => deleteCollection(libraryId, collectionId), + onSettled: () => { + queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); + }, + }); +}; + +/** + * Use this mutation to restore soft deleted collections in a library + */ +export const useRestoreCollection = (libraryId: string, collectionId: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async () => restoreCollection(libraryId, collectionId), + onSettled: () => { + queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); + queryClient.invalidateQueries({ predicate: (query) => libraryQueryPredicate(query, libraryId) }); + }, + }); +}; diff --git a/src/search-manager/BlockTypeLabel.test.tsx b/src/search-manager/BlockTypeLabel.test.tsx new file mode 100644 index 0000000000..385278b721 --- /dev/null +++ b/src/search-manager/BlockTypeLabel.test.tsx @@ -0,0 +1,73 @@ +import { render, screen } from '@testing-library/react'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; + +import BlockTypeLabel from './BlockTypeLabel'; +import messages from './messages'; + +const testCases = [ + { + blockType: 'annotatable', + count: undefined, + expectedLabel: messages['blockType.annotatable'].defaultMessage, + }, + { + blockType: 'chapter', + count: undefined, + expectedLabel: messages['blockType.chapter'].defaultMessage, + }, + { + blockType: 'chapter', + count: 10, + expectedLabel: messages['blockType.chapter'].defaultMessage, + }, + { + blockType: 'drag-and-drop-v2', + count: undefined, + expectedLabel: messages['blockType.drag-and-drop-v2'].defaultMessage, + }, + { + blockType: 'multiplechoiceresponse', + count: undefined, + expectedLabel: messages['blockType.multiplechoiceresponse'].defaultMessage, + }, + { + blockType: 'html', + count: undefined, + expectedLabel: messages['blockType.html'].defaultMessage, + }, + { + blockType: 'collection', + count: undefined, + expectedLabel: messages['blockType.collection'].defaultMessage, + }, + { + blockType: 'collection', + count: 0, + expectedLabel: messages['blockType.collection'].defaultMessage, + }, + { + blockType: 'collection', + count: 10, + expectedLabel: 'Collection (10)', + }, + // XBlock types without an explicit label are capitalized using the textTransform style + { + blockType: 'survey', + count: undefined, + expectedLabel: 'survey', + }, +]; + +describe('', () => { + test.each(testCases)( + 'render BlockTypeLabel for $blockType (count=$count)', + ({ blockType, expectedLabel, count }) => { + render( + + + , + ); + expect(screen.getByText(expectedLabel)).toBeInTheDocument(); + }, + ); +}); diff --git a/src/search-manager/BlockTypeLabel.tsx b/src/search-manager/BlockTypeLabel.tsx index 5f8a540733..f9602eb83e 100644 --- a/src/search-manager/BlockTypeLabel.tsx +++ b/src/search-manager/BlockTypeLabel.tsx @@ -5,18 +5,26 @@ import messages from './messages'; /** * Displays a friendly, localized text name for the given XBlock/component type * e.g. `vertical` becomes `"Unit"` + * + * Also accepts an optional `count` number, which will be displayed if + * it's non-zero and the block label supports it. */ -const BlockTypeLabel: React.FC<{ type: string }> = ({ type }) => { - // TODO: Load the localized list of Component names from Studio REST API? - const msg = messages[`blockType.${type}`]; +const BlockTypeLabel: React.FC<{ blockType: string, count?: number }> = ({ blockType, count }) => { + const msg = messages[`blockType.${blockType}`]; + const msgWithCount = messages[`blockType.${blockType}.with_count`]; + + if (count && msgWithCount) { + return ; + } if (msg) { return ; } + // Replace underscores and hypens with spaces, then let the browser capitalize this // in a locale-aware way to get a reasonable display value. // e.g. 'drag-and-drop-v2' -> "Drag And Drop V2" - return {type.replace(/[_-]/g, ' ')}; + return {blockType.replace(/[_-]/g, ' ')}; }; export default BlockTypeLabel; diff --git a/src/search-manager/FilterByBlockType.tsx b/src/search-manager/FilterByBlockType.tsx index eba53e96ea..cd887e3cc3 100644 --- a/src/search-manager/FilterByBlockType.tsx +++ b/src/search-manager/FilterByBlockType.tsx @@ -108,7 +108,7 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr >
- {' '} + {' '} {count}
{ Object.keys(problemTypes).length !== 0 && ( @@ -146,7 +146,7 @@ const ProblemFilterItem = ({ count, handleCheckboxChange } : ProblemFilterItemPr onChange={handleProblemCheckboxChange} >
- {' '} + {' '} {problemTypeCount}
@@ -199,7 +199,7 @@ const FilterItem = ({ blockType, count } : FilterItemProps) => { onChange={handleCheckboxChange} >
- {' '} + {' '} {count}
@@ -259,7 +259,7 @@ const FilterByBlockType: React.FC> = () => { }); const appliedFilters = [...blockTypesFilter, ...problemTypesFilter].map( - blockType => ({ label: }), + blockType => ({ label: }), ); return ( diff --git a/src/search-manager/data/api.mock.ts b/src/search-manager/data/api.mock.ts index bfed5a3694..e4673dd01b 100644 --- a/src/search-manager/data/api.mock.ts +++ b/src/search-manager/data/api.mock.ts @@ -51,14 +51,14 @@ export async function mockGetBlockTypes( noBlocks: {}, someBlocks: { problem: 1, html: 2 }, moreBlocks: { - advanced: 1, - discussion: 2, - library: 3, - drag_and_drop_v2: 4, - openassessment: 5, - html: 6, - problem: 7, - video: 8, + advanced: 8, + discussion: 7, + library: 6, + drag_and_drop_v2: 5, + openassessment: 4, + html: 3, + problem: 2, + video: 1, }, }; jest.spyOn(api, 'fetchBlockTypes').mockResolvedValue(mockResponseMap[mockResponse]); diff --git a/src/search-manager/messages.ts b/src/search-manager/messages.ts index ebda3cb91b..218b452e1e 100644 --- a/src/search-manager/messages.ts +++ b/src/search-manager/messages.ts @@ -56,6 +56,16 @@ const messages = defineMessages({ defaultMessage: 'Section', description: 'Name of the "Section" course outline level in Studio', }, + 'blockType.collection': { + id: 'course-authoring.course-search.blockType.collection', + defaultMessage: 'Collection', + description: 'Collection type text', + }, + 'blockType.collection.with_count': { + id: 'course-authoring.course-search.blockType.collectionWithCount', + defaultMessage: 'Collection ({count})', + description: 'Collection type text with children count', + }, 'blockType.discussion': { id: 'course-authoring.course-search.blockType.discussion', defaultMessage: 'Discussion', diff --git a/src/testUtils.tsx b/src/testUtils.tsx index 73b2518f16..6fc15089bd 100644 --- a/src/testUtils.tsx +++ b/src/testUtils.tsx @@ -34,6 +34,7 @@ let axiosMock: MockAdapter; let mockToastContext: ToastContextData = { showToast: jest.fn(), closeToast: jest.fn(), + toastAction: undefined, toastMessage: null, }; @@ -176,12 +177,17 @@ export function initializeMocks({ user = defaultUser, initialState = undefined } showToast: jest.fn(), closeToast: jest.fn(), toastMessage: null, + toastAction: undefined, }; + // Clear the call counts etc. of all mocks. This doesn't remove the mock's effects; just clears their history. + jest.clearAllMocks(); + return { reduxStore, axiosMock, mockShowToast: mockToastContext.showToast, + mockToastAction: mockToastContext.toastAction, }; }