[Response Ops] Es query rule "size" field re-initializes to 100 if set to 0 when editing (#213636)

Resolves https://github.com/elastic/kibana/issues/209427

## Summary

This PR fixes a bug when editing an es query rule with size set to 0. I
also refactored the tests to use react testing library.

### Checklist

Check the PR satisfies following conditions. 

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### To verify

1. Create an es query rule
2. Set the size to be 0
3. Save your rule 
4. Edit your rule and verify that the size is set to 0 when you open the
editor
This commit is contained in:
Alexi Doak 2025-03-14 12:19:43 -07:00 committed by GitHub
parent 67d6707715
commit 287eb3e5c6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 156 additions and 199 deletions

View file

@ -5,10 +5,10 @@
* 2.0.
*/
import React from 'react';
import React, { PropsWithChildren } from 'react';
import { fireEvent, render, waitFor, screen, act } from '@testing-library/react';
import { I18nProvider } from '@kbn/i18n-react';
import { of, Subject } from 'rxjs';
import { mountWithIntl, nextTick } from '@kbn/test-jest-helpers';
import { act } from 'react-dom/test-utils';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import { unifiedSearchPluginMock } from '@kbn/unified-search-plugin/public/mocks';
@ -106,6 +106,10 @@ const createDataPluginMock = () => {
return dataMock;
};
const AppWrapper = React.memo<PropsWithChildren<unknown>>(({ children }) => (
<I18nProvider>{children}</I18nProvider>
));
const dataMock = createDataPluginMock();
const dataViewMock = dataViewPluginMocks.createStartContract();
const unifiedSearchMock = unifiedSearchPluginMock.createStartContract();
@ -151,76 +155,71 @@ describe('EsQueryRuleTypeExpression', () => {
timeWindowSize: [],
};
const wrapper = mountWithIntl(
<EsQueryExpression
unifiedSearch={unifiedSearchMock}
ruleInterval="1m"
ruleThrottle="1m"
alertNotifyWhen="onThrottleInterval"
ruleParams={alertParams}
setRuleParams={() => {}}
setRuleProperty={() => {}}
errors={errors}
data={dataMock}
dataViews={dataViewMock}
defaultActionGroupId=""
actionGroups={[]}
charts={chartsStartMock}
onChangeMetaData={() => {}}
/>
return await act(async () =>
render(
<EsQueryExpression
unifiedSearch={unifiedSearchMock}
ruleInterval="1m"
ruleThrottle="1m"
alertNotifyWhen="onThrottleInterval"
ruleParams={alertParams}
setRuleParams={() => {}}
setRuleProperty={() => {}}
errors={errors}
data={dataMock}
dataViews={dataViewMock}
defaultActionGroupId=""
actionGroups={[]}
charts={chartsStartMock}
onChangeMetaData={() => {}}
/>,
{
wrapper: AppWrapper,
}
)
);
const update = async () =>
await act(async () => {
await nextTick();
wrapper.update();
});
await update();
return wrapper;
}
test('should render EsQueryRuleTypeExpression with expected components', async () => {
const wrapper = await setup(defaultEsQueryExpressionParams);
expect(wrapper.find('[data-test-subj="indexSelectPopover"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="sizeValueExpression"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="queryJsonEditor"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="thresholdExpression"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="forLastExpression"]').exists()).toBeTruthy();
const result = await setup(defaultEsQueryExpressionParams);
expect(result.getByTestId('indexSelectPopover')).toBeInTheDocument();
expect(result.getByTestId('sizeValueExpression')).toBeInTheDocument();
expect(result.getByTestId('queryJsonEditor')).toBeInTheDocument();
expect(result.getByTestId('thresholdPopover')).toBeInTheDocument();
expect(result.getByTestId('forLastExpression')).toBeInTheDocument();
expect(result.queryByTestId('testQuerySuccess')).not.toBeInTheDocument();
expect(result.queryByTestId('testQueryError')).not.toBeInTheDocument();
const excludeHitsButton = wrapper.find(
'[data-test-subj="excludeHitsFromPreviousRunExpression"]'
);
expect(excludeHitsButton.exists()).toBeTruthy();
expect(excludeHitsButton.first().prop('checked')).toBeTruthy();
expect(result.getByTestId('excludeHitsFromPreviousRunExpression')).toBeChecked();
const testQueryButton = wrapper.find('EuiButton[data-test-subj="testQuery"]');
expect(testQueryButton.exists()).toBeTruthy();
expect(testQueryButton.prop('disabled')).toBe(false);
expect(result.getByTestId('testQuery')).not.toBeDisabled();
});
test('should render Test Query button disabled if alert params are invalid', async () => {
const wrapper = await setup({
const result = await setup({
...defaultEsQueryExpressionParams,
timeField: null,
} as unknown as EsQueryRuleParams<SearchType.esQuery>);
const testQueryButton = wrapper.find('EuiButton[data-test-subj="testQuery"]');
expect(testQueryButton.exists()).toBeTruthy();
expect(testQueryButton.prop('disabled')).toBe(true);
expect(result.getByTestId('testQuery')).toBeDisabled();
});
test('should show excludeHitsFromPreviousRun unchecked by default', async () => {
const wrapper = await setup({
const result = await setup({
...defaultEsQueryExpressionParams,
excludeHitsFromPreviousRun: undefined,
} as unknown as EsQueryRuleParams<SearchType.esQuery>);
const excludeMatchesCheckBox = wrapper.find(
'EuiCheckbox[data-test-subj="excludeHitsFromPreviousRunExpression"]'
);
expect(excludeMatchesCheckBox.exists()).toBeTruthy();
expect(excludeMatchesCheckBox.prop('checked')).toBe(false);
expect(result.getByTestId('excludeHitsFromPreviousRunExpression')).not.toBeChecked();
});
test('should render EsQueryRuleTypeExpression with chosen size field', async () => {
const result = await setup({
...defaultEsQueryExpressionParams,
size: 0,
} as unknown as EsQueryRuleParams<SearchType.esQuery>);
expect(result.getByTestId('sizeValueExpression')).toHaveTextContent('Size 0');
});
test('should show success message if ungrouped Test Query is successful', async () => {
@ -232,19 +231,14 @@ describe('EsQueryRuleTypeExpression', () => {
},
});
dataMock.search.search.mockImplementation(() => searchResponseMock$);
const wrapper = await setup(defaultEsQueryExpressionParams);
const testQueryButton = wrapper.find('button[data-test-subj="testQuery"]');
testQueryButton.simulate('click');
expect(dataMock.search.search).toHaveBeenCalled();
await act(async () => {
await nextTick();
wrapper.update();
});
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeFalsy();
expect(wrapper.find('EuiText[data-test-subj="testQuerySuccess"]').text()).toEqual(
`Query matched 1234 documents in the last 15s.`
);
await setup(defaultEsQueryExpressionParams);
fireEvent.click(screen.getByTestId('testQuery'));
await waitFor(() => expect(dataMock.search.search).toBeCalled());
expect(screen.getByTestId('testQuerySuccess')).toBeInTheDocument();
expect(screen.getByText('Query matched 1234 documents in the last 15s.')).toBeInTheDocument();
expect(screen.queryByTestId('testQueryError')).not.toBeInTheDocument();
});
test('should show success message if grouped Test Query is successful', async () => {
@ -284,23 +278,18 @@ describe('EsQueryRuleTypeExpression', () => {
},
});
dataMock.search.search.mockImplementation(() => searchResponseMock$);
const wrapper = await setup({
await setup({
...defaultEsQueryExpressionParams,
termField: 'the-term',
termSize: 10,
});
const testQueryButton = wrapper.find('button[data-test-subj="testQuery"]');
testQueryButton.simulate('click');
expect(dataMock.search.search).toHaveBeenCalled();
await act(async () => {
await nextTick();
wrapper.update();
});
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeFalsy();
expect(wrapper.find('EuiText[data-test-subj="testQuerySuccess"]').text()).toEqual(
`Grouped query matched 5 groups in the last 15s.`
);
fireEvent.click(screen.getByTestId('testQuery'));
await waitFor(() => expect(dataMock.search.search).toBeCalled());
expect(screen.getByTestId('testQuerySuccess')).toBeInTheDocument();
expect(screen.getByText('Grouped query matched 5 groups in the last 15s.')).toBeInTheDocument();
expect(screen.queryByTestId('testQueryError')).not.toBeInTheDocument();
});
test('should show success message if Test Query is successful (with partial result)', async () => {
@ -319,41 +308,31 @@ describe('EsQueryRuleTypeExpression', () => {
};
const searchResponseMock$ = new Subject();
dataMock.search.search.mockImplementation(() => searchResponseMock$);
const wrapper = await setup(defaultEsQueryExpressionParams);
const testQueryButton = wrapper.find('button[data-test-subj="testQuery"]');
await setup(defaultEsQueryExpressionParams);
testQueryButton.simulate('click');
expect(dataMock.search.search).toHaveBeenCalled();
await act(async () => {
fireEvent.click(screen.getByTestId('testQuery'));
await waitFor(() => expect(dataMock.search.search).toBeCalled());
await waitFor(() => {
searchResponseMock$.next(partial);
searchResponseMock$.next(complete);
searchResponseMock$.complete();
await nextTick();
wrapper.update();
});
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeFalsy();
expect(wrapper.find('EuiText[data-test-subj="testQuerySuccess"]').text()).toEqual(
`Query matched 1234 documents in the last 15s.`
);
expect(screen.getByTestId('testQuerySuccess')).toBeInTheDocument();
expect(screen.getByText('Query matched 1234 documents in the last 15s.')).toBeInTheDocument();
expect(screen.queryByTestId('testQueryError')).not.toBeInTheDocument();
});
test('should show error message if Test Query is throws error', async () => {
dataMock.search.search.mockImplementation(() => {
throw new Error('What is this query');
});
const wrapper = await setup(defaultEsQueryExpressionParams);
const testQueryButton = wrapper.find('button[data-test-subj="testQuery"]');
await setup(defaultEsQueryExpressionParams);
testQueryButton.simulate('click');
expect(dataMock.search.search).toHaveBeenCalled();
await act(async () => {
await nextTick();
wrapper.update();
});
fireEvent.click(screen.getByTestId('testQuery'));
await waitFor(() => expect(dataMock.search.search).toBeCalled());
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeTruthy();
expect(screen.queryByTestId('testQuerySuccess')).not.toBeInTheDocument();
expect(screen.getByTestId('testQueryError')).toBeInTheDocument();
});
});

View file

@ -68,7 +68,7 @@ export const EsQueryExpression: React.FC<
timeWindowUnit: timeWindowUnit ?? DEFAULT_VALUES.TIME_WINDOW_UNIT,
threshold: threshold ?? DEFAULT_VALUES.THRESHOLD,
thresholdComparator: thresholdComparator ?? DEFAULT_VALUES.THRESHOLD_COMPARATOR,
size: size ? size : isServerless ? SERVERLESS_DEFAULT_VALUES.SIZE : DEFAULT_VALUES.SIZE,
size: size ?? (isServerless ? SERVERLESS_DEFAULT_VALUES.SIZE : DEFAULT_VALUES.SIZE),
esQuery: esQuery ?? DEFAULT_VALUES.QUERY,
aggType: aggType ?? DEFAULT_VALUES.AGGREGATION_TYPE,
groupBy: groupBy ?? DEFAULT_VALUES.GROUP_BY,
@ -198,6 +198,7 @@ export const EsQueryExpression: React.FC<
<Fragment>
<EuiFormRow
fullWidth
data-test-subj="indexSelectPopover"
label={
<FormattedMessage
id="xpack.stackAlerts.esQuery.ui.selectIndexPrompt"
@ -207,7 +208,6 @@ export const EsQueryExpression: React.FC<
>
<IndexSelectPopover
index={index}
data-test-subj="indexSelectPopover"
esFields={esFields}
timeField={timeField}
errors={errors}

View file

@ -5,23 +5,21 @@
* 2.0.
*/
import { mountWithIntl, nextTick } from '@kbn/test-jest-helpers';
import React from 'react';
import React, { PropsWithChildren } from 'react';
import { fireEvent, render, waitFor, screen, act } from '@testing-library/react';
import { I18nProvider } from '@kbn/i18n-react';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import { unifiedSearchPluginMock } from '@kbn/unified-search-plugin/public/mocks';
import { EsQueryRuleParams, SearchType } from '../types';
import { SearchSourceExpression } from './search_source_expression';
import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
import { act } from 'react-dom/test-utils';
import { Subject } from 'rxjs';
import { ISearchSource } from '@kbn/data-plugin/common';
import { IUiSettingsClient } from '@kbn/core/public';
import { findTestSubject } from '@elastic/eui/lib/test';
import { copyToClipboard, EuiLoadingSpinner } from '@elastic/eui';
import { copyToClipboard } from '@elastic/eui';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import { indexPatternEditorPluginMock as dataViewEditorPluginMock } from '@kbn/data-view-editor-plugin/public/mocks';
import { ReactWrapper } from 'enzyme';
import { DataPlugin } from '@kbn/data-plugin/public';
jest.mock('@elastic/eui', () => {
@ -147,6 +145,11 @@ describe('SearchSourceAlertTypeExpression', () => {
let dataMock: jest.Mocked<ReturnType<DataPlugin['start']>>;
let searchSourceMock: ISearchSource;
let mockSearchResult: Subject<unknown>;
const AppWrapper = React.memo<PropsWithChildren<unknown>>(({ children }) => (
<I18nProvider>{children}</I18nProvider>
));
beforeEach(() => {
mockSearchResult = new Subject();
searchSourceMock = {
@ -250,7 +253,7 @@ describe('SearchSourceAlertTypeExpression', () => {
};
const dataViewEditorMock = dataViewEditorPluginMock.createStartContract();
return mountWithIntl(
return render(
<KibanaContextProvider
services={{
dataViews: dataViewPluginMock,
@ -277,108 +280,85 @@ describe('SearchSourceAlertTypeExpression', () => {
metadata={{ adHocDataViewList: [] }}
onChangeMetaData={jest.fn()}
/>
</KibanaContextProvider>
</KibanaContextProvider>,
{
wrapper: AppWrapper,
}
);
};
test('should render correctly', async () => {
let wrapper = setup(defaultSearchSourceExpressionParams);
const result = setup(defaultSearchSourceExpressionParams);
expect(wrapper.find(EuiLoadingSpinner).exists()).toBeTruthy();
await waitFor(() =>
expect(result.getByTestId('searchSourceLoadingSpinner')).toBeInTheDocument()
);
await act(async () => {
await nextTick();
});
wrapper = await wrapper.update();
expect(findTestSubject(wrapper, 'thresholdExpression')).toBeTruthy();
expect(result.getByTestId('thresholdPopover')).toBeInTheDocument();
expect(result.getByTestId('excludeHitsFromPreviousRunExpression')).toBeChecked();
});
const excludeHitsCheckbox = findTestSubject(wrapper, 'excludeHitsFromPreviousRunExpression');
expect(excludeHitsCheckbox).toBeTruthy();
expect(excludeHitsCheckbox.prop('checked')).toBeTruthy();
test('should render chosen size field', async () => {
const result = await act(async () =>
setup({
...defaultSearchSourceExpressionParams,
size: 0,
})
);
expect(result.getByTestId('sizeValueExpression')).toHaveTextContent('Size 0');
});
test('should disable Test Query button if data view is not selected yet', async () => {
let wrapper = setup({ ...defaultSearchSourceExpressionParams, searchConfiguration: undefined });
await act(async () => {
await nextTick();
});
wrapper = await wrapper.update();
const result = await act(async () =>
setup({
...defaultSearchSourceExpressionParams,
searchConfiguration: undefined,
})
);
const testButton = findTestSubject(wrapper, 'testQuery');
expect(testButton.prop('disabled')).toBeTruthy();
expect(result.getByTestId('testQuery')).toBeDisabled();
});
test('should show success message if ungrouped Test Query is successful', async () => {
let wrapper = setup(defaultSearchSourceExpressionParams);
await act(async () => {
await nextTick();
});
wrapper = await wrapper.update();
await act(async () => {
const testButton = findTestSubject(wrapper, 'testQuery');
expect(testButton.prop('disabled')).toBeFalsy();
testButton.simulate('click');
wrapper.update();
});
wrapper = await wrapper.update();
await act(async () => setup(defaultSearchSourceExpressionParams));
await act(async () => {
fireEvent.click(screen.getByTestId('testQuery'));
await waitFor(() => {
mockSearchResult.next(testResultPartial);
mockSearchResult.next(testResultComplete);
mockSearchResult.complete();
await nextTick();
wrapper.update();
});
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeFalsy();
expect(wrapper.find('EuiText[data-test-subj="testQuerySuccess"]').text()).toEqual(
`Query matched 1234 documents in the last 15s.`
);
expect(screen.getByTestId('testQuerySuccess')).toBeInTheDocument();
expect(screen.getByText('Query matched 1234 documents in the last 15s.')).toBeInTheDocument();
expect(screen.queryByTestId('testQueryError')).not.toBeInTheDocument();
});
test('should show success message if grouped Test Query is successful', async () => {
let wrapper = setup({
...defaultSearchSourceExpressionParams,
termField: 'the-term',
termSize: 10,
});
await act(async () => {
await nextTick();
});
wrapper = await wrapper.update();
await act(async () => {
const testButton = findTestSubject(wrapper, 'testQuery');
expect(testButton.prop('disabled')).toBeFalsy();
testButton.simulate('click');
wrapper.update();
});
wrapper = await wrapper.update();
await act(async () =>
setup({
...defaultSearchSourceExpressionParams,
termField: 'the-term',
termSize: 10,
})
);
await act(async () => {
fireEvent.click(screen.getByTestId('testQuery'));
await waitFor(() => {
mockSearchResult.next(testResultPartial);
mockSearchResult.next(testResultGroupedComplete);
mockSearchResult.complete();
await nextTick();
wrapper.update();
});
expect(wrapper.find('[data-test-subj="testQuerySuccess"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="testQueryError"]').exists()).toBeFalsy();
expect(wrapper.find('EuiText[data-test-subj="testQuerySuccess"]').text()).toEqual(
`Grouped query matched 5 groups in the last 15s.`
);
expect(screen.getByTestId('testQuerySuccess')).toBeInTheDocument();
expect(screen.getByText('Grouped query matched 5 groups in the last 15s.')).toBeInTheDocument();
expect(screen.queryByTestId('testQueryError')).not.toBeInTheDocument();
});
it('should call copyToClipboard with the serialized query when the copy query button is clicked', async () => {
let wrapper = null as unknown as ReactWrapper;
await act(async () => {
wrapper = setup(defaultSearchSourceExpressionParams);
});
wrapper.update();
await act(async () => {
findTestSubject(wrapper, 'copyQuery').simulate('click');
});
wrapper.update();
await act(async () => setup(defaultSearchSourceExpressionParams));
fireEvent.click(screen.getByTestId('copyQuery'));
expect(copyToClipboard).toHaveBeenCalledWith(`{
\"fields\": [
{
@ -450,16 +430,13 @@ describe('SearchSourceAlertTypeExpression', () => {
(dataMock.search.searchSource.create as jest.Mock).mockImplementationOnce(() =>
Promise.reject(new Error('Cant find searchSource'))
);
let wrapper = setup(defaultSearchSourceExpressionParams);
const result = setup(defaultSearchSourceExpressionParams);
expect(wrapper.find(EuiLoadingSpinner).exists()).toBeTruthy();
expect(wrapper.text().includes('Cant find searchSource')).toBeFalsy();
await act(async () => {
await nextTick();
await waitFor(() => {
expect(screen.queryByText('Cant find searchSource')).not.toBeInTheDocument();
expect(result.getByTestId('searchSourceLoadingSpinner')).toBeInTheDocument();
});
wrapper = await wrapper.update();
expect(wrapper.text().includes('Cant find searchSource')).toBeTruthy();
expect(screen.getByText('Cant find searchSource')).toBeInTheDocument();
});
});

View file

@ -85,7 +85,7 @@ export const SearchSourceExpression = ({
timeWindowUnit: timeWindowUnit ?? DEFAULT_VALUES.TIME_WINDOW_UNIT,
threshold: threshold ?? DEFAULT_VALUES.THRESHOLD,
thresholdComparator: thresholdComparator ?? DEFAULT_VALUES.THRESHOLD_COMPARATOR,
size: size ? size : isServerless ? SERVERLESS_DEFAULT_VALUES.SIZE : DEFAULT_VALUES.SIZE,
size: size ?? (isServerless ? SERVERLESS_DEFAULT_VALUES.SIZE : DEFAULT_VALUES.SIZE),
aggType: aggType ?? DEFAULT_VALUES.AGGREGATION_TYPE,
aggField,
groupBy: groupBy ?? DEFAULT_VALUES.GROUP_BY,
@ -123,7 +123,11 @@ export const SearchSourceExpression = ({
}
if (!searchSource) {
return <EuiEmptyPrompt title={<EuiLoadingSpinner size="xl" />} />;
return (
<EuiEmptyPrompt
title={<EuiLoadingSpinner data-test-subj="searchSourceLoadingSpinner" size="xl" />}
/>
);
}
return (

View file

@ -115,11 +115,8 @@ export const SearchSourceExpressionForm = (props: SearchSourceExpressionFormProp
groupBy: ruleParams.groupBy ?? DEFAULT_VALUES.GROUP_BY,
termSize: ruleParams.termSize ?? DEFAULT_VALUES.TERM_SIZE,
termField: ruleParams.termField,
size: ruleParams.size
? ruleParams.size
: isServerless
? SERVERLESS_DEFAULT_VALUES.SIZE
: DEFAULT_VALUES.SIZE,
size:
ruleParams.size ?? (isServerless ? SERVERLESS_DEFAULT_VALUES.SIZE : DEFAULT_VALUES.SIZE),
excludeHitsFromPreviousRun:
ruleParams.excludeHitsFromPreviousRun ?? DEFAULT_VALUES.EXCLUDE_PREVIOUS_HITS,
sourceFields: ruleParams.sourceFields,

View file

@ -154,6 +154,7 @@ export const RuleCommonExpressions: React.FC<RuleCommonExpressionsProps> = ({
<EuiSpacer size="s" />
<EuiFormRow
fullWidth
data-test-subj="sizeValueExpression"
label={[
<FormattedMessage
id="xpack.stackAlerts.esQuery.ui.selectSizePrompt"
@ -174,7 +175,6 @@ export const RuleCommonExpressions: React.FC<RuleCommonExpressionsProps> = ({
description={i18n.translate('xpack.stackAlerts.esQuery.ui.sizeExpression', {
defaultMessage: 'Size',
})}
data-test-subj="sizeValueExpression"
value={size}
errors={errors.size}
display="fullWidth"