1
0
mirror of https://github.com/ansible/awx.git synced 2024-10-27 09:25:10 +03:00

Merge pull request #6562 from AlexSCorey/6333-SurveyCleanUp

Fixes several things about Survey List

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul
This commit is contained in:
softwarefactory-project-zuul[bot] 2020-04-15 18:33:07 +00:00 committed by GitHub
commit 7311ddf722
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 398 additions and 37 deletions

View File

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

View File

@ -21,4 +21,25 @@ describe('ErrorDetail', () => {
);
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();
});
});

View File

@ -0,0 +1,15 @@
export default function getErrorMessage(response) {
if (!response.data) {
return null;
}
if (typeof response.data === 'string') {
return response.data;
}
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

@ -6,9 +6,11 @@ import InventoryStep from './InventoryStep';
import CredentialsStep from './CredentialsStep';
import OtherPromptsStep from './OtherPromptsStep';
import PreviewStep from './PreviewStep';
import { InventoriesAPI } from '@api';
import { InventoriesAPI, CredentialsAPI, CredentialTypesAPI } from '@api';
jest.mock('@api/models/Inventories');
jest.mock('@api/models/CredentialTypes');
jest.mock('@api/models/Credentials');
let config;
const resource = {
@ -25,6 +27,10 @@ describe('LaunchPrompt', () => {
count: 1,
},
});
CredentialsAPI.read.mockResolvedValue({
data: { results: [{ id: 1 }], count: 1 },
});
CredentialTypesAPI.loadAllTypes({ data: { results: [{ type: 'ssh' }] } });
config = {
can_start_without_user_input: false,

View File

@ -5,7 +5,7 @@ import { Button, Tooltip } from '@patternfly/react-core';
import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro';
function ToolbarAddButton({ linkTo, onClick, i18n }) {
function ToolbarAddButton({ linkTo, onClick, i18n, isDisabled }) {
if (!linkTo && !onClick) {
throw new Error(
'ToolbarAddButton requires either `linkTo` or `onClick` prop'
@ -15,6 +15,7 @@ function ToolbarAddButton({ linkTo, onClick, i18n }) {
return (
<Tooltip content={i18n._(t`Add`)} position="top">
<Button
isDisabled={isDisabled}
component={Link}
to={linkTo}
variant="primary"

View File

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

View File

@ -43,6 +43,7 @@ describe('<SurveyList />', () => {
await act(async () => {
wrapper.find('Switch').invoke('onChange')(true);
});
wrapper.update();
expect(toggleSurvey).toHaveBeenCalled();
@ -53,14 +54,14 @@ describe('<SurveyList />', () => {
let wrapper;
await act(async () => {
wrapper = mountWithContexts(
<SurveyList survey={surveyData} deleteSurvey={deleteSurvey} />
<SurveyList survey={surveyData} deleteSurvey={deleteSurvey} canEdit />
);
});
wrapper.update();
expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
true
);
expect(
wrapper.find('Checkbox[aria-label="Select all"]').prop('isChecked')
).toBe(false);
@ -81,6 +82,7 @@ describe('<SurveyList />', () => {
act(() => {
wrapper.find('Button[variant="danger"]').invoke('onClick')();
});
wrapper.update();
await act(() =>
@ -88,6 +90,7 @@ describe('<SurveyList />', () => {
);
expect(deleteSurvey).toHaveBeenCalled();
});
test('should render Preview button ', async () => {
let wrapper;
@ -97,6 +100,7 @@ describe('<SurveyList />', () => {
expect(wrapper.find('Button[aria-label="Preview"]').length).toBe(1);
});
test('Preview button should render Modal', async () => {
let wrapper;
@ -108,6 +112,7 @@ describe('<SurveyList />', () => {
expect(wrapper.find('SurveyPreviewModal').length).toBe(1);
});
test('Modal close button should close modal', async () => {
let wrapper;
@ -119,11 +124,34 @@ describe('<SurveyList />', () => {
expect(wrapper.find('SurveyPreviewModal').length).toBe(1);
wrapper.find('Modal').prop('onClose')();
act(() => wrapper.find('Modal').prop('onClose')());
wrapper.update();
expect(wrapper.find('SurveyPreviewModal').length).toBe(0);
});
test('user without edit/delete permission cannot delete', async () => {
const deleteSurvey = jest.fn();
let wrapper;
await act(async () => {
wrapper = mountWithContexts(
<SurveyList survey={surveyData} deleteSurvey={deleteSurvey} />
);
});
expect(
wrapper
.find('DataToolbar')
.find('Checkbox')
.prop('isDisabled')
).toBe(true);
expect(wrapper.find('Switch').prop('isDisabled')).toBe(true);
expect(wrapper.find('ToolbarAddButton').prop('isDisabled')).toBe(true);
expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
true
);
});
});
describe('Survey with no questions', () => {

View File

@ -4,6 +4,8 @@ import { withI18n } from '@lingui/react';
import { Link } from 'react-router-dom';
import {
Button as _Button,
Chip,
ChipGroup,
DataListAction as _DataListAction,
DataListCheck,
DataListItemCells,
@ -27,8 +29,17 @@ const Button = styled(_Button)`
padding-bottom: 0;
padding-left: 0;
`;
const Required = styled.span`
color: var(--pf-global--danger-color--100);
margin-left: var(--pf-global--spacer--xs);
`;
const Label = styled.b`
margin-right: 20px;
`;
function SurveyListItem({
canEdit,
question,
i18n,
isLast,
@ -54,7 +65,7 @@ function SurveyListItem({
<Button
variant="plain"
aria-label={i18n._(t`move up`)}
isDisabled={isFirst}
isDisabled={isFirst || !canEdit}
onClick={() => onMoveUp(question)}
>
<CaretUpIcon />
@ -64,7 +75,7 @@ function SurveyListItem({
<Button
variant="plain"
aria-label={i18n._(t`move down`)}
isDisabled={isLast}
isDisabled={isLast || !canEdit}
onClick={() => onMoveDown(question)}
>
<CaretDownIcon />
@ -73,6 +84,7 @@ function SurveyListItem({
</Stack>
</DataListAction>
<DataListCheck
isDisabled={!canEdit}
checked={isChecked}
onChange={onSelect}
aria-labelledby="survey check"
@ -80,12 +92,46 @@ function SurveyListItem({
<DataListItemCells
dataListCells={[
<DataListCell key="name">
<Link to={`survey/edit/${question.variable}`}>
{question.question_name}
</Link>
<>
<Link to={`survey/edit/${question.variable}`}>
{question.question_name}
</Link>
{question.required && (
<Required
aria-label={i18n._(t`Required`)}
className="pf-c-form__label-required"
aria-hidden="true"
>
*
</Required>
)}
</>
</DataListCell>,
<DataListCell key="type">
<Label>{i18n._(t`Type:`)}</Label>
{question.type}
</DataListCell>,
<DataListCell key="default">
<Label>{i18n._(t`Default:`)}</Label>
{[question.type].includes('password') && (
<span>{i18n._(t`encrypted`).toUpperCase()}</span>
)}
{[question.type].includes('multiselect') &&
question.default.length > 0 && (
<ChipGroup numChips={5}>
{question.default.split('\n').map(chip => (
<Chip key={chip} isReadOnly>
{chip}
</Chip>
))}
</ChipGroup>
)}
{![question.type].includes('password') &&
![question.type].includes('multiselect') && (
<span>{question.default}</span>
)}
</DataListCell>,
<DataListCell key="type">{question.type}</DataListCell>,
<DataListCell key="default">{question.default}</DataListCell>,
]}
/>
</DataListItemRow>

View File

@ -25,6 +25,18 @@ describe('<SurveyListItem />', () => {
const moveDown = wrapper.find('Button[aria-label="move down"]');
expect(moveUp.length).toBe(1);
expect(moveDown.length).toBe(1);
expect(
wrapper
.find('b')
.at(0)
.text()
).toBe('Type:');
expect(
wrapper
.find('b')
.at(1)
.text()
).toBe('Default:');
expect(wrapper.find('DataListCheck').length).toBe(1);
expect(wrapper.find('DataListCell').length).toBe(3);
});
@ -44,4 +56,86 @@ describe('<SurveyListItem />', () => {
expect(moveUp).toBe(true);
expect(moveDown).toBe(true);
});
test('required item has required asterisk', () => {
const newItem = {
question_name: 'Foo',
default: 'Bar',
type: 'text',
id: 1,
required: true,
};
let wrapper;
act(() => {
wrapper = mountWithContexts(
<SurveyListItem question={newItem} isChecked={false} isFirst isLast />
);
});
expect(wrapper.find('span[aria-label="Required"]').length).toBe(1);
});
test('items that are not required should not have an asterisk', () => {
let wrapper;
act(() => {
wrapper = mountWithContexts(
<SurveyListItem question={item} isChecked={false} isFirst isLast />
);
});
expect(wrapper.find('span[aria-label="Required"]').length).toBe(0);
});
test('required item has required asterisk', () => {
const newItem = {
question_name: 'Foo',
default: 'a\nd\nb\ne\nf\ng\nh\ni\nk',
type: 'multiselect',
id: 1,
};
let wrapper;
act(() => {
wrapper = mountWithContexts(
<SurveyListItem question={newItem} isChecked={false} isFirst isLast />
);
});
expect(wrapper.find('Chip').length).toBe(6);
wrapper
.find('Chip')
.filter(chip => chip.prop('isOverFlowChip') !== true)
.map(chip => expect(chip.prop('isReadOnly')).toBe(true));
});
test('items that are no required should have no an asterisk', () => {
const newItem = {
question_name: 'Foo',
default: '$encrypted$',
type: 'password',
id: 1,
};
let wrapper;
act(() => {
wrapper = mountWithContexts(
<SurveyListItem question={newItem} isChecked={false} isFirst isLast />
);
});
expect(wrapper.find('span').text()).toBe('ENCRYPTED');
});
test('users without edit/delete permissions are unable to reorder the questions', () => {
let wrapper;
act(() => {
wrapper = mountWithContexts(
<SurveyListItem
question={item}
canAddAndDelete={false}
isChecked={false}
isFirst
isLast
/>
);
});
expect(wrapper.find('button[aria-label="move up"]').prop('disabled')).toBe(
true
);
expect(
wrapper.find('button[aria-label="move down"]').prop('disabled')
).toBe(true);
});
});

View File

@ -13,7 +13,7 @@ import FormField, {
FieldTooltip,
} from '@components/FormField';
import AnsibleSelect from '@components/AnsibleSelect';
import { required, noWhiteSpace, combine } from '@util/validators';
import { required, noWhiteSpace, combine, maxLength } from '@util/validators';
function AnswerTypeField({ i18n }) {
const [field] = useField({
@ -66,6 +66,17 @@ function SurveyQuestionForm({
submitError,
i18n,
}) {
const defaultIsNotAvailable = choices => {
return defaultValue => {
if (!choices.includes(defaultValue)) {
return i18n._(
t`Default choice must be answered from the choices listed.`
);
}
return undefined;
};
};
return (
<Formik
initialValues={{
@ -156,6 +167,7 @@ function SurveyQuestionForm({
<FormField
id="question-default"
name="default"
validate={maxLength(formik.values.max, i18n)}
type={formik.values.type === 'text' ? 'text' : 'number'}
label={i18n._(t`Default answer`)}
/>
@ -183,11 +195,15 @@ function SurveyQuestionForm({
type="textarea"
label={i18n._(t`Multiple Choice Options`)}
validate={required(null, i18n)}
tooltip={i18n._(
t`Each answer choice must be on a separate line.`
)}
isRequired
/>
<FormField
id="question-default"
name="default"
validate={defaultIsNotAvailable(formik.values.choices, i18n)}
type={
formik.values.type === 'multiplechoice'
? 'text'

View File

@ -2,9 +2,10 @@ import React from 'react';
import { useRouteMatch } from 'react-router-dom';
import { t } from '@lingui/macro';
import { withI18n } from '@lingui/react';
import styled from 'styled-components';
import {
DataToolbar,
DataToolbar as _DataToolbar,
DataToolbarContent,
DataToolbarGroup,
DataToolbarItem,
@ -12,7 +13,12 @@ import {
import { Switch, Checkbox, Button } from '@patternfly/react-core';
import { ToolbarAddButton } from '@components/PaginatedDataList';
const DataToolbar = styled(_DataToolbar)`
margin-left: 52px;
`;
function SurveyToolbar({
canEdit,
isAllSelected,
onSelectAll,
i18n,
@ -21,12 +27,14 @@ function SurveyToolbar({
isDeleteDisabled,
onToggleDeleteModal,
}) {
isDeleteDisabled = !canEdit || isDeleteDisabled;
const match = useRouteMatch();
return (
<DataToolbar id="survey-toolbar">
<DataToolbarContent>
<DataToolbarItem>
<Checkbox
isDisabled={!canEdit}
isChecked={isAllSelected}
onChange={isChecked => {
onSelectAll(isChecked);
@ -42,12 +50,16 @@ function SurveyToolbar({
label={i18n._(t`On`)}
labelOff={i18n._(t`Off`)}
isChecked={surveyEnabled}
isDisabled={!canEdit}
onChange={() => onToggleSurvey(!surveyEnabled)}
/>
</DataToolbarItem>
<DataToolbarGroup>
<DataToolbarItem>
<ToolbarAddButton linkTo={`${match.url}/add`} />
<ToolbarAddButton
isDisabled={!canEdit}
linkTo={`${match.url}/add`}
/>
</DataToolbarItem>
<DataToolbarItem>
<Button

View File

@ -36,6 +36,7 @@ describe('<SurveyToolbar />', () => {
isAllSelected
onToggleDeleteModal={jest.fn()}
onToggleSurvey={jest.fn()}
canEdit
/>
);
});
@ -84,4 +85,31 @@ describe('<SurveyToolbar />', () => {
expect(wrapper.find('Switch').length).toBe(1);
expect(wrapper.find('Switch').prop('isChecked')).toBe(true);
});
test('all action buttons in toolbar are disabled', () => {
let wrapper;
act(() => {
wrapper = mountWithContexts(
<SurveyToolbar
surveyEnabled
isDeleteDisabled={false}
onSelectAll={jest.fn()}
isAllSelected
onToggleDelete={jest.fn()}
onToggleSurvey={jest.fn()}
canEdit={false}
/>
);
});
expect(
wrapper
.find('DataToolbar')
.find('Checkbox')
.prop('isDisabled')
).toBe(true);
expect(wrapper.find('Switch').prop('isDisabled')).toBe(true);
expect(wrapper.find('ToolbarAddButton').prop('isDisabled')).toBe(true);
expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
true
);
});
});

View File

@ -71,6 +71,9 @@ function Template({ i18n, me, setBreadcrumb }) {
};
const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin;
const canAddAndEditSurvey =
template?.summary_fields?.user_capabilities.edit ||
template?.summary_fields?.user_capabilities.delete;
const tabsArray = [
{ name: i18n._(t`Details`), link: `${match.url}/details` },
@ -97,7 +100,7 @@ function Template({ i18n, me, setBreadcrumb }) {
link: `${match.url}/completed_jobs`,
},
{
name: i18n._(t`Survey`),
name: canAddAndEditSurvey ? i18n._(t`Survey`) : i18n._(t`View Survey`),
link: `${match.url}/survey`,
}
);
@ -200,7 +203,10 @@ function Template({ i18n, me, setBreadcrumb }) {
)}
{template && (
<Route path="/templates/:templateType/:id/survey">
<TemplateSurvey template={template} />
<TemplateSurvey
template={template}
canEdit={canAddAndEditSurvey}
/>
</Route>
)}
{!hasRolesandTemplateLoading && (

View File

@ -1,5 +1,5 @@
import React, { useState, useEffect, useCallback } from 'react';
import { Switch, Route, useParams } from 'react-router-dom';
import { Switch, Route, useParams, useLocation } from 'react-router-dom';
import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro';
import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '@api';
@ -9,10 +9,12 @@ import ErrorDetail from '@components/ErrorDetail';
import useRequest, { useDismissableError } from '@util/useRequest';
import { SurveyList, SurveyQuestionAdd, SurveyQuestionEdit } from './Survey';
function TemplateSurvey({ template, i18n }) {
function TemplateSurvey({ template, canEdit, i18n }) {
const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled);
const { templateType } = useParams();
const location = useLocation();
const {
result: survey,
request: fetchSurvey,
@ -28,9 +30,10 @@ function TemplateSurvey({ template, i18n }) {
return data;
}, [template.id, templateType])
);
useEffect(() => {
fetchSurvey();
}, [fetchSurvey]);
}, [fetchSurvey, location]);
const { request: updateSurvey, error: updateError } = useRequest(
useCallback(
@ -82,12 +85,22 @@ function TemplateSurvey({ template, i18n }) {
return (
<>
<Switch>
<Route path="/templates/:templateType/:id/survey/add">
<SurveyQuestionAdd survey={survey} updateSurvey={updateSurveySpec} />
</Route>
<Route path="/templates/:templateType/:id/survey/edit/:variable">
<SurveyQuestionEdit survey={survey} updateSurvey={updateSurveySpec} />
</Route>
{canEdit && (
<Route path="/templates/:templateType/:id/survey/add">
<SurveyQuestionAdd
survey={survey}
updateSurvey={updateSurveySpec}
/>
</Route>
)}
{canEdit && (
<Route path="/templates/:templateType/:id/survey/edit/:variable">
<SurveyQuestionEdit
survey={survey}
updateSurvey={updateSurveySpec}
/>
</Route>
)}
<Route path="/templates/:templateType/:id/survey" exact>
<SurveyList
isLoading={isLoading}
@ -96,6 +109,7 @@ function TemplateSurvey({ template, i18n }) {
toggleSurvey={toggleSurvey}
updateSurvey={updateSurveySpec}
deleteSurvey={deleteSurvey}
canEdit={canEdit}
/>
</Route>
</Switch>

View File

@ -116,6 +116,9 @@ class WorkflowJobTemplate extends Component {
const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin;
const canToggleNotifications = isNotifAdmin;
const canAddAndEditSurvey =
template?.summary_fields?.user_capabilities.edit ||
template?.summary_fields?.user_capabilities.delete;
const tabsArray = [
{ name: i18n._(t`Details`), link: `${match.url}/details` },
@ -145,7 +148,7 @@ class WorkflowJobTemplate extends Component {
link: `${match.url}/completed_jobs`,
});
tabsArray.push({
name: i18n._(t`Survey`),
name: canAddAndEditSurvey ? i18n._(t`Survey`) : i18n._(t`View Survey`),
link: `${match.url}/survey`,
});
@ -269,7 +272,10 @@ class WorkflowJobTemplate extends Component {
)}
{template && (
<Route path="/templates/:templateType/:id/survey">
<TemplateSurvey template={template} />
<TemplateSurvey
template={template}
canEdit={canAddAndEditSurvey}
/>
</Route>
)}
<Route key="not-found" path="*">

View File

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