Address PR feedback and merge conflicts

This commit is contained in:
Marliana Lara
2019-06-25 16:29:55 -04:00
parent 67d619f9cc
commit f3bf35311e
9 changed files with 20 additions and 38 deletions

View File

@@ -33,7 +33,7 @@ class AnsibleSelect extends React.Component {
key={datum.value} key={datum.value}
value={datum.value} value={datum.value}
label={datum.label} label={datum.label}
isDisabled={datum.disabled} isDisabled={datum.isDisabled}
/> />
))} ))}
</FormSelect> </FormSelect>

View File

@@ -49,7 +49,7 @@ describe('<AnsibleSelect />', () => {
data={mockData} data={mockData}
/> />
); );
// console.log(wrapper.debug());
expect(wrapper.find('FormSelect')).toHaveLength(1); expect(wrapper.find('FormSelect')).toHaveLength(1);
expect(wrapper.find('FormSelectOption')).toHaveLength(2); expect(wrapper.find('FormSelectOption')).toHaveLength(2);
}); });

View File

@@ -139,7 +139,7 @@ class OrganizationForm extends Component {
position="right" position="right"
content={i18n._(t`The maximum number of hosts allowed content={i18n._(t`The maximum number of hosts allowed
to be managed by this organization. Value defaults to to be managed by this organization. Value defaults to
0 which means no limit. Refer tothe Ansible 0 which means no limit. Refer to the Ansible
documentation for more details.`)} documentation for more details.`)}
> >
<QuestionCircleIcon /> <QuestionCircleIcon />

View File

@@ -75,6 +75,7 @@ class JobTemplateDetail extends Component {
}, },
hasTemplateLoading, hasTemplateLoading,
i18n, i18n,
match,
} = this.props; } = this.props;
const { instanceGroups, hasContentLoading, contentError } = this.state; const { instanceGroups, hasContentLoading, contentError } = this.state;
const verbosityOptions = [ const verbosityOptions = [
@@ -307,7 +308,7 @@ class JobTemplateDetail extends Component {
{summary_fields.user_capabilities.edit && ( {summary_fields.user_capabilities.edit && (
<Button <Button
component={Link} component={Link}
to="/home" to={`/templates/${match.params.templateType}/${match.params.id}/edit`}
aria-label={i18n._(t`Edit`)} aria-label={i18n._(t`Edit`)}
> >

View File

@@ -27,7 +27,6 @@ class Template extends Component {
hasContentError: false, hasContentError: false,
hasContentLoading: true, hasContentLoading: true,
template: null, template: null,
actions: null,
}; };
this.loadTemplate = this.loadTemplate.bind(this); this.loadTemplate = this.loadTemplate.bind(this);
} }
@@ -44,29 +43,15 @@ class Template extends Component {
} }
async loadTemplate () { async loadTemplate () {
const { actions: cachedActions } = this.state;
const { setBreadcrumb, match } = this.props; const { setBreadcrumb, match } = this.props;
const { id } = match.params; const { id } = match.params;
let optionsPromise;
if (cachedActions) {
optionsPromise = Promise.resolve({ data: { actions: cachedActions } });
} else {
optionsPromise = JobTemplatesAPI.readOptions();
}
const promises = Promise.all([
JobTemplatesAPI.readDetail(id),
optionsPromise
]);
this.setState({ hasContentError: false, hasContentLoading: true }); this.setState({ hasContentError: false, hasContentLoading: true });
try { try {
const [{ data }, { data: { actions } }] = await promises; const { data } = await JobTemplatesAPI.readDetail(id);
setBreadcrumb(data); setBreadcrumb(data);
this.setState({ this.setState({
template: data, template: data,
actions
}); });
} catch { } catch {
this.setState({ hasContentError: true }); this.setState({ hasContentError: true });
@@ -82,16 +67,12 @@ class Template extends Component {
location, location,
match, match,
} = this.props; } = this.props;
const { const {
actions,
hasContentError, hasContentError,
hasContentLoading, hasContentLoading,
template template
} = this.state; } = this.state;
const canAdd = actions && Object.prototype.hasOwnProperty.call(actions, 'POST');
const tabsArray = [ const tabsArray = [
{ name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 }, { name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 },
{ name: i18n._(t`Access`), link: '/home', id: 1 }, { name: i18n._(t`Access`), link: '/home', id: 1 },
@@ -154,7 +135,6 @@ class Template extends Component {
render={() => ( render={() => (
<TemplateEdit <TemplateEdit
template={template} template={template}
hasPermissions={canAdd}
/> />
)} )}
/> />

View File

@@ -7,10 +7,10 @@ describe('<Template />', () => {
mountWithContexts(<Template />); mountWithContexts(<Template />);
}); });
test('When component mounts API is called and the response is put in state', async (done) => { test('When component mounts API is called and the response is put in state', async (done) => {
const readTemplate = jest.spyOn(_Template.prototype, 'readTemplate'); const loadTemplate = jest.spyOn(_Template.prototype, 'loadTemplate');
const wrapper = mountWithContexts(<Template />); const wrapper = mountWithContexts(<Template />);
await waitForElement(wrapper, 'Template', (el) => el.state('hasContentLoading') === true); await waitForElement(wrapper, 'Template', (el) => el.state('hasContentLoading') === true);
expect(readTemplate).toHaveBeenCalled(); expect(loadTemplate).toHaveBeenCalled();
await waitForElement(wrapper, 'Template', (el) => el.state('hasContentLoading') === true); await waitForElement(wrapper, 'Template', (el) => el.state('hasContentLoading') === true);
done(); done();
}); });

View File

@@ -1,5 +1,4 @@
import React, { Component } from 'react'; import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { withRouter, Redirect } from 'react-router-dom'; import { withRouter, Redirect } from 'react-router-dom';
import { CardBody } from '@patternfly/react-core'; import { CardBody } from '@patternfly/react-core';
import TemplateForm from '../shared/TemplateForm'; import TemplateForm from '../shared/TemplateForm';
@@ -9,7 +8,6 @@ import { JobTemplate } from '@types';
class TemplateEdit extends Component { class TemplateEdit extends Component {
static propTypes = { static propTypes = {
template: JobTemplate.isRequired, template: JobTemplate.isRequired,
hasPermissions: PropTypes.bool.isRequired,
}; };
constructor (props) { constructor (props) {
@@ -40,10 +38,11 @@ class TemplateEdit extends Component {
} }
render () { render () {
const { template, hasPermissions } = this.props; const { template } = this.props;
const { error } = this.state; const { error } = this.state;
const canEdit = template.summary_fields.user_capabilities.edit;
if (!hasPermissions) { if (!canEdit) {
const { template: { id, type } } = this.props; const { template: { id, type } } = this.props;
return <Redirect to={`/templates/${type}/${id}/details`} />; return <Redirect to={`/templates/${type}/${id}/details`} />;
} }

View File

@@ -14,14 +14,18 @@ describe('<TemplateEdit />', () => {
inventory: 2, inventory: 2,
project: 3, project: 3,
playbook: 'Baz', playbook: 'Baz',
type: 'job_template' type: 'job_template',
summary_fields: {
user_capabilities: {
edit: true
}
}
}; };
test('initially renders successfully', () => { test('initially renders successfully', () => {
mountWithContexts( mountWithContexts(
<TemplateEdit <TemplateEdit
template={mockData} template={mockData}
hasPermissions
/> />
); );
}); });
@@ -30,7 +34,6 @@ describe('<TemplateEdit />', () => {
const wrapper = mountWithContexts( const wrapper = mountWithContexts(
<TemplateEdit <TemplateEdit
template={mockData} template={mockData}
hasPermissions
/> />
); );
const updatedTemplateData = { const updatedTemplateData = {
@@ -50,7 +53,6 @@ describe('<TemplateEdit />', () => {
const wrapper = mountWithContexts( const wrapper = mountWithContexts(
<TemplateEdit <TemplateEdit
template={mockData} template={mockData}
hasPermissions
/>, />,
{ context: { router: { history } } } { context: { router: { history } } }
); );

View File

@@ -38,9 +38,9 @@ class TemplateForm extends Component {
} = this.props; } = this.props;
const jobTypeOptions = [ const jobTypeOptions = [
{ value: '', label: i18n._(t`Choose a job type`), disabled: true }, { value: '', label: i18n._(t`Choose a job type`), isDisabled: true },
{ value: 'run', label: i18n._(t`Run`), disabled: false }, { value: 'run', label: i18n._(t`Run`), isDisabled: false },
{ value: 'check', label: i18n._(t`Check`), disabled: false } { value: 'check', label: i18n._(t`Check`), isDisabled: false }
]; ];
return ( return (