diff --git a/src/course-unit/unit-sidebar/UnitSidebarContext.tsx b/src/course-unit/unit-sidebar/UnitSidebarContext.tsx index 4069a805e..bb8b75cf8 100644 --- a/src/course-unit/unit-sidebar/UnitSidebarContext.tsx +++ b/src/course-unit/unit-sidebar/UnitSidebarContext.tsx @@ -94,9 +94,11 @@ export const UnitSidebarProvider = ({ ); }; -export function useUnitSidebarContext(): UnitSidebarContextData { +export function useUnitSidebarContext(raiseError?: true): UnitSidebarContextData; +export function useUnitSidebarContext(raiseError?: boolean): UnitSidebarContextData | undefined; +export function useUnitSidebarContext(raiseError: boolean = true): UnitSidebarContextData | undefined { const ctx = useContext(UnitSidebarContext); - if (ctx === undefined) { + if (ctx === undefined && raiseError) { /* istanbul ignore next */ throw new Error('useUnitSidebarContext() was used in a component without a ancestor.'); } diff --git a/src/course-unit/xblock-container-iframe/index.tsx b/src/course-unit/xblock-container-iframe/index.tsx index c24872969..6ee3335a4 100644 --- a/src/course-unit/xblock-container-iframe/index.tsx +++ b/src/course-unit/xblock-container-iframe/index.tsx @@ -58,7 +58,7 @@ const XBlockContainerIframe: FC = ({ const { setCurrentPageKey, setSelectedComponentId, - } = useUnitSidebarContext(); + } = useUnitSidebarContext(!readonly) || {}; // Useful to reload iframe const [iframeKey, setIframeKey] = useState(0); @@ -138,7 +138,7 @@ const XBlockContainerIframe: FC = ({ const onDeleteSubmit = async () => { if (deleteXBlockId) { await unitXBlockActions.handleDelete(deleteXBlockId); - setSelectedComponentId(undefined); + setSelectedComponentId?.(undefined); closeDeleteModal(); } }; @@ -192,7 +192,7 @@ const XBlockContainerIframe: FC = ({ const handleOpenManageTagsModal = (id: string) => { if (isUnitPageNewDesignEnabled()) { - setCurrentPageKey('align', id); + setCurrentPageKey?.('align', id); sendMessageToIframe(messageTypes.selectXblock, { locator: id }); } else { // Legacy manage tags modal @@ -219,7 +219,7 @@ const XBlockContainerIframe: FC = ({ }; const handleXBlockSelected = (id) => { - setCurrentPageKey('info', id); + setCurrentPageKey?.('info', id); }; const messageHandlers = useMessageHandlers({ diff --git a/src/data/apiHooks.test.tsx b/src/data/apiHooks.test.tsx index b5857d8f6..25538a800 100644 --- a/src/data/apiHooks.test.tsx +++ b/src/data/apiHooks.test.tsx @@ -29,20 +29,20 @@ describe('useWaffleFlags', () => { axiosMock.onGet(getApiWaffleFlagsUrl()).reply(() => promise); render(); - expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading'); - expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + expect(await screen.findByLabelText('isLoading')).toHaveTextContent('loading'); + expect(await screen.findByLabelText('isError')).toHaveTextContent('false'); // The default should be enabled, even before we hear back from the server: - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); // Then, the server responds with a new value: resolveResponse([200, { useNewCourseOutlinePage: false }]); // Now, we're no longer loading and we have the new value: - await waitFor(() => { - expect(screen.getByLabelText('isLoading')).toHaveTextContent('false'); + await waitFor(async () => { + expect(await screen.findByLabelText('isLoading')).toHaveTextContent('false'); }); - expect(screen.getByLabelText('isError')).toHaveTextContent('false'); - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); + expect(await screen.findByLabelText('isError')).toHaveTextContent('false'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); }); it('uses the default values if there\'s an error', async () => { @@ -53,20 +53,20 @@ describe('useWaffleFlags', () => { axiosMock.onGet(getApiWaffleFlagsUrl()).reply(() => promise); render(); - expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading'); - expect(screen.getByLabelText('isError')).toHaveTextContent('false'); + expect(await screen.findByLabelText('isLoading')).toHaveTextContent('loading'); + expect(await screen.findByLabelText('isError')).toHaveTextContent('false'); // The default should be enabled, even before we hear back from the server: - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); // Then, the server responds with an error resolveResponse([500, {}]); // Now, we're no longer loading, we have an error state, and we still have the default value: - await waitFor(() => { - expect(screen.getByLabelText('isLoading')).toHaveTextContent('false'); + await waitFor(async () => { + expect(await screen.findByLabelText('isLoading')).toHaveTextContent('false'); }); - expect(screen.getByLabelText('isError')).toHaveTextContent('error'); - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + expect(await screen.findByLabelText('isError')).toHaveTextContent('error'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); }); it('uses the global flag values while loading the course-specific flags', async () => { @@ -81,9 +81,9 @@ describe('useWaffleFlags', () => { // Check the global flag: render(); - await waitFor(() => { + await waitFor(async () => { // Once it loads the flags from the server, the global 'false' value will override the default 'true': - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); }); // Now check the course-specific flag: @@ -91,16 +91,16 @@ describe('useWaffleFlags', () => { render(); // Now, the course-specific value is loading but in the meantime we use the global default: - expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading'); - expect(screen.getByLabelText('isError')).toHaveTextContent('false'); - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); + expect(await screen.findByLabelText('isLoading')).toHaveTextContent('loading'); + expect(await screen.findByLabelText('isError')).toHaveTextContent('false'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled'); // Now the server responds: the course-specific flag is ON: resolveResponse([200, { useNewCourseOutlinePage: true }]); - await waitFor(() => { - expect(screen.getByLabelText('isLoading')).toHaveTextContent('false'); + await waitFor(async () => { + expect(await screen.findByLabelText('isLoading')).toHaveTextContent('false'); }); - expect(screen.getByLabelText('isError')).toHaveTextContent('false'); - expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); + expect(await screen.findByLabelText('isError')).toHaveTextContent('false'); + expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled'); }); });