From c70679da541db540817082e369d911843cb1394d Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 6 Oct 2023 20:35:51 +0530 Subject: [PATCH] fix: make max attempts setting fallback to default The max attempts setting for a problem xblock should be set to null for course default max attempt setting to take effect. This makes sure that xblock setting is updated if course default is updated. --- .../EditProblemView/SettingsWidget/hooks.js | 16 +++++++------- .../settingsComponents/ScoringCard.jsx | 2 ++ .../components/SelectTypeModal/hooks.js | 2 +- .../data/ReactStateSettingsParser.js | 2 +- .../ProblemEditor/data/SettingsParser.js | 22 ++++++++++++++----- src/editors/data/redux/problem/reducers.js | 7 ++---- .../data/redux/thunkActions/problem.js | 10 ++++----- 7 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.js b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.js index d8b058d9c..23e02b052 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.js +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.js @@ -115,29 +115,29 @@ export const scoringCardHooks = (scoring, updateSettings, defaultValue) => { const isUnlimited = event.target.checked; if (isUnlimited) { setAttemptDisplayValue(''); - updateSettings({ scoring: { ...scoring, attempts: { number: '', unlimited: true } } }); + updateSettings({ scoring: { ...scoring, attempts: { number: null, unlimited: true } } }); } else { setAttemptDisplayValue(`${defaultValue} (Default)`); - updateSettings({ scoring: { ...scoring, attempts: { number: defaultValue, unlimited: false } } }); + updateSettings({ scoring: { ...scoring, attempts: { number: null, unlimited: false } } }); } }; const handleMaxAttemptChange = (event) => { let unlimitedAttempts = false; let attemptNumber = parseInt(event.target.value); const { value } = event.target; + // TODO: impove below condition handling if (_.isNaN(attemptNumber)) { - if (value === '') { - attemptNumber = defaultValue; + if (value === '' && !_.isNil(defaultValue)) { + attemptNumber = null; setAttemptDisplayValue(`${defaultValue} (Default)`); - } else { - attemptNumber = ''; + } else if (_.isNil(defaultValue)) { + attemptNumber = null; unlimitedAttempts = true; } } else if (attemptNumber <= 0) { attemptNumber = 0; } else if (attemptNumber === defaultValue) { - const attemptNumberStr = value.replace(' (Default)'); - attemptNumber = parseInt(attemptNumberStr); + attemptNumber = null; } updateSettings({ scoring: { ...scoring, attempts: { number: attemptNumber, unlimited: unlimitedAttempts } } }); }; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ScoringCard.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ScoringCard.jsx index 74fc79edb..17c2b2a4a 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ScoringCard.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/ScoringCard.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import _ from 'lodash-es'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; @@ -71,6 +72,7 @@ export const ScoringCard = ({ className="mt-3 decoration-control-label" checked={scoring.attempts.unlimited} onChange={handleUnlimitedChange} + disabled={!_.isNil(defaultValue)} >
diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js index e28e3d306..c07663dfd 100644 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js @@ -25,7 +25,7 @@ export const onSelect = ({ selected, updateField, setBlockTitle }) => () => { setBlockTitle(AdvanceProblems[selected].title); } else { const newOLX = ProblemTypes[selected].template; - const { settings, ...newState } = getDataFromOlx({ rawOLX: newOLX, rawSettings: {} }); + const { settings, ...newState } = getDataFromOlx({ rawOLX: newOLX, rawSettings: {}, defaultSettings: {} }); updateField({ ...newState }); setBlockTitle(ProblemTypes[selected].title); } diff --git a/src/editors/containers/ProblemEditor/data/ReactStateSettingsParser.js b/src/editors/containers/ProblemEditor/data/ReactStateSettingsParser.js index 2c22dcb77..2d7ddf764 100644 --- a/src/editors/containers/ProblemEditor/data/ReactStateSettingsParser.js +++ b/src/editors/containers/ProblemEditor/data/ReactStateSettingsParser.js @@ -19,7 +19,7 @@ class ReactStateSettingsParser { let settings = {}; const stateSettings = this.problem.settings; - settings = popuplateItem(settings, 'number', 'max_attempts', stateSettings.scoring.attempts); + settings = popuplateItem(settings, 'number', 'max_attempts', stateSettings.scoring.attempts, true); settings = popuplateItem(settings, 'weight', 'weight', stateSettings.scoring); settings = popuplateItem(settings, 'on', 'showanswer', stateSettings.showAnswer); settings = popuplateItem(settings, 'afterAttempts', 'attempts_before_showanswer_button', stateSettings.showAnswer); diff --git a/src/editors/containers/ProblemEditor/data/SettingsParser.js b/src/editors/containers/ProblemEditor/data/SettingsParser.js index 68af5d47d..34590b1c6 100644 --- a/src/editors/containers/ProblemEditor/data/SettingsParser.js +++ b/src/editors/containers/ProblemEditor/data/SettingsParser.js @@ -2,21 +2,31 @@ import _ from 'lodash-es'; import { ShowAnswerTypes, RandomizationTypesKeys } from '../../../data/constants/problem'; -export const popuplateItem = (parentObject, itemName, statekey, metadata) => { +export const popuplateItem = (parentObject, itemName, statekey, metadata, allowNull=false) => { let parent = parentObject; const item = _.get(metadata, itemName, null); - if (!_.isNil(item)) { + if (!_.isNil(item) || allowNull) { parent = { ...parentObject, [statekey]: item }; } return parent; }; -export const parseScoringSettings = (metadata) => { +export const parseScoringSettings = (metadata, defaultSettings) => { let scoring = {}; let attempts = popuplateItem({}, 'max_attempts', 'number', metadata); - if (_.isEmpty(attempts) || _.isNaN(attempts.number)) { + // TODO: impove below condition handling + if ((_.isEmpty(attempts) || _.isNaN(attempts.number)) && _.isNaN(defaultSettings.max_attempts)) { attempts = { unlimited: true, number: '' }; + } else if (!_.isEmpty(attempts) && !_.isNaN(attempts.number)) { + attempts.unlimited = false; + attempts.number = attempts.number; + if (attempts.number < 0) { + attempts.number = 0; + } + } else if (!_.isNaN(defaultSettings.max_attempts)) { + attempts.unlimited = false; + attempts.number = defaultSettings.max_attempts; } else { attempts.unlimited = false; if (attempts.number < 0) { @@ -43,14 +53,14 @@ export const parseShowAnswer = (metadata) => { return showAnswer; }; -export const parseSettings = (metadata) => { +export const parseSettings = (metadata, defaultSettings) => { let settings = {}; if (_.isNil(metadata) || _.isEmpty(metadata)) { return settings; } - const scoring = parseScoringSettings(metadata); + const scoring = parseScoringSettings(metadata, defaultSettings); if (!_.isEmpty(scoring)) { settings = { ...settings, scoring }; } diff --git a/src/editors/data/redux/problem/reducers.js b/src/editors/data/redux/problem/reducers.js index 8c0683560..4173cbc9b 100644 --- a/src/editors/data/redux/problem/reducers.js +++ b/src/editors/data/redux/problem/reducers.js @@ -21,8 +21,8 @@ const initialState = { scoring: { weight: 1, attempts: { - unlimited: true, - number: '', + unlimited: false, + number: null, }, }, hints: [], @@ -210,9 +210,6 @@ const problem = createSlice({ setEnableTypeSelection: (state, { payload }) => { const { maxAttempts, showanswer, showResetButton } = payload; const attempts = { number: maxAttempts, unlimited: false }; - if (!maxAttempts) { - attempts.unlimited = true; - } return { ...state, settings: { diff --git a/src/editors/data/redux/thunkActions/problem.js b/src/editors/data/redux/thunkActions/problem.js index 0d29101de..4ed432660 100644 --- a/src/editors/data/redux/thunkActions/problem.js +++ b/src/editors/data/redux/thunkActions/problem.js @@ -25,7 +25,7 @@ export const isBlankProblem = ({ rawOLX }) => { return false; }; -export const getDataFromOlx = ({ rawOLX, rawSettings }) => { +export const getDataFromOlx = ({ rawOLX, rawSettings, defaultSettings }) => { let olxParser; let parsedProblem; try { @@ -33,13 +33,13 @@ export const getDataFromOlx = ({ rawOLX, rawSettings }) => { parsedProblem = olxParser.getParsedOLXData(); } catch (error) { console.error('The Problem Could Not Be Parsed from OLX. redirecting to Advanced editor.', error); - return { problemType: ProblemTypeKeys.ADVANCED, rawOLX, settings: parseSettings(rawSettings) }; + return { problemType: ProblemTypeKeys.ADVANCED, rawOLX, settings: parseSettings(rawSettings, defaultSettings) }; } if (parsedProblem?.problemType === ProblemTypeKeys.ADVANCED) { - return { problemType: ProblemTypeKeys.ADVANCED, rawOLX, settings: parseSettings(rawSettings) }; + return { problemType: ProblemTypeKeys.ADVANCED, rawOLX, settings: parseSettings(rawSettings, defaultSettings) }; } const { settings, ...data } = parsedProblem; - const parsedSettings = { ...settings, ...parseSettings(rawSettings) }; + const parsedSettings = { ...settings, ...parseSettings(rawSettings, defaultSettings) }; if (!_.isEmpty(rawOLX) && !_.isEmpty(data)) { return { ...data, rawOLX, settings: parsedSettings }; } @@ -50,7 +50,7 @@ export const loadProblem = ({ rawOLX, rawSettings, defaultSettings }) => (dispat if (isBlankProblem({ rawOLX })) { dispatch(actions.problem.setEnableTypeSelection(camelizeKeys(defaultSettings))); } else { - dispatch(actions.problem.load(getDataFromOlx({ rawOLX, rawSettings }))); + dispatch(actions.problem.load(getDataFromOlx({ rawOLX, rawSettings, defaultSettings }))); } };