From 4c1b0297e7df4ff75def59e52fa412dffdf90e63 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 3 Jun 2021 09:48:20 -0400 Subject: [PATCH 1/3] Fixes bug where checkbox list item was selecting things twice --- awx/ui_next/src/components/AddRole/SelectResourceStep.jsx | 4 +++- .../src/components/CheckboxListItem/CheckboxListItem.jsx | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index d74e08b823..ee20c2d706 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -100,7 +100,9 @@ function SelectResourceStep({ headerRow={ {sortColumns.map(({ name, key }) => ( - {name} + + {name} + ))} } diff --git a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx index 1467293026..c76d9af0d1 100644 --- a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx +++ b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx @@ -34,7 +34,6 @@ const CheckboxListItem = ({ select={{ rowIndex, isSelected, - onSelect: isSelected ? onDeselect : onSelect, variant: isRadio ? 'radio' : 'checkbox', }} name={name} @@ -43,7 +42,7 @@ const CheckboxListItem = ({ {columns?.length > 0 ? ( columns.map(col => ( - + {item[col.key]} )) From c2c93e7a665d126c51373eda6a4bbc15532e13b0 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 3 Jun 2021 13:50:26 -0400 Subject: [PATCH 2/3] Fix unit test failures --- .../AdHocCommands/AdHocCommands.test.jsx | 16 ++++------------ .../AdHocCommands/AdHocCommandsWizard.test.jsx | 4 ++-- .../AddRole/SelectResourceStep.test.jsx | 2 +- .../LaunchPrompt/steps/CredentialsStep.test.jsx | 6 +++--- .../Schedule/ScheduleAdd/ScheduleAdd.test.jsx | 6 +----- .../Schedule/ScheduleEdit/ScheduleEdit.test.jsx | 12 ++---------- .../Schedule/shared/ScheduleForm.test.jsx | 6 +----- .../UserAndTeamAccessAdd.test.jsx | 4 ++-- .../CredentialPluginPrompt.jsx | 1 - .../CredentialPluginPrompt.test.jsx | 2 +- .../Modals/NodeModals/NodeModal.test.jsx | 10 +++++----- 11 files changed, 22 insertions(+), 47 deletions(-) diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.jsx b/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.jsx index e03b2bf945..d6e24cf71f 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.jsx +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.jsx @@ -208,7 +208,7 @@ describe('', () => { wrapper .find('td#check-action-item-2') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); @@ -227,7 +227,7 @@ describe('', () => { wrapper .find('td#check-action-item-4') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); @@ -377,11 +377,7 @@ describe('', () => { wrapper .find('td#check-action-item-2') .find('input') - .simulate('change', { - target: { - checked: true, - }, - }); + .simulate('click'); }); wrapper.update(); @@ -400,11 +396,7 @@ describe('', () => { wrapper .find('td#check-action-item-4') .find('input') - .simulate('change', { - target: { - checked: true, - }, - }); + .simulate('click'); }); wrapper.update(); diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.jsx b/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.jsx index f1fe18338d..bfa9c154ca 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.jsx +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.jsx @@ -155,7 +155,7 @@ describe('', () => { wrapper .find('td#check-action-item-1') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); @@ -181,7 +181,7 @@ describe('', () => { wrapper .find('td#check-action-item-1') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx index 6ddfc677ff..5bbb8ceaab 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.test.jsx @@ -122,7 +122,7 @@ describe('', () => { checkboxListItemWrapper .first() .find('input[type="checkbox"]') - .simulate('change', { target: { checked: true } }); + .simulate('click'); expect(handleRowClick).toHaveBeenCalledWith(data.results[0]); }); }); diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/CredentialsStep.test.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/CredentialsStep.test.jsx index 1b6e37c15f..3b6855f0b9 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/CredentialsStep.test.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/CredentialsStep.test.jsx @@ -183,7 +183,7 @@ describe('CredentialsStep', () => { wrapper .find('td#check-action-item-2') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); expect(wrapper.find('Alert').length).toBe(1); @@ -244,7 +244,7 @@ describe('CredentialsStep', () => { wrapper .find('td#check-action-item-5') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); expect(wrapper.find('Alert').length).toBe(0); @@ -321,7 +321,7 @@ describe('CredentialsStep', () => { wrapper .find('td#check-action-item-33') .find('input') - .simulate('change', { target: { checked: true } }); + .simulate('click'); }); wrapper.update(); expect(wrapper.find('Alert').length).toBe(0); diff --git a/awx/ui_next/src/components/Schedule/ScheduleAdd/ScheduleAdd.test.jsx b/awx/ui_next/src/components/Schedule/ScheduleAdd/ScheduleAdd.test.jsx index b95212e755..01962c8ba8 100644 --- a/awx/ui_next/src/components/Schedule/ScheduleAdd/ScheduleAdd.test.jsx +++ b/awx/ui_next/src/components/Schedule/ScheduleAdd/ScheduleAdd.test.jsx @@ -343,11 +343,7 @@ describe('', () => { wrapper .find('td#check-action-item-1') .find('input') - .simulate('change', { - target: { - checked: true, - }, - }); + .simulate('click'); }); wrapper.update(); expect( diff --git a/awx/ui_next/src/components/Schedule/ScheduleEdit/ScheduleEdit.test.jsx b/awx/ui_next/src/components/Schedule/ScheduleEdit/ScheduleEdit.test.jsx index 9b1b2f4c62..133b22ed33 100644 --- a/awx/ui_next/src/components/Schedule/ScheduleEdit/ScheduleEdit.test.jsx +++ b/awx/ui_next/src/components/Schedule/ScheduleEdit/ScheduleEdit.test.jsx @@ -475,11 +475,7 @@ describe('', () => { wrapper .find('td#check-action-item-1') .find('input') - .simulate('change', { - target: { - checked: true, - }, - }); + .simulate('click'); }); wrapper.update(); @@ -607,11 +603,7 @@ describe('', () => { wrapper .find('td#check-action-item-2') .find('input') - .simulate('change', { - target: { - checked: true, - }, - }); + .simulate('click'); }); wrapper.update(); diff --git a/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx b/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx index 9e95e4d009..5cc4490d9a 100644 --- a/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx +++ b/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx @@ -345,11 +345,7 @@ describe('', () => { promptWrapper .find('td#check-action-item-1') .find('input') - .simulate('change', { - target: { - checked: true, - }, - }); + .simulate('click'); }); promptWrapper.update(); expect( diff --git a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx index 343eca7126..87b79bf780 100644 --- a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx +++ b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx @@ -152,7 +152,7 @@ describe('', () => { .find('CheckboxListItem') .first() .find('input[type="checkbox"]') - .simulate('change', { target: { checked: true } }) + .simulate('click') ); wrapper.update(); @@ -235,7 +235,7 @@ describe('', () => { .find('CheckboxListItem') .first() .find('input[type="checkbox"]') - .simulate('change', { target: { checked: true } }) + .simulate('click') ); wrapper.update(); diff --git a/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.jsx b/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.jsx index 9ac1491d07..219fa8ae5a 100644 --- a/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.jsx +++ b/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.jsx @@ -1,7 +1,6 @@ import React, { useCallback } from 'react'; import { func, shape } from 'prop-types'; import { Formik, useField } from 'formik'; - import { t } from '@lingui/macro'; import { Button, diff --git a/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.test.jsx b/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.test.jsx index ce765d19b9..9b36b8f076 100644 --- a/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.test.jsx +++ b/awx/ui_next/src/screens/Credential/shared/CredentialPlugins/CredentialPluginPrompt/CredentialPluginPrompt.test.jsx @@ -134,7 +134,7 @@ describe('', () => { wrapper .find('td#check-action-item-1') .find('input') - .invoke('onChange')(true); + .simulate('click'); }); wrapper.update(); expect( diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx index 1885a52219..ac0d2fb3ac 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx @@ -267,7 +267,7 @@ describe('NodeModal', () => { wrapper .find('td#check-action-item-1') .find('input') - .prop('onChange')(true); + .simulate('click'); }); wrapper.update(); await act(async () => { @@ -337,7 +337,7 @@ describe('NodeModal', () => { wrapper .find('td#check-action-item-1') .find('input') - .prop('onChange')(true); + .simulate('click'); }); wrapper.update(); await act(async () => { @@ -376,7 +376,7 @@ describe('NodeModal', () => { wrapper .find('td#check-action-item-1') .find('input') - .prop('onChange')(true); + .simulate('click'); }); wrapper.update(); await act(async () => { @@ -415,7 +415,7 @@ describe('NodeModal', () => { wrapper .find('td#check-action-item-1') .find('input') - .prop('onChange')(true) + .simulate('click') ); wrapper.update(); @@ -673,7 +673,7 @@ describe('Edit existing node', () => { newWrapper .find('td#check-action-item-1') .find('input') - .prop('onChange')(); + .simulate('click'); newWrapper.update(); }); newWrapper.update(); From 177901eca6d40619562478d11343d761ae1c2e99 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 3 Jun 2021 14:01:04 -0400 Subject: [PATCH 3/3] Adds assertion to ensure that only one chip is shown when a checkbox list item is selected --- awx/ui_next/src/components/AddRole/AddResourceRole.test.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/ui_next/src/components/AddRole/AddResourceRole.test.jsx b/awx/ui_next/src/components/AddRole/AddResourceRole.test.jsx index 5be9c860d1..651368d927 100644 --- a/awx/ui_next/src/components/AddRole/AddResourceRole.test.jsx +++ b/awx/ui_next/src/components/AddRole/AddResourceRole.test.jsx @@ -95,6 +95,7 @@ describe('<_AddResourceRole />', () => { // Step 2 await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); + expect(wrapper.find('Chip').length).toBe(0); act(() => wrapper.find('CheckboxListItem[name="foo"]').invoke('onSelect')(true) ); @@ -102,6 +103,7 @@ describe('<_AddResourceRole />', () => { expect( wrapper.find('CheckboxListItem[name="foo"]').prop('isSelected') ).toBe(true); + expect(wrapper.find('Chip').length).toBe(1); act(() => wrapper.find('Button[type="submit"]').prop('onClick')()); wrapper.update();