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] FinalEdit: Add name and kql_query fields + shared components #193828

Merged
merged 50 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
12bdeae
integrate state and components
maximpn Sep 13, 2024
8ecf7ae
move tab related components to components folder
maximpn Sep 25, 2024
2294396
adopt SplitAccordion
maximpn Sep 25, 2024
204933f
reduce type casting to bare minimum
maximpn Sep 26, 2024
64ddaa7
improve component's naming
maximpn Sep 26, 2024
6b51235
improve function naming
maximpn Sep 26, 2024
1a0fa9a
remove unused code
maximpn Sep 26, 2024
c3bcaae
integrate state and components
maximpn Sep 13, 2024
f991566
Set up data passage to make field forms work
nikitaindik Sep 24, 2024
ee92867
Add `FieldFormWrapper` to encapsulate form setup
nikitaindik Sep 24, 2024
2684b22
Extract common fields into a separate component
nikitaindik Sep 24, 2024
a09e90c
Merge branch 'maximpn_integrate-prebuilt-rule-upgrade-state-and-compo…
nikitaindik Sep 27, 2024
ac4048e
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Sep 27, 2024
798592a
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Sep 30, 2024
d753724
Account for renames in Maxim's PR
nikitaindik Sep 30, 2024
076c7a0
Account for more renames
nikitaindik Sep 30, 2024
718c088
Pass `diffableRule` and field value setter via context
nikitaindik Oct 1, 2024
91e47fd
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Oct 2, 2024
0407cbe
Make `kql_query` work
nikitaindik Oct 4, 2024
a1b0000
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Oct 4, 2024
19e4101
Merge branch 'main' into 3wd-field-edit-base
maximpn Oct 6, 2024
54183c7
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Oct 10, 2024
9509411
Add a spacer to the callout and add plurals to callout copy
nikitaindik Oct 10, 2024
51637f4
Extract hidden fields into a separate variable
nikitaindik Oct 10, 2024
83c476d
Simplify setting resolved value
nikitaindik Oct 10, 2024
e45796e
Refactor deserializtion code
nikitaindik Oct 10, 2024
022ef98
Refactor `onSubmit` validation handling
nikitaindik Oct 10, 2024
f92d273
Refactor `getDefaultValue`
nikitaindik Oct 10, 2024
caa3064
Use `i18n` for button labels
nikitaindik Oct 10, 2024
e491ad7
Wrap form handlers in `useCallback`
nikitaindik Oct 10, 2024
bdd4937
Move calculation of `useRuleIndexPattern` parameters into a separate …
nikitaindik Oct 10, 2024
f9a6ba1
Remove braces
nikitaindik Oct 10, 2024
265c8c8
Extract `FinalSide` mode values into an enum
nikitaindik Oct 10, 2024
db49679
Refactor `FinalEditContextProvider` to accept children as `PropsWithC…
nikitaindik Oct 10, 2024
3d185d1
Fix Storybook types
nikitaindik Oct 11, 2024
233fb28
Make TS happy
nikitaindik Oct 11, 2024
2fdfabd
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Oct 11, 2024
e8a7b5e
Merge branch 'main' into 3wd-field-edit-base
nikitaindik Oct 12, 2024
02710b3
Update x-pack/plugins/security_solution/public/detection_engine/rule_…
nikitaindik Oct 12, 2024
9a4741b
Update code comment text
nikitaindik Oct 14, 2024
05563a5
Fix a typo in `isNonUpgradeableFieldName`
nikitaindik Oct 14, 2024
61326b8
Simplify `isTimelineSearchOpen`/`toggleIsTimelineSearchOpen` implemen…
nikitaindik Oct 14, 2024
d0c6f55
Move getting saved query ID into a function
nikitaindik Oct 14, 2024
5b3ebf9
Refactor `getUseRuleIndexPatternParameters`
nikitaindik Oct 14, 2024
2079647
Move field-related types and constants into `fields.ts`
nikitaindik Oct 14, 2024
9749c8c
Make a separate type for field components
nikitaindik Oct 14, 2024
048dfac
Add explicit field component return types
nikitaindik Oct 14, 2024
0a98280
Refactor passing data via local context
nikitaindik Oct 14, 2024
655d126
Remove duplicated `RuleUpgradeState` type
nikitaindik Oct 14, 2024
7b4265c
Merge remote-tracking branch 'upstream/main' into 3wd-field-edit-base
nikitaindik Oct 14, 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 @@ -24,7 +24,7 @@ import * as i18n from './translations';
interface ComparisonSideProps<FieldName extends keyof DiffableAllFields> {
fieldName: FieldName;
fieldThreeWayDiff: ThreeWayDiff<DiffableAllFields[FieldName]>;
resolvedValue?: DiffableAllFields[FieldName];
resolvedValue: DiffableAllFields[FieldName];
}

export function ComparisonSide<FieldName extends keyof DiffableAllFields>({
Expand All @@ -39,6 +39,7 @@ export function ComparisonSide<FieldName extends keyof DiffableAllFields>({
const [oldVersionType, newVersionType] = selectedVersions.split('_') as [Version, Version];

const oldFieldValue = pickFieldValueForVersion(oldVersionType, fieldThreeWayDiff, resolvedValue);

const newFieldValue = pickFieldValueForVersion(newVersionType, fieldThreeWayDiff, resolvedValue);

const subfieldChanges = getSubfieldChanges(fieldName, oldFieldValue, newFieldValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ import type {
*
* @param version - The version for which the field value is to be picked.
* @param fieldThreeWayDiff - The three-way diff object containing the field values for different versions.
* @param resolvedValue - The user-set resolved value resolved value. Used if it is set and the version is final.
* @param resolvedValue - Field value that the upgraded rule will have.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
* @param resolvedValue - Field value that the upgraded rule will have.
* @param resolvedValue - User accepted field value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this one. resolvedValue might not be user accepted. Even before user modifications we still have resolvedValue which is equal to our merge proposal or current value. Any ideas how to phrase it better?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe User accepted or system suggested field value or A value field will be upgraded to?

* @returns - The field value for the specified version
*/
export function pickFieldValueForVersion<FieldName extends keyof DiffableAllFields>(
version: Version,
fieldThreeWayDiff: ThreeWayDiff<DiffableAllFields[FieldName]>,
resolvedValue?: DiffableAllFields[FieldName]
resolvedValue: DiffableAllFields[FieldName]
): DiffableAllFields[FieldName] | undefined {
if (version === Version.Final) {
return resolvedValue;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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 type { DiffableAllFields } from '../../../../../../../common/api/detection_engine';

type NonEditableFields = Readonly<Set<keyof DiffableAllFields>>;

/* These fields are not visible in the comparison UI and are not editable */
export const HIDDEN_FIELDS: NonEditableFields = new Set([
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
export const HIDDEN_FIELDS: NonEditableFields = new Set([
export const NON_EDITABLE_UPON_UPGRADE_DIFFABLE_FIELDS: NonEditableFields = new Set([

'alert_suppression',
'author',
'rule_id',
'license',
'version',
]);
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { css } from '@emotion/css';
import { SplitAccordion } from '../../../../../../common/components/split_accordion';
import type {
DiffableAllFields,
DiffableRule,
RuleFieldsDiff,
ThreeWayDiff,
} from '../../../../../../../common/api/detection_engine';
Expand All @@ -20,23 +19,25 @@ import type { FieldUpgradeState } from '../../../../model/prebuilt_rule_upgrade'
import { ComparisonSide } from '../comparison_side/comparison_side';
import { FinalSide } from '../final_side/final_side';
import { FieldUpgradeConflictsResolverHeader } from './field_upgrade_conflicts_resolver_header';
import { useDiffableRuleContext } from '../diffable_rule_context';
import type { UpgradeableDiffableFields } from '../../../../model/prebuilt_rule_upgrade/types';

interface FieldUpgradeConflictsResolverProps<FieldName extends keyof RuleFieldsDiff> {
interface FieldUpgradeConflictsResolverProps<FieldName extends UpgradeableDiffableFields> {
fieldName: FieldName;
fieldUpgradeState: FieldUpgradeState;
fieldThreeWayDiff: RuleFieldsDiff[FieldName];
finalDiffableRule: DiffableRule;
}

export function FieldUpgradeConflictsResolver<FieldName extends keyof RuleFieldsDiff>({
export function FieldUpgradeConflictsResolver<FieldName extends UpgradeableDiffableFields>({
fieldName,
fieldUpgradeState,
fieldThreeWayDiff,
finalDiffableRule,
}: FieldUpgradeConflictsResolverProps<FieldName>): JSX.Element {
const { euiTheme } = useEuiTheme();
const hasConflict = fieldThreeWayDiff.conflict !== ThreeWayDiffConflict.NONE;

const { finalDiffableRule } = useDiffableRuleContext();

return (
<>
<SplitAccordion
Expand Down Expand Up @@ -65,7 +66,7 @@ export function FieldUpgradeConflictsResolver<FieldName extends keyof RuleFields
`}
/>
<EuiFlexItem grow={1}>
<FinalSide fieldName={fieldName} finalDiffableRule={finalDiffableRule} />
<FinalSide fieldName={fieldName} />
</EuiFlexItem>
</EuiFlexGroup>
</SplitAccordion>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ export function RuleUpgradeCallout({ ruleUpgradeState }: RuleUpgradeCalloutProps
}

return (
<EuiCallOut title={i18n.RULE_IS_READY_FOR_UPGRADE} iconType="warning" color="success" size="s">
<EuiCallOut
title={i18n.RULE_IS_READY_FOR_UPGRADE}
iconType="checkInCircleFilled"
color="success"
size="s"
>
<p>{i18n.RULE_IS_READY_FOR_UPGRADE_DESCRIPTION}</p>
</EuiCallOut>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const RULE_HAS_NON_SOLVABLE_CONFLICTS = (count: number) =>
{
values: { count },
defaultMessage:
'{count} of the fields has a unsolved conflict. Please review and modify accordingly.',
'{count} of the fields {count, plural, one {has} other {have}} an unsolved conflict. Please review and modify accordingly.',
}
);

Expand All @@ -31,7 +31,7 @@ export const RULE_HAS_SOLVABLE_CONFLICTS = (count: number) =>
{
values: { count },
defaultMessage:
'{count} of the fields has an update conflict, please review the suggested update being updating.',
'{count} of the fields {count, plural, one {has} other {have}} an update conflict, please review the suggested update being updating.',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,39 @@
*/

import React from 'react';
import type {
RuleUpgradeState,
SetRuleFieldResolvedValueFn,
} from '../../../../model/prebuilt_rule_upgrade';
import type { RuleUpgradeState } from '../../../../model/prebuilt_rule_upgrade';
import { FieldUpgradeConflictsResolver } from './field_upgrade_conflicts_resolver';
import type { NonUpgradeableDiffableFields } from '../../../../model/prebuilt_rule_upgrade/types';
import { inNonUpgradeableFieldName } from '../../../../model/prebuilt_rule_upgrade/constants';

type FieldDiffEntries<FieldsDiff, ExcludedFields extends keyof FieldsDiff = never> = Array<
[
Exclude<keyof FieldsDiff, ExcludedFields>,
Required<FieldsDiff>[Exclude<keyof FieldsDiff, ExcludedFields>]
]
>;

interface RuleUpgradeConflictsResolverProps {
ruleUpgradeState: RuleUpgradeState;
setRuleFieldResolvedValue: SetRuleFieldResolvedValueFn;
}

export function RuleUpgradeConflictsResolver({
ruleUpgradeState,
setRuleFieldResolvedValue,
}: RuleUpgradeConflictsResolverProps): JSX.Element {
const fieldDiffEntries = Object.entries(ruleUpgradeState.diff.fields) as Array<
[
keyof typeof ruleUpgradeState.diff.fields,
Required<typeof ruleUpgradeState.diff.fields>[keyof typeof ruleUpgradeState.diff.fields]
]
const fieldDiffEntries = Object.entries(ruleUpgradeState.diff.fields) as FieldDiffEntries<
typeof ruleUpgradeState.diff.fields
>;
const fields = fieldDiffEntries.map(([fieldName, fieldDiff]) => (

const fields = fieldDiffEntries.filter(([fieldName]) => {
return inNonUpgradeableFieldName(fieldName) === false;
}) as FieldDiffEntries<typeof ruleUpgradeState.diff.fields, NonUpgradeableDiffableFields>;

fields.map(([fieldName, fieldDiff]) => (
<FieldUpgradeConflictsResolver
key={fieldName}
fieldName={fieldName}
fieldUpgradeState={ruleUpgradeState.fieldsUpgradeState[fieldName]}
fieldThreeWayDiff={fieldDiff}
finalDiffableRule={ruleUpgradeState.finalRule}
/>
));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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, { createContext, useContext } from 'react';
import type { DiffableRule } from '../../../../../../common/api/detection_engine';
import { invariant } from '../../../../../../common/utils/invariant';
import type { SetRuleFieldResolvedValueFn } from '../../../model/prebuilt_rule_upgrade/set_rule_field_resolved_value';

interface DiffableRuleContextType {
finalDiffableRule: DiffableRule;
setRuleFieldResolvedValue: SetRuleFieldResolvedValueFn;
}

const DiffableRuleContext = createContext<DiffableRuleContextType | null>(null);

interface DiffableRuleContextProviderProps {
finalDiffableRule: DiffableRule;
setRuleFieldResolvedValue: SetRuleFieldResolvedValueFn;
children: React.ReactNode;
}

export function DiffableRuleContextProvider({
finalDiffableRule,
setRuleFieldResolvedValue,
children,
}: DiffableRuleContextProviderProps) {
const contextValue = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Context value changes every render. Have you checked if this causes underlying components re-rendering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just checked. Once you hit "Save" after editing a field value the state in UpgradePrebuiltRulesTableContextProvider gets updated and it causes a lot of components to re-render, including UpgradePrebuiltRulesTable and the whole flyout.

Scherm­afbeelding 2024-10-14 om 15 15 44

We'll def want to optimize this at some point. Added a task to our todo list in the ticket.

finalDiffableRule,
setRuleFieldResolvedValue,
};

return (
<DiffableRuleContext.Provider value={contextValue}>{children}</DiffableRuleContext.Provider>
);
}

export function useDiffableRuleContext() {
const context = useContext(DiffableRuleContext);

invariant(
context !== null,
'useDiffableRuleContext must be used inside a DiffableRuleContextProvider'
);

return context;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* 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 { FieldFormWrapper } from './field_form_wrapper';
import { NameEdit, nameSchema } from './fields/name';
import type { UpgradeableCommonFields } from '../../../../model/prebuilt_rule_upgrade/types';
interface CommonRuleFieldEditProps {
fieldName: UpgradeableCommonFields;
}

export function CommonRuleFieldEdit({ fieldName }: CommonRuleFieldEditProps) {
switch (fieldName) {
case 'name':
return <FieldFormWrapper component={NameEdit} fieldFormSchema={nameSchema} />;
default:
return null; // Will be replaced with `assertUnreachable(fieldName)` once all fields are implemented
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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 { FieldFormWrapper } from './field_form_wrapper';
import {
KqlQueryEdit,
kqlQuerySchema,
kqlQuerySerializer,
kqlQueryDeserializer,
} from './fields/kql_query';
import type { UpgradeableCustomQueryFields } from '../../../../model/prebuilt_rule_upgrade/types';

interface CustomQueryRuleFieldEditProps {
fieldName: UpgradeableCustomQueryFields;
}

export function CustomQueryRuleFieldEdit({ fieldName }: CustomQueryRuleFieldEditProps) {
switch (fieldName) {
case 'kql_query':
return (
<FieldFormWrapper
component={KqlQueryEdit}
fieldFormSchema={kqlQuerySchema}
serializer={kqlQuerySerializer}
deserializer={kqlQueryDeserializer}
/>
);
default:
return null; // Will be replaced with `assertUnreachable(fieldName)` once all fields are implemented
}
}
Loading