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

CARDS-2580 - Number question: allow min/max value to be specified but not enforced #1810

Merged
merged 8 commits into from
Oct 15, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const GHOST_SENTINEL = "custom-input";
function MultipleChoice(props) {
let { classes, customInput, customInputProps, existingAnswer, input, textbox, onUpdate, onChange, additionalInputProps, muiInputProps, naValue, noneOfTheAboveValue, error, questionName, ...rest } = props;
let { maxAnswers, minAnswers, displayMode, enableSeparatorDetection } = {...props.questionDefinition, ...props};
let { validate, validationErrorText, liveValidation } = {...props.questionDefinition, ...props};
let { validate, validationErrorText, liveValidation, softValidation } = {...props.questionDefinition, ...props};
// pageActive should be passed to the Answer component, so we make sure to include it in the `rest` variable above
let { instanceId, pageActive } = props;

Expand Down Expand Up @@ -388,8 +388,12 @@ function MultipleChoice(props) {
:
<TextField
variant="standard"
error={inputError}
helperText={inputError ? validationErrorText : maxAnswers !== 1 && "Press ENTER to add a new option"}
error={error || inputError}
helperText={
inputError
? <FormattedText variant="caption">{ validationErrorText }</FormattedText>
: maxAnswers !== 1 && !error && "Press ENTER to add a new option"
}
className={classes.textField + (isRadio ? (' ' + classes.nestedInput) : '')}
onChange={ghostUpdateEvent}
disabled={disabled}
Expand All @@ -401,7 +405,7 @@ function MultipleChoice(props) {
// We need to stop the event so that it doesn't trigger a form submission
event.preventDefault();
event.stopPropagation();
acceptEnteredOption(true);
acceptEnteredOption(!softValidation);
}
},
tabIndex: isRadio ? -1 : undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ const useSliderStyles = makeStyles(theme => ({
// />
function NumberQuestion(props) {
const { existingAnswer, errorText, classes, pageActive, disableValueInstructions, ...rest} = props;
const { dataType,displayMode, minAnswers, minValue, maxValue, isRange,
const { dataType,displayMode, minAnswers, minValue, maxValue, disableMinMaxValueEnforcement, messageForValuesOutsideMinMax, isRange,
sliderStep, sliderMarkStep, sliderOrientation, minValueLabel, maxValueLabel }
= {sliderOrientation: "horizontal", ...props.questionDefinition, ...props};
const answerNodeType = props.answerNodeType || DATA_TO_NODE_TYPE[dataType];
Expand Down Expand Up @@ -177,10 +177,9 @@ function NumberQuestion(props) {
{ height: Math.max(100, sliderMarks.length*30) + "px" } : undefined

// Callback function for our min/max
let hasError = (text) => {
let hasMinMaxValueError = (text) => {
let value = 0;

if (typeof(text) === "undefined") {
if (typeof(text) === "undefined" || text === "") {
// The custom input has been unset
return false;
}
Expand Down Expand Up @@ -210,17 +209,12 @@ function NumberQuestion(props) {
return false;
}

// Callback for a change of MultipleChoice input to check for errors on the input
let findError = (text) => {
setMinMaxError(text && hasError(text));
}

React.useEffect(() => {
if (!isRange) return;
// Check for invalid range limits
setMinMaxError(
lowerLimit && hasError(lowerLimit) ||
upperLimit && hasError(upperLimit)
lowerLimit && hasMinMaxValueError(lowerLimit) ||
upperLimit && hasMinMaxValueError(upperLimit)
);
setRangeError(
typeof(lowerLimit) == 'undefined' && typeof(upperLimit) != 'undefined' ||
Expand All @@ -231,7 +225,7 @@ function NumberQuestion(props) {
React.useEffect(() => {
if (!isSlider || isRange) return;
setMinMaxError(
sliderValue && hasError(sliderValue)
sliderValue && hasMinMaxValueError(sliderValue)
);
}, [sliderValue]);

Expand All @@ -247,7 +241,7 @@ function NumberQuestion(props) {
const textFieldProps = {
min: minValue,
max: maxValue,
allowNegative: (typeof minValue === "undefined" || minValue < 0),
allowNegative: (typeof minValue === "undefined" || minValue < 0 || disableMinMaxValueEnforcement),
decimalScale: dataType === "long" ? 0 : undefined
};
const muiInputProps = {
Expand All @@ -266,17 +260,21 @@ function NumberQuestion(props) {
// * minValue = 0
// * displayMode = slider
let minMaxMessage = "";
if ((minValue || typeof maxValue !== "undefined") && !isSlider && !disableValueInstructions) {
minMaxMessage = "Please enter values ";
if (typeof minValue !== "undefined" && typeof maxValue !== "undefined") {
minMaxMessage = `${minMaxMessage} between ${minValue} and ${maxValue}`;
} else if (typeof minValue !== "undefined") {
minMaxMessage = `${minMaxMessage} of at least ${minValue}`;
if ((typeof minValue !== "undefined" || typeof maxValue !== "undefined") && !isSlider && !disableValueInstructions) {
if (disableMinMaxValueEnforcement && typeof messageForValuesOutsideMinMax != "undefined") {
minMaxMessage = messageForValuesOutsideMinMax;
} else {
minMaxMessage = `${minMaxMessage} of at most ${maxValue}`;
}
if (hasAnswerOptions) {
minMaxMessage = `${minMaxMessage} or select one of the options`;
minMaxMessage = "Please enter values ";
if (typeof minValue !== "undefined" && typeof maxValue !== "undefined") {
minMaxMessage = `${minMaxMessage} between ${minValue} and ${maxValue}`;
} else if (typeof minValue !== "undefined") {
minMaxMessage = `${minMaxMessage} of at least ${minValue}`;
} else {
minMaxMessage = `${minMaxMessage} of at most ${maxValue}`;
}
if (hasAnswerOptions) {
minMaxMessage = `${minMaxMessage} or select one of the options`;
}
}
}

Expand Down Expand Up @@ -343,7 +341,7 @@ function NumberQuestion(props) {
{ errorText }
</Typography>
}
{ pageActive && minMaxMessage &&
{ pageActive && minMaxMessage && !disableMinMaxValueEnforcement &&
<Typography
component="p"
color={minMaxError ? 'error' : 'textSecondary'}
Expand Down Expand Up @@ -438,12 +436,15 @@ function NumberQuestion(props) {
valueType={valueType}
input={displayMode === "input" || displayMode === "list+input"}
textbox={displayMode === "textbox"}
onUpdate={findError}
onUpdate={text => setMinMaxError(!!text && hasMinMaxValueError(text))}
additionalInputProps={textFieldProps}
muiInputProps={muiInputProps}
error={minMaxError}
error={!disableMinMaxValueEnforcement && minMaxError}
existingAnswer={existingAnswer}
pageActive={pageActive}
validate={disableMinMaxValueEnforcement ? value => !hasMinMaxValueError(value) : undefined}
validationErrorText={minMaxMessage}
softValidation={disableMinMaxValueEnforcement}
{...rest}
/>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"entryMode": "Specifies the source of the value for this question: \n* manually entered by the **user**,\n* **computed** using a specified expression and answers to other questions, \n* copied over from another **reference** answer to a question specified by a path, or \n* **autocreated** by a backend script (the logic for populating autocreated answers cannot be configured from the user interface).",
"expression": "Supports javascript syntax, with special placeholder variables being populated with answers to other questions in the form.\n\nExample:\n\n```return @{laps} * @{lapLength:-100}```\n\n where `laps` and `lapLength` are the names of questions in the same form, `:-` is an optional marker for a default value, and `100` is the optional default value.",
"enableUserEntry": "If enabled, the user is not restricted to using one of the suggestions in the dropdown, and may save their own text as the free-form answer to the question.",
"disableMinMaxValueEnforcement": "If set, the value is not _required_ to be within the specified (**Min value**, **Max value**) limits, and an out of answer value will NOT be marked as invalid. A message will be displayed under the input, warning the user that the value they entered appears abnormal, without further consequences.",
"messageForValuesOutsideMinMax": "Message that guides the user to enter values between **Min value** and **Max value**, displayed when **Min value** and/or **Max value** are specified but not enforced. If absent, a generic message 'Please enter values between ...' will be displayed.",
"question": "Path to a question, either in this or a different questionnaire. Example: `/Questionnaires/Demographics/name`",
"defaultCountry": "The default country code (ISO 3166), for example 'ca'.",
"regions": "In the country dropdown, show only countries from certain regions or subregions, specified as a comma-separated list. For example:\n* Regions - `america, europe, asia, oceania, africa`);\n* Subregions -`north-america, south-america, central-america, carribean, eu-union, ex-ussr, ex-yugos, baltic, middle-east, north-africa`.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"minAnswers": "long",
"maxAnswers": "long",
"validationRegexp": "string",
"validationErrorText": "string",
"validationErrorText": "markdown",
"liveValidation": "boolean",
"enableSeparatorDetection": "boolean",
"displayMode": {
Expand Down Expand Up @@ -133,6 +133,8 @@
"maxAnswers": "long",
"minValue": "double",
"maxValue": "double",
marta- marked this conversation as resolved.
Show resolved Hide resolved
"disableMinMaxValueEnforcement" : "boolean",
"messageForValuesOutsideMinMax" : "markdown",
"displayMode": {
"input": {
"isRange" : "boolean"
Expand Down Expand Up @@ -195,6 +197,8 @@
"maxAnswers": "long",
"minValue": "double",
"maxValue": "double",
"disableMinMaxValueEnforcement" : "boolean",
"messageForValuesOutsideMinMax" : "markdown",
"displayMode": {
"input": {
"isRange" : "boolean"
Expand Down Expand Up @@ -257,6 +261,8 @@
"maxAnswers": "long",
"minValue": "double",
"maxValue": "double",
"disableMinMaxValueEnforcement" : "boolean",
"messageForValuesOutsideMinMax" : "markdown",
"displayMode": {
"input": {
"isRange" : "boolean"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@
- minValue (double)
- maxValue (double)

// For numeric answers with min/max values, specify if the limits are a hard or soft requirement.
// If they are a soft requirement, a value outside limits does NOT make the answer (and the form) invalid.
- disableMinMaxValueEnforcement (boolean)
// When the limits are a soft requirement and the value is outside limits, an optional custom message is displayed to the user entering the data
- messageForValuesOutsideMinMax (string)

// For questions that have a list of options, whether all the options are displayed on a single line (compact), or one per line (default)
- compact (boolean)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@
@Component(immediate = true)
public class MinMaxValueValidator implements AnswerValidator
{
private static final String DATA_TYPE_PROP = "dataType";

private static final String MAX_VALUE_PROP = "maxValue";

private static final String MIN_VALUE_PROP = "minValue";

private static final String DISABLE_MIN_MAX_ENFORCEMENT_PROP = "disableMinMaxValueEnforcement";

private static final Set<String> SUPPORTED_TYPES = Set.of("long", "double", "decimal");

@Override
Expand All @@ -50,16 +58,15 @@ public void validate(final NodeBuilder answer, final Node question, final boolea
final Map<String, Boolean> flags)
{
try {
final String type = question.getProperty("dataType").getString();
if (!SUPPORTED_TYPES.contains(type)) {
// This only works on numerical types, nothing to do if this is not one of them
if (!isMinMaxValidationApplicable(question)) {
// If this isn't a numeric value with required limits, don't validate
return;
}
if (answer.hasProperty(PROP_VALUE)) {
final double minValue =
question.hasProperty("minValue") ? question.getProperty("minValue").getDouble() : Double.NaN;
final double maxValue =
question.hasProperty("maxValue") ? question.getProperty("maxValue").getDouble() : Double.NaN;
final double minValue = question.hasProperty(MIN_VALUE_PROP)
? question.getProperty(MIN_VALUE_PROP).getDouble() : Double.NaN;
final double maxValue = question.hasProperty(MAX_VALUE_PROP)
? question.getProperty(MAX_VALUE_PROP).getDouble() : Double.NaN;

final PropertyState answerProp = answer.getProperty(PROP_VALUE);
// if any value is out of range, set FLAG_INVALID to true
Expand All @@ -77,4 +84,24 @@ public void validate(final NodeBuilder answer, final Node question, final boolea
// If something goes wrong do nothing
}
}

private boolean isMinMaxValidationApplicable(final Node question)
{
try {
final String type = question.getProperty(DATA_TYPE_PROP).getString();
if (!SUPPORTED_TYPES.contains(type)) {
// This only works on numerical types, nothing to do if this is not one of them
return false;
}
final Boolean limitsNotEnforced = question.hasProperty(DISABLE_MIN_MAX_ENFORCEMENT_PROP)
? question.getProperty(DISABLE_MIN_MAX_ENFORCEMENT_PROP).getBoolean() : Boolean.FALSE;
if (limitsNotEnforced) {
// Value limits are not enforced, only suggested. Do not add the INVALID flag
return false;
}
} catch (final RepositoryException ex) {
// If something goes wrong do nothing
}
return true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->

<node>
<name>NumberMinMaxTest</name>
<primaryNodeType>cards:Questionnaire</primaryNodeType>
<property>
<name>title</name>
<value>Number Min / Max Value Test</value>
<type>String</type>
</property>
<property>
<name>description</name>
<value>CARDS-2580</value>
<type>String</type>
</property>
<property>
<name>requiredSubjectTypes</name>
<values>
<value>/SubjectTypes/Patient</value>
</values>
<type>Reference</type>
</property>
<node>
<name>long_1</name>
<primaryNodeType>cards:Question</primaryNodeType>
<property>
<name>text</name>
<value>Long, enforced min value and max value</value>
<type>String</type>
</property>
<property>
<name>minAnswers</name>
<value>1</value>
<type>Long</type>
</property>
<property>
<name>dataType</name>
<value>long</value>
<type>String</type>
</property>
<property>
<name>minValue</name>
<value>-10</value>
<type>Double</type>
</property>
<property>
<name>maxValue</name>
<value>10</value>
<type>Double</type>
</property>
</node>
<node>
<name>long_2</name>
<primaryNodeType>cards:Question</primaryNodeType>
<property>
<name>text</name>
<value>Long with min value and max value, **not enforced**</value>
<type>String</type>
</property>
<property>
<name>description</name>
<value>Expecting values between 0 and 20. A message will be displayed if avalue outside these limits is entered.</value>
<type>String</type>
</property>
<property>
<name>minAnswers</name>
<value>1</value>
<type>Long</type>
</property>
<property>
<name>dataType</name>
<value>long</value>
<type>String</type>
</property>
<property>
<name>displayMode</name>
<value>input</value>
<type>String</type>
</property>
<property>
<name>minValue</name>
<value>0</value>
<type>Double</type>
</property>
<property>
<name>maxValue</name>
<value>20</value>
<type>Double</type>
</property>
<property>
<name>disableMinMaxValueEnforcement</name>
<value>True</value>
<type>Boolean</type>
</property>
<property>
<name>messageForValuesOutsideMinMax</name>
<value>Please check that your entry is correct.
Expected values are between 0 and 20.</value>
<type>String</type>
</property>
</node>
</node>