[Security Solution][Detections] Chore: Fix rule form warnings (#79590)

* Prevent react warnings for unknown `isVisible` prop

Unknown config options ultimately become attributes on an anchor/button
element, so we need to not overload these config objects.

* Fix react warning about unknown `isDisabled` prop

There is no `isDisabled` prop on the underlying `EuiFieldText`
component, so we should use the native `disabled` attribute instead.

* Fix warning about switching between un-/controlled componenents

On the first few renders, our input refs have not been set while a
default value has, so the length of our values is 1 while our refs is 0.
This causes us to hit the 'else' condition of our ternary, leading to an
input with no value. When the ref is then populated, a value is
generated and so is the error.

The previous logic here was always evaluating to `value: item` and so was
simplified.

* Fix jest warning on unexpected state update

* removes unnecessary use of act()
* renders our component within the test
  * This allows our test to synchronize with react and prevents the
    warning; I suspect it has something to do with the scoping of
    `wrapper` or the `beforeEach` not being async, but I haven't had
    time to investigate.

* Remove unnecessary test guards

These tets no longer throw warnings.
This commit is contained in:
Ryland Herrick 2020-10-06 15:38:26 -05:00 committed by GitHub
parent c9d2398572
commit a4a5393d1f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 18 additions and 42 deletions

View file

@ -147,10 +147,7 @@ export const AddItem = ({
...(index === values.length - 1
? { inputRef: handleLastInputRef.bind(null, index) }
: {}),
...((inputsRef.current[index] != null && inputsRef.current[index].value !== item) ||
inputsRef.current[index] == null
? { value: item }
: {}),
value: item,
isInvalid: validate == null ? false : showValidation && validate(item),
};
return (

View file

@ -12,16 +12,6 @@ import { TestProviders, useFormFieldMock } from '../../../../common/mock';
jest.mock('../../../../common/lib/kibana');
describe('SelectRuleType', () => {
// I do this to avoid the messy warning from happening
// Warning: React does not recognize the `isVisible` prop on a DOM element.
beforeEach(() => {
jest.spyOn(console, 'error').mockImplementation(jest.fn());
});
afterEach(() => {
jest.spyOn(console, 'error').mockRestore();
});
it('renders correctly', () => {
const Component = () => {
const field = useFormFieldMock();

View file

@ -56,18 +56,16 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
() => ({
onClick: setEql,
isSelected: isEqlRule(ruleType),
isVisible: !isUpdateView || isEqlRule(ruleType),
}),
[ruleType, setEql, isUpdateView]
[ruleType, setEql]
);
const querySelectableConfig = useMemo(
() => ({
onClick: setQuery,
isSelected: isQueryRule(ruleType),
isVisible: !isUpdateView || isQueryRule(ruleType),
}),
[ruleType, setQuery, isUpdateView]
[ruleType, setQuery]
);
const mlSelectableConfig = useMemo(
@ -75,27 +73,24 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
isDisabled: !hasValidLicense || !isMlAdmin,
onClick: setMl,
isSelected: isMlRule(ruleType),
isVisible: !isUpdateView || isMlRule(ruleType),
}),
[ruleType, setMl, isUpdateView, hasValidLicense, isMlAdmin]
[ruleType, setMl, hasValidLicense, isMlAdmin]
);
const thresholdSelectableConfig = useMemo(
() => ({
onClick: setThreshold,
isSelected: isThresholdRule(ruleType),
isVisible: !isUpdateView || isThresholdRule(ruleType),
}),
[ruleType, setThreshold, isUpdateView]
[ruleType, setThreshold]
);
const threatMatchSelectableConfig = useMemo(
() => ({
onClick: setThreatMatch,
isSelected: isThreatMatchRule(ruleType),
isVisible: !isUpdateView || isThreatMatchRule(ruleType),
}),
[ruleType, setThreatMatch, isUpdateView]
[ruleType, setThreatMatch]
);
return (
@ -106,7 +101,7 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
label={field.label}
>
<EuiFlexGrid columns={3}>
{querySelectableConfig.isVisible && (
{(!isUpdateView || querySelectableConfig.isSelected) && (
<EuiFlexItem>
<EuiCard
data-test-subj="customRuleType"
@ -120,7 +115,7 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
/>
</EuiFlexItem>
)}
{mlSelectableConfig.isVisible && (
{(!isUpdateView || mlSelectableConfig.isSelected) && (
<EuiFlexItem>
<EuiCard
data-test-subj="machineLearningRuleType"
@ -140,7 +135,7 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
/>
</EuiFlexItem>
)}
{thresholdSelectableConfig.isVisible && (
{(!isUpdateView || thresholdSelectableConfig.isSelected) && (
<EuiFlexItem>
<EuiCard
data-test-subj="thresholdRuleType"
@ -154,7 +149,7 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
/>
</EuiFlexItem>
)}
{eqlSelectableConfig.isVisible && (
{(!isUpdateView || eqlSelectableConfig.isSelected) && (
<EuiFlexItem>
<EuiCard
data-test-subj="eqlRuleType"
@ -168,7 +163,7 @@ export const SelectRuleType: React.FC<SelectRuleTypeProps> = ({
/>
</EuiFlexItem>
)}
{threatMatchSelectableConfig.isVisible && (
{(!isUpdateView || threatMatchSelectableConfig.isSelected) && (
<EuiFlexItem>
<EuiCard
data-test-subj="threatMatchRuleType"

View file

@ -269,7 +269,7 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
'data-test-subj': 'detectionEngineStepAboutRuleLicense',
euiFieldProps: {
fullWidth: true,
isDisabled: isLoading,
disabled: isLoading,
placeholder: '',
},
}}

View file

@ -5,8 +5,7 @@
*/
import React from 'react';
import { shallow, mount, ReactWrapper } from 'enzyme';
import { act } from 'react-dom/test-utils';
import { shallow, mount } from 'enzyme';
import '../../../../../common/mock/match_media';
import '../../../../../common/mock/formatted_relative';
@ -181,9 +180,8 @@ describe('AllRules', () => {
});
describe('rules tab', () => {
let wrapper: ReactWrapper;
beforeEach(() => {
wrapper = mount(
it('renders correctly', async () => {
const wrapper = mount(
<TestProviders>
<AllRules
createPrePackagedRules={jest.fn()}
@ -199,14 +197,10 @@ describe('AllRules', () => {
/>
</TestProviders>
);
});
it('renders correctly', async () => {
await act(async () => {
await waitFor(() => {
expect(wrapper.exists('[data-test-subj="monitoring-table"]')).toBeFalsy();
expect(wrapper.exists('[data-test-subj="rules-table"]')).toBeTruthy();
});
await waitFor(() => {
expect(wrapper.exists('[data-test-subj="monitoring-table"]')).toBeFalsy();
expect(wrapper.exists('[data-test-subj="rules-table"]')).toBeTruthy();
});
});
});