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] Display Modified badge for customized fields in Rule Upgrade flyout #203968

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

maximpn
Copy link
Contributor

@maximpn maximpn commented Dec 12, 2024

Resolves: #203718

Summary

This PR adds Modified badge to customized fields in rules upgrade flyout.

Details

_review API endpoint contains fields diff providing enough information on what field values were involved in the comparison. In particular diff_outcome is used to determine if a field was customized i.e. the rule was edited and field value has a different value than a stock value. Modified badge is show for fields CustomizedValueCanUpdate, CustomizedValueSameUpdate and CustomizedValueNoUpdate.

Screenshot

image

@maximpn maximpn added release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. v9.0.0 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:version Backport to applied version labels v8.18.0 labels Dec 12, 2024
@maximpn maximpn self-assigned this Dec 12, 2024
@maximpn maximpn requested a review from nikitaindik December 12, 2024 19:43
@maximpn maximpn marked this pull request as ready for review December 12, 2024 19:44
@maximpn maximpn requested a review from a team as a code owner December 12, 2024 19:44
@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)

Copy link
Contributor

@nikitaindik nikitaindik left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @maximpn. I've reviewed the code and tested it locally.

I have a couple of concerns that are related to the product side of the feature.

  1. The term "Modified" is too generic; it can mean both "Current differs from Base" and "you've made changes in the Final Update section." But right now I can't think of a better term to avoid this confusion.
  2. IMO, we should not use the term "stock value" since it might be unclear what it refers to. Maybe "original value", although it's also not perfect. This seems like a decision for the Kseniia / docs folks regarding precise wording.
Scherm­afbeelding 2024-12-13 om 10 26 33

@maximpn maximpn force-pushed the add-field-modified-badge branch from b087f2a to 3acaec6 Compare December 13, 2024 10:08
@maximpn maximpn enabled auto-merge (squash) December 13, 2024 10:08
@maximpn maximpn force-pushed the add-field-modified-badge branch from 3acaec6 to 1b6cf09 Compare December 13, 2024 10:15
@maximpn
Copy link
Contributor Author

maximpn commented Dec 13, 2024

@nikitaindik thanks for your review 🙏

The term "Modified" is too generic; it can mean both "Current differs from Base" and "you've made changes in the Final Update section." But right now I can't think of a better term to avoid this confusion.

IMO, we should not use the term "stock value" since it might be unclear what it refers to. Maybe "original value", although it's also not perfect. This seems like a decision for the Kseniia / docs folks regarding precise wording.

Doc folks may have some suggestion here. I tweaked the texts a bit for better clarity.

Let's discuss both items above at our product sync meeting.

@maximpn maximpn merged commit 87079ff into elastic:main Dec 13, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@maximpn maximpn deleted the add-field-modified-badge branch December 13, 2024 11:44
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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 14.7MB 14.7MB +448.0B

History

cc @maximpn

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 13, 2024
…ule Upgrade flyout (elastic#203968)

**Resolves:** elastic#203718

## Summary

This PR adds `Modified` badge to customized fields in rules upgrade flyout.

## Details

`_review` API endpoint contains fields diff providing enough information on what field values were involved in the comparison. In particular `diff_outcome` is used to determine if a field was customized i.e. the rule was edited and field value has a different value than a stock value. `Modified` badge is show for fields `CustomizedValueCanUpdate`, `CustomizedValueSameUpdate` and `CustomizedValueNoUpdate`.

## Screenshot

![image](https://github.com/user-attachments/assets/8f773f45-7ab5-4883-9ef7-fee8f3bde768)

(cherry picked from commit 87079ff)
@kibanamachine
Copy link
Contributor

💚 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

kibanamachine added a commit that referenced this pull request Dec 13, 2024
…s in Rule Upgrade flyout (#203968) (#204195)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Display Modified badge for customized fields in
Rule Upgrade flyout
(#203968)](#203968)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Maxim
Palenov","email":"maxim.palenov@elastic.co"},"sourceCommit":{"committedDate":"2024-12-13T11:43:45Z","message":"[Security
Solution] Display Modified badge for customized fields in Rule Upgrade
flyout (#203968)\n\n**Resolves:**
https://github.com/elastic/kibana/issues/203718\r\n\r\n##
Summary\r\n\r\nThis PR adds `Modified` badge to customized fields in
rules upgrade flyout.\r\n\r\n## Details\r\n\r\n`_review` API endpoint
contains fields diff providing enough information on what field values
were involved in the comparison. In particular `diff_outcome` is used to
determine if a field was customized i.e. the rule was edited and field
value has a different value than a stock value. `Modified` badge is show
for fields `CustomizedValueCanUpdate`, `CustomizedValueSameUpdate` and
`CustomizedValueNoUpdate`.\r\n\r\n##
Screenshot\r\n\r\n![image](https://github.com/user-attachments/assets/8f773f45-7ab5-4883-9ef7-fee8f3bde768)","sha":"87079ff226b616d3b11370f7307f5c7448001b28","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","impact:medium","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","backport:version","v8.18.0"],"title":"[Security Solution]
Display Modified badge for customized fields in Rule Upgrade
flyout","number":203968,"url":"https://github.com/elastic/kibana/pull/203968","mergeCommit":{"message":"[Security
Solution] Display Modified badge for customized fields in Rule Upgrade
flyout (#203968)\n\n**Resolves:**
https://github.com/elastic/kibana/issues/203718\r\n\r\n##
Summary\r\n\r\nThis PR adds `Modified` badge to customized fields in
rules upgrade flyout.\r\n\r\n## Details\r\n\r\n`_review` API endpoint
contains fields diff providing enough information on what field values
were involved in the comparison. In particular `diff_outcome` is used to
determine if a field was customized i.e. the rule was edited and field
value has a different value than a stock value. `Modified` badge is show
for fields `CustomizedValueCanUpdate`, `CustomizedValueSameUpdate` and
`CustomizedValueNoUpdate`.\r\n\r\n##
Screenshot\r\n\r\n![image](https://github.com/user-attachments/assets/8f773f45-7ab5-4883-9ef7-fee8f3bde768)","sha":"87079ff226b616d3b11370f7307f5c7448001b28"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203968","number":203968,"mergeCommit":{"message":"[Security
Solution] Display Modified badge for customized fields in Rule Upgrade
flyout (#203968)\n\n**Resolves:**
https://github.com/elastic/kibana/issues/203718\r\n\r\n##
Summary\r\n\r\nThis PR adds `Modified` badge to customized fields in
rules upgrade flyout.\r\n\r\n## Details\r\n\r\n`_review` API endpoint
contains fields diff providing enough information on what field values
were involved in the comparison. In particular `diff_outcome` is used to
determine if a field was customized i.e. the rule was edited and field
value has a different value than a stock value. `Modified` badge is show
for fields `CustomizedValueCanUpdate`, `CustomizedValueSameUpdate` and
`CustomizedValueNoUpdate`.\r\n\r\n##
Screenshot\r\n\r\n![image](https://github.com/user-attachments/assets/8f773f45-7ab5-4883-9ef7-fee8f3bde768)","sha":"87079ff226b616d3b11370f7307f5c7448001b28"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Maxim Palenov <maxim.palenov@elastic.co>
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 impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. 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.

[Security Solution] Show Modified badge for fields customized by users
4 participants