From 7e4ecff4e8705118b23a3695fa5ceb8c4b688428 Mon Sep 17 00:00:00 2001 From: Peter Kulko <93188219+PKulkoRaccoonGang@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:05:48 -0800 Subject: [PATCH] feat: handle edit modals from advanced xblocks (#1445) Adds new message types, updates message handlers, and implements a new modal iframe for legacy XBlock editing. --- src/constants.js | 10 ++ src/course-unit/CourseUnit.test.jsx | 120 ++++++++++++++++++ src/course-unit/constants.js | 6 + src/course-unit/data/thunk.js | 17 +++ .../hooks/tests/hooks.test.tsx | 62 +++++---- .../xblock-container-iframe/hooks/types.ts | 4 + .../hooks/useMessageHandlers.tsx | 13 +- .../xblock-container-iframe/index.tsx | 38 +++++- .../xblock-container-iframe/messages.ts | 5 + .../xblock-container-iframe/utils.ts | 11 ++ src/generic/modal-iframe/ModalIframe.test.tsx | 34 +++++ src/generic/modal-iframe/index.scss | 13 ++ src/generic/modal-iframe/index.tsx | 43 +++++++ .../saving-error-alert/SavingErrorAlert.jsx | 1 + src/generic/styles.scss | 1 + 15 files changed, 353 insertions(+), 25 deletions(-) create mode 100644 src/generic/modal-iframe/ModalIframe.test.tsx create mode 100644 src/generic/modal-iframe/index.scss create mode 100644 src/generic/modal-iframe/index.tsx diff --git a/src/constants.js b/src/constants.js index 80e7cdd778..7439961ffa 100644 --- a/src/constants.js +++ b/src/constants.js @@ -78,6 +78,16 @@ export const REGEX_RULES = { noSpaceRule: /^\S*$/, }; +/** + * Feature policy for iframe, allowing access to certain courseware-related media. + * + * We must use the wildcard (*) origin for each feature, as courseware content + * may be embedded in external iframes. Notably, xblock-lti-consumer is a popular + * block that iframes external course content. + + * This policy was selected in conference with the edX Security Working Group. + * Changes to it should be vetted by them (security@edx.org). + */ export const IFRAME_FEATURE_POLICY = ( 'microphone *; camera *; midi *; geolocation *; encrypted-media *; clipboard-write *' ); diff --git a/src/course-unit/CourseUnit.test.jsx b/src/course-unit/CourseUnit.test.jsx index 029a157ceb..7d81a9c7b0 100644 --- a/src/course-unit/CourseUnit.test.jsx +++ b/src/course-unit/CourseUnit.test.jsx @@ -233,6 +233,126 @@ describe('', () => { }); }); + it('displays an error alert when a studioAjaxError message is received', async () => { + const { getByTitle, getByTestId } = render(); + + await waitFor(() => { + const xblocksIframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(xblocksIframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.studioAjaxError, { + error: 'Some error text...', + }); + }); + expect(getByTestId('saving-error-alert')).toBeInTheDocument(); + }); + + it('renders XBlock iframe and opens legacy edit modal on editXBlock message', async () => { + const { getByTitle } = render(); + + await waitFor(() => { + const xblocksIframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(xblocksIframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.editXBlock, { id: blockId }); + + const legacyXBlockEditModalIframe = getByTitle( + xblockContainerIframeMessages.legacyEditModalIframeTitle.defaultMessage, + ); + expect(legacyXBlockEditModalIframe).toBeInTheDocument(); + }); + }); + + it('closes the legacy edit modal when closeXBlockEditorModal message is received', async () => { + const { getByTitle, queryByTitle } = render(); + + await waitFor(() => { + const xblocksIframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(xblocksIframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.closeXBlockEditorModal, { id: blockId }); + + const legacyXBlockEditModalIframe = queryByTitle( + xblockContainerIframeMessages.legacyEditModalIframeTitle.defaultMessage, + ); + expect(legacyXBlockEditModalIframe).not.toBeInTheDocument(); + }); + }); + + it('closes legacy edit modal and updates course unit sidebar after saveEditedXBlockData message', async () => { + const { getByTitle, queryByTitle, getByTestId } = render(); + + await waitFor(() => { + const xblocksIframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(xblocksIframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.saveEditedXBlockData); + + const legacyXBlockEditModalIframe = queryByTitle( + xblockContainerIframeMessages.legacyEditModalIframeTitle.defaultMessage, + ); + expect(legacyXBlockEditModalIframe).not.toBeInTheDocument(); + }); + + axiosMock + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, { + ...courseUnitIndexMock, + has_changes: true, + published_by: userName, + }); + + await waitFor(() => { + const courseUnitSidebar = getByTestId('course-unit-sidebar'); + expect( + within(courseUnitSidebar).getByText(sidebarMessages.sidebarTitleDraftUnpublishedChanges.defaultMessage), + ).toBeInTheDocument(); + expect( + within(courseUnitSidebar).getByText(sidebarMessages.releaseStatusTitle.defaultMessage), + ).toBeInTheDocument(); + expect( + within(courseUnitSidebar).getByText(sidebarMessages.sidebarBodyNote.defaultMessage), + ).toBeInTheDocument(); + expect( + within(courseUnitSidebar).queryByRole('button', { + name: sidebarMessages.actionButtonPublishTitle.defaultMessage, + }), + ).toBeInTheDocument(); + }); + }); + + it('updates course unit sidebar after receiving refreshPositions message', async () => { + const { getByTitle, getByTestId } = render(); + + await waitFor(() => { + const xblocksIframe = getByTitle(xblockContainerIframeMessages.xblockIframeTitle.defaultMessage); + expect(xblocksIframe).toBeInTheDocument(); + simulatePostMessageEvent(messageTypes.refreshPositions); + }); + + axiosMock + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, { + ...courseUnitIndexMock, + has_changes: true, + published_by: userName, + }); + + await waitFor(() => { + const courseUnitSidebar = getByTestId('course-unit-sidebar'); + expect( + within(courseUnitSidebar).getByText(sidebarMessages.sidebarTitleDraftUnpublishedChanges.defaultMessage), + ).toBeInTheDocument(); + expect( + within(courseUnitSidebar).getByText(sidebarMessages.releaseStatusTitle.defaultMessage), + ).toBeInTheDocument(); + expect( + within(courseUnitSidebar).getByText(sidebarMessages.sidebarBodyNote.defaultMessage), + ).toBeInTheDocument(); + expect( + within(courseUnitSidebar).queryByRole('button', { + name: sidebarMessages.actionButtonPublishTitle.defaultMessage, + }), + ).toBeInTheDocument(); + }); + }); + it('checks whether xblock is removed when the corresponding delete button is clicked and the sidebar is the updated', async () => { const { getByTitle, getByText, queryByRole, getAllByRole, getByRole, diff --git a/src/course-unit/constants.js b/src/course-unit/constants.js index c46e1af416..2b03e789a0 100644 --- a/src/course-unit/constants.js +++ b/src/course-unit/constants.js @@ -69,4 +69,10 @@ export const messageTypes = { addXBlock: 'addXBlock', scrollToXBlock: 'scrollToXBlock', handleViewXBlockContent: 'handleViewXBlockContent', + editXBlock: 'editXBlock', + closeXBlockEditorModal: 'closeXBlockEditorModal', + saveEditedXBlockData: 'saveEditedXBlockData', + completeXBlockEditing: 'completeXBlockEditing', + studioAjaxError: 'studioAjaxError', + refreshPositions: 'refreshPositions', }; diff --git a/src/course-unit/data/thunk.js b/src/course-unit/data/thunk.js index 548511f1da..c6cdde3188 100644 --- a/src/course-unit/data/thunk.js +++ b/src/course-unit/data/thunk.js @@ -318,3 +318,20 @@ export function patchUnitItemQuery({ } }; } + +export function updateCourseUnitSidebar(itemId) { + return async (dispatch) => { + dispatch(updateSavingStatus({ status: RequestStatus.PENDING })); + dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.saving)); + + try { + const courseUnit = await getCourseUnitData(itemId); + dispatch(fetchCourseItemSuccess(courseUnit)); + dispatch(hideProcessingNotification()); + dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL })); + } catch (error) { + dispatch(hideProcessingNotification()); + handleResponseErrors(error, dispatch, updateSavingStatus); + } + }; +} diff --git a/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx b/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx index f4dd037308..b39d30d191 100644 --- a/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx +++ b/src/course-unit/xblock-container-iframe/hooks/tests/hooks.test.tsx @@ -175,34 +175,52 @@ describe('useLoadBearingHook', () => { }); describe('useMessageHandlers', () => { - it('calls handleScrollToXBlock after debounce delay', () => { - const mockHandleScrollToXBlock = jest.fn(); - const courseId = 'course-v1:Test+101+2025'; - const navigate = jest.fn(); - const dispatch = jest.fn(); - const setIframeOffset = jest.fn(); - const handleDeleteXBlock = jest.fn(); - const handleDuplicateXBlock = jest.fn(); - const handleManageXBlockAccess = jest.fn(); - - const { result } = renderHook(() => useMessageHandlers({ - courseId, - navigate, - dispatch, - setIframeOffset, - handleDeleteXBlock, - handleDuplicateXBlock, - handleScrollToXBlock: mockHandleScrollToXBlock, - handleManageXBlockAccess, - })); + let handlers; + let result; + + beforeEach(() => { + handlers = { + courseId: 'course-v1:Test+101+2025', + navigate: jest.fn(), + dispatch: jest.fn(), + setIframeOffset: jest.fn(), + handleDeleteXBlock: jest.fn(), + handleDuplicateXBlock: jest.fn(), + handleScrollToXBlock: jest.fn(), + handleManageXBlockAccess: jest.fn(), + handleShowLegacyEditXBlockModal: jest.fn(), + handleCloseLegacyEditorXBlockModal: jest.fn(), + handleSaveEditedXBlockData: jest.fn(), + handleFinishXBlockDragging: jest.fn(), + }; + + ({ result } = renderHook(() => useMessageHandlers(handlers))); + }); + it('calls handleScrollToXBlock after debounce delay', () => { act(() => { result.current[messageTypes.scrollToXBlock]({ scrollOffset: 200 }); }); jest.advanceTimersByTime(3000); - expect(mockHandleScrollToXBlock).toHaveBeenCalledTimes(1); - expect(mockHandleScrollToXBlock).toHaveBeenCalledWith(200); + expect(handlers.handleScrollToXBlock).toHaveBeenCalledTimes(1); + expect(handlers.handleScrollToXBlock).toHaveBeenCalledWith(200); + }); + + it.each([ + [messageTypes.editXBlock, { id: 'test-xblock-id' }, 'handleShowLegacyEditXBlockModal', 'test-xblock-id'], + [messageTypes.closeXBlockEditorModal, {}, 'handleCloseLegacyEditorXBlockModal', undefined], + [messageTypes.saveEditedXBlockData, {}, 'handleSaveEditedXBlockData', undefined], + [messageTypes.refreshPositions, {}, 'handleFinishXBlockDragging', undefined], + ])('calls %s with correct arguments', (messageType, payload, handlerKey, expectedArg) => { + act(() => { + result.current[messageType](payload); + }); + + expect(handlers[handlerKey]).toHaveBeenCalledTimes(1); + if (expectedArg !== undefined) { + expect(handlers[handlerKey]).toHaveBeenCalledWith(expectedArg); + } }); }); diff --git a/src/course-unit/xblock-container-iframe/hooks/types.ts b/src/course-unit/xblock-container-iframe/hooks/types.ts index c759f403f9..840d82dbfc 100644 --- a/src/course-unit/xblock-container-iframe/hooks/types.ts +++ b/src/course-unit/xblock-container-iframe/hooks/types.ts @@ -7,6 +7,10 @@ export type UseMessageHandlersTypes = { handleScrollToXBlock: (scrollOffset: number) => void; handleDuplicateXBlock: (blockType: string, usageId: string) => void; handleManageXBlockAccess: (usageId: string) => void; + handleShowLegacyEditXBlockModal: (id: string) => void; + handleCloseLegacyEditorXBlockModal: () => void; + handleSaveEditedXBlockData: () => void; + handleFinishXBlockDragging: () => void; }; export type MessageHandlersTypes = Record void>; diff --git a/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx b/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx index 8633e63e96..1c2879ec9a 100644 --- a/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx +++ b/src/course-unit/xblock-container-iframe/hooks/useMessageHandlers.tsx @@ -1,7 +1,9 @@ import { useMemo } from 'react'; import { debounce } from 'lodash'; +import { handleResponseErrors } from '../../../generic/saving-error-alert/utils'; import { copyToClipboard } from '../../../generic/data/thunks'; +import { updateSavingStatus } from '../../data/slice'; import { messageTypes } from '../../constants'; import { MessageHandlersTypes, UseMessageHandlersTypes } from './types'; @@ -20,16 +22,25 @@ export const useMessageHandlers = ({ handleDuplicateXBlock, handleScrollToXBlock, handleManageXBlockAccess, + handleShowLegacyEditXBlockModal, + handleCloseLegacyEditorXBlockModal, + handleSaveEditedXBlockData, + handleFinishXBlockDragging, }: UseMessageHandlersTypes): MessageHandlersTypes => useMemo(() => ({ [messageTypes.copyXBlock]: ({ usageId }) => dispatch(copyToClipboard(usageId)), [messageTypes.deleteXBlock]: ({ usageId }) => handleDeleteXBlock(usageId), [messageTypes.newXBlockEditor]: ({ blockType, usageId }) => navigate(`/course/${courseId}/editor/${blockType}/${usageId}`), [messageTypes.duplicateXBlock]: ({ blockType, usageId }) => handleDuplicateXBlock(blockType, usageId), [messageTypes.manageXBlockAccess]: ({ usageId }) => handleManageXBlockAccess(usageId), - [messageTypes.scrollToXBlock]: debounce(({ scrollOffset }) => handleScrollToXBlock(scrollOffset), 3000), + [messageTypes.scrollToXBlock]: debounce(({ scrollOffset }) => handleScrollToXBlock(scrollOffset), 1000), [messageTypes.toggleCourseXBlockDropdown]: ({ courseXBlockDropdownHeight, }: { courseXBlockDropdownHeight: number }) => setIframeOffset(courseXBlockDropdownHeight), + [messageTypes.editXBlock]: ({ id }) => handleShowLegacyEditXBlockModal(id), + [messageTypes.closeXBlockEditorModal]: handleCloseLegacyEditorXBlockModal, + [messageTypes.saveEditedXBlockData]: handleSaveEditedXBlockData, + [messageTypes.studioAjaxError]: ({ error }) => handleResponseErrors(error, dispatch, updateSavingStatus), + [messageTypes.refreshPositions]: handleFinishXBlockDragging, }), [ courseId, handleDeleteXBlock, diff --git a/src/course-unit/xblock-container-iframe/index.tsx b/src/course-unit/xblock-container-iframe/index.tsx index d0a8528378..6e6df33abe 100644 --- a/src/course-unit/xblock-container-iframe/index.tsx +++ b/src/course-unit/xblock-container-iframe/index.tsx @@ -8,17 +8,20 @@ import { useNavigate } from 'react-router-dom'; import DeleteModal from '../../generic/delete-modal/DeleteModal'; import ConfigureModal from '../../generic/configure-modal/ConfigureModal'; +import ModalIframe from '../../generic/modal-iframe'; import { IFRAME_FEATURE_POLICY } from '../../constants'; import supportedEditors from '../../editors/supportedEditors'; import { useIframe } from '../context/hooks'; +import { updateCourseUnitSidebar } from '../data/thunk'; import { useMessageHandlers, useIframeContent, useIframeMessages, useIFrameBehavior, } from './hooks'; -import { formatAccessManagedXBlockData, getIframeUrl } from './utils'; +import { formatAccessManagedXBlockData, getIframeUrl, getLegacyEditModalUrl } from './utils'; import messages from './messages'; +import { messageTypes } from '../constants'; import { XBlockContainerIframeProps, @@ -39,10 +42,12 @@ const XBlockContainerIframe: FC = ({ const [iframeOffset, setIframeOffset] = useState(0); const [deleteXBlockId, setDeleteXBlockId] = useState(null); const [configureXBlockId, setConfigureXBlockId] = useState(null); + const [showLegacyEditModal, setShowLegacyEditModal] = useState(false); const iframeUrl = useMemo(() => getIframeUrl(blockId), [blockId]); + const legacyEditModalUrl = useMemo(() => getLegacyEditModalUrl(configureXBlockId), [configureXBlockId]); - const { setIframeRef } = useIframe(); + const { setIframeRef, sendMessageToIframe } = useIframe(); const { iframeHeight } = useIFrameBehavior({ id: blockId, iframeUrl }); useIframeContent(iframeRef, setIframeRef); @@ -96,6 +101,25 @@ const XBlockContainerIframe: FC = ({ }); }; + const handleShowLegacyEditXBlockModal = (id: string) => { + setConfigureXBlockId(id); + setShowLegacyEditModal(true); + }; + + const handleCloseLegacyEditorXBlockModal = () => { + setConfigureXBlockId(null); + setShowLegacyEditModal(false); + }; + + const handleSaveEditedXBlockData = () => { + sendMessageToIframe(messageTypes.completeXBlockEditing, { locator: configureXBlockId }); + dispatch(updateCourseUnitSidebar(blockId)); + }; + + const handleFinishXBlockDragging = () => { + dispatch(updateCourseUnitSidebar(blockId)); + }; + const messageHandlers = useMessageHandlers({ courseId, navigate, @@ -105,12 +129,22 @@ const XBlockContainerIframe: FC = ({ handleDuplicateXBlock, handleManageXBlockAccess, handleScrollToXBlock, + handleShowLegacyEditXBlockModal, + handleCloseLegacyEditorXBlockModal, + handleSaveEditedXBlockData, + handleFinishXBlockDragging, }); useIframeMessages(messageHandlers); return ( <> + {showLegacyEditModal && ( + + )} `${getConfig().STUDIO_BASE_URL}/container_embed/${blockId}`; + +/** + * Generates the legacy edit modal URL for the given block ID. + * + * @param {string | null} blockId - The unique identifier of the block. + * + * @returns {string} - The generated URL for editing the XBlock in the legacy modal. + */ +export const getLegacyEditModalUrl = ( + blockId: string | null, +): string => (blockId ? `${getConfig().STUDIO_BASE_URL}/xblock/${blockId}/action/edit` : ''); diff --git a/src/generic/modal-iframe/ModalIframe.test.tsx b/src/generic/modal-iframe/ModalIframe.test.tsx new file mode 100644 index 0000000000..6ff63b3427 --- /dev/null +++ b/src/generic/modal-iframe/ModalIframe.test.tsx @@ -0,0 +1,34 @@ +import { render } from '@testing-library/react'; + +import { IFRAME_FEATURE_POLICY } from '../../constants'; +import ModalIframe, { SANDBOX_OPTIONS } from '.'; + +describe('ModalIframe Component', () => { + const title = 'Legacy Edit Modal'; + const src = 'edit/xblock'; + + it('renders without crashing', async () => { + const { getByRole } = render(); + + expect(getByRole('dialog')).toBeInTheDocument(); + }); + + it('renders iframe with correct src', () => { + const { getByTitle } = render(); + + const iframe = getByTitle(title); + expect(iframe).toBeInTheDocument(); + expect(iframe).toHaveAttribute('src', src); + expect(iframe).toHaveAttribute('allow', IFRAME_FEATURE_POLICY); + expect(iframe).toHaveAttribute('class', 'modal-iframe'); + expect(iframe).toHaveAttribute('referrerpolicy', 'origin'); + expect(iframe).toHaveAttribute('sandbox', SANDBOX_OPTIONS); + expect(iframe).toHaveAttribute('scrolling', 'no'); + }); + + it('does not render when showLegacyEditModal is false', () => { + const { container } = render(); + + expect(container.firstChild).not.toBeNull(); + }); +}); diff --git a/src/generic/modal-iframe/index.scss b/src/generic/modal-iframe/index.scss new file mode 100644 index 0000000000..dd462ffa50 --- /dev/null +++ b/src/generic/modal-iframe/index.scss @@ -0,0 +1,13 @@ +.modal-iframe { + position: fixed; + top: 0; + left: 0; + width: 100%; + height: 100%; + z-index: $zindex-modal; + + iframe { + width: inherit; + height: inherit; + } +} diff --git a/src/generic/modal-iframe/index.tsx b/src/generic/modal-iframe/index.tsx new file mode 100644 index 0000000000..f6a57155c6 --- /dev/null +++ b/src/generic/modal-iframe/index.tsx @@ -0,0 +1,43 @@ +import { forwardRef, ForwardedRef, IframeHTMLAttributes } from 'react'; +import classNames from 'classnames'; + +import { IFRAME_FEATURE_POLICY } from '../../constants'; + +interface ModalIframeProps extends IframeHTMLAttributes { + title: string; + className?: string; +} + +export const SANDBOX_OPTIONS = [ + 'allow-forms', + 'allow-modals', + 'allow-popups', + 'allow-popups-to-escape-sandbox', + 'allow-presentation', + 'allow-same-origin', + 'allow-scripts', + 'allow-top-navigation-by-user-activation', +].join(' '); + +const ModalIframe = forwardRef( + ({ title, className, ...props }, ref: ForwardedRef) => ( +