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

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

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,7 +56,7 @@ 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();
     });
   });
@@ -63,7 +64,7 @@ describe('SettingsWidget', () => {
   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';
+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';
 
-describe('SwitchEditorCard', () => {
-  const mockSwitchEditor = jest.fn().mockName('switchEditor');
+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('');
   });
 });

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

@@ -10,6 +10,7 @@ import {
 } from '@openedx/paragon';
 
 import PropTypes from 'prop-types';
+import { useEditorContext } from '@src/editors/EditorContext';
 import AnswerWidget from './AnswerWidget';
 import SettingsWidget from './SettingsWidget';
 import QuestionWidget from './QuestionWidget';
@@ -41,9 +42,9 @@ const EditProblemView = ({ returnFunction }) => {
   const isDirty = useSelector(selectors.problem.isDirty);
 
   const isMarkdownEditorEnabledSelector = useSelector(selectors.problem.isMarkdownEditorEnabled);
-  const isMarkdownEditorEnabledForCourse = useSelector(selectors.app.isMarkdownEditorEnabledForCourse);
+  const { isMarkdownEditorEnabledForContext } = useEditorContext();
 
-  const isMarkdownEditorEnabled = isMarkdownEditorEnabledSelector && isMarkdownEditorEnabledForCourse;
+  const isMarkdownEditorEnabled = isMarkdownEditorEnabledSelector && isMarkdownEditorEnabledForContext;
 
   const isAdvancedProblemType = problemType === ProblemTypeKeys.ADVANCED;
 

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

@@ -43,7 +43,6 @@ jest.mock('./hooks', () => ({
 const initialState: PartialEditorState = {
   app: {
     lmsEndpointUrl: null,
-    isMarkdownEditorEnabledForCourse: false,
   },
   problem: {
     problemType: null,
@@ -89,7 +88,6 @@ describe('EditProblemView', () => {
     const modifiedInitialState: PartialEditorState = {
       app: {
         lmsEndpointUrl: null,
-        isMarkdownEditorEnabledForCourse: true,
       },
       problem: {
         problemType: null,

src/editors/data/redux/app/reducer.test.js

@@ -21,7 +21,6 @@ describe('app reducer', () => {
           blockId: 'anID',
           learningContextId: 'OTHERid',
           blockType: 'someTYPE',
-          isMarkdownEditorEnabledForCourse: true,
         };
         expect(reducer(
           testingState,

src/editors/data/redux/app/reducer.ts

@@ -21,7 +21,6 @@ const initialState: EditorState['app'] = {
   videos: {},
   courseDetails: {},
   showRawEditor: false,
-  isMarkdownEditorEnabledForCourse: false,
 };
 
 // eslint-disable-next-line no-unused-vars
@@ -36,7 +35,6 @@ const app = createSlice({
       blockId: payload.blockId,
       learningContextId: payload.learningContextId,
       blockType: payload.blockType,
-      isMarkdownEditorEnabledForCourse: payload.isMarkdownEditorEnabledForCourse,
       blockValue: null,
     }),
     setUnitUrl: (state, { payload }) => ({ ...state, unitUrl: payload }),

src/editors/data/redux/app/selectors.test.ts

@@ -48,7 +48,6 @@ describe('app selectors unit tests', () => {
         simpleKeys.images,
         simpleKeys.videos,
         simpleKeys.showRawEditor,
-        simpleKeys.isMarkdownEditorEnabledForCourse,
       ].map(testSimpleSelector);
     });
   });

src/editors/data/redux/app/selectors.ts

@@ -26,7 +26,6 @@ export const simpleSelectors = {
   images: mkSimpleSelector(app => app.images),
   videos: mkSimpleSelector(app => app.videos),
   showRawEditor: mkSimpleSelector(app => app.showRawEditor),
-  isMarkdownEditorEnabledForCourse: mkSimpleSelector(app => app.isMarkdownEditorEnabledForCourse),
 };
 
 export const returnUrl = createSelector(

src/editors/data/redux/index.ts

@@ -108,7 +108,6 @@ export interface EditorState {
     videos: Record<string, any>;
     courseDetails: Record<string, any>;
     showRawEditor: boolean;
-    isMarkdownEditorEnabledForCourse: boolean;
   },
   requests: Record<keyof typeof RequestKeys, {
     status: keyof typeof RequestStates;
@@ -158,6 +157,12 @@ export interface EditorState {
     rawOLX: string;
     rawMarkdown: string;
     problemType: null | ProblemType | AdvancedProblemType;
+    /**
+     * Is the "markdown" editor currently active (as opposed to visual or advanced editors)
+     * This is confusingly named, and different from `isMarkdownEditorEnabledForContext`
+     * which is a waffle flag that determines whether the user is allowed to use the
+     * "markdown" editor at all in this course/library.
+     */
     isMarkdownEditorEnabled: boolean;
     question: string;
     answers: any[];

src/editors/hooks.test.jsx

@@ -56,7 +56,6 @@ describe('hooks', () => {
         blockId: 'blockId',
         studioEndpointUrl: 'studioEndpointUrl',
         learningContextId: 'learningContextId',
-        isMarkdownEditorEnabledForCourse: true,
       };
       hooks.useInitializeApp({ dispatch, data: fakeData });
       expect(dispatch).not.toHaveBeenCalledWith(fakeData);
@@ -65,7 +64,6 @@ describe('hooks', () => {
         fakeData.blockId,
         fakeData.studioEndpointUrl,
         fakeData.learningContextId,
-        fakeData.isMarkdownEditorEnabledForCourse,
       ]);
       cb();
       expect(dispatch).toHaveBeenCalledWith(thunkActions.app.initialize(fakeData));

src/editors/hooks.ts

@@ -12,7 +12,7 @@ export const useInitializeApp = ({ dispatch, data }) => {
     setLoading(true);
     dispatch(thunkActions.app.initialize(data));
     setLoading(false);
-  }, [data?.blockId, data?.studioEndpointUrl, data?.learningContextId, data?.isMarkdownEditorEnabledForCourse]);
+  }, [data?.blockId, data?.studioEndpointUrl, data?.learningContextId]);
   return loading;
 };