[8.8] [Security Solution] [Controls] [Fix] Move fieldFilterPredicate function out of redux (#157004) (#158906)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] [Controls] [Fix] Move fieldFilterPredicate
function out of redux
(#157004)](https://github.com/elastic/kibana/pull/157004)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jatin
Kathuria","email":"jatin.kathuria@elastic.co"},"sourceCommit":{"committedDate":"2023-05-17T15:09:08Z","message":"[Security
Solution] [Controls] [Fix] Move fieldFilterPredicate function out of
redux (#157004)\n\n## Summary\r\n\r\nThis PR handles:
#156996\r\n\r\n@elastic/kibana-presentation team: Currently, this PR
moves,\r\n`fieldFilterPredicate` out of settings. Now it is being used
as the prop\r\nof the `ControlGroupRenderer`. Below video demonstrates
the difference\r\nbetween both:\r\n\r\n|Before|After|\r\n|---|---|\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/236853473-343c4da6-ebe2-43d2-ae43-84d46dedae7c.mov\"/>\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/236851603-45aed5da-f185-4c43-9b93-571aa44709c5.mov\"\r\n/>
|\r\n\r\n\r\nPlease let me know if there could be a better approach for
achieving the\r\nsame objective.\r\n\r\n\r\n### Checklist\r\n\r\nDelete
any items that are not applicable to this PR.\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n- [x] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"d2fb381350064a5aacdf1fe5fde8a08b642e2e74","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","backport:skip","Team:Security
Solution
Platform","v8.9.0"],"number":157004,"url":"https://github.com/elastic/kibana/pull/157004","mergeCommit":{"message":"[Security
Solution] [Controls] [Fix] Move fieldFilterPredicate function out of
redux (#157004)\n\n## Summary\r\n\r\nThis PR handles:
#156996\r\n\r\n@elastic/kibana-presentation team: Currently, this PR
moves,\r\n`fieldFilterPredicate` out of settings. Now it is being used
as the prop\r\nof the `ControlGroupRenderer`. Below video demonstrates
the difference\r\nbetween both:\r\n\r\n|Before|After|\r\n|---|---|\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/236853473-343c4da6-ebe2-43d2-ae43-84d46dedae7c.mov\"/>\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/236851603-45aed5da-f185-4c43-9b93-571aa44709c5.mov\"\r\n/>
|\r\n\r\n\r\nPlease let me know if there could be a better approach for
achieving the\r\nsame objective.\r\n\r\n\r\n### Checklist\r\n\r\nDelete
any items that are not applicable to this PR.\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n- [x] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"d2fb381350064a5aacdf1fe5fde8a08b642e2e74"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/157004","number":157004,"mergeCommit":{"message":"[Security
Solution] [Controls] [Fix] Move fieldFilterPredicate function out of
redux (#157004)\n\n## Summary\r\n\r\nThis PR handles:
#156996\r\n\r\n@elastic/kibana-presentation team: Currently, this PR
moves,\r\n`fieldFilterPredicate` out of settings. Now it is being used
as the prop\r\nof the `ControlGroupRenderer`. Below video demonstrates
the difference\r\nbetween both:\r\n\r\n|Before|After|\r\n|---|---|\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/236853473-343c4da6-ebe2-43d2-ae43-84d46dedae7c.mov\"/>\r\n|
<video\r\nsrc=\"https://user-images.githubusercontent.com/7485038/236851603-45aed5da-f185-4c43-9b93-571aa44709c5.mov\"\r\n/>
|\r\n\r\n\r\nPlease let me know if there could be a better approach for
achieving the\r\nsame objective.\r\n\r\n\r\n### Checklist\r\n\r\nDelete
any items that are not applicable to this PR.\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n- [x] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"d2fb381350064a5aacdf1fe5fde8a08b642e2e74"}}]}]
BACKPORT-->
This commit is contained in:
Jatin Kathuria 2023-06-02 04:29:46 -07:00 committed by GitHub
parent 80d17c6788
commit 8c954da6e3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 84 additions and 16 deletions

View file

@ -98,9 +98,6 @@ export const ControlEditor = ({
const controlGroup = useControlGroupContainer();
const editorConfig = controlGroup.select((state) => state.componentState.editorConfig);
const customFilterPredicate = controlGroup.select(
(state) => state.componentState.fieldFilterPredicate
);
const [defaultTitle, setDefaultTitle] = useState<string>();
const [currentTitle, setCurrentTitle] = useState(title ?? '');
@ -200,7 +197,7 @@ export const ControlEditor = ({
<EuiFormRow label={ControlGroupStrings.manageControl.getFieldTitle()}>
<FieldPicker
filterPredicate={(field: DataViewField) => {
const customPredicate = customFilterPredicate ? customFilterPredicate(field) : true;
const customPredicate = controlGroup.fieldFilterPredicate?.(field) ?? true;
return Boolean(fieldRegistry?.[field.name]) && customPredicate;
}}
selectedFieldName={selectedField}

View file

@ -26,6 +26,7 @@ import {
ControlPanelState,
ControlsPanels,
CONTROL_GROUP_TYPE,
FieldFilterPredicate,
} from '../types';
import {
cachedChildEmbeddableOrder,
@ -98,11 +99,14 @@ export class ControlGroupContainer extends Container<
public onFiltersPublished$: Subject<Filter[]>;
public onControlRemoved$: Subject<string>;
public fieldFilterPredicate: FieldFilterPredicate | undefined;
constructor(
reduxToolsPackage: ReduxToolsPackage,
initialInput: ControlGroupInput,
parent?: Container,
settings?: ControlGroupSettings
settings?: ControlGroupSettings,
fieldFilterPredicate?: FieldFilterPredicate
) {
super(
initialInput,
@ -141,6 +145,8 @@ export class ControlGroupContainer extends Container<
this.setupSubscriptions();
this.initialized$.next(true);
});
this.fieldFilterPredicate = fieldFilterPredicate;
}
private setupSubscriptions = () => {

View file

@ -19,7 +19,12 @@ import { Container, EmbeddableFactoryDefinition } from '@kbn/embeddable-plugin/p
import { lazyLoadReduxToolsPackage } from '@kbn/presentation-util-plugin/public';
import { EmbeddablePersistableStateService } from '@kbn/embeddable-plugin/common';
import { ControlGroupInput, ControlGroupSettings, CONTROL_GROUP_TYPE } from '../types';
import {
ControlGroupInput,
ControlGroupSettings,
CONTROL_GROUP_TYPE,
FieldFilterPredicate,
} from '../types';
import {
createControlGroupExtract,
createControlGroupInject,
@ -52,10 +57,17 @@ export class ControlGroupContainerFactory implements EmbeddableFactoryDefinition
public create = async (
initialInput: ControlGroupInput,
parent?: Container,
settings?: ControlGroupSettings
settings?: ControlGroupSettings,
fieldFilterPredicate?: FieldFilterPredicate
) => {
const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
const { ControlGroupContainer } = await import('./control_group_container');
return new ControlGroupContainer(reduxEmbeddablePackage, initialInput, parent, settings);
return new ControlGroupContainer(
reduxEmbeddablePackage,
initialInput,
parent,
settings,
fieldFilterPredicate
);
};
}

View file

@ -53,6 +53,7 @@ describe('control group renderer', () => {
expect(mockControlGroupFactory.create).toHaveBeenCalledWith(
expect.objectContaining({ controlStyle: 'twoLine' }),
undefined,
undefined,
undefined
);
});

View file

@ -77,7 +77,7 @@ export const ControlGroupRenderer = forwardRef<AwaitingControlGroupAPI, ControlG
> & {
create: ControlGroupContainerFactory['create'];
};
const { initialInput, settings } =
const { initialInput, settings, fieldFilterPredicate } =
(await getCreationOptions?.(getDefaultControlGroupInput(), controlGroupInputBuilder)) ??
{};
const newControlGroup = (await factory?.create(
@ -87,7 +87,8 @@ export const ControlGroupRenderer = forwardRef<AwaitingControlGroupAPI, ControlG
...initialInput,
},
undefined,
settings
settings,
fieldFilterPredicate
)) as ControlGroupContainer;
if (canceled) {

View file

@ -22,9 +22,12 @@ export type ControlGroupReduxState = ReduxEmbeddableState<
ControlGroupSettings
>;
export type FieldFilterPredicate = (f: DataViewField) => boolean;
export interface ControlGroupCreationOptions {
initialInput?: Partial<ControlGroupInput>;
settings?: ControlGroupSettings;
fieldFilterPredicate?: FieldFilterPredicate;
}
export interface ControlGroupSettings {
@ -35,7 +38,6 @@ export interface ControlGroupSettings {
hideWidthSettings?: boolean;
hideAdditionalSettings?: boolean;
};
fieldFilterPredicate?: (field: DataViewField) => boolean;
}
export {

View file

@ -11,10 +11,12 @@ import {
CONTROL_FRAMES,
CONTROL_FRAME_TITLE,
FILTER_GROUP_CHANGED_BANNER,
FILTER_GROUP_EDIT_CONTROL_PANEL_ITEMS,
FILTER_GROUP_SAVE_CHANGES_POPOVER,
OPTION_LIST_LABELS,
OPTION_LIST_VALUES,
OPTION_SELECTABLE,
FILTER_GROUP_CONTROL_ACTION_EDIT,
} from '../../screens/common/filter_group';
import { createRule } from '../../tasks/api_calls/rules';
import { cleanKibana } from '../../tasks/common';
@ -37,6 +39,7 @@ import { navigateFromHeaderTo } from '../../tasks/security_header';
import { ALERTS, CASES } from '../../screens/security_header';
import {
addNewFilterGroupControlValues,
cancelFieldEditing,
deleteFilterGroupControl,
discardFilterGroupControls,
editFilterGroupControl,
@ -280,6 +283,7 @@ describe('Detections : Page Filters', { testIsolation: false }, () => {
it('Custom filters from URLS are populated & changed banner is displayed', () => {
visitAlertsPageWithCustomFilters(customFilters);
waitForPageFilters();
assertFilterControlsWithFilterObject(customFilters);
@ -288,14 +292,14 @@ describe('Detections : Page Filters', { testIsolation: false }, () => {
it('Changed banner should hide on saving changes', () => {
visitAlertsPageWithCustomFilters(customFilters);
waitForPageFilters();
cy.get(FILTER_GROUP_CHANGED_BANNER).should('be.visible');
saveFilterGroupControls();
cy.get(FILTER_GROUP_CHANGED_BANNER).should('not.exist');
});
it('Changed banner should hide on discarding changes', () => {
visitAlertsPageWithCustomFilters(customFilters);
waitForPageFilters();
cy.get(FILTER_GROUP_CHANGED_BANNER).should('be.visible');
discardFilterGroupControls();
cy.get(FILTER_GROUP_CHANGED_BANNER).should('not.exist');
@ -303,7 +307,23 @@ describe('Detections : Page Filters', { testIsolation: false }, () => {
it('Changed banner should hide on Reset', () => {
visitAlertsPageWithCustomFilters(customFilters);
waitForPageFilters();
resetFilters();
cy.get(FILTER_GROUP_CHANGED_BANNER).should('not.exist');
});
it('Number fields are not visible in field edit panel', () => {
const idx = 3;
const { FILTER_FIELD_TYPE, FIELD_TYPES } = FILTER_GROUP_EDIT_CONTROL_PANEL_ITEMS;
editFilterGroupControls();
cy.get(CONTROL_FRAME_TITLE).eq(idx).trigger('mouseover');
cy.get(FILTER_GROUP_CONTROL_ACTION_EDIT(idx)).trigger('click', { force: true });
cy.get(FILTER_FIELD_TYPE).should('be.visible').trigger('click');
cy.get(FIELD_TYPES.STRING).should('be.visible');
cy.get(FIELD_TYPES.BOOLEAN).should('be.visible');
cy.get(FIELD_TYPES.IP).should('be.visible');
cy.get(FIELD_TYPES.NUMBER).should('not.exist');
cancelFieldEditing();
discardFilterGroupControls();
});
});

View file

@ -5,6 +5,8 @@
* 2.0.
*/
import { getDataTestSubjectSelector } from '../../helpers/common';
export const FILTER_GROUP_LOADING = '[data-test-subj="filter-group__loading"]';
export const FILTER_GROUP_ITEMS = '[data-test-subj="filter-group__items"]';
export const FILTER_GROUP_CLEAR = '[data-test-subj="filter-group__clear"]';
@ -67,6 +69,14 @@ export const FILTER_GROUP_EDIT_CONTROL_PANEL_ITEMS = {
FIELD_PICKER: (fieldName: string) => `[data-test-subj="field-picker-select-${fieldName}"]`,
FIELD_LABEL: '[data-test-subj="control-editor-title-input"]',
SAVE: '[data-test-subj="control-editor-save"]',
CANCEL: getDataTestSubjectSelector('control-editor-cancel'),
FILTER_FIELD_TYPE: getDataTestSubjectSelector('toggleFieldFilterButton'),
FIELD_TYPES: {
STRING: getDataTestSubjectSelector('typeFilter-string'),
BOOLEAN: getDataTestSubjectSelector('typeFilter-boolean'),
IP: getDataTestSubjectSelector('typeFilter-ip'),
NUMBER: getDataTestSubjectSelector('typeFilter-number'),
},
};
export const FILTER_GROUP_CONTROL_ACTION_DELETE = (idx: number) => {

View file

@ -49,6 +49,12 @@ export const editFilterGroupControls = () => {
cy.get(FILTER_GROUP_CONTEXT_EDIT_CONTROLS).trigger('click');
};
export const cancelFieldEditing = () => {
cy.get(FILTER_GROUP_EDIT_CONTROLS_PANEL).should('be.visible');
cy.get(FILTER_GROUP_EDIT_CONTROL_PANEL_ITEMS.CANCEL).should('be.visible').trigger('click');
cy.get(FILTER_GROUP_EDIT_CONTROLS_PANEL).should('not.exist');
};
export const saveFilterGroupControls = () => {
cy.get(FILTER_GROUP_SAVE_CHANGES).trigger('click');
cy.get(FILTER_GROUP_SAVE_CHANGES).should('not.exist');

View file

@ -27,7 +27,10 @@ import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import type { Subscription } from 'rxjs';
import styled from 'styled-components';
import { cloneDeep, debounce, isEqual } from 'lodash';
import type { ControlGroupCreationOptions } from '@kbn/controls-plugin/public/control_group/types';
import type {
ControlGroupCreationOptions,
FieldFilterPredicate,
} from '@kbn/controls-plugin/public/control_group/types';
import { useInitializeUrlParam } from '../../utils/global_query_string';
import { URL_PARAM_KEY } from '../../hooks/use_url_state';
import type { FilterGroupProps, FilterItemObj } from './types';
@ -298,6 +301,8 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
return resultControls;
}, [initialUrlParam, initialControls, getStoredControlInput]);
const fieldFilterPredicate: FieldFilterPredicate = useCallback((f) => f.type !== 'number', []);
const getCreationOptions: ControlGroupRendererProps['getCreationOptions'] = useCallback(
async (
defaultInput: Partial<ControlGroupInput>,
@ -334,7 +339,6 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
return {
initialInput,
settings: {
fieldFilterPredicate: (f) => f.type !== 'number',
showAddButton: false,
staticDataViewId: dataViewId ?? '',
editorConfig: {
@ -343,9 +347,18 @@ const FilterGroupComponent = (props: PropsWithChildren<FilterGroupProps>) => {
hideAdditionalSettings: true,
},
},
fieldFilterPredicate,
} as ControlGroupCreationOptions;
},
[dataViewId, timeRange, filters, chainingSystem, query, selectControlsWithPriority]
[
dataViewId,
timeRange,
filters,
chainingSystem,
query,
selectControlsWithPriority,
fieldFilterPredicate,
]
);
useFilterUpdatesToUrlSync({