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

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

nikitaindik
Copy link
Contributor

@nikitaindik nikitaindik commented Nov 15, 2024

Partially addresses: #171520

Summary

Changes in this PR:

  • history_window_start and new_terms_fields are now editable in the Rule Upgrade flyout
  • Extracted fields into separate components that are easier to reuse (NewTermsFieldsEdit and HistoryWindowStartEdit)
Scherm­afbeelding 2024-11-15 om 15 51 04

Testing

  • Ensure the prebuiltRulesCustomizationEnabled feature flag is enabled.
  • To simulate the availability of prebuilt rule upgrades, downgrade a currently installed prebuilt rule using the PATCH api/detection_engine/rules API.
    • Set version: 1 in the request body to downgrade it to version 1.
    • Modify other rule fields in the request body as needed to test the changes.

@nikitaindik nikitaindik added release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 15, 2024
@nikitaindik nikitaindik self-assigned this Nov 15, 2024
@nikitaindik nikitaindik marked this pull request as ready for review November 15, 2024 14:54
@nikitaindik nikitaindik requested review from a team as code owners November 15, 2024 14:54
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@nikitaindik nikitaindik requested review from maximpn and removed request for dplumlee November 15, 2024 14:55
@banderror banderror added v9.0.0 backport:version Backport to applied version labels v8.17.0 and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 15, 2024
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

Hi @nikitaindik,

I looked through the code and left comments according to my findings.

Testing revealed a problem that validation doesn't run in Prebuilt Rules Customization workflow

  • it's possible to save empty fields
    Screenshot 2024-11-18 at 09 58 38
Screen.Recording.2024-11-18.at.10.00.33.mov
  • it's possible to save zero history window size
Screen.Recording.2024-11-18.at.10.02.37.mov

while the validation works in the rule editing form
Screenshot 2024-11-18 at 10 01 50

Comment on lines 15 to 19
if (historyStart.startsWith('now-')) {
return historyStart.substring(4);
} else {
return historyStart;
}
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
if (historyStart.startsWith('now-')) {
return historyStart.substring(4);
} else {
return historyStart;
}
if (historyStart.startsWith('now-')) {
return historyStart.substring(4);
}
return historyStart;

import { ScheduleItemField } from '../schedule_item_field';
import { UseField } from '../../../../shared_imports';

const componentProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Constants should have capital case COMPONENT_PROPS.

Comment on lines 18 to 24
export function NewTermsFieldsEdit({ path, browserFields }: NewTermsFieldsEditProps): JSX.Element {
const componentProps = useMemo(() => ({ browserFields }), [browserFields]);

return (
<UseField path={path} component={NewTermsFieldsEditField} componentProps={componentProps} />
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You could memoize the whole component to make the code a bit shorter

Suggested change
export function NewTermsFieldsEdit({ path, browserFields }: NewTermsFieldsEditProps): JSX.Element {
const componentProps = useMemo(() => ({ browserFields }), [browserFields]);
return (
<UseField path={path} component={NewTermsFieldsEditField} componentProps={componentProps} />
);
}
export const NewTermsFieldsEdit = memo(function NewTermsFieldsEdit({ path, browserFields }: NewTermsFieldsEditProps): JSX.Element {
return (
<UseField path={path} component={NewTermsFieldsEditField} componentProps={{browserFields}} />
);
});

Wrapping a named function instead of an arrow function provides a display name to React.


interface NewTermsFieldsEditProps {
path: string;
browserFields: DataViewFieldBase[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know what browser in that context mean?

It looks simpler to stick to simple fieldNames: string[].

@@ -39,4 +39,4 @@ export const NewTermsFieldsComponent: React.FC<NewTermsFieldsProps> = ({
return <Field field={field} idAria={fieldDescribedByIds} euiFieldProps={fieldEuiFieldProps} />;
};

export const NewTermsFields = React.memo(NewTermsFieldsComponent);
export const NewTermsFieldsEditField = React.memo(NewTermsFieldsEditFieldComponent);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It looks like too much memoization here. When the top component NewTermsFieldsEdit is memoized it doesn't look like useMemo and memo are required here since the component will be anyway re-rendered when fields change.

finalDiffableRule,
}: NewTermsFieldsEditAdapterProps): JSX.Element {
const { dataView } = useDiffableRuleDataView(finalDiffableRule);
const termsAggregationFields = useTermsAggregationFields(dataView?.fields || []);
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
const termsAggregationFields = useTermsAggregationFields(dataView?.fields || []);
const termsAggregationFields = useTermsAggregationFields(dataView?.fields ?? []);

Comment on lines 30 to 40
function deserializer(defaultValue: FormData): NewTermsFieldsFormData {
return {
newTermsFields: defaultValue.new_terms_fields,
};
}

function serializer(formData: FormData): { new_terms_fields: NewTermsFields } {
return {
new_terms_fields: formData.newTermsFields,
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use new_terms_fields path in NewTermsFieldsEditAdapter and omit deserializer/serializer?

* @param historyStart - History start string to convert. For example, "now-30d".
* @returns Converted size string. For example, "30d".
*/
export const convertHistoryStartToSize = (historyStart: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

now-30d is called Date Math. The output is rather duration than size.

Let's rename this pair of functions to convertDateMathToDuration and convertDurationToDateMath.


export const NewTermsFieldsComponent: React.FC<NewTermsFieldsProps> = ({
export 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.

It's better to name it NewTermsFieldsField for coherence.

}

export const historyWindowFormSchema = {
historyWindowSize: schema.historyWindowSize,
Copy link
Contributor

Choose a reason for hiding this comment

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

From architectural POV it looks better to bring historyWindowSize and newTermsFields configurations like label, helpText and etc to the corresponding edit components. When edit components rendered conditionally only for new terms rule types validation doesn't required check for the rule type.

The only thing which needs to be addressed is user input persistence. But it can be easily done via a hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea! I was also considering a way to share schemas but couldn't figure out how. Your approach actually works. I've refactored the code accordingly.

@nikitaindik
Copy link
Contributor Author

Thanks for the review and testing, @maximpn! I've addressed your feedback. Please take a look.

@nikitaindik nikitaindik requested review from maximpn and xcrzx November 18, 2024 20:32
Copy link
Contributor

@nkhristinin nkhristinin left a comment

Choose a reason for hiding this comment

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

tested locally - LGTM

Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@nikitaindik thanks for addressing my comments 🙏

I found new issues and left comments about them.

{
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.


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 {
  // ...
});

@@ -555,106 +553,8 @@ export const schema: FormSchema<DefineStepRule> = {
},
],
},
newTermsFields: {
Copy link
Contributor

Choose a reason for hiding this comment

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

After removing labels from the schema labels should be added to x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/index.tsx in getDescriptionItem() as if (filed === 'newTermsFields') and if (field === 'historyWindowSize ') entries where labels could be imported from components. Without that readonly view looks like

Screenshot 2024-11-19 at 13 03 11

And on main
Screenshot 2024-11-19 at 13 06 29

Comment on lines 17 to 18
const field = form.getFields()[path] as FieldHook<string> | undefined;
const value = field?.value ?? '';
Copy link
Contributor

Choose a reason for hiding this comment

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

value should be extracted from args

const [{ value, path, form }] = args;

Comment on lines 20 to 48
const numberMatchResult = value.match(/\d+/g);

if (numberMatchResult === null) {
return {
code: 'ERR_NOT_INT_NUMBER',
path,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.historyWindowSize.errNumber',
{
defaultMessage: 'History window size must be a positive number.',
}
),
};
}

const numericValue = parseInt(numberMatchResult[0], 10);

if (numericValue <= 0) {
return {
code: 'ERR_MIN_LENGTH',
path,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.historyWindowSize.errMin',
{
defaultMessage: 'History window size must be greater than 0.',
}
),
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The validation logic could be simplified by using parseInt() only and checking for NaN

Suggested change
const numberMatchResult = value.match(/\d+/g);
if (numberMatchResult === null) {
return {
code: 'ERR_NOT_INT_NUMBER',
path,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.historyWindowSize.errNumber',
{
defaultMessage: 'History window size must be a positive number.',
}
),
};
}
const numericValue = parseInt(numberMatchResult[0], 10);
if (numericValue <= 0) {
return {
code: 'ERR_MIN_LENGTH',
path,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.historyWindowSize.errMin',
{
defaultMessage: 'History window size must be greater than 0.',
}
),
};
}
const historyWindowSize = parseInt(value, 10);
if (Number.isNaN(historyWindowSize)) {
return {
code: 'ERR_NOT_INT_NUMBER',
path,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.historyWindowSize.errNumber',
{
defaultMessage: 'History window size must be a positive number.',
}
),
};
}
if (historyWindowSize <= 0) {
return {
code: 'ERR_MIN_LENGTH',
path,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.historyWindowSize.errMin',
{
defaultMessage: 'History window size must be greater than 0.',
}
),
};
}

Comment on lines 12 to 14
export function historyWindowStartValidationFactory(
...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.

The function should be a factory. It means it constructs a functions and returns it. The implementation is a validator function. Correct implementation should look like

export function historyWindowStartValidationFactory(): ValidationFunc<FormData, ERROR_CODE, unknown> {
  return (...args) => {
    // Your implementation goes here
  };
}

Comment on lines 12 to 34
export function newTermsFieldsValidatorFactory(
...args: Parameters<ValidationFunc>
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined {
return (
fieldValidators.emptyField(
i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.newTermsFieldsMin',
{
defaultMessage: 'A minimum of one field is required.',
}
)
)(...args) ??
fieldValidators.maxLengthField({
length: MAX_NUMBER_OF_NEW_TERMS_FIELDS,
message: i18n.translate(
'xpack.securitySolution.detectionEngine.validations.stepDefineRule.newTermsFieldsMax',
{
defaultMessage: 'Number of fields must be 3 or less.',
}
),
})(...args)
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't compose validators like that. The best place to compose validators is NewTermsFieldsEdit component. new_terms_fields_edit.tsx should have the following

const NEW_TERMS_FIELDS_CONFIG = {
  ...
  validations: [
    {
      validator: fieldValidators.emptyField(
        i18n.translate(
          'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.newTermsFieldsMin',
          {
            defaultMessage: 'A minimum of one field is required.',
          }
        )
      ),
    },
    {
      validator: fieldValidators.maxLengthField({
        length: MAX_NUMBER_OF_NEW_TERMS_FIELDS,
        message: i18n.translate(
          'xpack.securitySolution.detectionEngine.validations.stepDefineRule.newTermsFieldsMax',
          {
            defaultMessage: 'Number of fields must be 3 or less.',
          }
        ),
      }),
    },
  ],
};

i18n messages should be extracted to translations.ts.


As a side comment. fieldValidators.maxLengthField is a validator factory. It's a high order function returning a validator function.

@@ -16,6 +16,10 @@ interface NewTermsFieldsReadOnlyProps {
}

export function NewTermsFieldsReadOnly({ newTermsFields }: NewTermsFieldsReadOnlyProps) {
if (!newTermsFields.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like newTermsFields.length should always be more than zero according to form validation. Why do we need this condition? Event if newTermsFields has zero length it should render properly.

}

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.

Comment on lines 50 to 57
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(),
},
],

@nikitaindik nikitaindik requested a review from maximpn November 19, 2024 17:04
@nikitaindik
Copy link
Contributor Author

Thanks for reviewing again, @maximpn! I've addressed the comments. Please take another look.

Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@nikitaindik I had a look a left a few comments. The PR works well but let's do the final polishing.

...args: Parameters<ValidationFunc>
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined =>
historyWindowStartValidationFactory(...args),
validator: (...args: Parameters<ValidationFunc>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Parameters<ValidationFunc> could be omitted.

Comment on lines 42 to 61
const [{ path, value }] = args;

const historyWindowSize = Number.parseInt(String(value), 10);

if (Number.isNaN(historyWindowSize)) {
return {
code: 'ERR_NOT_INT_NUMBER',
path,
message: i18n.MUST_BE_POSITIVE_INTEGER_VALIDATION_ERROR,
};
}

if (historyWindowSize <= 0) {
return {
code: 'ERR_MIN_LENGTH',
path,
message: i18n.MUST_BE_GREATER_THAN_ZERO_VALIDATION_ERROR,
};
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend to move the validation to a separate function in a separate file for better readability.

import { i18n } from '@kbn/i18n';
import type { ERROR_CODE, ValidationFunc } from '../../../../shared_imports';
import { FIELD_TYPES, UseField } from '../../../../shared_imports';
// import { i18n } from '@kbn/i18n';
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be removed?

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.

@nikitaindik nikitaindik requested a review from maximpn November 20, 2024 14:12
@nikitaindik
Copy link
Contributor Author

@maximpn New feedback addressed. Please take a new look.

@maximpn maximpn added v8.18.0 and removed v8.17.0 labels Nov 21, 2024
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@nikitaindik Thanks for improvements it LGTM 👍

@banderror banderror marked this pull request as draft December 4, 2024 16:14
@nikitaindik
Copy link
Contributor Author

Thanks, everyone, for your reviews and testing! @xcrzx still needs to review it. I've transitioned this to draft since the team is now focusing on high-priority Rule Customization tickets, and we want to avoid daily notifications from less important PRs like this one. I will reopen it for review once we finish the other work and notify @xcrzx to re-review.

@nikitaindik nikitaindik marked this pull request as ready for review December 16, 2024 09:10
@nikitaindik nikitaindik force-pushed the new-terms-editable-fields branch from 4221186 to 71c52f0 Compare December 17, 2024 11:46
Copy link
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

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

I manually tested the changes in this PR locally and verified the affected fields: history window, new terms, and schedule. I tested various scenarios involving modifications to these fields using both the rule editing form and the rule upgrade form. I also confirmed that the fields display correctly on the rule details page and tested rule upgrade flows with the feature flag disabled. I didn’t encounter any issues.

I’m skipping a review of the code changes since @maximpn has already reviewed them.

@nikitaindik nikitaindik enabled auto-merge (squash) December 18, 2024 15:20
@nikitaindik nikitaindik merged commit 639143a into elastic:main Dec 18, 2024
9 checks passed
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #76 / Visualizations - Group 3 lens app - TSVB Open in Lens Table should convert aggregate by to split row dimension

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6435 6449 +14

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 21.4MB 21.4MB +2.8KB

History

cc @nikitaindik

@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12397843961

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- [index management] Better privilege checking for component index templates (#202251)

Manual backport

To create the backport manually run:

node scripts/backport --pr 200304

Questions ?

Please refer to the Backport tool documentation

@nikitaindik
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

nikitaindik added a commit to nikitaindik/kibana that referenced this pull request Dec 18, 2024
… editable fields (elastic#200304)

**Partially addresses: elastic#171520

## Summary
**Changes in this PR**:
- `history_window_start` and `new_terms_fields` are now editable in the
Rule Upgrade flyout
- Extracted fields into separate components that are easier to reuse
(`NewTermsFieldsEdit` and `HistoryWindowStartEdit`)

<img width="1392" alt="Scherm­afbeelding 2024-11-15 om 15 51 04"
src="https://github.com/user-attachments/assets/d00b7b3d-7c01-4041-b940-390660a069a9">

### Testing
- Ensure the `prebuiltRulesCustomizationEnabled` feature flag is
enabled.
- To simulate the availability of prebuilt rule upgrades, downgrade a
currently installed prebuilt rule using the `PATCH
api/detection_engine/rules` API.
   - Set `version: 1` in the request body to downgrade it to version 1.
- Modify other rule fields in the request body as needed to test the
changes.
nikitaindik added a commit that referenced this pull request Dec 18, 2024
…ields` editable fields (#200304) (#204830)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Add `history_window_start` and `new_terms_fields`
editable fields
(#200304)](#200304)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Nikita
Indik","email":"nikita.indik@elastic.co"},"sourceCommit":{"committedDate":"2024-12-18T17:07:35Z","message":"[Security
Solution] Add `history_window_start` and `new_terms_fields` editable
fields (#200304)\n\n**Partially addresses:
https://github.com/elastic/kibana/issues/171520**\n\n##
Summary\n**Changes in this PR**:\n- `history_window_start` and
`new_terms_fields` are now editable in the\nRule Upgrade flyout\n-
Extracted fields into separate components that are easier to
reuse\n(`NewTermsFieldsEdit` and `HistoryWindowStartEdit`)\n\n<img
width=\"1392\" alt=\"Scherm­afbeelding 2024-11-15 om 15 51
04\"\nsrc=\"https://github.com/user-attachments/assets/d00b7b3d-7c01-4041-b940-390660a069a9\">\n\n###
Testing\n- Ensure the `prebuiltRulesCustomizationEnabled` feature flag
is\nenabled.\n- To simulate the availability of prebuilt rule upgrades,
downgrade a\ncurrently installed prebuilt rule using the
`PATCH\napi/detection_engine/rules` API.\n - Set `version: 1` in the
request body to downgrade it to version 1.\n- Modify other rule fields
in the request body as needed to test
the\nchanges.","sha":"639143ac59e9bb8bf2e629d30a4ffe363f974cce","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","backport:version","v8.18.0"],"number":200304,"url":"https://github.com/elastic/kibana/pull/200304","mergeCommit":{"message":"[Security
Solution] Add `history_window_start` and `new_terms_fields` editable
fields (#200304)\n\n**Partially addresses:
https://github.com/elastic/kibana/issues/171520**\n\n##
Summary\n**Changes in this PR**:\n- `history_window_start` and
`new_terms_fields` are now editable in the\nRule Upgrade flyout\n-
Extracted fields into separate components that are easier to
reuse\n(`NewTermsFieldsEdit` and `HistoryWindowStartEdit`)\n\n<img
width=\"1392\" alt=\"Scherm­afbeelding 2024-11-15 om 15 51
04\"\nsrc=\"https://github.com/user-attachments/assets/d00b7b3d-7c01-4041-b940-390660a069a9\">\n\n###
Testing\n- Ensure the `prebuiltRulesCustomizationEnabled` feature flag
is\nenabled.\n- To simulate the availability of prebuilt rule upgrades,
downgrade a\ncurrently installed prebuilt rule using the
`PATCH\napi/detection_engine/rules` API.\n - Set `version: 1` in the
request body to downgrade it to version 1.\n- Modify other rule fields
in the request body as needed to test
the\nchanges.","sha":"639143ac59e9bb8bf2e629d30a4ffe363f974cce"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200304","number":200304,"mergeCommit":{"message":"[Security
Solution] Add `history_window_start` and `new_terms_fields` editable
fields (#200304)\n\n**Partially addresses:
https://github.com/elastic/kibana/issues/171520**\n\n##
Summary\n**Changes in this PR**:\n- `history_window_start` and
`new_terms_fields` are now editable in the\nRule Upgrade flyout\n-
Extracted fields into separate components that are easier to
reuse\n(`NewTermsFieldsEdit` and `HistoryWindowStartEdit`)\n\n<img
width=\"1392\" alt=\"Scherm­afbeelding 2024-11-15 om 15 51
04\"\nsrc=\"https://github.com/user-attachments/assets/d00b7b3d-7c01-4041-b940-390660a069a9\">\n\n###
Testing\n- Ensure the `prebuiltRulesCustomizationEnabled` feature flag
is\nenabled.\n- To simulate the availability of prebuilt rule upgrades,
downgrade a\ncurrently installed prebuilt rule using the
`PATCH\napi/detection_engine/rules` API.\n - Set `version: 1` in the
request body to downgrade it to version 1.\n- Modify other rule fields
in the request body as needed to test
the\nchanges.","sha":"639143ac59e9bb8bf2e629d30a4ffe363f974cce"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
… editable fields (elastic#200304)

**Partially addresses: elastic#171520

## Summary
**Changes in this PR**:
- `history_window_start` and `new_terms_fields` are now editable in the
Rule Upgrade flyout
- Extracted fields into separate components that are easier to reuse
(`NewTermsFieldsEdit` and `HistoryWindowStartEdit`)

<img width="1392" alt="Scherm­afbeelding 2024-11-15 om 15 51 04"
src="https://github.com/user-attachments/assets/d00b7b3d-7c01-4041-b940-390660a069a9">

### Testing
- Ensure the `prebuiltRulesCustomizationEnabled` feature flag is
enabled.
- To simulate the availability of prebuilt rule upgrades, downgrade a
currently installed prebuilt rule using the `PATCH
api/detection_engine/rules` API.
   - Set `version: 1` in the request body to downgrade it to version 1.
- Modify other rule fields in the request body as needed to test the
changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants