Skip to content

Commit 82a3c28

Browse files
authored
feat: enable markdown to OLX conversion (#2518)
1 parent 191be55 commit 82a3c28

File tree

15 files changed

+368
-161
lines changed

15 files changed

+368
-161
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import React from 'react';
2+
3+
type ProblemEditorRef = React.MutableRefObject<unknown> | React.RefObject<unknown> | null;
4+
5+
export interface ProblemEditorContextValue {
6+
editorRef: ProblemEditorRef;
7+
}
8+
9+
export type ProblemEditorContextInit = {
10+
editorRef?: ProblemEditorRef;
11+
};
12+
13+
const context = React.createContext<ProblemEditorContextValue | undefined>(undefined);
14+
15+
export function useProblemEditorContext() {
16+
const ctx = React.useContext(context);
17+
if (ctx === undefined) {
18+
/* istanbul ignore next */
19+
throw new Error('This component needs to be wrapped in <ProblemEditorContextProvider>');
20+
}
21+
return ctx;
22+
}
23+
24+
export const ProblemEditorContextProvider: React.FC<{ children: React.ReactNode; } & ProblemEditorContextInit> = ({
25+
children,
26+
editorRef = null,
27+
}) => {
28+
const ctx: ProblemEditorContextValue = React.useMemo(() => ({ editorRef }), [editorRef]);
29+
30+
return <context.Provider value={ctx}>{children}</context.Provider>;
31+
};

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ const SettingsWidget = ({
4545
isMarkdownEditorEnabledForContext,
4646
} = useEditorContext();
4747
const rawMarkdown = useSelector(selectors.problem.rawMarkdown);
48+
const isMarkdownEditorEnabled = useSelector(selectors.problem.isMarkdownEditorEnabled);
4849
const showMarkdownEditorButton = isMarkdownEditorEnabledForContext && rawMarkdown;
4950
const { isAdvancedCardsVisible, showAdvancedCards } = showAdvancedSettingsCards();
5051
const feedbackCard = () => {
@@ -161,7 +162,7 @@ const SettingsWidget = ({
161162
<div className="my-3">
162163
<SwitchEditorCard problemType={problemType} editorType="advanced" />
163164
</div>
164-
{ showMarkdownEditorButton
165+
{ (showMarkdownEditorButton && !isMarkdownEditorEnabled) // Only show button if not already in markdown editor
165166
&& (
166167
<div className="my-3">
167168
<SwitchEditorCard problemType={problemType} editorType="markdown" />

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

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import React from 'react';
22

33
import { ProblemTypeKeys } from '@src/editors/data/constants/problem';
44
import { screen, initializeMocks } from '@src/testUtils';
5-
import { editorRender } from '@src/editors/editorTestRender';
5+
import { editorRender, type PartialEditorState } from '@src/editors/editorTestRender';
66
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
77
import * as hooks from './hooks';
88
import { SettingsWidgetInternal as SettingsWidget } from '.';
9+
import { ProblemEditorContextProvider } from '../ProblemEditorContext';
910

1011
jest.mock('./settingsComponents/GeneralFeedback', () => 'GeneralFeedback');
1112
jest.mock('./settingsComponents/GroupFeedback', () => 'GroupFeedback');
@@ -23,7 +24,6 @@ describe('SettingsWidget', () => {
2324
const showAdvancedSettingsCardsBaseProps = {
2425
isAdvancedCardsVisible: false,
2526
showAdvancedCards: jest.fn().mockName('showAdvancedSettingsCards.showAdvancedCards'),
26-
setResetTrue: jest.fn().mockName('showAdvancedSettingsCards.setResetTrue'),
2727
};
2828

2929
const props = {
@@ -49,22 +49,34 @@ describe('SettingsWidget', () => {
4949

5050
};
5151

52+
const editorRef = { current: null };
53+
54+
const renderSettingsWidget = (
55+
overrideProps = {},
56+
options = {},
57+
) => editorRender(
58+
<ProblemEditorContextProvider editorRef={editorRef}>
59+
<SettingsWidget {...props} {...overrideProps} />
60+
</ProblemEditorContextProvider>,
61+
options,
62+
);
63+
5264
beforeEach(() => {
5365
initializeMocks();
5466
});
5567

5668
describe('behavior', () => {
5769
it('calls showAdvancedSettingsCards when initialized', () => {
5870
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsBaseProps);
59-
editorRender(<SettingsWidget {...props} />);
71+
renderSettingsWidget();
6072
expect(hooks.showAdvancedSettingsCards).toHaveBeenCalled();
6173
});
6274
});
6375

6476
describe('renders', () => {
6577
test('renders Settings widget page', () => {
6678
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsBaseProps);
67-
editorRender(<SettingsWidget {...props} />);
79+
renderSettingsWidget();
6880
expect(screen.getByText('Show advanced settings')).toBeInTheDocument();
6981
});
7082

@@ -74,7 +86,7 @@ describe('SettingsWidget', () => {
7486
isAdvancedCardsVisible: true,
7587
};
7688
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
77-
const { container } = editorRender(<SettingsWidget {...props} />);
89+
const { container } = renderSettingsWidget();
7890
expect(screen.queryByText('Show advanced settings')).not.toBeInTheDocument();
7991
expect(container.querySelector('showanswercard')).toBeInTheDocument();
8092
expect(container.querySelector('resetcard')).toBeInTheDocument();
@@ -86,12 +98,49 @@ describe('SettingsWidget', () => {
8698
isAdvancedCardsVisible: true,
8799
};
88100
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
89-
const { container } = editorRender(
90-
<SettingsWidget {...props} problemType={ProblemTypeKeys.ADVANCED} />,
91-
);
101+
const { container } = renderSettingsWidget({ problemType: ProblemTypeKeys.ADVANCED });
92102
expect(container.querySelector('randomization')).toBeInTheDocument();
93103
});
94104
});
105+
describe('SwitchEditorCard rendering (markdown vs advanced)', () => {
106+
test('shows two SwitchEditorCard components when markdown is available and not currently enabled', () => {
107+
const showAdvancedSettingsCardsProps = {
108+
...showAdvancedSettingsCardsBaseProps,
109+
isAdvancedCardsVisible: true,
110+
};
111+
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
112+
const modifiedInitialState: PartialEditorState = {
113+
problem: {
114+
problemType: null, // non-advanced problem
115+
isMarkdownEditorEnabled: false, // currently in advanced/raw (or standard) editor
116+
rawOLX: '<problem></problem>',
117+
rawMarkdown: '## Problem', // markdown content exists so button should appear
118+
isDirty: false,
119+
},
120+
};
121+
const { container } = renderSettingsWidget({}, { initialState: modifiedInitialState });
122+
expect(container.querySelectorAll('switcheditorcard')).toHaveLength(2);
123+
});
124+
125+
test('shows only the advanced SwitchEditorCard when already in markdown mode', () => {
126+
const showAdvancedSettingsCardsProps = {
127+
...showAdvancedSettingsCardsBaseProps,
128+
isAdvancedCardsVisible: true,
129+
};
130+
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
131+
const modifiedInitialState: PartialEditorState = {
132+
problem: {
133+
problemType: null,
134+
isMarkdownEditorEnabled: true, // already in markdown editor, so markdown button hidden
135+
rawOLX: '<problem></problem>',
136+
rawMarkdown: '## Problem',
137+
isDirty: false,
138+
},
139+
};
140+
const { container } = renderSettingsWidget({}, { initialState: modifiedInitialState });
141+
expect(container.querySelectorAll('switcheditorcard')).toHaveLength(1);
142+
});
143+
});
95144

96145
describe('isLibrary', () => {
97146
const libraryProps = {
@@ -100,7 +149,7 @@ describe('SettingsWidget', () => {
100149
};
101150
test('renders Settings widget page', () => {
102151
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsBaseProps);
103-
const { container } = editorRender(<SettingsWidget {...libraryProps} />);
152+
const { container } = renderSettingsWidget(libraryProps);
104153
expect(container.querySelector('timercard')).not.toBeInTheDocument();
105154
expect(container.querySelector('resetcard')).not.toBeInTheDocument();
106155
expect(container.querySelector('typecard')).toBeInTheDocument();
@@ -114,7 +163,7 @@ describe('SettingsWidget', () => {
114163
isAdvancedCardsVisible: true,
115164
};
116165
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
117-
const { container } = editorRender(<SettingsWidget {...libraryProps} />);
166+
const { container } = renderSettingsWidget(libraryProps);
118167
expect(screen.queryByText('Show advanced settings')).not.toBeInTheDocument();
119168
expect(container.querySelector('showanswearscard')).not.toBeInTheDocument();
120169
expect(container.querySelector('resetcard')).not.toBeInTheDocument();
@@ -128,7 +177,7 @@ describe('SettingsWidget', () => {
128177
isAdvancedCardsVisible: true,
129178
};
130179
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
131-
const { container } = editorRender(<SettingsWidget {...libraryProps} problemType={ProblemTypeKeys.ADVANCED} />);
180+
const { container } = renderSettingsWidget({ ...libraryProps, problemType: ProblemTypeKeys.ADVANCED });
132181
expect(container.querySelector('randomization')).toBeInTheDocument();
133182
});
134183
});

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/messages.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,13 @@ const messages = defineMessages({
169169
},
170170
'ConfirmSwitchMessage-advanced': {
171171
id: 'authoring.problemeditor.settings.switchtoeditor.ConfirmSwitchMessage.advanced',
172-
defaultMessage: 'If you use the advanced editor, this problem will be converted to OLX and you will not be able to return to the simple editor.',
172+
defaultMessage: 'If you use the advanced editor, this problem will be converted to OLX. Depending on what edits you make to the OLX, you may not be able to return to the simple editor.',
173173
description: 'message to confirm that a user wants to use the advanced editor',
174174
},
175175
'ConfirmSwitchMessage-markdown': {
176176
id: 'authoring.problemeditor.settings.switchtoeditor.ConfirmSwitchMessage.markdown',
177-
defaultMessage: 'If you use the markdown editor, this problem will be converted to markdown and you will not be able to return to the simple editor.',
178-
description: 'message to confirm that a user wants to use the advanced editor',
177+
defaultMessage: 'Some edits that are possible with the markdown editor are not supported by the simple editor, so you may not be able to change back to the simple editor.',
178+
description: 'message to confirm that a user wants to use the markdown editor',
179179
},
180180
'ConfirmSwitchMessageTitle-advanced': {
181181
id: 'authoring.problemeditor.settings.switchtoeditor.ConfirmSwitchMessageTitle.advanced',

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,24 @@
11
import React from 'react';
2-
import { useDispatch, useSelector } from 'react-redux';
2+
import { useDispatch } from 'react-redux';
33
import { FormattedMessage } from '@edx/frontend-platform/i18n';
44
import { Card } from '@openedx/paragon';
55
import PropTypes from 'prop-types';
6-
import { useEditorContext } from '@src/editors/EditorContext';
7-
import { selectors, thunkActions } from '@src/editors/data/redux';
6+
import { thunkActions } from '@src/editors/data/redux';
87
import BaseModal from '@src/editors/sharedComponents/BaseModal';
98
import Button from '@src/editors/sharedComponents/Button';
109
import { ProblemTypeKeys } from '@src/editors/data/constants/problem';
1110
import messages from '../messages';
1211
import { handleConfirmEditorSwitch } from '../hooks';
12+
import { useProblemEditorContext } from '../../ProblemEditorContext';
1313

1414
const SwitchEditorCard = ({
1515
editorType,
1616
problemType,
1717
}) => {
1818
const [isConfirmOpen, setConfirmOpen] = React.useState(false);
19-
const { isMarkdownEditorEnabledForContext } = useEditorContext();
20-
const isMarkdownEditorEnabled = useSelector(selectors.problem.isMarkdownEditorEnabled);
2119
const dispatch = useDispatch();
22-
23-
const isMarkdownEditorActive = isMarkdownEditorEnabled && isMarkdownEditorEnabledForContext;
24-
if (isMarkdownEditorActive || problemType === ProblemTypeKeys.ADVANCED) { return null; }
20+
const { editorRef } = useProblemEditorContext();
21+
if (problemType === ProblemTypeKeys.ADVANCED) { return null; }
2522

2623
return (
2724
<Card className="border border-light-700 shadow-none">
@@ -33,7 +30,7 @@ const SwitchEditorCard = ({
3330
<Button
3431
/* istanbul ignore next */
3532
onClick={() => handleConfirmEditorSwitch({
36-
switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType)),
33+
switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType, editorRef)),
3734
setConfirmOpen,
3835
})}
3936
variant="primary"

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

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event';
44
import { editorRender } from '@src/editors/editorTestRender';
55
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
66
import { thunkActions } from '@src/editors/data/redux';
7+
import { ProblemEditorContextProvider } from '../../ProblemEditorContext';
78
import SwitchEditorCard from './SwitchEditorCard';
89

910
const switchEditorSpy = jest.spyOn(thunkActions.problem, 'switchEditor');
@@ -13,6 +14,13 @@ describe('SwitchEditorCard - markdown', () => {
1314
problemType: 'stringresponse',
1415
editorType: 'markdown',
1516
};
17+
const editorRef = { current: null };
18+
19+
const renderSwitchEditorCard = (overrideProps = {}) => editorRender(
20+
<ProblemEditorContextProvider editorRef={editorRef}>
21+
<SwitchEditorCard {...baseProps} {...overrideProps} />
22+
</ProblemEditorContextProvider>,
23+
);
1624

1725
beforeEach(() => {
1826
initializeMocks();
@@ -23,7 +31,7 @@ describe('SwitchEditorCard - markdown', () => {
2331
mockWaffleFlags({ useReactMarkdownEditor: true });
2432
// The markdown editor is not currently active (default)
2533

26-
editorRender(<SwitchEditorCard {...baseProps} />);
34+
renderSwitchEditorCard();
2735
const user = userEvent.setup();
2836
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
2937
const switchButton = screen.getByRole('button', { name: 'Switch to markdown editor' });
@@ -38,7 +46,7 @@ describe('SwitchEditorCard - markdown', () => {
3846
mockWaffleFlags({ useReactMarkdownEditor: true });
3947
// The markdown editor is not currently active (default)
4048

41-
editorRender(<SwitchEditorCard {...baseProps} />);
49+
renderSwitchEditorCard();
4250
const user = userEvent.setup();
4351
const switchButton = screen.getByRole('button', { name: 'Switch to markdown editor' });
4452
expect(switchButton).toBeInTheDocument();
@@ -49,28 +57,12 @@ describe('SwitchEditorCard - markdown', () => {
4957
expect(confirmButton).toBeInTheDocument();
5058
expect(switchEditorSpy).not.toHaveBeenCalled();
5159
await user.click(confirmButton);
52-
expect(switchEditorSpy).toHaveBeenCalledWith('markdown');
60+
expect(switchEditorSpy).toHaveBeenCalledWith('markdown', editorRef);
5361
// Markdown editor would now be active.
5462
});
5563

5664
test('renders nothing for advanced problemType', () => {
57-
const { container } = editorRender(<SwitchEditorCard {...baseProps} problemType="advanced" />);
58-
const reduxWrapper = (container.firstChild as HTMLElement | null);
59-
expect(reduxWrapper?.innerHTML).toBe('');
60-
});
61-
62-
test('returns null when editor is already Markdown', () => {
63-
// Markdown Editor support is on for this course:
64-
mockWaffleFlags({ useReactMarkdownEditor: true });
65-
// The markdown editor *IS* currently active (default)
66-
67-
const { container } = editorRender(<SwitchEditorCard {...baseProps} />, {
68-
initialState: {
69-
problem: {
70-
isMarkdownEditorEnabled: true,
71-
},
72-
},
73-
});
65+
const { container } = renderSwitchEditorCard({ problemType: 'advanced' });
7466
const reduxWrapper = (container.firstChild as HTMLElement | null);
7567
expect(reduxWrapper?.innerHTML).toBe('');
7668
});

src/editors/containers/ProblemEditor/components/EditProblemView/hooks.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ export const parseState = ({
9090
return {
9191
settings: {
9292
...settings,
93-
...(isMarkdownEditorEnabled && { markdown: contentString }),
93+
// If the save action isn’t triggered from the Markdown editor, the Markdown content might be outdated. Since the
94+
// Markdown editor shouldn't be displayed in future in this case, we’re sending `null` instead.
95+
// TODO: Implement OLX-to-Markdown conversion to properly handle this scenario.
96+
markdown: isMarkdownEditorEnabled ? contentString : null,
9497
markdown_edited: isMarkdownEditorEnabled,
9598
},
9699
olx: isAdvanced || isMarkdownEditorEnabled ? rawOLX : reactBuiltOlx,

src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ describe('EditProblemView hooks parseState', () => {
165165
assets: {},
166166
})();
167167
expect(res.olx).toBe(mockRawOLX);
168+
expect(res.settings.markdown).toBe(null);
168169
});
169170
it('markdown problem', () => {
170171
const res = hooks.parseState({
@@ -306,13 +307,16 @@ describe('EditProblemView hooks parseState', () => {
306307
show_reset_button: false,
307308
submission_wait_seconds: 0,
308309
attempts_before_showanswer_button: 0,
310+
markdown: null,
311+
markdown_edited: false,
309312
};
310313
const openSaveWarningModal = jest.fn();
311314

312315
it('default visual save and returns parseState data', () => {
313316
const problem = { ...problemState, problemType: ProblemTypeKeys.NUMERIC, answers: [{ id: 'A', title: 'problem', correct: true }] };
314317
const content = hooks.getContent({
315318
isAdvancedProblemType: false,
319+
isMarkdownEditorEnabled: false,
316320
problemState: problem,
317321
editorRef,
318322
assets,
@@ -339,6 +343,7 @@ describe('EditProblemView hooks parseState', () => {
339343
};
340344
const { settings } = hooks.getContent({
341345
isAdvancedProblemType: false,
346+
isMarkdownEditorEnabled: false,
342347
problemState: problem,
343348
editorRef,
344349
assets,
@@ -353,12 +358,15 @@ describe('EditProblemView hooks parseState', () => {
353358
attempts_before_showanswer_button: 0,
354359
submission_wait_seconds: 0,
355360
weight: 1,
361+
markdown: null,
362+
markdown_edited: false,
356363
});
357364
});
358365

359366
it('default advanced save and returns parseState data', () => {
360367
const content = hooks.getContent({
361368
isAdvancedProblemType: true,
369+
isMarkdownEditorEnabled: false,
362370
problemState,
363371
editorRef,
364372
assets,

0 commit comments

Comments
 (0)