Merge pull request #6149 from jlmitch5/fixMultiSelectCred

update multi select credential logic vault credential logic, add notice, and update multicred tests

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx

@@ -3,7 +3,7 @@ import { withRouter } from 'react-router-dom';
 import PropTypes from 'prop-types';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
-import { ToolbarItem } from '@patternfly/react-core';
+import { ToolbarItem, Alert } from '@patternfly/react-core';
 import { CredentialsAPI, CredentialTypesAPI } from '@api';
 import AnsibleSelect from '@components/AnsibleSelect';
 import CredentialChip from '@components/CredentialChip';
@@ -77,7 +77,7 @@ function MultiCredentialsLookup(props) {
     />
   );
 
-  const isMultiple = selectedType && selectedType.kind === 'vault';
+  const isVault = selectedType?.kind === 'vault';
 
   return (
     <Lookup
@@ -91,6 +91,16 @@ function MultiCredentialsLookup(props) {
       renderOptionsList={({ state, dispatch, canDelete }) => {
         return (
           <Fragment>
+            {isVault && (
+              <Alert
+                variant="info"
+                isInline
+                css="margin-bottom: 20px;"
+                title={i18n._(
+                  t`You cannot select multiple vault credentials with the same vault ID. Doing so will automatically deselect the other with the same vault ID.`
+                )}
+              />
+            )}
             {credentialTypes && credentialTypes.length > 0 && (
               <ToolbarItem css=" display: flex; align-items: center;">
                 <div css="flex: 0 0 25%; margin-right: 32px">
@@ -140,17 +150,18 @@ function MultiCredentialsLookup(props) {
                   key: 'name',
                 },
               ]}
-              multiple={isMultiple}
+              multiple={isVault}
               header={i18n._(t`Credentials`)}
               name="credentials"
               qsConfig={QS_CONFIG}
               readOnly={!canDelete}
               selectItem={item => {
-                if (isMultiple) {
+                const hasSameVaultID = val =>
-                  return dispatch({ type: 'SELECT_ITEM', item });
+                  val?.inputs?.vault_id !== undefined &&
-                }
+                  val?.inputs?.vault_id === item?.inputs?.vault_id;
-                const selectedItems = state.selectedItems.filter(
+                const hasSameKind = val => val.kind === item.kind;
-                  i => i.kind !== item.kind
+                const selectedItems = state.selectedItems.filter(i =>
+                  isVault ? !hasSameVaultID(i) : !hasSameKind(i)
                 );
                 selectedItems.push(item);
                 return dispatch({

awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx

@@ -12,7 +12,8 @@ describe('<MultiCredentialsLookup />', () => {
   const credentials = [
     { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
     { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' },
-    { name: 'Gatsby', id: 21, kind: 'vault' },
+    { name: 'Gatsby', id: 21, kind: 'vault', inputs: { vault_id: '1' } },
+    { name: 'Gatsby 2', id: 23, kind: 'vault' },
     { name: 'Gatsby', id: 8, kind: 'Machine' },
   ];
 
@@ -80,14 +81,15 @@ describe('<MultiCredentialsLookup />', () => {
       );
     });
     const chip = wrapper.find('CredentialChip');
-    expect(chip).toHaveLength(4);
+    expect(chip).toHaveLength(5);
     const button = chip.at(1).find('ChipButton');
     await act(async () => {
       button.invoke('onClick')();
     });
     expect(onChange).toBeCalledWith([
       { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
-      { id: 21, kind: 'vault', name: 'Gatsby' },
+      { id: 21, inputs: { vault_id: '1' }, kind: 'vault', name: 'Gatsby' },
+      { id: 23, kind: 'vault', name: 'Gatsby 2' },
       { id: 8, kind: 'Machine', name: 'Gatsby' },
     ]);
   });
@@ -161,12 +163,13 @@ describe('<MultiCredentialsLookup />', () => {
     expect(onChange).toBeCalledWith([
       { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
       { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' },
-      { id: 21, kind: 'vault', name: 'Gatsby' },
+      { id: 21, inputs: { vault_id: '1' }, kind: 'vault', name: 'Gatsby' },
+      { id: 23, kind: 'vault', name: 'Gatsby 2' },
       { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' },
     ]);
   });
 
-  test('should allow multiple vault credentials', async () => {
+  test('should allow multiple vault credentials with no vault id', async () => {
     const onChange = jest.fn();
     await act(async () => {
       wrapper = mountWithContexts(
@@ -193,7 +196,7 @@ describe('<MultiCredentialsLookup />', () => {
     act(() => {
       optionsList.invoke('selectItem')({
         id: 5,
-        kind: 'Machine',
+        kind: 'vault',
         name: 'Cred 5',
         url: 'www.google.com',
       });
@@ -205,9 +208,115 @@ describe('<MultiCredentialsLookup />', () => {
     expect(onChange).toBeCalledWith([
       { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
       { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' },
-      { id: 21, kind: 'vault', name: 'Gatsby' },
+      { id: 21, kind: 'vault', name: 'Gatsby', inputs: { vault_id: '1' } },
+      { id: 23, kind: 'vault', name: 'Gatsby 2' },
       { id: 8, kind: 'Machine', name: 'Gatsby' },
-      { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' },
+      { id: 5, kind: 'vault', name: 'Cred 5', url: 'www.google.com' },
+    ]);
+  });
+
+  test('should allow multiple vault credentials with different vault ids', async () => {
+    const onChange = jest.fn();
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <MultiCredentialsLookup
+          value={credentials}
+          tooltip="This is credentials look up"
+          onChange={onChange}
+          onError={() => {}}
+        />
+      );
+    });
+    const searchButton = await waitForElement(wrapper, 'SearchButton');
+    await act(async () => {
+      searchButton.invoke('onClick')();
+    });
+    wrapper.update();
+    const typeSelect = wrapper.find('AnsibleSelect');
+    act(() => {
+      typeSelect.invoke('onChange')({}, 500);
+    });
+    wrapper.update();
+    const optionsList = wrapper.find('OptionsList');
+    expect(optionsList.prop('multiple')).toEqual(true);
+    act(() => {
+      optionsList.invoke('selectItem')({
+        id: 5,
+        kind: 'vault',
+        name: 'Cred 5',
+        url: 'www.google.com',
+        inputs: { vault_id: '2' },
+      });
+    });
+    wrapper.update();
+    act(() => {
+      wrapper.find('Button[variant="primary"]').invoke('onClick')();
+    });
+    expect(onChange).toBeCalledWith([
+      { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
+      { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' },
+      { id: 21, kind: 'vault', name: 'Gatsby', inputs: { vault_id: '1' } },
+      { id: 23, kind: 'vault', name: 'Gatsby 2' },
+      { id: 8, kind: 'Machine', name: 'Gatsby' },
+      {
+        id: 5,
+        kind: 'vault',
+        name: 'Cred 5',
+        url: 'www.google.com',
+        inputs: { vault_id: '2' },
+      },
+    ]);
+  });
+
+  test('should not select multiple vault credentials with same vault id', async () => {
+    const onChange = jest.fn();
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <MultiCredentialsLookup
+          value={credentials}
+          tooltip="This is credentials look up"
+          onChange={onChange}
+          onError={() => {}}
+        />
+      );
+    });
+    const searchButton = await waitForElement(wrapper, 'SearchButton');
+    await act(async () => {
+      searchButton.invoke('onClick')();
+    });
+    wrapper.update();
+    const typeSelect = wrapper.find('AnsibleSelect');
+    act(() => {
+      typeSelect.invoke('onChange')({}, 500);
+    });
+    wrapper.update();
+    const optionsList = wrapper.find('OptionsList');
+    expect(optionsList.prop('multiple')).toEqual(true);
+    act(() => {
+      optionsList.invoke('selectItem')({
+        id: 24,
+        kind: 'vault',
+        name: 'Cred 5',
+        url: 'www.google.com',
+        inputs: { vault_id: '1' },
+      });
+    });
+    wrapper.update();
+    act(() => {
+      wrapper.find('Button[variant="primary"]').invoke('onClick')();
+    });
+    expect(onChange).toBeCalledWith([
+      { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' },
+      { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' },
+      { id: 23, kind: 'vault', name: 'Gatsby 2' },
+      { id: 8, kind: 'Machine', name: 'Gatsby' },
+      {
+        id: 24,
+        kind: 'vault',
+        name: 'Cred 5',
+        url: 'www.google.com',
+        inputs: { vault_id: '1' },
+      },
     ]);
   });
 });