From 880d205cbbab952e819950d27f7129ed8b64aaf1 Mon Sep 17 00:00:00 2001 From: connorhaugh <49422820+connorhaugh@users.noreply.github.com> Date: Tue, 10 Jan 2023 09:42:44 -0500 Subject: [PATCH] Feat: raw editor ingress and egress logic (#179) * feat: conditional rendering of olx editor. * fix: open the raw editor if advanced is chosen * fix: add test fix * feat: add button to switch visual->advanced * fix: add tests + lint for visual->advanced button * feat: revert to advanced if parser fails * fix: improve coverage * feat: add confirm dialog to switch * fix: load settings with advanced * fix: refactor + lint fix --- .../__snapshots__/index.test.jsx.snap | 10 +++ .../EditProblemView/SettingsWidget/index.jsx | 4 ++ .../SettingsWidget/messages.js | 20 ++++++ .../SwitchToAdvancedEditorCard.jsx | 53 ++++++++++++++++ .../SwitchToAdvancedEditorCard.test.jsx | 12 ++++ .../SwitchToAdvancedEditorCard.test.jsx.snap | 48 ++++++++++++++ .../components/EditProblemView/index.jsx | 6 +- .../components/SelectTypeModal/hooks.js | 3 +- .../components/SelectTypeModal/hooks.test.js | 1 + .../ProblemEditor/data/OLXParser.js | 15 ++++- .../ProblemEditor/data/OLXParser.test.js | 12 ++++ .../data/mockData/olxTestData.js | 14 +++++ .../containers/ProblemEditor/index.jsx | 4 +- .../data/redux/thunkActions/problem.js | 62 +++++++++++++------ .../data/redux/thunkActions/problem.test.js | 51 +++++++++++++++ www/package-lock.json | 1 + 16 files changed, 288 insertions(+), 28 deletions(-) create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.test.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/__snapshots__/SwitchToAdvancedEditorCard.test.jsx.snap create mode 100644 src/editors/data/redux/thunkActions/problem.test.js diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/__snapshots__/index.test.jsx.snap index 346860fd2..8a6aa9fa0 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/__snapshots__/index.test.jsx.snap @@ -77,6 +77,11 @@ exports[`SettingsWidget snapshot snapshot: renders Settings widget page 1`] = ` > + + + @@ -163,6 +168,11 @@ exports[`SettingsWidget snapshot snapshot: renders Settings widget page advanced > + + + diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx index 90e5f904a..b97c357f6 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx @@ -13,6 +13,7 @@ import ResetCard from './settingsComponents/ResetCard'; import MatlabCard from './settingsComponents/MatlabCard'; import TimerCard from './settingsComponents/TimerCard'; import TypeCard from './settingsComponents/TypeCard'; +import SwitchToAdvancedEditorCard from './settingsComponents/SwitchToAdvancedEditorCard'; import messages from './messages'; import { showAdvancedSettingsCards } from './hooks'; @@ -75,6 +76,9 @@ export const SettingsWidget = ({ + + + diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/messages.js b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/messages.js index 4539decc3..395acf755 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/messages.js +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/messages.js @@ -149,6 +149,26 @@ export const messages = { defaultMessage: 'Type', description: 'Type settings card title', }, + SwitchButtonLabel: { + id: 'authoring.problemeditor.settings.switchtoadvancededitor.label', + defaultMessage: 'Switch To Advanced Editor', + description: 'button to switch to the advanced mode of the editor.', + }, + ConfirmSwitchMessage: { + id: 'authoring.problemeditor.settings.switchtoadvancededitor.message', + 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.', + description: 'message to confirm that a user wants to use the advanced editor', + }, + ConfirmSwitchMessageTitle: { + id: 'authoring.problemeditor.settings.switchtoadvancededitor.message', + defaultMessage: 'Convert to OLX?', + description: 'message to confirm that a user wants to use the advanced editor', + }, + ConfirmSwitchButtonLabel: { + id: 'authoring.problemeditor.settings.switchtoadvancededitor.message', + defaultMessage: 'Switch To Advanced Editor', + description: 'message to confirm that a user wants to use the advanced editor', + }, }; export default messages; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.jsx new file mode 100644 index 000000000..297854a5b --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.jsx @@ -0,0 +1,53 @@ +import React from 'react'; +import { connect } from 'react-redux'; +import { injectIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; +import { Button, Card } from '@edx/paragon'; +import PropTypes from 'prop-types'; +import messages from '../messages'; +import { thunkActions } from '../../../../../../data/redux'; +import BaseModal from '../../../../../TextEditor/components/BaseModal'; + +export const SwitchToAdvancedEditorCard = ({ + switchToAdvancedEditor, +}) => { + const [isConfirmOpen, setConfirmOpen] = React.useState(false); + return ( + + { setConfirmOpen(false); }} + title={()} + confirmAction={( + + )} + size="md" + > + + + + + ); +}; + +SwitchToAdvancedEditorCard.propTypes = { + switchToAdvancedEditor: PropTypes.func.isRequired, +}; + +export const mapStateToProps = () => ({ +}); +export const mapDispatchToProps = { + switchToAdvancedEditor: thunkActions.problem.switchToAdvancedEditor, +}; + +export default injectIntl(connect(mapStateToProps, mapDispatchToProps)(SwitchToAdvancedEditorCard)); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.test.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.test.jsx new file mode 100644 index 000000000..96d0b4a5c --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchToAdvancedEditorCard.test.jsx @@ -0,0 +1,12 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import { SwitchToAdvancedEditorCard } from './SwitchToAdvancedEditorCard'; + +describe('SwitchToAdvancedEditorCard snapshot', () => { + const mockSwitchToAdvancedEditor = jest.fn().mockName('switchToAdvancedEditor'); + test('snapshot: SwitchToAdvancedEditorCard', () => { + expect( + shallow(), + ).toMatchSnapshot(); + }); +}); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/__snapshots__/SwitchToAdvancedEditorCard.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/__snapshots__/SwitchToAdvancedEditorCard.test.jsx.snap new file mode 100644 index 000000000..07768a93c --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/__snapshots__/SwitchToAdvancedEditorCard.test.jsx.snap @@ -0,0 +1,48 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SwitchToAdvancedEditorCard snapshot snapshot: SwitchToAdvancedEditorCard 1`] = ` + + + + + } + footerAction={null} + isOpen={false} + size="md" + title={ + + } + > + + + + +`; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx index 5600f9e3a..d832f05d4 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/index.jsx @@ -10,7 +10,7 @@ import { EditorContainer } from '../../../EditorContainer'; import { selectors } from '../../../../data/redux'; import ReactStateSettingsParser from '../../data/ReactStateSettingsParser'; import ReactStateOLXParser from '../../data/ReactStateOLXParser'; -import { AdvanceProblemKeys } from '../../../../data/constants/problem'; +import { ProblemTypeKeys } from '../../../../data/constants/problem'; export const EditProblemView = ({ problemType, @@ -24,8 +24,8 @@ export const EditProblemView = ({ olx: reactOLXParser.buildOLX(), }; }; - if (Object.values(AdvanceProblemKeys).includes(problemType)) { - return `hello raw editor with ${problemType}`; + if (problemType === ProblemTypeKeys.ADVANCED) { + return 'hello raw editor'; } return ( diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js index f1dace285..1b225aee4 100644 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js @@ -19,7 +19,8 @@ export const selectHooks = () => { export const onSelect = (setProblemType, selected, updateField) => () => { if (Object.values(AdvanceProblemKeys).includes(selected)) { - updateField({ rawOLX: AdvanceProblems[selected].template }); + updateField({ problemType: ProblemTypeKeys.ADVANCED, rawOLX: AdvanceProblems[selected].template }); + return; } setProblemType({ selected }); }; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js index 50829221e..9cb24caf5 100644 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js @@ -47,6 +47,7 @@ describe('SelectTypeModal hooks', () => { test('updateField is called with selected templated if selected is an Advanced Problem', () => { module.onSelect(mockSetProblemType, mockAdvancedSelected, mockUpdateField)(); expect(mockUpdateField).toHaveBeenCalledWith({ + problemType: ProblemTypeKeys.ADVANCED, rawOLX: AdvanceProblems[mockAdvancedSelected].template, }); }); diff --git a/src/editors/containers/ProblemEditor/data/OLXParser.js b/src/editors/containers/ProblemEditor/data/OLXParser.js index 18126a039..e01fa50e1 100644 --- a/src/editors/containers/ProblemEditor/data/OLXParser.js +++ b/src/editors/containers/ProblemEditor/data/OLXParser.js @@ -331,10 +331,16 @@ export class OLXParser { getProblemType() { const problemKeys = Object.keys(this.problem); const intersectedProblems = _.intersection(Object.values(ProblemTypeKeys), problemKeys); - if (intersectedProblems.length === 0) { - return null; + // a blank problem is a problem which contains only `` as it's olx. + // blank problems are not given types, so that a type may be selected. + if (problemKeys.length === 1 && problemKeys[0] === '#text' && this.problem[problemKeys[0]] === '') { + return null; + } + // if we have no matching problem type, the problem is advanced. + return ProblemTypeKeys.ADVANCED; } + // make sure compound problems are treated as advanced if (intersectedProblems.length > 1) { return ProblemTypeKeys.ADVANCED; } @@ -369,7 +375,10 @@ export class OLXParser { answersObject = this.parseMultipleChoiceAnswers(ProblemTypeKeys.SINGLESELECT, 'choicegroup', 'choice'); break; case ProblemTypeKeys.ADVANCED: - break; + return { + problemType, + settings: {}, + }; default: // if problem is unset, return null return {}; diff --git a/src/editors/containers/ProblemEditor/data/OLXParser.test.js b/src/editors/containers/ProblemEditor/data/OLXParser.test.js index 3ae946638..5823e4b6e 100644 --- a/src/editors/containers/ProblemEditor/data/OLXParser.test.js +++ b/src/editors/containers/ProblemEditor/data/OLXParser.test.js @@ -7,6 +7,8 @@ import { textInputWithFeedbackAndHintsOLX, mutlipleChoiceWithFeedbackAndHintsOLX, textInputWithFeedbackAndHintsOLXWithMultipleAnswers, + advancedProblemOlX, + blankProblemOLX, } from './mockData/olxTestData'; import { ProblemTypeKeys } from '../../../data/constants/problem'; @@ -36,6 +38,16 @@ describe('Check OLXParser problem type', () => { const problemType = olxparser.getProblemType(); expect(problemType).toBe(ProblemTypeKeys.TEXTINPUT); }); + test('Test Advanced Problem Type', () => { + const olxparser = new OLXParser(advancedProblemOlX.rawOLX); + const problemType = olxparser.getProblemType(); + expect(problemType).toBe(ProblemTypeKeys.ADVANCED); + }); + test('Test Blank Problem Type', () => { + const olxparser = new OLXParser(blankProblemOLX.rawOLX); + const problemType = olxparser.getProblemType(); + expect(problemType).toBe(null); + }); }); describe('Check OLXParser hints', () => { diff --git a/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js b/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js index cc9e2de9d..22ab47cf0 100644 --- a/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js +++ b/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js @@ -553,3 +553,17 @@ export const numericInputWithFeedbackAndHintsOLXException = { `, }; +export const advancedProblemOlX = { + rawOLX: ` + +

You can use this template as a guide to the OLX markup to use for math expression problems. Edit this component to replace the example with your own assessment.

+ + You can add an optional tip or note related to the prompt like this. Example: To test this example, the correct answer is R_1*R_2/R_3 + + + +`, +}; +export const blankProblemOLX = { + rawOLX: '', +}; diff --git a/src/editors/containers/ProblemEditor/index.jsx b/src/editors/containers/ProblemEditor/index.jsx index 542fcabaa..b96a400c7 100644 --- a/src/editors/containers/ProblemEditor/index.jsx +++ b/src/editors/containers/ProblemEditor/index.jsx @@ -17,8 +17,6 @@ export const ProblemEditor = ({ blockValue, initializeProblemEditor, }) => { - React.useEffect(() => initializeProblemEditor(blockValue), [blockValue]); - // TODO: INTL MSG, Add LOAD FAILED ERROR using BLOCKFAILED if (!blockFinished || !studioViewFinished) { return (
@@ -31,6 +29,8 @@ export const ProblemEditor = ({ ); } // once data is loaded, init store + React.useEffect(() => initializeProblemEditor(blockValue), [blockValue]); + // TODO: INTL MSG, Add LOAD FAILED ERROR using BLOCKFAILED if (problemType === null) { return (); diff --git a/src/editors/data/redux/thunkActions/problem.js b/src/editors/data/redux/thunkActions/problem.js index d1902d46f..82caf7928 100644 --- a/src/editors/data/redux/thunkActions/problem.js +++ b/src/editors/data/redux/thunkActions/problem.js @@ -1,31 +1,55 @@ import _ from 'lodash-es'; -import * as requests from './requests'; import { actions } from '..'; import { OLXParser } from '../../../containers/ProblemEditor/data/OLXParser'; import { parseSettings } from '../../../containers/ProblemEditor/data/SettingsParser'; +import { ProblemTypeKeys } from '../../constants/problem'; +import ReactStateOLXParser from '../../../containers/ProblemEditor/data/ReactStateOLXParser'; +import { blankProblemOLX } from '../../../containers/ProblemEditor/data/mockData/olxTestData'; + +export const switchToAdvancedEditor = () => (dispatch, getState) => { + const state = getState(); + const reactOLXParser = new ReactStateOLXParser({ problem: state.problem }); + const rawOlx = reactOLXParser.buildOLX(); + dispatch(actions.problem.updateField({ problemType: ProblemTypeKeys.ADVANCED, rawOlx })); +}; + +export const isBlankProblem = ({ rawOLX }) => { + if (rawOLX === blankProblemOLX.rawOLX) { + return true; + } + return false; +}; + +export const getDataFromOlx = ({ rawOLX, rawSettings }) => { + let olxParser; + let parsedProblem; + try { + olxParser = new OLXParser(rawOLX); + parsedProblem = olxParser.getParsedOLXData(); + } catch { + console.error('The Problem Could Not Be Parsed from OLX. redirecting to Advanced editor.'); + return { problemType: ProblemTypeKeys.ADVANCED, rawOLX, settings: parseSettings(rawSettings) }; + } + if (parsedProblem?.problemType === ProblemTypeKeys.ADVANCED) { + return { problemType: ProblemTypeKeys.ADVANCED, rawOLX, settings: parseSettings(rawSettings) }; + } + const { settings, ...data } = parsedProblem; + const parsedSettings = { ...settings, ...parseSettings(rawSettings) }; + if (!_.isEmpty(rawOLX) && !_.isEmpty(data)) { + return { ...data, rawOLX, settings: parsedSettings }; + } + return {}; +}; export const initializeProblem = (blockValue) => (dispatch) => { const rawOLX = _.get(blockValue, 'data.data', {}); - const olxParser = new OLXParser(rawOLX); + const rawSettings = _.get(blockValue, 'data.metadata', {}); - const parsedProblem = olxParser.getParsedOLXData(); - if (_.isEmpty(parsedProblem)) { - // if problem is blank, enable selection. + if (isBlankProblem({ rawOLX })) { dispatch(actions.problem.setEnableTypeSelection()); + } else { + dispatch(actions.problem.load(getDataFromOlx({ rawOLX, rawSettings }))); } - const { settings, ...data } = parsedProblem; - const parsedSettings = { ...settings, ...parseSettings(_.get(blockValue, 'data.metadata', {})) }; - if (!_.isEmpty(rawOLX) && !_.isEmpty(data)) { - dispatch(actions.problem.load({ ...data, rawOLX, settings: parsedSettings })); - } - dispatch(requests.fetchAdvanceSettings({ - onSuccess: (response) => { - console.log(response); - if (response.data.allow_unsupported_xblocks.value) { - console.log(response.allow_unsupported_xblocks.value); - } - }, - })); }; -export default { initializeProblem }; +export default { initializeProblem, switchToAdvancedEditor }; diff --git a/src/editors/data/redux/thunkActions/problem.test.js b/src/editors/data/redux/thunkActions/problem.test.js new file mode 100644 index 000000000..c48c82bd8 --- /dev/null +++ b/src/editors/data/redux/thunkActions/problem.test.js @@ -0,0 +1,51 @@ +import { actions } from '..'; +import { initializeProblem, switchToAdvancedEditor } from './problem'; +import { checkboxesOLXWithFeedbackAndHintsOLX, advancedProblemOlX, blankProblemOLX } from '../../../containers/ProblemEditor/data/mockData/olxTestData'; +import { ProblemTypeKeys } from '../../constants/problem'; + +const mockOlx = 'SOmEVALue'; +const mockBuildOlx = jest.fn(() => mockOlx); +jest.mock('../../../containers/ProblemEditor/data/ReactStateOLXParser', () => jest.fn().mockImplementation(() => ({ buildOLX: mockBuildOlx }))); + +jest.mock('..', () => ({ + actions: { + problem: { + load: () => {}, + setEnableTypeSelection: () => {}, + updateField: (args) => args, + }, + }, +})); + +describe('problem thunkActions', () => { + let dispatch; + let getState; + beforeEach(() => { + dispatch = jest.fn((action) => ({ dispatch: action })); + getState = jest.fn(() => ({ + problem: { + }, + })); + }); + test('initializeProblem visual Problem :', () => { + const blockValue = { data: { data: checkboxesOLXWithFeedbackAndHintsOLX.rawOLX } }; + initializeProblem(blockValue)(dispatch); + expect(dispatch).toHaveBeenCalledWith(actions.problem.load()); + }); + test('initializeProblem advanced Problem', () => { + const blockValue = { data: { data: advancedProblemOlX.rawOLX } }; + initializeProblem(blockValue)(dispatch); + expect(dispatch).toHaveBeenCalledWith(actions.problem.load()); + }); + test('initializeProblem blank Problem', () => { + const blockValue = { data: { data: blankProblemOLX.rawOLX } }; + initializeProblem(blockValue)(dispatch); + expect(dispatch).toHaveBeenCalledWith(actions.problem.setEnableTypeSelection()); + }); + test('switchToAdvancedEditor visual Problem', () => { + switchToAdvancedEditor()(dispatch, getState); + expect(dispatch).toHaveBeenCalledWith( + actions.problem.updateField({ problemType: ProblemTypeKeys.ADVANCED, rawOlx: mockOlx }), + ); + }); +}); diff --git a/www/package-lock.json b/www/package-lock.json index e1efceca8..6dc2344c5 100644 --- a/www/package-lock.json +++ b/www/package-lock.json @@ -26,6 +26,7 @@ } }, "..": { + "name": "@edx/frontend-lib-content-components", "version": "1.0.0-semantically-released", "license": "AGPL-3.0", "dependencies": {