-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[RAC] [o11y] add permission in alerts table from kibana privilege/consumer #109759
Conversation
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
x-pack/plugins/observability/public/pages/alerts/alerts_table_t_grid.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/timelines/public/components/t_grid/standalone/index.tsx
Outdated
Show resolved
Hide resolved
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.
Code LGTM!
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.
Do you think it's worth adding a few tests for the permissions and selection logic?
x-pack/plugins/observability/public/hooks/use_alert_permission.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/timelines/public/components/t_grid/body/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/timelines/public/components/t_grid/body/index.tsx
Outdated
Show resolved
Hide resolved
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!
In general when fixing bugs it's beneficial to add a sanity-check test to ensure we are covering the bug with these changes and to ensure future changes do not re-introduce this bug. |
d5be667
to
ff6c520
Compare
932528e
to
42a1678
Compare
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 tested this locally. Looks good.
💚 Build SucceededMetrics [docs]Public APIs missing comments
Async chunks
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
…sumer (elastic#109759) * add alert permission in o11y * review I * review II * fix selection all when checkbox disabled * fix selected on bulk actions
…sumer (elastic#109759) * add alert permission in o11y * review I * review II * fix selection all when checkbox disabled * fix selected on bulk actions
Summary
#109109