-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Alerts table] Edit tags UI #243792
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
[Alerts table] Edit tags UI #243792
Conversation
|
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
I removed useItemsAction here because that hook was designed for the alert update api, which does not easily support updating workflow tags. I moved the logic to useTagsAction instead. useItemsAction may come in use in the future if we decide to directly write to the tags field of an alert.
|
I picked this up earlier today. Note: I removed useItemsAction here because that hook was designed for the alert update api, which does not easily support updating workflow tags. I moved the logic to useTagsAction instead. useItemsAction may come in use in the future if we decide to directly write to the tags field of an alert. |
other improvements
13c0b95 to
af1c1ce
Compare
|
Pinging @elastic/actionable-obs-team (Team:actionable-obs) |
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#9973[❌] x-pack/platform/test/alerting_api_integration/spaces_only/tests/alerting/group4/config.ts: 6/25 tests passed. |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#9975[✅] x-pack/platform/test/alerting_api_integration/spaces_only/tests/alerting/group4/config.ts: 25/25 tests passed. |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#9980[✅] x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/detection_engine/exceptions/operators_data_types/float/basic_license_essentials_tier/configs/serverless.config.ts: 25/25 tests passed. |
cnasikas
left a comment
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.
LGMT! I found a bug. If you run Kibana with --run-examles and you create an Always firing rule, and then you go to the detailed page of the rule you see the following:
Screen.Recording.2025-12-05.at.1.42.14.PM.mov
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.
nit: Should we test that the onAction* callbacks are being called correctly?
| }); | ||
| }); | ||
|
|
||
| it('handles multiple alerts from different indices', async () => { |
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.
Note: Maybe we can remove this test as the indices are not being taken into account anymore.
| expect(onError).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should update loading state during mutation', async () => { |
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.
Thank you so much for this great coverage!
nit: Maybe we can remove this test as it tests React query and there is no custom logic regarding the isLoading and isSuccess booleans.
| focusButtonRef?: React.Ref<HTMLButtonElement>; | ||
| } | ||
|
|
||
| const FlyoutBodyCss = css` |
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.
A reminder that the new flyout system is coming elastic/eui#9202 and these changes here may affect it.
src/platform/packages/shared/response-ops/alerts-table/components/tags/edit_tags_flyout.tsx
Show resolved
Hide resolved
| <h2 data-test-subj="alerts-edit-tags-flyout-title">{i18n.EDIT_TAGS}</h2> | ||
| </EuiTitle> | ||
| <EuiText color="subdued"> | ||
| <p>{headerSubtitle as string}</p> |
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.
nit:
| <p>{headerSubtitle as string}</p> | |
| <p>{headerSubtitle}</p> |
| </EuiText> | ||
| </EuiFlyoutHeader> | ||
| <EuiFlyoutBody css={FlyoutBodyCss}> | ||
| {isLoading ? ( |
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.
nit: As this is always set to false, probably we do not need this logic here.
cnasikas
left a comment
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.
LGTM! Thanks for your patience with my comments!
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#9995[❌] x-pack/platform/test/alerting_api_integration/spaces_only/tests/alerting/group4/config.ts: 174/200 tests passed. |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#9998[✅] x-pack/platform/test/alerting_api_integration/spaces_only/tests/alerting/group4/config.ts: 25/25 tests passed. |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
## Summary Closes elastic#240377 Closes elastic#240376 (with [this PR](elastic#243792)) <img width="1467" height="297" alt="Screenshot 2025-11-25 at 5 20 01 PM" src="https://github.com/user-attachments/assets/124ab28e-db22-4bc8-914d-31088930afea" /> <img width="1469" height="640" alt="Screenshot 2025-11-25 at 5 18 06 PM" src="https://github.com/user-attachments/assets/0a4874e1-6fbf-4355-a8b8-8d26c68b8492" /> ## Release Notes Users can now view and filter by workflow tags manually added to alerts --------- Co-authored-by: Faisal Kanout <[email protected]>
## Release Notes Users can now manually add workflow tags directly to alerts (see [this PR as well](elastic#244251)) ## Summary Part of elastic#240376 - Adds "Edit tags" in alert actions - Opens flyout when clicking on "Edit tags" - Flyout shows list of all `kibana.alert.workflow_tags` from the selected alerts - "Edit tags" will be disabled when all alerts are selected (as there can be 1000s of alerts with many tags) - "Edit tags" will not be available for Security rules --------- Co-authored-by: Bailey Cash <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]>
Release Notes
Users can now manually add workflow tags directly to alerts (see this PR as well)
Summary
Resolves #240376
kibana.alert.workflow_tagsfrom the selected alerts