refactor: move 'isMarkdownEditorEnabledForContext' into EditorContext (#2398)

This just moves one single state variable, `isMarkdownEditorEnabledForCourse` out of the Redux state and into the `EditorContext`.

Author: bradenmacdonald

src/editors/Editor.tsx

@@ -5,8 +5,6 @@ import { useDispatch } from 'react-redux';
 
 import * as hooks from './hooks';
 
-import { useWaffleFlags } from '../data/apiHooks';
-import { isCourseKey } from '../generic/key-utils';
 import supportedEditors from './supportedEditors';
 import type { EditorComponent } from './EditorComponent';
 import AdvancedEditor from './AdvancedEditor';
@@ -28,15 +26,12 @@ const Editor: React.FC<Props> = ({
   onClose = null,
   returnFunction = null,
 }) => {
-  const courseIdIfCourse = isCourseKey(learningContextId) ? learningContextId : undefined;
-  const isMarkdownEditorEnabledForCourse = useWaffleFlags(courseIdIfCourse).useReactMarkdownEditor;
   const dispatch = useDispatch();
   const loading = hooks.useInitializeApp({
     dispatch,
     data: {
       blockId,
       blockType,
-      isMarkdownEditorEnabledForCourse,
       learningContextId,
       lmsEndpointUrl,
       studioEndpointUrl,

src/editors/EditorContext.tsx

@@ -1,3 +1,5 @@
+import { useWaffleFlags } from '@src/data/apiHooks';
+import { isCourseKey } from '@src/generic/key-utils';
 import React from 'react';
 
 /**
@@ -6,9 +8,19 @@ import React from 'react';
  * Note: we're in the process of moving things from redux into this.
  */
 export interface EditorContext {
+  /**
+   * The ID of the current course or library.
+   * Use `isCourseKey()` from '@src/generic/key-utils' if you need to check what type it is.
+   */
   learningContextId: string;
+  /** Is the so-called "Markdown" problem editor available in this learning context? */
+  isMarkdownEditorEnabledForContext: boolean;
 }
 
+export type EditorContextInit = {
+  learningContextId: string;
+};
+
 const context = React.createContext<EditorContext | undefined>(undefined);
 
 /** Hook to get the editor context (shared context) */
@@ -21,10 +33,20 @@ export function useEditorContext() {
   return ctx;
 }
 
-export const EditorContextProvider: React.FC<{
-  children: React.ReactNode,
-  learningContextId: string;
-}> = ({ children, ...contextData }) => {
-  const ctx: EditorContext = React.useMemo(() => ({ ...contextData }), []);
+export const EditorContextProvider: React.FC<{ children: React.ReactNode; } & EditorContextInit> = ({
+  children,
+  learningContextId,
+}) => {
+  const courseIdIfCourse = isCourseKey(learningContextId) ? learningContextId : undefined;
+  const isMarkdownEditorEnabledForContext = useWaffleFlags(courseIdIfCourse).useReactMarkdownEditor;
+
+  const ctx: EditorContext = React.useMemo(() => ({
+    learningContextId,
+    isMarkdownEditorEnabledForContext,
+  }), [
+    // Dependencies - make sure we update the context object if any of these values change:
+    learningContextId,
+    isMarkdownEditorEnabledForContext,
+  ]);
   return <context.Provider value={ctx}>{children}</context.Provider>;
 };

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx

@@ -1,11 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { FormattedMessage } from '@edx/frontend-platform/i18n';
-import { connect } from 'react-redux';
+import { connect, useSelector } from 'react-redux';
 import {
   Button, Collapsible,
 } from '@openedx/paragon';
-import { selectors, actions } from '../../../../../data/redux';
+import { useEditorContext } from '@src/editors/EditorContext';
+import { selectors, actions } from '@src/editors/data/redux';
 import ScoringCard from './settingsComponents/ScoringCard';
 import ShowAnswerCard from './settingsComponents/ShowAnswerCard';
 import HintsCard from './settingsComponents/HintsCard';
@@ -38,9 +39,13 @@ const SettingsWidget = ({
   defaultSettings,
   images,
   isLibrary,
-  learningContextId,
-  showMarkdownEditorButton,
 }) => {
+  const {
+    learningContextId,
+    isMarkdownEditorEnabledForContext,
+  } = useEditorContext();
+  const rawMarkdown = useSelector(selectors.problem.rawMarkdown);
+  const showMarkdownEditorButton = isMarkdownEditorEnabledForContext && rawMarkdown;
   const { isAdvancedCardsVisible, showAdvancedCards } = showAdvancedSettingsCards();
   const feedbackCard = () => {
     if ([ProblemTypeKeys.MULTISELECT].includes(problemType)) {
@@ -199,11 +204,9 @@ SettingsWidget.propTypes = {
     rerandomize: PropTypes.string,
   }).isRequired,
   images: PropTypes.shape({}).isRequired,
-  learningContextId: PropTypes.string.isRequired,
   isLibrary: PropTypes.bool.isRequired,
   // eslint-disable-next-line
   settings: PropTypes.any.isRequired,
-  showMarkdownEditorButton: PropTypes.bool.isRequired,
 };
 
 const mapStateToProps = (state) => ({
@@ -215,9 +218,6 @@ const mapStateToProps = (state) => ({
   defaultSettings: selectors.problem.defaultSettings(state),
   images: selectors.app.images(state),
   isLibrary: selectors.app.isLibrary(state),
-  learningContextId: selectors.app.learningContextId(state),
-  showMarkdownEditorButton: selectors.app.isMarkdownEditorEnabledForCourse(state)
-  && selectors.problem.rawMarkdown(state),
 });
 
 export const mapDispatchToProps = {

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.test.tsx

@@ -1,9 +1,9 @@
 import React from 'react';
 
 import { ProblemTypeKeys } from '@src/editors/data/constants/problem';
-import {
-  render, screen, initializeMocks,
-} from '@src/testUtils';
+import { screen, initializeMocks } from '@src/testUtils';
+import { editorRender } from '@src/editors/editorTestRender';
+import { mockWaffleFlags } from '@src/data/apiHooks.mock';
 import * as hooks from './hooks';
 import { SettingsWidgetInternal as SettingsWidget } from '.';
 
@@ -17,6 +17,7 @@ jest.mock('./settingsComponents/ShowAnswerCard', () => 'ShowAnswerCard');
 jest.mock('./settingsComponents/SwitchEditorCard', () => 'SwitchEditorCard');
 jest.mock('./settingsComponents/TimerCard', () => 'TimerCard');
 jest.mock('./settingsComponents/TypeCard', () => 'TypeCard');
+mockWaffleFlags();
 
 describe('SettingsWidget', () => {
   const showAdvancedSettingsCardsBaseProps = {
@@ -55,15 +56,15 @@ describe('SettingsWidget', () => {
   describe('behavior', () => {
     it('calls showAdvancedSettingsCards when initialized', () => {
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsBaseProps);
-      render(<SettingsWidget {...props} />);
+      editorRender(<SettingsWidget {...props} />);
       expect(hooks.showAdvancedSettingsCards).toHaveBeenCalled();
     });
   });
 
   describe('renders', () => {
     test('renders Settings widget page', () => {
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsBaseProps);
-      render(<SettingsWidget {...props} />);
+      editorRender(<SettingsWidget {...props} />);
       expect(screen.getByText('Show advanced settings')).toBeInTheDocument();
     });
 
@@ -73,7 +74,7 @@ describe('SettingsWidget', () => {
         isAdvancedCardsVisible: true,
       };
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
-      const { container } = render(<SettingsWidget {...props} />);
+      const { container } = editorRender(<SettingsWidget {...props} />);
       expect(screen.queryByText('Show advanced settings')).not.toBeInTheDocument();
       expect(container.querySelector('showanswercard')).toBeInTheDocument();
       expect(container.querySelector('resetcard')).toBeInTheDocument();
@@ -85,7 +86,7 @@ describe('SettingsWidget', () => {
         isAdvancedCardsVisible: true,
       };
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
-      const { container } = render(
+      const { container } = editorRender(
         <SettingsWidget {...props} problemType={ProblemTypeKeys.ADVANCED} />,
       );
       expect(container.querySelector('randomization')).toBeInTheDocument();
@@ -99,7 +100,7 @@ describe('SettingsWidget', () => {
     };
     test('renders Settings widget page', () => {
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsBaseProps);
-      const { container } = render(<SettingsWidget {...libraryProps} />);
+      const { container } = editorRender(<SettingsWidget {...libraryProps} />);
       expect(container.querySelector('timercard')).not.toBeInTheDocument();
       expect(container.querySelector('resetcard')).not.toBeInTheDocument();
       expect(container.querySelector('typecard')).toBeInTheDocument();
@@ -113,7 +114,7 @@ describe('SettingsWidget', () => {
         isAdvancedCardsVisible: true,
       };
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
-      const { container } = render(<SettingsWidget {...libraryProps} />);
+      const { container } = editorRender(<SettingsWidget {...libraryProps} />);
       expect(screen.queryByText('Show advanced settings')).not.toBeInTheDocument();
       expect(container.querySelector('showanswearscard')).not.toBeInTheDocument();
       expect(container.querySelector('resetcard')).not.toBeInTheDocument();
@@ -127,7 +128,7 @@ describe('SettingsWidget', () => {
         isAdvancedCardsVisible: true,
       };
       jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
-      const { container } = render(<SettingsWidget {...libraryProps} problemType={ProblemTypeKeys.ADVANCED} />);
+      const { container } = editorRender(<SettingsWidget {...libraryProps} problemType={ProblemTypeKeys.ADVANCED} />);
       expect(container.querySelector('randomization')).toBeInTheDocument();
     });
   });

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.jsx

@@ -1,24 +1,27 @@
 import React from 'react';
-import { connect } from 'react-redux';
+import { useDispatch, useSelector } from 'react-redux';
 import { FormattedMessage } from '@edx/frontend-platform/i18n';
 import { Card } from '@openedx/paragon';
 import PropTypes from 'prop-types';
+import { useEditorContext } from '@src/editors/EditorContext';
+import { selectors, thunkActions } from '@src/editors/data/redux';
+import BaseModal from '@src/editors/sharedComponents/BaseModal';
+import Button from '@src/editors/sharedComponents/Button';
+import { ProblemTypeKeys } from '@src/editors/data/constants/problem';
 import messages from '../messages';
-import { selectors, thunkActions } from '../../../../../../data/redux';
-import BaseModal from '../../../../../../sharedComponents/BaseModal';
-import Button from '../../../../../../sharedComponents/Button';
 import { handleConfirmEditorSwitch } from '../hooks';
-import { ProblemTypeKeys } from '../../../../../../data/constants/problem';
 
 const SwitchEditorCard = ({
   editorType,
   problemType,
-  switchEditor,
-  isMarkdownEditorEnabled,
 }) => {
   const [isConfirmOpen, setConfirmOpen] = React.useState(false);
+  const { isMarkdownEditorEnabledForContext } = useEditorContext();
+  const isMarkdownEditorEnabled = useSelector(selectors.problem.isMarkdownEditorEnabled);
+  const dispatch = useDispatch();
 
-  if (isMarkdownEditorEnabled || problemType === ProblemTypeKeys.ADVANCED) { return null; }
+  const isMarkdownEditorActive = isMarkdownEditorEnabled && isMarkdownEditorEnabledForContext;
+  if (isMarkdownEditorActive || problemType === ProblemTypeKeys.ADVANCED) { return null; }
 
   return (
     <Card className="border border-light-700 shadow-none">
@@ -29,7 +32,10 @@ const SwitchEditorCard = ({
         confirmAction={(
           <Button
             /* istanbul ignore next */
-            onClick={() => handleConfirmEditorSwitch({ switchEditor: () => switchEditor(editorType), setConfirmOpen })}
+            onClick={() => handleConfirmEditorSwitch({
+              switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType)),
+              setConfirmOpen,
+            })}
             variant="primary"
           >
             <FormattedMessage {...messages[`ConfirmSwitchButtonLabel-${editorType}`]} />
@@ -52,19 +58,8 @@ const SwitchEditorCard = ({
 };
 
 SwitchEditorCard.propTypes = {
-  switchEditor: PropTypes.func.isRequired,
-  isMarkdownEditorEnabled: PropTypes.bool.isRequired,
   problemType: PropTypes.string.isRequired,
   editorType: PropTypes.string.isRequired,
 };
 
-export const mapStateToProps = (state) => ({
-  isMarkdownEditorEnabled: selectors.problem.isMarkdownEditorEnabled(state)
-     && selectors.app.isMarkdownEditorEnabledForCourse(state),
-});
-export const mapDispatchToProps = {
-  switchEditor: thunkActions.problem.switchEditor,
-};
-
-export const SwitchEditorCardInternal = SwitchEditorCard; // For testing only
-export default connect(mapStateToProps, mapDispatchToProps)(SwitchEditorCard);
+export default SwitchEditorCard;

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.test.tsx

@@ -1,58 +1,77 @@
 import React from 'react';
-import {
-  render, screen, initializeMocks, fireEvent,
-} from '@src/testUtils';
-import { SwitchEditorCardInternal as SwitchEditorCard, mapDispatchToProps } from './SwitchEditorCard';
-import { thunkActions } from '../../../../../../data/redux';
-
-describe('SwitchEditorCard', () => {
-  const mockSwitchEditor = jest.fn().mockName('switchEditor');
+import { screen, initializeMocks, within } from '@src/testUtils';
+import userEvent from '@testing-library/user-event';
+import { editorRender } from '@src/editors/editorTestRender';
+import { mockWaffleFlags } from '@src/data/apiHooks.mock';
+import { thunkActions } from '@src/editors/data/redux';
+import SwitchEditorCard from './SwitchEditorCard';
+
+const switchEditorSpy = jest.spyOn(thunkActions.problem, 'switchEditor');
+
+describe('SwitchEditorCard - markdown', () => {
   const baseProps = {
-    switchEditor: mockSwitchEditor,
     problemType: 'stringresponse',
     editorType: 'markdown',
-    isMarkdownEditorEnabled: false,
   };
 
   beforeEach(() => {
     initializeMocks();
   });
 
-  test('renders SwitchEditorCard', () => {
-    render(<SwitchEditorCard {...baseProps} />);
+  test('renders SwitchEditorCard', async () => {
+    // Markdown Editor support is on for this course:
+    mockWaffleFlags({ useReactMarkdownEditor: true });
+    // The markdown editor is not currently active (default)
+
+    editorRender(<SwitchEditorCard {...baseProps} />);
+    const user = userEvent.setup();
     expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
     const switchButton = screen.getByRole('button', { name: 'Switch to markdown editor' });
     expect(switchButton).toBeInTheDocument();
-    fireEvent.click(switchButton);
+    await user.click(switchButton);
+    // A confirmation dialog is shown:
     expect(screen.getByRole('dialog')).toBeInTheDocument();
   });
 
-  test('calls switchEditor function when confirm button is clicked', () => {
-    render(<SwitchEditorCard {...baseProps} />);
+  test('calls switchEditor function when confirm button is clicked', async () => {
+    // Markdown Editor support is on for this course:
+    mockWaffleFlags({ useReactMarkdownEditor: true });
+    // The markdown editor is not currently active (default)
+
+    editorRender(<SwitchEditorCard {...baseProps} />);
+    const user = userEvent.setup();
     const switchButton = screen.getByRole('button', { name: 'Switch to markdown editor' });
     expect(switchButton).toBeInTheDocument();
-    fireEvent.click(switchButton);
-    const confirmButton = screen.getByRole('button', { name: 'Switch to markdown editor' });
+    await user.click(switchButton);
+
+    const modal = screen.getByRole('dialog');
+    const confirmButton = within(modal).getByRole('button', { name: 'Switch to markdown editor' });
     expect(confirmButton).toBeInTheDocument();
-    fireEvent.click(confirmButton);
-    expect(mockSwitchEditor).toHaveBeenCalledWith('markdown');
+    expect(switchEditorSpy).not.toHaveBeenCalled();
+    await user.click(confirmButton);
+    expect(switchEditorSpy).toHaveBeenCalledWith('markdown');
+    // Markdown editor would now be active.
   });
 
   test('renders nothing for advanced problemType', () => {
-    const { container } = render(<SwitchEditorCard {...baseProps} problemType="advanced" />);
+    const { container } = editorRender(<SwitchEditorCard {...baseProps} problemType="advanced" />);
     const reduxWrapper = (container.firstChild as HTMLElement | null);
     expect(reduxWrapper?.innerHTML).toBe('');
   });
 
-  test('snapshot: SwitchEditorCard returns null when editor is Markdown', () => {
-    const { container } = render(<SwitchEditorCard {...baseProps} editorType="markdown" isMarkdownEditorEnabled />);
-    const reduxWrapper = (container.firstChild as HTMLElement | null);
-    expect(reduxWrapper?.innerHTML).toBe('');
-  });
+  test('returns null when editor is already Markdown', () => {
+    // Markdown Editor support is on for this course:
+    mockWaffleFlags({ useReactMarkdownEditor: true });
+    // The markdown editor *IS* currently active (default)
 
-  describe('mapDispatchToProps', () => {
-    test('updateField from actions.problem.updateField', () => {
-      expect(mapDispatchToProps.switchEditor).toEqual(thunkActions.problem.switchEditor);
+    const { container } = editorRender(<SwitchEditorCard {...baseProps} />, {
+      initialState: {
+        problem: {
+          isMarkdownEditorEnabled: true,
+        },
+      },
     });
+    const reduxWrapper = (container.firstChild as HTMLElement | null);
+    expect(reduxWrapper?.innerHTML).toBe('');
   });
 });