Adds error handling and validation.

Also adresses small PR issues
This commit is contained in:
Alex Corey
2020-04-08 17:16:24 -04:00
parent ed3b6385f1
commit a95632c349
17 changed files with 169 additions and 48 deletions

View File

@@ -9,6 +9,7 @@ import {
CardBody as PFCardBody, CardBody as PFCardBody,
Expandable as PFExpandable, Expandable as PFExpandable,
} from '@patternfly/react-core'; } from '@patternfly/react-core';
import getErrorMessage from './getErrorMessage';
const Card = styled(PFCard)` const Card = styled(PFCard)`
background-color: var(--pf-global--BackgroundColor--200); background-color: var(--pf-global--BackgroundColor--200);
@@ -52,14 +53,7 @@ class ErrorDetail extends Component {
renderNetworkError() { renderNetworkError() {
const { error } = this.props; const { error } = this.props;
const { response } = error; const { response } = error;
const message = getErrorMessage(response);
let message = '';
if (response?.data) {
message =
typeof response.data === 'string'
? response.data
: response.data?.detail;
}
return ( return (
<Fragment> <Fragment>
@@ -67,7 +61,17 @@ class ErrorDetail extends Component {
{response?.config?.method.toUpperCase()} {response?.config?.url}{' '} {response?.config?.method.toUpperCase()} {response?.config?.url}{' '}
<strong>{response?.status}</strong> <strong>{response?.status}</strong>
</CardBody> </CardBody>
<CardBody>{message}</CardBody> <CardBody>
{Array.isArray(message) ? (
<ul>
{message.map(m => (
<li key={m}>{m}</li>
))}
</ul>
) : (
message
)}
</CardBody>
</Fragment> </Fragment>
); );
} }

View File

@@ -21,4 +21,26 @@ describe('ErrorDetail', () => {
); );
expect(wrapper).toHaveLength(1); expect(wrapper).toHaveLength(1);
}); });
test('testing errors', () => {
const wrapper = mountWithContexts(
<ErrorDetail
error={
new Error({
response: {
config: {
method: 'patch',
},
data: {
project: ['project error'],
inventory: ['inventory error'],
},
},
})
}
/>
);
wrapper.find('Expandable').prop('onToggle')();
wrapper.update();
// console.log(wrapper.find('ErrorDetail').prop('error'));
});
}); });

View File

@@ -0,0 +1,15 @@
export default function getErrorMessage(response) {
if (typeof response.data === 'string') {
return response.data;
}
if (!response.data) {
return null;
}
if (response.data.detail) {
return response.data.detail;
}
return Object.values(response.data).reduce(
(acc, currentValue) => acc.concat(currentValue),
[]
);
}

View File

@@ -0,0 +1,60 @@
import getErrorMessage from './getErrorMessage';
describe('getErrorMessage', () => {
test('should return data string', () => {
const response = {
data: 'error response',
};
expect(getErrorMessage(response)).toEqual('error response');
});
test('should return detail string', () => {
const response = {
data: {
detail: 'detail string',
},
};
expect(getErrorMessage(response)).toEqual('detail string');
});
test('should return an array of strings', () => {
const response = {
data: {
project: ['project error response'],
},
};
expect(getErrorMessage(response)).toEqual(['project error response']);
});
test('should consolidate error messages from multiple keys into an array', () => {
const response = {
data: {
project: ['project error response'],
inventory: ['inventory error response'],
organization: ['org error response'],
},
};
expect(getErrorMessage(response)).toEqual([
'project error response',
'inventory error response',
'org error response',
]);
});
test('should handle no response.data', () => {
const response = {};
expect(getErrorMessage(response)).toEqual(null);
});
test('should consolidate multiple error messages from multiple keys into an array', () => {
const response = {
data: {
project: ['project error response'],
inventory: [
'inventory error response',
'another inventory error response',
],
},
};
expect(getErrorMessage(response)).toEqual([
'project error response',
'inventory error response',
'another inventory error response',
]);
});
});

View File

@@ -322,6 +322,7 @@ exports[`<DeleteRoleConfirmationModal /> should render initially 1`] = `
className="pf-c-backdrop" className="pf-c-backdrop"
> >
<FocusTrap <FocusTrap
_createFocusTrap={[Function]}
active={true} active={true}
className="pf-l-bullseye" className="pf-l-bullseye"
focusTrapOptions={ focusTrapOptions={
@@ -330,6 +331,7 @@ exports[`<DeleteRoleConfirmationModal /> should render initially 1`] = `
} }
} }
paused={false} paused={false}
tag="div"
> >
<div <div
className="pf-l-bullseye" className="pf-l-bullseye"

View File

@@ -22,7 +22,7 @@ function SurveyList({
toggleSurvey, toggleSurvey,
updateSurvey, updateSurvey,
deleteSurvey, deleteSurvey,
canAddAndEditSurvey, canEdit,
i18n, i18n,
}) { }) {
const questions = survey?.spec || []; const questions = survey?.spec || [];
@@ -98,7 +98,7 @@ function SurveyList({
onSelect={() => handleSelect(question)} onSelect={() => handleSelect(question)}
onMoveUp={moveUp} onMoveUp={moveUp}
onMoveDown={moveDown} onMoveDown={moveDown}
canAddAndEditSurvey={canAddAndEditSurvey} canEdit={canEdit}
/> />
))} ))}
{isPreviewModalOpen && ( {isPreviewModalOpen && (
@@ -171,7 +171,7 @@ function SurveyList({
surveyEnabled={surveyEnabled} surveyEnabled={surveyEnabled}
onToggleSurvey={toggleSurvey} onToggleSurvey={toggleSurvey}
isDeleteDisabled={selected?.length === 0} isDeleteDisabled={selected?.length === 0}
canAddAndEditSurvey={canAddAndEditSurvey} canEdit={canEdit}
onToggleDeleteModal={() => setIsDeleteModalOpen(true)} onToggleDeleteModal={() => setIsDeleteModalOpen(true)}
/> />
{content} {content}

View File

@@ -54,11 +54,7 @@ describe('<SurveyList />', () => {
let wrapper; let wrapper;
await act(async () => { await act(async () => {
wrapper = mountWithContexts( wrapper = mountWithContexts(
<SurveyList <SurveyList survey={surveyData} deleteSurvey={deleteSurvey} canEdit />
survey={surveyData}
deleteSurvey={deleteSurvey}
canAddAndEditSurvey
/>
); );
}); });
wrapper.update(); wrapper.update();

View File

@@ -4,7 +4,8 @@ import { withI18n } from '@lingui/react';
import { Link } from 'react-router-dom'; import { Link } from 'react-router-dom';
import { import {
Button as _Button, Button as _Button,
Chip as _Chip, Chip,
ChipGroup,
DataListAction as _DataListAction, DataListAction as _DataListAction,
DataListCheck, DataListCheck,
DataListItemCells, DataListItemCells,
@@ -29,11 +30,8 @@ const Button = styled(_Button)`
padding-left: 0; padding-left: 0;
`; `;
const Required = styled.span` const Required = styled.span`
color: red; color: var(--pf-global--danger-color--100);
margin-left: 5px; margin-left: var(--pf-global--spacer--xs);
`;
const Chip = styled(_Chip)`
margin-right: 5px;
`; `;
const Label = styled.b` const Label = styled.b`
@@ -41,7 +39,7 @@ const Label = styled.b`
`; `;
function SurveyListItem({ function SurveyListItem({
canAddAndEditSurvey, canEdit,
question, question,
i18n, i18n,
isLast, isLast,
@@ -67,7 +65,7 @@ function SurveyListItem({
<Button <Button
variant="plain" variant="plain"
aria-label={i18n._(t`move up`)} aria-label={i18n._(t`move up`)}
isDisabled={isFirst || !canAddAndEditSurvey} isDisabled={isFirst || !canEdit}
onClick={() => onMoveUp(question)} onClick={() => onMoveUp(question)}
> >
<CaretUpIcon /> <CaretUpIcon />
@@ -77,7 +75,7 @@ function SurveyListItem({
<Button <Button
variant="plain" variant="plain"
aria-label={i18n._(t`move down`)} aria-label={i18n._(t`move down`)}
isDisabled={isLast || !canAddAndEditSurvey} isDisabled={isLast || !canEdit}
onClick={() => onMoveDown(question)} onClick={() => onMoveDown(question)}
> >
<CaretDownIcon /> <CaretDownIcon />
@@ -86,7 +84,7 @@ function SurveyListItem({
</Stack> </Stack>
</DataListAction> </DataListAction>
<DataListCheck <DataListCheck
isDisabled={!canAddAndEditSurvey} isDisabled={!canEdit}
checked={isChecked} checked={isChecked}
onChange={onSelect} onChange={onSelect}
aria-labelledby="survey check" aria-labelledby="survey check"
@@ -120,12 +118,15 @@ function SurveyListItem({
<span>{i18n._(t`encrypted`).toUpperCase()}</span> <span>{i18n._(t`encrypted`).toUpperCase()}</span>
)} )}
{[question.type].includes('multiselect') && {[question.type].includes('multiselect') &&
question.default.length > 0 && question.default.length > 0 && (
question.default.split('\n').map(chip => ( <ChipGroup numChips={5}>
<Chip key={chip} isReadOnly> {question.default.split('\n').map(chip => (
{chip} <Chip key={chip} isReadOnly>
</Chip> {chip}
))} </Chip>
))}
</ChipGroup>
)}
{![question.type].includes('password') && {![question.type].includes('password') &&
![question.type].includes('multiselect') && ( ![question.type].includes('multiselect') && (
<span>{question.default}</span> <span>{question.default}</span>

View File

@@ -96,9 +96,10 @@ describe('<SurveyListItem />', () => {
<SurveyListItem question={newItem} isChecked={false} isFirst isLast /> <SurveyListItem question={newItem} isChecked={false} isFirst isLast />
); );
}); });
expect(wrapper.find('Chip').length).toBe(9); expect(wrapper.find('Chip').length).toBe(6);
wrapper wrapper
.find('Chip') .find('Chip')
.filter(chip => chip.prop('isOverFlowChip') !== true)
.map(chip => expect(chip.prop('isReadOnly')).toBe(true)); .map(chip => expect(chip.prop('isReadOnly')).toBe(true));
}); });
test('items that are no required should have no an asterisk', () => { test('items that are no required should have no an asterisk', () => {

View File

@@ -13,7 +13,13 @@ import FormField, {
FieldTooltip, FieldTooltip,
} from '@components/FormField'; } from '@components/FormField';
import AnsibleSelect from '@components/AnsibleSelect'; import AnsibleSelect from '@components/AnsibleSelect';
import { required, noWhiteSpace, combine } from '@util/validators'; import {
required,
noWhiteSpace,
combine,
maxLength,
defaultIsNotAvailable,
} from '@util/validators';
function AnswerTypeField({ i18n }) { function AnswerTypeField({ i18n }) {
const [field] = useField({ const [field] = useField({
@@ -156,6 +162,7 @@ function SurveyQuestionForm({
<FormField <FormField
id="question-default" id="question-default"
name="default" name="default"
validate={maxLength(formik.values.max, i18n)}
type={formik.values.type === 'text' ? 'text' : 'number'} type={formik.values.type === 'text' ? 'text' : 'number'}
label={i18n._(t`Default answer`)} label={i18n._(t`Default answer`)}
/> />
@@ -191,6 +198,7 @@ function SurveyQuestionForm({
<FormField <FormField
id="question-default" id="question-default"
name="default" name="default"
validate={defaultIsNotAvailable(formik.values.choices, i18n)}
type={ type={
formik.values.type === 'multiplechoice' formik.values.type === 'multiplechoice'
? 'text' ? 'text'

View File

@@ -18,7 +18,7 @@ const DataToolbar = styled(_DataToolbar)`
`; `;
function SurveyToolbar({ function SurveyToolbar({
canAddAndEditSurvey, canEdit,
isAllSelected, isAllSelected,
onSelectAll, onSelectAll,
i18n, i18n,
@@ -27,14 +27,14 @@ function SurveyToolbar({
isDeleteDisabled, isDeleteDisabled,
onToggleDeleteModal, onToggleDeleteModal,
}) { }) {
isDeleteDisabled = !canAddAndEditSurvey || isDeleteDisabled; isDeleteDisabled = !canEdit || isDeleteDisabled;
const match = useRouteMatch(); const match = useRouteMatch();
return ( return (
<DataToolbar id="survey-toolbar"> <DataToolbar id="survey-toolbar">
<DataToolbarContent> <DataToolbarContent>
<DataToolbarItem> <DataToolbarItem>
<Checkbox <Checkbox
isDisabled={!canAddAndEditSurvey} isDisabled={!canEdit}
isChecked={isAllSelected} isChecked={isAllSelected}
onChange={isChecked => { onChange={isChecked => {
onSelectAll(isChecked); onSelectAll(isChecked);
@@ -50,14 +50,14 @@ function SurveyToolbar({
label={i18n._(t`On`)} label={i18n._(t`On`)}
labelOff={i18n._(t`Off`)} labelOff={i18n._(t`Off`)}
isChecked={surveyEnabled} isChecked={surveyEnabled}
isDisabled={!canAddAndEditSurvey} isDisabled={!canEdit}
onChange={() => onToggleSurvey(!surveyEnabled)} onChange={() => onToggleSurvey(!surveyEnabled)}
/> />
</DataToolbarItem> </DataToolbarItem>
<DataToolbarGroup> <DataToolbarGroup>
<DataToolbarItem> <DataToolbarItem>
<ToolbarAddButton <ToolbarAddButton
isDisabled={!canAddAndEditSurvey} isDisabled={!canEdit}
linkTo={`${match.url}/add`} linkTo={`${match.url}/add`}
/> />
</DataToolbarItem> </DataToolbarItem>

View File

@@ -36,7 +36,7 @@ describe('<SurveyToolbar />', () => {
isAllSelected isAllSelected
onToggleDeleteModal={jest.fn()} onToggleDeleteModal={jest.fn()}
onToggleSurvey={jest.fn()} onToggleSurvey={jest.fn()}
canAddAndEditSurvey canEdit
/> />
); );
}); });
@@ -96,7 +96,7 @@ describe('<SurveyToolbar />', () => {
isAllSelected isAllSelected
onToggleDelete={jest.fn()} onToggleDelete={jest.fn()}
onToggleSurvey={jest.fn()} onToggleSurvey={jest.fn()}
canAddAndEditSurvey={false} canEdit={false}
/> />
); );
}); });

View File

@@ -205,7 +205,7 @@ function Template({ i18n, me, setBreadcrumb }) {
<Route path="/templates/:templateType/:id/survey"> <Route path="/templates/:templateType/:id/survey">
<TemplateSurvey <TemplateSurvey
template={template} template={template}
canAddAndEditSurvey={canAddAndEditSurvey} canEdit={canAddAndEditSurvey}
/> />
</Route> </Route>
)} )}

View File

@@ -9,7 +9,7 @@ import ErrorDetail from '@components/ErrorDetail';
import useRequest, { useDismissableError } from '@util/useRequest'; import useRequest, { useDismissableError } from '@util/useRequest';
import { SurveyList, SurveyQuestionAdd, SurveyQuestionEdit } from './Survey'; import { SurveyList, SurveyQuestionAdd, SurveyQuestionEdit } from './Survey';
function TemplateSurvey({ template, canAddAndEditSurvey, i18n }) { function TemplateSurvey({ template, canEdit, i18n }) {
const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled); const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled);
const { templateType } = useParams(); const { templateType } = useParams();
@@ -85,7 +85,7 @@ function TemplateSurvey({ template, canAddAndEditSurvey, i18n }) {
return ( return (
<> <>
<Switch> <Switch>
{canAddAndEditSurvey && ( {canEdit && (
<Route path="/templates/:templateType/:id/survey/add"> <Route path="/templates/:templateType/:id/survey/add">
<SurveyQuestionAdd <SurveyQuestionAdd
survey={survey} survey={survey}
@@ -93,7 +93,7 @@ function TemplateSurvey({ template, canAddAndEditSurvey, i18n }) {
/> />
</Route> </Route>
)} )}
{canAddAndEditSurvey && ( {canEdit && (
<Route path="/templates/:templateType/:id/survey/edit/:variable"> <Route path="/templates/:templateType/:id/survey/edit/:variable">
<SurveyQuestionEdit <SurveyQuestionEdit
survey={survey} survey={survey}
@@ -109,7 +109,7 @@ function TemplateSurvey({ template, canAddAndEditSurvey, i18n }) {
toggleSurvey={toggleSurvey} toggleSurvey={toggleSurvey}
updateSurvey={updateSurveySpec} updateSurvey={updateSurveySpec}
deleteSurvey={deleteSurvey} deleteSurvey={deleteSurvey}
canAddAndEditSurvey={canAddAndEditSurvey} canEdit={canEdit}
/> />
</Route> </Route>
</Switch> </Switch>

View File

@@ -274,7 +274,7 @@ class WorkflowJobTemplate extends Component {
<Route path="/templates/:templateType/:id/survey"> <Route path="/templates/:templateType/:id/survey">
<TemplateSurvey <TemplateSurvey
template={template} template={template}
canAddAndEditSurvey={canAddAndEditSurvey} canEdit={canAddAndEditSurvey}
/> />
</Route> </Route>
)} )}

View File

@@ -48,6 +48,7 @@ describe('<WorkflowJobTemplate/>', () => {
{ name: 'Label 3', id: 3 }, { name: 'Label 3', id: 3 },
], ],
}, },
user_capabilities: {},
}, },
related: { related: {
webhook_key: '/api/v2/workflow_job_templates/57/webhook_key/', webhook_key: '/api/v2/workflow_job_templates/57/webhook_key/',

View File

@@ -36,6 +36,17 @@ export function minMaxValue(min, max, i18n) {
}; };
} }
export function defaultIsNotAvailable(choices, i18n) {
return defaultValue => {
if (!choices.includes(defaultValue)) {
return i18n._(
t`Default choice must be answered from the choices listed.`
);
}
return undefined;
};
}
export function requiredEmail(i18n) { export function requiredEmail(i18n) {
return value => { return value => {
if (!value) { if (!value) {