-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Custom Threshold Rule] Fix custom threshold deletion/update #171919
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked metric threshold and we don't have this issue there, can we use a similar approach without adding an id to the rule's saved object and API? Since rule creation API is used by SRE, I rather not add extra fields there if it is not necessary.
Screen.Recording.2023-11-27.at.08.23.52.mov
@@ -75,6 +75,7 @@ export function thresholdRuleType( | |||
threshold: schema.arrayOf(schema.number()), | |||
comparator: oneOfLiterals(Object.values(Comparator)), | |||
timeUnit: schema.string(), | |||
id: schema.maybe(schema.string()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id is only used in UI and isn't a required field when let's say user is using an API.
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
@shahzad31 I noticed that the issue only happens for the aggregation and equation section and I fixed it by making sure these sections will re-render if deletion happens. Here is the fix: #172187 |
closing in favor of #172187 |
Summary
Fix custom threshold deletion/update !!
Set id in expression items to make sure uniqueness
Fixes #171623