Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Add history_window_start and new_terms_fields editable fields #200304

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d79f204
Add `new_terms_fields`
nikitaindik Nov 14, 2024
8f9cc18
Add `history_window_start` and refactor `new_terms_fields`
nikitaindik Nov 15, 2024
1170e92
Add `assertUnreachable` for type safety
nikitaindik Nov 15, 2024
a67e1b0
Removed commented out code
nikitaindik Nov 15, 2024
7a786be
Merge remote-tracking branch 'upstream/main' into new-terms-editable-…
nikitaindik Nov 18, 2024
a9591c7
Fix validation for `new_terms_fields`
nikitaindik Nov 18, 2024
2bfd937
Fix validation for `history_window_start`
nikitaindik Nov 18, 2024
65d5d9a
Refactor to address code review feedback
nikitaindik Nov 18, 2024
e7942e2
Move validation into components to enhance reusability
nikitaindik Nov 18, 2024
4284e9a
Address more code review feedback
nikitaindik Nov 19, 2024
ead30e0
Merge remote-tracking branch 'upstream/main' into new-terms-editable-…
nikitaindik Nov 19, 2024
28081a2
Address new code review feedback
nikitaindik Nov 20, 2024
762530b
Merge remote-tracking branch 'upstream/main' into new-terms-editable-…
nikitaindik Nov 20, 2024
3e3fe80
Fix typo
nikitaindik Nov 26, 2024
399285c
Merge remote-tracking branch 'upstream/main' into new-terms-editable-…
nikitaindik Nov 26, 2024
09e68ef
Merge remote-tracking branch 'upstream/main' into new-terms-editable-…
nikitaindik Nov 29, 2024
c312627
Merge branch 'main' into new-terms-editable-fields
nikitaindik Dec 5, 2024
d8ed050
Merge branch 'main' into new-terms-editable-fields
nikitaindik Dec 6, 2024
3ce17c7
Merge branch 'main' into new-terms-editable-fields
nikitaindik Dec 9, 2024
b88f566
Merge branch 'main' into new-terms-editable-fields
nikitaindik Dec 10, 2024
2668271
Merge remote-tracking branch 'upstream/main' into new-terms-editable-…
nikitaindik Dec 11, 2024
adc3091
Merge branch 'main' into new-terms-editable-fields
nikitaindik Dec 14, 2024
7a3f474
Merge branch 'main' into new-terms-editable-fields
nikitaindik Dec 16, 2024
4221186
Update imports to reflect latest directory structure changes
nikitaindik Dec 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@
* 2.0.
*/

import { useMemo } from 'react';
import type { DataViewFieldBase } from '@kbn/es-query';
import type { FieldSpec } from '@kbn/data-plugin/common';

import { getTermsAggregationFields } from '../../../../detection_engine/rule_creation_ui/components/step_define_rule/utils';
import { useRuleFields } from '../../../../detection_engine/rule_management/logic/use_rule_fields';
import { useMlCapabilities } from './use_ml_capabilities';
import { useMlRuleValidations } from './use_ml_rule_validations';
import { hasMlAdminPermissions } from '../../../../../common/machine_learning/has_ml_admin_permissions';
import { hasMlLicense } from '../../../../../common/machine_learning/has_ml_license';
import { useTermsAggregationFields } from '../../../hooks/use_terms_aggregation_fields';

export interface UseMlRuleConfigReturn {
hasMlAdminPermissions: boolean;
Expand Down Expand Up @@ -45,10 +44,7 @@ export const useMLRuleConfig = ({
const { loading: mlFieldsLoading, fields: mlFields } = useRuleFields({
machineLearningJobId,
});
const mlSuppressionFields = useMemo(
() => getTermsAggregationFields(mlFields as FieldSpec[]),
[mlFields]
);
const mlSuppressionFields = useTermsAggregationFields(mlFields);

return {
hasMlAdminPermissions: hasMlAdminPermissions(mlCapabilities),
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/public/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ export const RISK_SCORE_HIGH = 73;
export const RISK_SCORE_CRITICAL = 99;

export const ONBOARDING_VIDEO_SOURCE = '//play.vidyard.com/K6kKDBbP9SpXife9s2tHNP.html?';

export const DEFAULT_HISTORY_WINDOW_SIZE = '7d';
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { useMemo } from 'react';
import type { FieldSpec } from '@kbn/data-views-plugin/common';
import type { DataViewFieldBase } from '@kbn/es-query';
import { getTermsAggregationFields } from '../../detection_engine/rule_creation_ui/components/step_define_rule/utils';

export function useTermsAggregationFields(fields?: DataViewFieldBase[]) {
const termsAggregationFields = useMemo(
/**
* Typecasting to FieldSpec because fields is
* typed as DataViewFieldBase[] which does not have
* the 'aggregatable' property, however the type is incorrect
*
* fields does contain elements with the aggregatable property.
* We will need to determine where these types are defined and
* figure out where the discrepancy is.
*/
() => getTermsAggregationFields((fields as FieldSpec[]) ?? []),
[fields]
);

return termsAggregationFields;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

/**
* Converts a date math string to a duration string by removing the 'now-' prefix.
*
* @param historyStart - Date math string to convert. For example, "now-30d".
* @returns Resulting duration string. For example, "30d".
*/
export const convertDateMathToDuration = (dateMathString: string): string => {
if (dateMathString.startsWith('now-')) {
return dateMathString.substring(4);
}

return dateMathString;
};

/**
* Converts a duration string to a dateMath string by adding the 'now-' prefix.
*
* @param durationString - Duration string to convert. For example, "30d".
* @returns Resulting date math string. For example, "now-30d".
*/
export const convertDurationToDateMath = (durationString: string): string =>
`now-${durationString}`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { i18n } from '@kbn/i18n';
import { ScheduleItemField } from '../schedule_item_field';
import type { ERROR_CODE, ValidationFunc } from '../../../../shared_imports';
import { type FieldConfig, UseField } from '../../../../shared_imports';
import { type HistoryWindowStart } from '../../../../../common/api/detection_engine';
import { historyWindowStartValidationFactory } from '../../../rule_creation_ui/validators/history_window_start_validator_factory';

const COMPONENT_PROPS = {
idAria: 'historyWindowSize',
dataTestSubj: 'historyWindowSize',
timeTypes: ['m', 'h', 'd'],
};

interface HistoryWindowStartEditProps {
path: string;
}

export function HistoryWindowStartEdit({ path }: HistoryWindowStartEditProps): JSX.Element {
return (
<UseField
path={path}
config={HISTORY_WINDOW_START_FIELD_CONFIG}
component={ScheduleItemField}
componentProps={COMPONENT_PROPS}
/>
);
}

const HISTORY_WINDOW_START_FIELD_CONFIG: FieldConfig<HistoryWindowStart> = {
label: i18n.translate(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Translations could be moved to translations.ts.

'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.historyWindowSizeLabel',
{
defaultMessage: 'History Window Size',
}
),
helpText: i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepScheduleRule.historyWindowSizeHelpText',
{
defaultMessage: "New terms rules only alert if terms don't appear in historical data.",
}
),
validations: [
{
validator: (
...args: Parameters<ValidationFunc>
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined =>
historyWindowStartValidationFactory(...args),
},
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
validations: [
{
validator: (
...args: Parameters<ValidationFunc>
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined =>
historyWindowStartValidationFactory(...args),
},
],
validations: [
{
validator: historyWindowStartValidationFactory(),
},
],

};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export { HistoryWindowStartEdit } from './history_window_start_edit';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export { NewTermsFieldsEdit } from './new_terms_fields_edit';
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { memo } from 'react';
import { i18n } from '@kbn/i18n';
import type { ERROR_CODE, ValidationFunc } from '../../../../shared_imports';
import { FIELD_TYPES, UseField } from '../../../../shared_imports';
import { NewTermsFieldsField } from './new_terms_fields_field';
import { newTermsFieldsValidatorFactory } from '../../../rule_creation_ui/validators/new_terms_fields_validator_factory';

interface NewTermsFieldsEditProps {
path: string;
fieldNames: string[];
}

export const NewTermsFieldsEdit = memo(function NewTermsFieldsEdit({
path,
fieldNames,
}: NewTermsFieldsEditProps): JSX.Element {
return (
<UseField
path={path}
config={NEW_TERMS_FIELDS_CONFIG}
component={NewTermsFieldsField}
componentProps={{ fieldNames }}
/>
);
});

const NEW_TERMS_FIELDS_CONFIG = {
type: FIELD_TYPES.COMBO_BOX,
label: i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.newTermsFieldsLabel',
{
defaultMessage: 'Fields',
}
),
helpText: i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldNewTermsFieldHelpText',
{
defaultMessage: 'Select a field to check for new terms.',
}
),
validations: [
{
validator: (
...args: Parameters<ValidationFunc>
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Return type could be omitted.

return newTermsFieldsValidatorFactory(...args);
},
},
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,36 @@
* 2.0.
*/

import React, { useMemo } from 'react';
import React from 'react';

import type { DataViewFieldBase } from '@kbn/es-query';
import type { FieldHook } from '../../../../shared_imports';
import { Field } from '../../../../shared_imports';
import { NEW_TERMS_FIELD_PLACEHOLDER } from './translations';

interface NewTermsFieldsProps {
browserFields: DataViewFieldBase[];
fieldNames: DataViewFieldBase[];
field: FieldHook;
}

const FIELD_COMBO_BOX_WIDTH = 410;

const fieldDescribedByIds = 'detectionEngineStepDefineRuleNewTermsField';
const fieldDescribedByIds = 'newTermsFieldEdit';

export const NewTermsFieldsComponent: React.FC<NewTermsFieldsProps> = ({
browserFields,
const NewTermsFieldsEditFieldComponent: React.FC<NewTermsFieldsProps> = ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Component suffix looks excessive here. Could we name it NewTermsFieldsField or NewTermsFieldsFormField?

The function could be defined simpler

const NewTermsFieldsField = memo(function NewTermsFieldsField(): JSX.Element {
  // ...
});

fieldNames,
field,
}: NewTermsFieldsProps) => {
const fieldEuiFieldProps = useMemo(
() => ({
fullWidth: true,
noSuggestions: false,
options: browserFields.map((browserField) => ({ label: browserField.name })),
placeholder: NEW_TERMS_FIELD_PLACEHOLDER,
onCreateOption: undefined,
style: { width: `${FIELD_COMBO_BOX_WIDTH}px` },
}),
[browserFields]
);
const fieldEuiFieldProps = {
fullWidth: true,
noSuggestions: false,
options: fieldNames.map((name) => ({ label: name })),
placeholder: NEW_TERMS_FIELD_PLACEHOLDER,
onCreateOption: undefined,
style: { width: `${FIELD_COMBO_BOX_WIDTH}px` },
};

return <Field field={field} idAria={fieldDescribedByIds} euiFieldProps={fieldEuiFieldProps} />;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could use ComboBoxField directly. It doesn't look like changing a field type makes sense here.

};

export const NewTermsFields = React.memo(NewTermsFieldsComponent);
export const NewTermsFieldsField = React.memo(NewTermsFieldsEditFieldComponent);
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export { ScheduleItemField } from './schedule_item_field';
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
import React from 'react';
import { mount, shallow } from 'enzyme';

import { ScheduleItem } from '.';
import { ScheduleItemField } from './schedule_item_field';
import { TestProviders, useFormFieldMock } from '../../../../common/mock';

describe('ScheduleItem', () => {
describe('ScheduleItemField', () => {
it('renders correctly', () => {
const mockField = useFormFieldMock<string>();
const wrapper = shallow(
<ScheduleItem
<ScheduleItemField
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
Expand All @@ -30,7 +30,7 @@ describe('ScheduleItem', () => {
const mockField = useFormFieldMock<string>();
const wrapper = mount(
<TestProviders>
<ScheduleItem
<ScheduleItemField
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
Expand All @@ -53,7 +53,7 @@ describe('ScheduleItem', () => {
const mockField = useFormFieldMock<string>();
const wrapper = mount(
<TestProviders>
<ScheduleItem
<ScheduleItemField
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
Expand All @@ -77,7 +77,7 @@ describe('ScheduleItem', () => {
const mockField = useFormFieldMock<string>();
const wrapper = mount(
<TestProviders>
<ScheduleItem
<ScheduleItemField
dataTestSubj="schedule-item"
idAria="idAria"
isDisabled={false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const getNumberFromUserInput = (input: string, minimumValue = 0): number => {
}
};

export const ScheduleItem = ({
export const ScheduleItemField = ({
dataTestSubj,
field,
idAria,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const getIsRulePreviewDisabled = ({
threatMapping,
machineLearningJobId,
queryBar,
newTermsFields,
newTermsFields = [],
}: {
ruleType: Type;
isQueryBarValid: boolean;
Expand Down
Loading