Addresses PR Issues

Improves credential ID variable in JT model. Removes unused prop from Lookup ComponentDidMount. Removed unused function call from Credentials ComponentDidMount. Streamlines toggleCredential function and moves it to JobTemplateForm. This was done because the JobTemplateForm should handle the credential values passed to the CredentialsLookup. Adds tests for JobTemplateForm to ensure toggleCredentialSelection function is putting proper values in state. Removed withRouter wrapper on CredentialsLookup export. Improved CredentialsLookup test to ensure that onChange is called when user removes a credential from the input.
2026-01-15 20:00:43 -03:30 · 2019-10-23 09:59:23 -04:00 · 2019-10-23 09:59:23 -04:00 · 491f4824b0
commit 491f4824b0
parent 91721e09df
7 changed files with 124 additions and 100 deletions
--- a/awx/ui_next/src/api/models/JobTemplates.js
+++ b/awx/ui_next/src/api/models/JobTemplates.js
@ -45,15 +45,15 @@ class JobTemplates extends InstanceGroupsMixin(NotificationsMixin(Base)) {
    return this.http.get(`${this.baseUrl}${id}/credentials/`, { params });
  }

-  associateCredentials(id, credential) {
+  associateCredentials(id, credentialId) {
    return this.http.post(`${this.baseUrl}${id}/credentials/`, {
-      id: credential,
+      id: credentialId,
    });
  }

-  disassociateCredentials(id, credential) {
+  disassociateCredentials(id, credentialId) {
    return this.http.post(`${this.baseUrl}${id}/credentials/`, {
-      id: credential,
+      id: credentialId,
      disassociate: true,
    });
  }
--- a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx
+++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx
@ -1,7 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { withI18n } from '@lingui/react';
-import { withRouter } from 'react-router-dom';
 import { t } from '@lingui/macro';
 import { FormGroup, Tooltip } from '@patternfly/react-core';
 import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons';
@ -19,31 +18,18 @@ class CredentialsLookup extends React.Component {
    super(props);

    this.state = {
-      credentials: props.credentials,
      selectedCredentialType: { label: 'Machine', id: 1, kind: 'ssh' },
      credentialTypes: [],
    };
-
-    this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind(
-      this
-    );
-    this.loadCredentials = this.loadCredentials.bind(this);
    this.loadCredentialTypes = this.loadCredentialTypes.bind(this);
-    this.toggleCredential = this.toggleCredential.bind(this);
+    this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind(this);
+    this.loadCredentials = this.loadCredentials.bind(this);
  }

  componentDidMount() {
-    this.loadCredentials({ page: 1, page_size: 5, order_by: 'name' });
    this.loadCredentialTypes();
  }

-  componentDidUpdate(prevState) {
-    const { selectedType } = this.state;
-    if (prevState.selectedType !== selectedType) {
-      Promise.all([this.loadCredentials()]);
-    }
-  }
-
  async loadCredentialTypes() {
    const { onError } = this.props;
    try {
@ -57,6 +43,7 @@ class CredentialsLookup extends React.Component {
            // require different field values.
            cred = {
              id: cred.id,
+              key: cred.id,
              kind: cred.kind,
              type: cred.namespace,
              value: cred.name,
@ -85,43 +72,9 @@ class CredentialsLookup extends React.Component {
    this.setState({ selectedCredentialType: selectedType[0] });
  }

-  toggleCredential(item) {
-    const { credentials: stateToUpdate, selectedCredentialType } = this.state;
-    const { onChange } = this.props;
-    const index = stateToUpdate.findIndex(
-      credential => credential.id === item.id
-    );
-    if (index > -1) {
-      const newCredentialsList = stateToUpdate.filter(
-        cred => cred.id !== item.id
-      );
-      this.setState({ credentials: newCredentialsList });
-      onChange(newCredentialsList);
-      return;
-    }
-
-    const credentialTypeOccupied = stateToUpdate.some(
-      cred => cred.kind === item.kind
-    );
-    if (selectedCredentialType.value === 'Vault' || !credentialTypeOccupied) {
-      item.credentialType = selectedCredentialType;
-      this.setState({ credentials: [...stateToUpdate, item] });
-      onChange([...stateToUpdate, item]);
-    } else {
-      const credsList = [...stateToUpdate];
-      const occupyingCredIndex = stateToUpdate.findIndex(
-        occupyingCred => occupyingCred.kind === item.kind
-      );
-      credsList.splice(occupyingCredIndex, 1, item);
-      this.setState({ credentials: credsList });
-      onChange(credsList);
-    }
-  }
-
  render() {
-    const { tooltip, i18n } = this.props;
-    const { credentials, selectedCredentialType, credentialTypes } = this.state;
-
+    const { selectedCredentialType, credentialTypes } = this.state;
+    const { tooltip, i18n, credentials, onChange } = this.props;
    return (
      <FormGroup label={i18n._(t`Credentials`)} fieldId="org-credentials">
        {tooltip && (
@ -134,7 +87,7 @@ class CredentialsLookup extends React.Component {
            selectCategoryOptions={credentialTypes}
            selectCategory={this.handleCredentialTypeSelect}
            selectedCategory={selectedCredentialType}
-            onToggleItem={this.toggleCredential}
+            onToggleItem={onChange}
            onloadCategories={this.loadCredentialTypes}
            id="org-credentials"
            lookupHeader={i18n._(t`Credentials`)}
@ -162,7 +115,6 @@ class CredentialsLookup extends React.Component {

 CredentialsLookup.propTypes = {
  tooltip: PropTypes.string,
-  onChange: PropTypes.func.isRequired,
 };

 CredentialsLookup.defaultProps = {
@ -170,4 +122,4 @@ CredentialsLookup.defaultProps = {
 };
 export { CredentialsLookup as _CredentialsLookup };

-export default withI18n()(withRouter(CredentialsLookup));
+export default withI18n()(CredentialsLookup);
--- a/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx
+++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx
@ -1,6 +1,6 @@
 import React from 'react';
 import { mountWithContexts } from '@testUtils/enzymeHelpers';
-import CredentialsLookup, { _CredentialsLookup } from './CredentialsLookup';
+import CredentialsLookup from './CredentialsLookup';
 import { CredentialsAPI, CredentialTypesAPI } from '@api';

 jest.mock('@api');
@ -9,6 +9,7 @@ describe('<CredentialsLookup />', () => {
  let wrapper;
  let lookup;
  let credLookup;
+  let onChange;

  const credentials = [
    { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
@ -42,12 +43,12 @@ describe('<CredentialsLookup />', () => {
        count: 3,
      },
    });
-
+    onChange = jest.fn();
    wrapper = mountWithContexts(
      <CredentialsLookup
        onError={() => {}}
        credentials={credentials}
-        onChange={() => {}}
+        onChange={onChange}
        tooltip="This is credentials look up"
      />
    );
@ -62,17 +63,22 @@ describe('<CredentialsLookup />', () => {

  test('CredentialsLookup renders properly', () => {
    expect(wrapper.find('CredentialsLookup')).toHaveLength(1);
+    expect(CredentialTypesAPI.read).toHaveBeenCalled();
  });
-  test('Removes credential from directly from input', () => {
-    const chip = wrapper.find('PFChip').at(0);
-    expect(chip).toHaveLength(1);
-    chip.find('ChipButton').invoke('onClick')();
-    expect(wrapper.find('PFChip')).toHaveLength(1);
+
+  test('onChange is called when you click to remove a credential from input', () => {
+    const chip = wrapper.find('PFChip');
+    const button = chip.at(1).find('Button');
+    expect(chip).toHaveLength(2);
+    button.prop('onClick')();
+    expect(onChange).toBeCalledTimes(1);
  });
-  test('can change credential types', async () => {
+
+  test('can change credential types', () => {
    lookup.prop('selectCategory')({}, 'Vault');
    expect(credLookup.state('selectedCredentialType')).toEqual({
      id: 500,
+      key: 500,
      kind: 'vault',
      type: 'buzz',
      value: 'Vault',
@ -81,23 +87,4 @@ describe('<CredentialsLookup />', () => {
    });
    expect(CredentialsAPI.read).toHaveBeenCalled();
  });
-  test('Toggle credentials properly adds credentials', async () => {
-    function callOnToggle(item, index) {
-      lookup.prop('onToggleItem')(item);
-      expect(credLookup.state('credentials')[index].name).toEqual(
-        `${item.name}`
-      );
-    }
-    callOnToggle({ name: 'Gatsby', id: 8, kind: 'Machine' }, 2);
-    callOnToggle({ name: 'Party', id: 9, kind: 'Machine' }, 2);
-    callOnToggle({ name: 'Animal', id: 10, kind: 'Ansible' }, 3);
-
-    lookup.prop('onToggleItem')({
-      id: 1,
-      kind: 'cloud',
-      name: 'Foo',
-      url: 'www.google.com',
-    });
-    expect(credLookup.state('credentials').length).toBe(3);
-  });
 });
--- a/awx/ui_next/src/components/Lookup/Lookup.jsx
+++ b/awx/ui_next/src/components/Lookup/Lookup.jsx
@ -84,12 +84,7 @@ class Lookup extends React.Component {
  }

  componentDidMount() {
-    const { onLoadCredentialTypes } = this.props;
-    if (onLoadCredentialTypes) {
-      Promise.all([onLoadCredentialTypes(), this.getData()]);
-    } else {
-      this.getData();
-    }
+      Promise.all([this.getData()]);
  }

  componentDidUpdate(prevProps) {
--- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx
+++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx
@ -110,7 +110,6 @@ class JobTemplateEdit extends Component {
      instanceGroups,
      initialInstanceGroups,
      credentials,
-      initialCredentials,
      ...remainingValues
    } = values;

--- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx
+++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx
@ -77,9 +77,11 @@ class JobTemplateForm extends Component {
      project: props.template.summary_fields.project,
      inventory: props.template.summary_fields.inventory,
      allowCallbacks: !!props.template.host_config_key,
+      credentials: props.template.summary_fields.credentials,
    };
    this.handleProjectValidation = this.handleProjectValidation.bind(this);
    this.loadRelatedInstanceGroups = this.loadRelatedInstanceGroups.bind(this);
+    this.toggleCredentialSelection = this.toggleCredentialSelection.bind(this);
  }

  componentDidMount() {
@ -106,6 +108,31 @@ class JobTemplateForm extends Component {
    }
  }

+  toggleCredentialSelection(newCredential) {
+    const { credentials: credentialsToUpdate } = this.state;
+    const { setFieldValue } = this.props;
+
+    let newCredentialsList;
+    const isSelectedCredentialInState =
+      credentialsToUpdate.filter(cred => cred.id === newCredential.id).length >
+      0;
+
+    if (isSelectedCredentialInState) {
+      newCredentialsList = credentialsToUpdate.filter(
+        cred => cred.id !== newCredential.id
+      );
+    } else {
+      newCredentialsList = credentialsToUpdate.filter(
+        credential =>
+          credential.kind === 'vault' || credential.kind !== newCredential.kind
+      );
+      newCredentialsList = [...newCredentialsList, newCredential];
+    }
+
+    setFieldValue('credentials', newCredentialsList);
+    this.setState({ credentials: newCredentialsList });
+  }
+
  handleProjectValidation() {
    const { i18n, touched } = this.props;
    const { project } = this.state;
@ -127,6 +154,7 @@ class JobTemplateForm extends Component {
      inventory,
      project,
      allowCallbacks,
+      credentials,
    } = this.state;
    const {
      handleCancel,
@ -324,11 +352,11 @@ class JobTemplateForm extends Component {
          <Field
            name="credentials"
            fieldId="template-credentials"
-            render={({ form }) => (
+            render={() => (
              <CredentialsLookup
+                credentials={credentials}
+                onChange={this.toggleCredentialSelection}
                onError={err => this.setState({ contentError: err })}
-                credentials={template.summary_fields.credentials}
-                onChange={value => form.setFieldValue('credentials', value)}
                tooltip={i18n._(
                  t`Select credentials that allow Tower to access the nodes this job will be ran against. You can only select one credential of each type. For machine credentials (SSH), checking "Prompt on launch" without selecting credentials will require you to select a machine credential at run time. If you select credentials and check "Prompt on launch", the selected credential(s) become the defaults that can be updated at run time.`
                )}
@ -604,8 +632,7 @@ const FormikApp = withFormik({
      organizationId: summary_fields.inventory.organization_id || null,
      initialInstanceGroups: [],
      instanceGroups: [],
-      initialCredentials: summary_fields.credentials || [],
-      credentials: [],
+      credentials: summary_fields.credentials || [],
    };
  },
  handleSubmit: (values, { props }) => props.handleSubmit(values),
--- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx
+++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx
@ -3,7 +3,13 @@ import { act } from 'react-dom/test-utils';
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
 import { sleep } from '@testUtils/testUtils';
 import JobTemplateForm from './JobTemplateForm';
-import { LabelsAPI, JobTemplatesAPI, ProjectsAPI } from '@api';
+import {
+  LabelsAPI,
+  JobTemplatesAPI,
+  ProjectsAPI,
+  CredentialTypesAPI,
+  CredentialsAPI,
+} from '@api';

 jest.mock('@api');

@ -59,10 +65,21 @@ describe('<JobTemplateForm />', () => {
      policy_instance_list: [],
    },
  ];
+  const mockCredentials = [
+    { id: 1, kind: 'cloud', name: 'Cred 1', url: 'www.google.com' },
+    { id: 2, kind: 'ssh', name: 'Cred 2', url: 'www.google.com' },
+    { id: 3, kind: 'Ansible', name: 'Cred 3', url: 'www.google.com' },
+    { id: 4, kind: 'Machine', name: 'Cred 4', url: 'www.google.com' },
+    { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' },
+  ];
+
  beforeEach(() => {
    LabelsAPI.read.mockReturnValue({
      data: mockData.summary_fields.labels,
    });
+    CredentialsAPI.read.mockReturnValue({
+      data: { results: mockCredentials },
+    });
    JobTemplatesAPI.readInstanceGroups.mockReturnValue({
      data: { results: mockInstanceGroups },
    });
@ -138,6 +155,13 @@ describe('<JobTemplateForm />', () => {
      target: { value: 'new baz type', name: 'playbook' },
    });
    expect(form.state('values').playbook).toEqual('new baz type');
+    wrapper
+      .find('CredentialChip')
+      .at(0)
+      .prop('onClick')();
+    expect(form.state('values').credentials).toEqual([
+      { id: 2, kind: 'ssh', name: 'Bar' },
+    ]);
  });

  test('should call handleSubmit when Submit button is clicked', async () => {
@ -176,4 +200,44 @@ describe('<JobTemplateForm />', () => {
    wrapper.find('button[aria-label="Cancel"]').invoke('onClick')();
    expect(handleCancel).toBeCalled();
  });
+  test('toggleCredentialSelection should handle credential selection properly', async () => {
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <JobTemplateForm
+          template={mockData}
+          handleSubmit={jest.fn()}
+          handleCancel={jest.fn()}
+        />
+      );
+    });
+
+    function callToggleCredSelection(credential, formState) {
+      JobTempForm.instance().toggleCredentialSelection(credential);
+
+      expect(form.state('values').credentials).toEqual(formState);
+    }
+    const form = wrapper.find('Formik');
+    const JobTempForm = wrapper.find('JobTemplateForm');
+
+    callToggleCredSelection(
+      { id: 3, kind: 'vault', name: 'Vault Credential' },
+      [
+        { id: 1, kind: 'cloud', name: 'Foo' },
+        { id: 2, kind: 'ssh', name: 'Bar' },
+        { id: 3, kind: 'vault', name: 'Vault Credential' },
+      ]
+    );
+    callToggleCredSelection({ id: 4, kind: 'ssh', name: 'New Bar' }, [
+      { id: 1, kind: 'cloud', name: 'Foo' },
+      { id: 3, kind: 'vault', name: 'Vault Credential' },
+      { id: 4, kind: 'ssh', name: 'New Bar' },
+    ]);
+    callToggleCredSelection({ id: 5, kind: 'vault', name: 'New Vault' }, [
+      { id: 1, kind: 'cloud', name: 'Foo' },
+      { id: 3, kind: 'vault', name: 'Vault Credential' },
+      { id: 4, kind: 'ssh', name: 'New Bar' },
+      { id: 5, kind: 'vault', name: 'New Vault' },
+    ]);
+  });
 });