Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: answer range format validation and error message in Numerical input #1557

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { FeedbackBox } from './components/Feedback';
import * as hooks from './hooks';
import { ProblemTypeKeys } from '../../../../../data/constants/problem';
import ExpandableTextArea from '../../../../../sharedComponents/ExpandableTextArea';
import { answerRangeFormatRegex } from '../../../data/OLXParser';

const AnswerOption = ({
answer,
Expand All @@ -42,6 +43,11 @@ const AnswerOption = ({
const setUnselectedFeedback = hooks.setUnselectedFeedback({ answer, hasSingleAnswer, dispatch });
const { isFeedbackVisible, toggleFeedback } = hooks.useFeedback(answer);

const validateAnswerTitle = (value) => {
const cleanedValue = value.replace(/^\s+|\s+$/g, '');
return !cleanedValue.length || answerRangeFormatRegex.test(cleanedValue);
};

const getInputArea = () => {
if ([ProblemTypeKeys.SINGLESELECT, ProblemTypeKeys.MULTISELECT].includes(problemType)) {
return (
Expand Down Expand Up @@ -72,8 +78,9 @@ const AnswerOption = ({
);
}
// Return Answer Range View
const isValidValue = validateAnswerTitle(answer.title);
return (
<div>
<Form.Group isInvalid={!isValidValue}>
<Form.Control
as="textarea"
className="answer-option-textarea text-gray-500 small"
Expand All @@ -83,11 +90,15 @@ const AnswerOption = ({
onChange={setAnswerTitle}
placeholder={intl.formatMessage(messages.answerRangeTextboxPlaceholder)}
/>
{!isValidValue && (
<Form.Control.Feedback type="invalid">
<FormattedMessage {...messages.answerRangeErrorText} />
</Form.Control.Feedback>
)}
<div className="pgn__form-switch-helper-text">
<FormattedMessage {...messages.answerRangeHelperText} />
</div>
</div>

</Form.Group>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('AnswerOption', () => {
};
const answerRange = {
id: 'A',
title: 'Answer 1',
title: '[2,5]',
correct: true,
selectedFeedback: 'selected feedback',
unselectedFeedback: 'unselected feedback',
Expand Down Expand Up @@ -72,6 +72,9 @@ describe('AnswerOption', () => {
test('snapshot: renders correct option with numeric input problem and answer range', () => {
expect(shallow(<AnswerOption {...props} problemType="numericalresponse" answer={answerRange} />).snapshot).toMatchSnapshot();
});
test('snapshot: renders incorrect option with numeric input problem and answer range', () => {
expect(shallow(<AnswerOption {...props} problemType="numericalresponse" answer={{ ...answerRange, title: '[2.5]' }} />).snapshot).toMatchSnapshot();
});
});

describe('mapStateToProps', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ exports[`AnswerOption render snapshot: renders correct option with numeric input
"id": "A",
"isAnswerRange": true,
"selectedFeedback": "selected feedback",
"title": "Answer 1",
"title": "[2,5]",
"unselectedFeedback": "unselected feedback",
}
}
Expand All @@ -190,15 +190,17 @@ exports[`AnswerOption render snapshot: renders correct option with numeric input
<div
className="ml-1 flex-grow-1"
>
<div>
<Form.Group
isInvalid={false}
>
<Form.Control
as="textarea"
autoResize={true}
className="answer-option-textarea text-gray-500 small"
onChange={[Function]}
placeholder="Enter an answer range"
rows={1}
value="Answer 1"
value="[2,5]"
/>
<div
className="pgn__form-switch-helper-text"
Expand All @@ -209,7 +211,7 @@ exports[`AnswerOption render snapshot: renders correct option with numeric input
id="authoring.answerwidget.answer.answerRangeHelperText"
/>
</div>
</div>
</Form.Group>
<Body>
<injectIntl(ShimmedIntlComponent)
answer={
Expand All @@ -218,7 +220,7 @@ exports[`AnswerOption render snapshot: renders correct option with numeric input
"id": "A",
"isAnswerRange": true,
"selectedFeedback": "selected feedback",
"title": "Answer 1",
"title": "[2,5]",
"unselectedFeedback": "unselected feedback",
}
}
Expand Down Expand Up @@ -340,3 +342,109 @@ exports[`AnswerOption render snapshot: renders correct option with selected unse
</div>
</Advanced>
`;

exports[`AnswerOption render snapshot: renders incorrect option with numeric input problem and answer range 1`] = `
<Advanced
className="answer-option d-flex flex-row justify-content-between flex-nowrap pb-2 pt-2"
onToggle={[Function]}
open={false}
>
<div
className="mr-1 d-flex"
>
<Checker
answer={
{
"correct": true,
"id": "A",
"isAnswerRange": true,
"selectedFeedback": "selected feedback",
"title": "[2.5]",
"unselectedFeedback": "unselected feedback",
}
}
disabled={true}
hasSingleAnswer={false}
setAnswer={[Function]}
/>
</div>
<div
className="ml-1 flex-grow-1"
>
<Form.Group
isInvalid={true}
>
<Form.Control
as="textarea"
autoResize={true}
className="answer-option-textarea text-gray-500 small"
onChange={[Function]}
placeholder="Enter an answer range"
rows={1}
value="[2.5]"
/>
<Form.Control.Feedback
type="invalid"
>
<FormattedMessage
defaultMessage="Error: Invalid range format. Use brackets or parentheses with values separated by a comma."
description="Error text describing wrong format of answer ranges"
id="authoring.answerwidget.answer.answerRangeErrorText"
/>
</Form.Control.Feedback>
<div
className="pgn__form-switch-helper-text"
>
<FormattedMessage
defaultMessage="Enter min and max values separated by a comma. Use a bracket to include the number next to it in the range, or a parenthesis to exclude the number. For example, to identify the correct answers as 5, 6, or 7, but not 8, specify [5,8)."
description="Helper text describing usage of answer ranges"
id="authoring.answerwidget.answer.answerRangeHelperText"
/>
</div>
</Form.Group>
<Body>
<injectIntl(ShimmedIntlComponent)
answer={
{
"correct": true,
"id": "A",
"isAnswerRange": true,
"selectedFeedback": "selected feedback",
"title": "[2.5]",
"unselectedFeedback": "unselected feedback",
}
}
images={{}}
intl={
{
"formatMessage": [Function],
}
}
isLibrary={false}
learningContextId="course+org+run"
problemType="numericalresponse"
setSelectedFeedback={[Function]}
setUnselectedFeedback={[Function]}
/>
</Body>
</div>
<div
className="d-flex flex-row flex-nowrap"
>
<Trigger
aria-label="Toggle feedback"
className="btn-icon btn-icon-primary btn-icon-md align-items-center"
>
<Icon
alt="Toggle feedback"
/>
</Trigger>
<IconButton
alt="Delete answer"
iconAs="Icon"
onClick={[Function]}
variant="primary"
/>
</div>
</Advanced>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ const messages = defineMessages({
defaultMessage: 'Enter min and max values separated by a comma. Use a bracket to include the number next to it in the range, or a parenthesis to exclude the number. For example, to identify the correct answers as 5, 6, or 7, but not 8, specify [5,8).',
description: 'Helper text describing usage of answer ranges',
},
answerRangeErrorText: {
id: 'authoring.answerwidget.answer.answerRangeErrorText',
defaultMessage: 'Error: Invalid range format. Use brackets or parentheses with values separated by a comma.',
description: 'Error text describing wrong format of answer ranges',
},
});

export default messages;
4 changes: 3 additions & 1 deletion src/editors/containers/ProblemEditor/data/OLXParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ export const responseKeys = [
'choicetextresponse',
];

export const answerRangeFormatRegex = /^[([]\s*\d+(\.\d+)?\s*,\s*\d+(\.\d+)?\s*[)\]]$/m;

export const stripNonTextTags = ({ input, tag }) => {
const stripedTags = {};
Object.entries(input).forEach(([key, value]) => {
Expand Down Expand Up @@ -423,7 +425,7 @@ export class OLXParser {
[type]: defaultValue,
};
}
const isAnswerRange = /[([]\s*\d*,\s*\d*\s*[)\]]/gm.test(numericalresponse['@_answer']);
const isAnswerRange = answerRangeFormatRegex.test(numericalresponse['@_answer']);
answers.push({
id: indexToLetterMap[answers.length],
title: numericalresponse['@_answer'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,16 +413,18 @@
const lowerBoundFloat = Number(numerator) / Number(denominator);
lowerBoundInt = lowerBoundFloat;
} else {
// these regex replaces remove everything that is not a decimal or positive/negative numer
// these regex replaces remove everything that is not a decimal or positive/negative number
lowerBoundInt = Number(rawLowerBound.replace(/[^0-9-.]/gm, ''));
}
if (rawUpperBound.includes('/')) {
if (!rawUpperBound) {
upperBoundInt = lowerBoundInt;

Check warning on line 420 in src/editors/containers/ProblemEditor/data/ReactStateOLXParser.js

View check run for this annotation

Codecov / codecov/patch

src/editors/containers/ProblemEditor/data/ReactStateOLXParser.js#L420

Added line #L420 was not covered by tests
} else if (rawUpperBound.includes('/')) {
upperBoundFraction = rawUpperBound.replace(/[^0-9-/]/gm, '');
const [numerator, denominator] = upperBoundFraction.split('/');
const upperBoundFloat = Number(numerator) / Number(denominator);
upperBoundInt = upperBoundFloat;
} else {
// these regex replaces remove everything that is not a decimal or positive/negative numer
// these regex replaces remove everything that is not a decimal or positive/negative number
upperBoundInt = Number(rawUpperBound.replace(/[^0-9-.]/gm, ''));
}
if (lowerBoundInt > upperBoundInt) {
Expand Down
Loading