From 7b3e5cd8d5e5c8d7cbadc24dc0b1626baa1477d7 Mon Sep 17 00:00:00 2001
From: Marliana Lara <mlara@redhat.com>
Date: Tue, 25 Jun 2019 10:47:29 -0400
Subject: [PATCH] Refactor AnsibleSelect data prop to accept an array of option
 objects

* First custom_virtualenv in options list is always default
---
 .../AnsibleSelect/AnsibleSelect.jsx           | 25 +++++++------------
 .../AnsibleSelect/AnsibleSelect.test.jsx      | 21 ++++++++++------
 .../OrganizationAdd/OrganizationAdd.test.jsx  |  3 ++-
 .../Organization/shared/OrganizationForm.jsx  | 18 +++++++++----
 .../shared/OrganizationForm.test.jsx          |  3 ++-
 5 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/src/components/AnsibleSelect/AnsibleSelect.jsx b/src/components/AnsibleSelect/AnsibleSelect.jsx
index fe6e352811..5360081412 100644
--- a/src/components/AnsibleSelect/AnsibleSelect.jsx
+++ b/src/components/AnsibleSelect/AnsibleSelect.jsx
@@ -20,7 +20,8 @@ class AnsibleSelect extends React.Component {
   }
 
   render () {
-    const { label, value, data, defaultSelected, i18n } = this.props;
+    const { value, data, i18n } = this.props;
+
     return (
       <FormSelect
         value={value}
@@ -28,15 +29,12 @@ class AnsibleSelect extends React.Component {
         aria-label={i18n._(t`Select Input`)}
       >
         {data.map((datum) => (
-          datum === defaultSelected ? (
-            <FormSelectOption
-              key=""
-              value=""
-              label={i18n._(t`Use Default ${label}`)}
-            />
-          ) : (
-            <FormSelectOption key={datum} value={datum} label={datum} />
-          )
+          <FormSelectOption
+            key={datum.value}
+            value={datum.value}
+            label={datum.label}
+            isDisabled={datum.disabled}
+          />
         ))}
       </FormSelect>
     );
@@ -45,15 +43,10 @@ class AnsibleSelect extends React.Component {
 
 AnsibleSelect.defaultProps = {
   data: [],
-  label: 'Ansible Select',
-  defaultSelected: null,
 };
 
 AnsibleSelect.propTypes = {
-  data: PropTypes.arrayOf(PropTypes.string),
-  defaultSelected: PropTypes.string,
-  label: PropTypes.string,
-  name: PropTypes.string.isRequired,
+  data: PropTypes.arrayOf(PropTypes.object),
   onChange: PropTypes.func.isRequired,
   value: PropTypes.string.isRequired,
 };
diff --git a/src/components/AnsibleSelect/AnsibleSelect.test.jsx b/src/components/AnsibleSelect/AnsibleSelect.test.jsx
index d3be54f5d7..8d074dc000 100644
--- a/src/components/AnsibleSelect/AnsibleSelect.test.jsx
+++ b/src/components/AnsibleSelect/AnsibleSelect.test.jsx
@@ -2,8 +2,17 @@ import React from 'react';
 import { mountWithContexts } from '../../../testUtils/enzymeHelpers';
 import AnsibleSelect, { _AnsibleSelect } from './AnsibleSelect';
 
-const label = 'test select';
-const mockData = ['/venv/baz/', '/venv/ansible/'];
+const mockData = [
+  {
+    label: 'Baz',
+    value: '/venv/baz/'
+  },
+  {
+    label: 'Default',
+    value: '/venv/ansible/'
+  }
+];
+
 describe('<AnsibleSelect />', () => {
   test('initially renders succesfully', async () => {
     mountWithContexts(
@@ -11,7 +20,6 @@ describe('<AnsibleSelect />', () => {
         value="foo"
         name="bar"
         onChange={() => { }}
-        label={label}
         data={mockData}
       />
     );
@@ -24,7 +32,6 @@ describe('<AnsibleSelect />', () => {
         value="foo"
         name="bar"
         onChange={() => { }}
-        label={label}
         data={mockData}
       />
     );
@@ -33,17 +40,17 @@ describe('<AnsibleSelect />', () => {
     expect(spy).toHaveBeenCalled();
   });
 
-  test('Returns correct select options if defaultSelected props is passed', () => {
+  test('Returns correct select options', () => {
     const wrapper = mountWithContexts(
       <AnsibleSelect
         value="foo"
         name="bar"
         onChange={() => { }}
-        label={label}
         data={mockData}
-        defaultSelected={mockData[1]}
       />
     );
+    // console.log(wrapper.debug());
     expect(wrapper.find('FormSelect')).toHaveLength(1);
+    expect(wrapper.find('FormSelectOption')).toHaveLength(2);
   });
 });
diff --git a/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx b/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx
index 595854955a..386344f577 100644
--- a/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx
+++ b/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx
@@ -105,7 +105,8 @@ describe('<OrganizationAdd />', () => {
       { context: { config } }
     ).find('AnsibleSelect');
     expect(wrapper.find('FormSelect')).toHaveLength(1);
-    expect(wrapper.find('FormSelectOption')).toHaveLength(2);
+    expect(wrapper.find('FormSelectOption')).toHaveLength(3);
+    expect(wrapper.find('FormSelectOption').first().prop('value')).toEqual('/venv/ansible/');
   });
 
   test('AnsibleSelect component does not render if there are 0 virtual environments', () => {
diff --git a/src/screens/Organization/shared/OrganizationForm.jsx b/src/screens/Organization/shared/OrganizationForm.jsx
index 9a200b15af..cb5765be22 100644
--- a/src/screens/Organization/shared/OrganizationForm.jsx
+++ b/src/screens/Organization/shared/OrganizationForm.jsx
@@ -93,7 +93,11 @@ class OrganizationForm extends Component {
   render () {
     const { organization, handleCancel, i18n, me } = this.props;
     const { instanceGroups, formIsValid, error } = this.state;
-    const defaultVenv = '/venv/ansible/';
+    const defaultVenv = {
+      label: i18n._(t`Use Default Ansible Environment`),
+      value: '/venv/ansible/',
+      key: 'default'
+    };
 
     return (
       <Formik
@@ -133,7 +137,10 @@ class OrganizationForm extends Component {
                       {(
                         <Tooltip
                           position="right"
-                          content="The maximum number of hosts allowed to be managed by this organization. Value defaults to 0 which means no limit. Refer to the Ansible documentation for more details."
+                          content={i18n._(t`The maximum number of hosts allowed
+                          to be managed by this organization. Value defaults to
+                          0 which means no limit. Refer tothe Ansible
+                          documentation for more details.`)}
                         >
                           <QuestionCircleIcon />
                         </Tooltip>
@@ -156,9 +163,10 @@ class OrganizationForm extends Component {
                           label={i18n._(t`Ansible Environment`)}
                         >
                           <AnsibleSelect
-                            data={custom_virtualenvs}
-                            defaultSelected={defaultVenv}
-                            label={i18n._(t`Ansible Environment`)}
+                            data={[defaultVenv, ...custom_virtualenvs
+                              .filter(datum => datum !== defaultVenv.value)
+                              .map(datum => ({ label: datum, value: datum, key: datum }))
+                            ]}
                             {...field}
                           />
                         </FormGroup>
diff --git a/src/screens/Organization/shared/OrganizationForm.test.jsx b/src/screens/Organization/shared/OrganizationForm.test.jsx
index 1be9d34de3..e1bd876e54 100644
--- a/src/screens/Organization/shared/OrganizationForm.test.jsx
+++ b/src/screens/Organization/shared/OrganizationForm.test.jsx
@@ -144,7 +144,8 @@ describe('<OrganizationForm />', () => {
       }
     );
     expect(wrapper.find('FormSelect')).toHaveLength(1);
-    expect(wrapper.find('FormSelectOption')).toHaveLength(2);
+    expect(wrapper.find('FormSelectOption')).toHaveLength(3);
+    expect(wrapper.find('FormSelectOption').first().prop('value')).toEqual('/venv/ansible/');
   });
 
   test('calls handleSubmit when form submitted', async () => {