From c7ca4bf1d819dbb578c278d80dd75233fd0d7eca Mon Sep 17 00:00:00 2001 From: Alyssa Wang Date: Fri, 15 Sep 2023 10:43:57 -0400 Subject: [PATCH] FI-2075: Input validation (#388) * prevent modal from closing when inputs have been edited * remove log * FI-2003: Add codecov.yml (#384) * add codecov * turn codecov on * modify backend target * add patch coverage --------- Co-authored-by: Alyssa Wang * add carryforward * check edits on serial inputs * add close modal button to inputs dialog * keep edited status consistent between input types * add change listening to inputs * add required input warning * update oauth serial parsing * include null checks on oauth refresh nested fields --------- Co-authored-by: Alyssa Wang --- .../src/components/InputsModal/FieldLabel.tsx | 5 +- .../InputsModal/InputCheckboxGroup.tsx | 16 +++++- .../InputsModal/InputOAuthCredentials.tsx | 49 +++++++++++++------ .../components/InputsModal/InputTextArea.tsx | 13 +++-- .../components/InputsModal/InputTextField.tsx | 13 +++-- .../components/InputsModal/InputsModal.tsx | 6 +-- .../InputsModal/RequiredInputWarning.tsx | 12 ++++- client/src/components/InputsModal/styles.tsx | 3 +- 8 files changed, 86 insertions(+), 31 deletions(-) diff --git a/client/src/components/InputsModal/FieldLabel.tsx b/client/src/components/InputsModal/FieldLabel.tsx index 14439b272..1fd1084ea 100644 --- a/client/src/components/InputsModal/FieldLabel.tsx +++ b/client/src/components/InputsModal/FieldLabel.tsx @@ -2,12 +2,14 @@ import React, { FC } from 'react'; import LockIcon from '@mui/icons-material/Lock'; import { TestInput } from '~/models/testSuiteModels'; import useStyles from './styles'; +import RequiredInputWarning from './RequiredInputWarning'; export interface FieldLabelProps { requirement: TestInput; + isMissingInput?: boolean; } -const FieldLabel: FC = ({ requirement }) => { +const FieldLabel: FC = ({ requirement, isMissingInput = false }) => { const { classes } = useStyles(); const fieldLabelText = requirement.title || requirement.name; @@ -20,6 +22,7 @@ const FieldLabel: FC = ({ requirement }) => { return ( <> + {isMissingInput && } {fieldLabelText} {requiredLabel} {lockedIcon} diff --git a/client/src/components/InputsModal/InputCheckboxGroup.tsx b/client/src/components/InputsModal/InputCheckboxGroup.tsx index 203ae7bb4..725b7142f 100644 --- a/client/src/components/InputsModal/InputCheckboxGroup.tsx +++ b/client/src/components/InputsModal/InputCheckboxGroup.tsx @@ -26,6 +26,7 @@ const InputCheckboxGroup: FC = ({ setInputsMap, }) => { const { classes } = useStyles(); + const [hasBeenModified, setHasBeenModified] = React.useState(false); const [values, setValues] = React.useState(() => { // Default values should be in form ['value'] where all values are checked @@ -61,6 +62,9 @@ const InputCheckboxGroup: FC = ({ return startingValues as CheckboxValues; }); + const isMissingInput = + hasBeenModified && !requirement.optional && inputsMap.get(requirement.name) === '[]'; + useEffect(() => { // Make sure starting values get set in inputsMap inputsMap.set(requirement.name, transformValuesToJSONArray(values)); @@ -75,6 +79,7 @@ const InputCheckboxGroup: FC = ({ inputsMap.set(requirement.name, transformValuesToJSONArray(newValues)); setInputsMap(new Map(inputsMap)); setValues(newValues); + setHasBeenModified(true); }; // Convert map from item name to checked status back to array of checked values @@ -93,11 +98,13 @@ const InputCheckboxGroup: FC = ({ component="fieldset" id={`requirement${index}_input`} disabled={requirement.locked} + required={!requirement.optional} + error={isMissingInput} fullWidth className={classes.inputField} > - - + + {requirement.description && ( {requirement.description} @@ -111,6 +118,11 @@ const InputCheckboxGroup: FC = ({ color="secondary" name={option.value} checked={values[option.value as keyof CheckboxValues] || false} + onBlur={(e) => { + if (e.currentTarget === e.target) { + setHasBeenModified(true); + } + }} onChange={handleChange} /> } diff --git a/client/src/components/InputsModal/InputOAuthCredentials.tsx b/client/src/components/InputsModal/InputOAuthCredentials.tsx index 2b04e5f58..adfa2796d 100644 --- a/client/src/components/InputsModal/InputOAuthCredentials.tsx +++ b/client/src/components/InputsModal/InputOAuthCredentials.tsx @@ -12,6 +12,7 @@ import { import { OAuthCredentials, TestInput } from '~/models/testSuiteModels'; import FieldLabel from './FieldLabel'; import useStyles from './styles'; +import RequiredInputWarning from './RequiredInputWarning'; export interface InputOAuthCredentialsProps { requirement: TestInput; @@ -35,22 +36,20 @@ const InputOAuthCredentials: FC = ({ setInputsMap, }) => { const { classes } = useStyles(); + const [hasBeenModified, setHasBeenModified] = React.useState({}); // Convert OAuth string to Object // OAuth should be an Object while in this component but should be converted to a string // before being updated in the inputs map - const oAuthCredentials = ( - inputsMap.get(requirement.name) - ? JSON.parse(inputsMap.get(requirement.name) as string) - : { - access_token: '', - refresh_token: '', - expires_in: '', - client_id: '', - client_secret: '', - token_url: '', - } - ) as OAuthCredentials; + const oAuthCredentials = { + access_token: '', + refresh_token: '', + expires_in: '', + client_id: '', + client_secret: '', + token_url: '', + ...JSON.parse((inputsMap.get(requirement.name) as string) || '{}'), + } as OAuthCredentials; const showRefreshDetails = !!oAuthCredentials.refresh_token; @@ -91,16 +90,32 @@ const InputOAuthCredentials: FC = ({ }, ]; + const getIsMissingInput = (field: InputOAuthField) => { + return ( + hasBeenModified[field.name as keyof typeof hasBeenModified] && + field.required && + !oAuthCredentials[field.name as keyof OAuthCredentials] + ); + }; + const oAuthField = (field: InputOAuthField) => { - const fieldLabel = field.required + const fieldName = field.required ? `${(field.label || field.name) as string} (required)` : field.label || field.name; + + const fieldLabel = ( + <> + {getIsMissingInput(field) && } + {fieldName} + + ); + return ( = ({ variant="standard" color="secondary" fullWidth + onBlur={(e) => { + if (e.currentTarget === e.target) { + setHasBeenModified({ ...hasBeenModified, [field.name]: true }); + } + }} onChange={(event) => { const value = event.target.value; oAuthCredentials[field.name as keyof OAuthCredentials] = value; @@ -129,7 +149,6 @@ const InputOAuthCredentials: FC = ({ required={!requirement.optional} disabled={requirement.locked} className={classes.inputLabel} - shrink > diff --git a/client/src/components/InputsModal/InputTextArea.tsx b/client/src/components/InputsModal/InputTextArea.tsx index 807f42f6e..68db3194e 100644 --- a/client/src/components/InputsModal/InputTextArea.tsx +++ b/client/src/components/InputsModal/InputTextArea.tsx @@ -16,27 +16,34 @@ const InputTextArea: FC = ({ requirement, index, inputsMap, const { classes } = useStyles(); const [hasBeenModified, setHasBeenModified] = React.useState(false); + const isMissingInput = + hasBeenModified && !requirement.optional && !inputsMap.get(requirement.name); + return ( } + label={} helperText={requirement.description} value={inputsMap.get(requirement.name)} multiline rows={4} + onBlur={(e) => { + if (e.currentTarget === e.target) { + setHasBeenModified(true); + } + }} onChange={(event) => { const value = event.target.value; inputsMap.set(requirement.name, value); setInputsMap(new Map(inputsMap)); - setHasBeenModified(true); }} FormHelperTextProps={{ sx: { '&.Mui-disabled': { color: lightTheme.palette.common.grayDark } }, diff --git a/client/src/components/InputsModal/InputTextField.tsx b/client/src/components/InputsModal/InputTextField.tsx index 841354628..e54298be4 100644 --- a/client/src/components/InputsModal/InputTextField.tsx +++ b/client/src/components/InputsModal/InputTextField.tsx @@ -21,25 +21,32 @@ const InputTextField: FC = ({ const { classes } = useStyles(); const [hasBeenModified, setHasBeenModified] = React.useState(false); + const isMissingInput = + hasBeenModified && !requirement.optional && !inputsMap.get(requirement.name); + return ( } + label={} helperText={requirement.description} value={inputsMap.get(requirement.name)} + onBlur={(e) => { + if (e.currentTarget === e.target) { + setHasBeenModified(true); + } + }} onChange={(event) => { const value = event.target.value; inputsMap.set(requirement.name, value); setInputsMap(new Map(inputsMap)); - setHasBeenModified(true); }} InputLabelProps={{ shrink: true }} FormHelperTextProps={{ diff --git a/client/src/components/InputsModal/InputsModal.tsx b/client/src/components/InputsModal/InputsModal.tsx index 97e94cef4..d78cf5be0 100644 --- a/client/src/components/InputsModal/InputsModal.tsx +++ b/client/src/components/InputsModal/InputsModal.tsx @@ -98,8 +98,7 @@ const InputsModal: FC = ({ ) as OAuthCredentials; const accessTokenIsEmpty = oAuthJSON.access_token === ''; const refreshIsEmpty = - oAuthJSON.refresh_token !== '' && - (oAuthJSON.token_url === '' || oAuthJSON.client_id === ''); + oAuthJSON.refresh_token !== '' && (!oAuthJSON.token_url || !oAuthJSON.client_id); oAuthMissingRequiredInput = (accessTokenIsEmpty && !input.optional) || refreshIsEmpty; } catch (e: unknown) { const errorMessage = e instanceof Error ? e.message : String(e); @@ -281,8 +280,9 @@ const InputsModal: FC = ({ const parsedChanges = parseSerialChanges(serialChanges); if (parsedChanges !== undefined && parsedChanges.keys !== undefined) { parsedChanges.forEach((change: TestInput) => { - if (!change.locked && change.value !== undefined) + if (!change.locked && change.value !== undefined) { inputsMap.set(change.name, change.value || ''); + } }); } handleSetInputsMap(new Map(inputsMap), true); diff --git a/client/src/components/InputsModal/RequiredInputWarning.tsx b/client/src/components/InputsModal/RequiredInputWarning.tsx index d0855a55f..51bef2437 100644 --- a/client/src/components/InputsModal/RequiredInputWarning.tsx +++ b/client/src/components/InputsModal/RequiredInputWarning.tsx @@ -1,11 +1,19 @@ import React, { FC } from 'react'; -import WarningIcon from '@mui/icons-material/Warning'; +import { Report } from '@mui/icons-material'; import CustomTooltip from '~/components/_common/CustomTooltip'; const RequiredInputWarning: FC = () => { return ( - + ); }; diff --git a/client/src/components/InputsModal/styles.tsx b/client/src/components/InputsModal/styles.tsx index 75279782e..729e6b272 100644 --- a/client/src/components/InputsModal/styles.tsx +++ b/client/src/components/InputsModal/styles.tsx @@ -23,9 +23,8 @@ export default makeStyles()((theme: Theme) => ({ }, }, inputLabel: { - color: theme.palette.common.grayDarkest, + color: theme.palette.common.grayDarker, fontWeight: 600, - fontSize: '.75rem', }, lockedIcon: { marginLeft: '5px',