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.
openedx · Feb 20, 2025 · 7e4ecff · 7e4ecff
1 parent 2befd82
commit 7e4ecff
Show file tree

Hide file tree

Showing 15 changed files with 353 additions and 25 deletions.
diff --git 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 ([email protected]).
+ */
 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
@@ -233,6 +233,126 @@ describe('<CourseUnit />', () => {
     });
   });
 
+  it('displays an error alert when a studioAjaxError message is received', async () => {
+    const { getByTitle, getByTestId } = render(<RootWrapper />);
+
+    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(<RootWrapper />);
+
+    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(<RootWrapper />);
+
+    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(<RootWrapper />);
+
+    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(<RootWrapper />);
+
+    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
@@ -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
@@ -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
@@ -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
@@ -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<string, (payload: any) => void>;

diff --git 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
@@ -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<XBlockContainerIframeProps> = ({
   const [iframeOffset, setIframeOffset] = useState(0);
   const [deleteXBlockId, setDeleteXBlockId] = useState<string | null>(null);
   const [configureXBlockId, setConfigureXBlockId] = useState<string | null>(null);
+  const [showLegacyEditModal, setShowLegacyEditModal] = useState<boolean>(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<XBlockContainerIframeProps> = ({
     });
   };
 
+  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<XBlockContainerIframeProps> = ({
     handleDuplicateXBlock,
     handleManageXBlockAccess,
     handleScrollToXBlock,
+    handleShowLegacyEditXBlockModal,
+    handleCloseLegacyEditorXBlockModal,
+    handleSaveEditedXBlockData,
+    handleFinishXBlockDragging,
   });
 
   useIframeMessages(messageHandlers);
 
   return (
     <>
+      {showLegacyEditModal && (
+        <ModalIframe
+          title={intl.formatMessage(messages.legacyEditModalIframeTitle)}
+          src={legacyEditModalUrl}
+        />
+      )}
       <DeleteModal
         category="component"
         isOpen={isDeleteModalOpen}

diff --git a/src/course-unit/xblock-container-iframe/messages.ts b/src/course-unit/xblock-container-iframe/messages.ts
@@ -6,6 +6,11 @@ const messages = defineMessages({
     defaultMessage: 'Course unit iframe',
     description: 'Title for the xblock iframe',
   },
+  legacyEditModalIframeTitle: {
+    id: 'course-authoring.course-unit.legacy.modal.xblock-edit.iframe.title',
+    defaultMessage: 'Legacy xBlock edit modal',
+    description: 'Title for the legacy xblock edit modal iframe',
+  },
   xblockIframeLabel: {
     id: 'course-authoring.course-unit.xblock.iframe.label',
     defaultMessage: '{xblockCount} xBlocks inside the frame',