Skip to content

[Security Solution] Fix alerts table potentially not applying alert assignees#219460

Merged
PhilippeOberti merged 2 commits intoelastic:mainfrom
PhilippeOberti:fix-assignee-action
May 1, 2025
Merged

[Security Solution] Fix alerts table potentially not applying alert assignees#219460
PhilippeOberti merged 2 commits intoelastic:mainfrom
PhilippeOberti:fix-assignee-action

Conversation

@PhilippeOberti
Copy link
Copy Markdown
Contributor

Summary

This PR implements a similar change that was just merged a few hours ago. While that change was made to the alert tags not always working on the alerts table, this current change is applied to the alert assignees that faced a potential similar issue. The alert assignee code was introduced in this PR, and I believe the code was using the similar logic of the alert tag PR.

The issue is related to the fact that we have a useRef for a function that is returned before the useEffect in the same hook runs, and setting the value of the function returned is happening within that useEffect. This has not caused any issues because the few places where this code is being used (the alerts page alerts table) is extremely not efficient and renders multiple times. This gives enough tries to the hook to actually get a value and return the correct function.

This PR fixes that by returning the function directly.

Here's a video showing that the functionality still works correctly for bulk actions:

Screen.Recording.2025-04-28.at.3.59.32.PM.mov

And also for normal row actions:

Screen.Recording.2025-04-28.at.3.59.49.PM.mov

Checklist

@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team v9.1.0 labels Apr 28, 2025
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner April 28, 2025 21:10
@PhilippeOberti PhilippeOberti requested a review from xcrzx April 28, 2025 21:10
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

@PhilippeOberti PhilippeOberti requested a review from e40pud April 28, 2025 21:10
Copy link
Copy Markdown
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

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

Hey @PhilippeOberti, I think our team is being required to review this PR due to a misconfiguration. We don’t own any files under the bulk_actions folder.

Could you please correct this line in the CODEOWNERS file to make sure the right team is pinged for review?

/x-pack/solutions/security/plugins/security_solution/public/common/components/toolbar/bulk_actions @elastic/security-detection-rule-management

@PhilippeOberti PhilippeOberti force-pushed the fix-assignee-action branch from d2cb862 to 0704047 Compare May 1, 2025 14:04
@PhilippeOberti PhilippeOberti force-pushed the fix-assignee-action branch from 0704047 to 586ff1e Compare May 1, 2025 14:05
@prodsecmachine
Copy link
Copy Markdown
Collaborator

prodsecmachine commented May 1, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@PhilippeOberti
Copy link
Copy Markdown
Contributor Author

Hey @PhilippeOberti, I think our team is being required to review this PR due to a misconfiguration. We don’t own any files under the bulk_actions folder.

Could you please correct this line in the CODEOWNERS file to make sure the right team is pinged for review?

/x-pack/solutions/security/plugins/security_solution/public/common/components/toolbar/bulk_actions @elastic/security-detection-rule-management

Codeowners file updated!

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #53 / console app misc console behavior keyboard shortcuts should execute the request when Ctrl+Enter is pressed

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.1MB 9.1MB -52.0B

History

Copy link
Copy Markdown
Contributor

@dplumlee dplumlee left a comment

Choose a reason for hiding this comment

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

Pulled down and tested, everything seems to work the same as before the alerts table update 👍 . Also talked with @PhilippeOberti about this offline - same bug/fix that's in #219410.

LGTM, thanks for switching this code over to the detection engine team ownership.

Might be nice to see if this fix would also allow us to unskip the related cypress tests for this feature (x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/detection_alerts/assignments/assignments.cy.ts)

@PhilippeOberti PhilippeOberti merged commit 254477e into elastic:main May 1, 2025
9 checks passed
@PhilippeOberti PhilippeOberti deleted the fix-assignee-action branch May 1, 2025 21:09
kapral18 added a commit to kapral18/kibana that referenced this pull request May 4, 2025
…ends-crash

* main: (111 commits)
  [ResponseOps][Rules] Cases action title length too long (elastic#219226)
  [main] Sync bundled packages with Package Storage (elastic#219839)
  Fix ignored dynamic templates (elastic#219875)
  Enforce dependency review by kibana-security workflow (elastic#219262)
  [Security Solution] [Detections] Removes tech preview text from eql seq suppression ui (elastic#219870)
  [Security Solution] Fix alerts table potentially not applying alert assignees (elastic#219460)
  fix(slo): alert deletion (elastic#219876)
  [AI4DSOC] fix styling to address cutoff when screen is narrow (elastic#219306)
  [Security Solution][Endpoint] Response action create and history log API updates in of space awareness (elastic#218674)
  Update publish_oas_docs.sh to deploy Kibana Serverless API docs (elastic#219867)
  feat(slo): lock resource installation (elastic#219747)
  [AI4DSOC] Alert flyout code cleanup (elastic#219810)
  [fleet] fixing `isAgentlessDefault` config usage and readability improvements to `isAgentlessSetupDefault` (elastic#219423)
  feat(slo): Bulk delete UI (elastic#219634)
  m1 demo prep (elastic#219588)
  [Security Solution] Replace sourcerer in EQL tab with dataview picker (elastic#218897)
  [AI4DSOC] Attack discovery widget follow up follow up (elastic#219849)
  [AI Assistant] Fix some OpenAI models not accepting temperature for Inference service (elastic#218887)
  Update dependency msw to ~2.7.5 (main) (elastic#219289)
  Use new client URLs in doc link service (elastic#219600)
  ...
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
…ssignees (elastic#219460)

## Summary

This PR implements a similar change that was just merged a few hours
ago. While [that change](elastic#219410)
was made to the alert tags not always working on the alerts table, this
current change is applied to the alert assignees that faced a potential
similar issue. The alert assignee code was introduced in [this
PR](elastic#170579), and I believe the
code was using the similar logic of [the alert tag
PR](elastic#157786).

The issue is related to the fact that we have a `useRef` for a function
that is returned before the `useEffect` in the same hook runs, and
setting the value of the function returned is happening within that
`useEffect`. This has not caused any issues because the few places where
this code is being used (the alerts page alerts table) is extremely not
efficient and renders multiple times. This gives enough tries to the
hook to actually get a value and return the correct function.

This PR fixes that by returning the function directly.

Here's a video showing that the functionality still works correctly for
bulk actions:


https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea

And also for normal row actions:


https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
PhilippeOberti added a commit to PhilippeOberti/kibana that referenced this pull request May 30, 2025
…ssignees (elastic#219460)

## Summary

This PR implements a similar change that was just merged a few hours
ago. While [that change](elastic#219410)
was made to the alert tags not always working on the alerts table, this
current change is applied to the alert assignees that faced a potential
similar issue. The alert assignee code was introduced in [this
PR](elastic#170579), and I believe the
code was using the similar logic of [the alert tag
PR](elastic#157786).

The issue is related to the fact that we have a `useRef` for a function
that is returned before the `useEffect` in the same hook runs, and
setting the value of the function returned is happening within that
`useEffect`. This has not caused any issues because the few places where
this code is being used (the alerts page alerts table) is extremely not
efficient and renders multiple times. This gives enough tries to the
hook to actually get a value and return the correct function.

This PR fixes that by returning the function directly.

Here's a video showing that the functionality still works correctly for
bulk actions:

https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea

And also for normal row actions:

https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 254477e)
qn895 pushed a commit to qn895/kibana that referenced this pull request Jun 3, 2025
…ssignees (elastic#219460)

## Summary

This PR implements a similar change that was just merged a few hours
ago. While [that change](elastic#219410)
was made to the alert tags not always working on the alerts table, this
current change is applied to the alert assignees that faced a potential
similar issue. The alert assignee code was introduced in [this
PR](elastic#170579), and I believe the
code was using the similar logic of [the alert tag
PR](elastic#157786).

The issue is related to the fact that we have a `useRef` for a function
that is returned before the `useEffect` in the same hook runs, and
setting the value of the function returned is happening within that
`useEffect`. This has not caused any issues because the few places where
this code is being used (the alerts page alerts table) is extremely not
efficient and renders multiple times. This gives enough tries to the
hook to actually get a value and return the correct function.

This PR fixes that by returning the function directly.

Here's a video showing that the functionality still works correctly for
bulk actions:


https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea

And also for normal row actions:


https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
PhilippeOberti added a commit to PhilippeOberti/kibana that referenced this pull request Jun 4, 2025
…ssignees (elastic#219460)

## Summary

This PR implements a similar change that was just merged a few hours
ago. While [that change](elastic#219410)
was made to the alert tags not always working on the alerts table, this
current change is applied to the alert assignees that faced a potential
similar issue. The alert assignee code was introduced in [this
PR](elastic#170579), and I believe the
code was using the similar logic of [the alert tag
PR](elastic#157786).

The issue is related to the fact that we have a `useRef` for a function
that is returned before the `useEffect` in the same hook runs, and
setting the value of the function returned is happening within that
`useEffect`. This has not caused any issues because the few places where
this code is being used (the alerts page alerts table) is extremely not
efficient and renders multiple times. This gives enough tries to the
hook to actually get a value and return the correct function.

This PR fixes that by returning the function directly.

Here's a video showing that the functionality still works correctly for
bulk actions:

https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea

And also for normal row actions:

https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 254477e)
PhilippeOberti added a commit to PhilippeOberti/kibana that referenced this pull request Jun 4, 2025
…ssignees (elastic#219460)

## Summary

This PR implements a similar change that was just merged a few hours
ago. While [that change](elastic#219410)
was made to the alert tags not always working on the alerts table, this
current change is applied to the alert assignees that faced a potential
similar issue. The alert assignee code was introduced in [this
PR](elastic#170579), and I believe the
code was using the similar logic of [the alert tag
PR](elastic#157786).

The issue is related to the fact that we have a `useRef` for a function
that is returned before the `useEffect` in the same hook runs, and
setting the value of the function returned is happening within that
`useEffect`. This has not caused any issues because the few places where
this code is being used (the alerts page alerts table) is extremely not
efficient and renders multiple times. This gives enough tries to the
hook to actually get a value and return the correct function.

This PR fixes that by returning the function directly.

Here's a video showing that the functionality still works correctly for
bulk actions:

https://github.com/user-attachments/assets/b3394ffe-8333-4e0a-9bf7-831ef8ea8aea

And also for normal row actions:

https://github.com/user-attachments/assets/5f8c9d23-f0ef-4c65-b7de-4dc34478a8e7

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 254477e)
PhilippeOberti added a commit that referenced this pull request Jun 4, 2025
…) (#222074)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[AI4DSOC] Alert summary page routing and initialization
(#214889)](#214889)
- [[AI4DSOC] Alert summary landing page
(#215246)](#215246)
- [[AI4DSOC] Alert summary dataview
(#215265)](#215265)
- [[AI4DSOC] Alert summary KQL bar
[#215586]](#215586)
- [[AI4DSOC] Alert summary KPI charts
[#215585]](#215585)
- [[AI4DSOR] Alert summary integrations section
[#215266]](#215266)
- [[AI4DSOC] Fix issue with filtering by integrations
[#216574]](#216574)
- [[AI4DSOC] Alert summary table setup
[#216744]](#216744)
- [Alerty summary table flyout setup
[#217421]](#217421)
- [[AI4DSOC] Alert summary alert actions in table and flyout
[#217696]](#217696)
- [[AI4DSOC] Alert summary table custom cell renderers
[#217124]](#217124)
- [[AI4DSOC] Alert summary table and flyout ai assistant
[#217744]](#217744)
- [[AI4DSOC] Alert summary page performance improvements
[#218632]](#218632)
- [[AI4DSOC] Change the Attack Discovery page to use the AI for SOC
alerts table [#218736]](#218736)
- [[AI4DSOC] Change the Cases page to use the AI for SOC alerts table
[#218742]](#218742)
- [[AI4DSOC] Fix spacing issue on alert summary landing page integration
card [#218868]](#218868)
- [[AI4DSOC][ResponseOps] Fix alerts table not handling undefined
maintenanceWindow capability
[#218999]](#218999)
- [[AI4DSOC] Fix link to the new integrations page
[#219030]](#219030)
- [[AI4DSOC] Disable CellActions and PreviewLinks on the Attack
discovery page [#219033]](#219033)
- [[AI4DSOC] Add cell renderer for datetime fields to the alert summary
table [#219126]](#219126)
- [[AI4DSOC] Remove Assistant icon from row action in alert summary
table [#219141]](#219141)
- [[AI4DSOC] Add checkboxes to the alert summary table
[#219169]](#219169)
- [[Security Solution][AI4DSOC] Fix table not applying alert tags for
Attack discovery and Cases pages in AI4DSOC
[#219410]](#219410)
- [[AI4DSOC] Fix logic that renders the group title when grouping by
integrations [#219430]](#219430)
- [[AI4DSOC] Alert summary table truncates long values and display the
field/value pair in tooltip
[#219438]](#219438)
- [[Security Solution] Fix alerts table potentially not applying alert
assignees [#219460]](#219460)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants