[Security Solution] Prebuilt rule customizations flyout refactor

[dplumlee]

Fixes: #227805
Follow-up to: #225939

Summary

Addresses comments from previous PR and refactors code to a more readable structure and adds labels to the "Investigation guide" and "Setup guide" sections of the Rule Details page for better displays to the user.

Screenshots

Rule details panel

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Unit or functional tests were updated or added to match the most common scenarios
• Flaky Test Runner was used on any tests changed

[elasticmachine]

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

[elasticmachine]

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

[rylnd]

DE changes LGTM.

[rylnd]

Should these live within this file, now, or do you intend to reuse them?

[dplumlee]

I think they are fine to live here for now, we don't display either of these fields this way anywhere else - they usually get split up into their own section or tab. I broke them out into the smaller fragment stubs for overall readability in the main component

[maximpn]

While these two components aren't explicitly reused they are actually reused, please check setup and note components from the Prebuilt Rule Upgrade Flyout.

Ideally we should reuse Investigation Guide and Setup Guide components. You may see some readonly components are defined in rule_definition_section.tsx. note component in the Prebuilt Rule Upgrade Flyout should be renamed to investigation_guide.

nit: IMHO functions have higher readability

interface MyComponentProps {
  propA: string;
}

function MyComponent(props: MyComponentProps): JSX.Element {
  // implementation...
}

vs

interface MyComponentProps {
  propA: string;
}

const MyComponent = (props: MyComponentProps) => {
  // implementation...
};

[maximpn]

@dplumlee Thanks for addressing my previous comments and making our UI code better 🙏

I pushed a little change to get rid of setCustomizationsRule in RuleCustomizationsContextProvider and replace it with a simpler implementation.

On top of that I left a couple non critical comments regarding further improvements. These two don't block the PR but it'd be nice if there is time to address them.

While testing I noticed that we have two tooltips appearing after hovering one a badge for some time. One from the browser and another one you've added. Maybe we could disable the first one?

[maximpn]

While these two components aren't explicitly reused they are actually reused, please check setup and note components from the Prebuilt Rule Upgrade Flyout.

Ideally we should reuse Investigation Guide and Setup Guide components. You may see some readonly components are defined in rule_definition_section.tsx. note component in the Prebuilt Rule Upgrade Flyout should be renamed to investigation_guide.

nit: IMHO functions have higher readability

interface MyComponentProps {
  propA: string;
}

function MyComponent(props: MyComponentProps): JSX.Element {
  // implementation...
}

vs

interface MyComponentProps {
  propA: string;
}

const MyComponent = (props: MyComponentProps) => {
  // implementation...
};

[maximpn]

Can we have one utility function to convert field names to huma/display names? There is already getHumanizedFieldName which maps field names to human field names.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: ccc74fd
• Cloud Deployment
• Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-227355-ccc74fdedddd
• Security Deployment

Metrics [docs]

Async chunks

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

id	before	after	diff
securitySolution	9.8MB	9.8MB	-769.0B

History

• 💚 Build #320169 succeeded 8087a48
• 💚 Build #320115 succeeded 775c2be
• 💚 Build #319481 succeeded 94dccaa
• 💚 Build #319429 succeeded cb74710

cc @dplumlee

[kibanamachine]

Starting backport for target branches: 8.19, 9.1

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

[marshallmain]

@maximpn This commit is not a small one imo, so by adding it to this PR and approving the PR as well you've effectively approved your own commits to the repo and bypassed the approval process. In general I think changes like this should be made by the PR author if they're available so the author retains ownership and responsibility for the code being introduced.

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts You might need to backport the following PRs to 8.19: - [Investigations][DataViews] - Remove non-performant toSpec usage (#225726)
❌	9.1	Backport failed because of merge conflicts You might need to backport the following PRs to 9.1: - [Investigations][DataViews] - Remove non-performant toSpec usage (#225726)

Manual backport

To create the backport manually run:

node scripts/backport --pr 227355

Questions ?

Please refer to the Backport tool documentation

[dplumlee]

💔 Some backports could not be created

Status	Branch	Result
✅	9.1
❌	8.19	Conflict resolution was aborted by the user

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 227355

Questions ?

Please refer to the Backport tool documentation

[dplumlee]

💔 Some backports could not be created

Status	Branch	Result
✅	9.1
❌	8.19	Conflict resolution was aborted by the user

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 227355

Questions ?

Please refer to the Backport tool documentation

[dplumlee]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

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

Questions ?

Please refer to the Backport tool documentation

[maximpn]

@marshallmain,

Yes, sorry about the mess. Looking at the change from this point it’s not necessary and I could just leave a comment.

My commit contains only two functional changes

• RuleCustomizationsContextProvider was moved inside Rule Details Page component
• rule param was added to RuleCustomizationsContextProvider to make the context initialization much simpler

However, since I moved connect and wrapped Rule Detail Page component to connect the diff looks huge.