-
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
[Security Solution][Detection Alerts] Fixes alert page refresh issues #111042
Conversation
22c3ee6
to
23dd074
Compare
@elasticmachine merge upstream |
Generic comment about adding tests.™ 💙 (I know there's been a lot of movement and refactoring between all these components and it's no small effort to add robust test coverage around all this functionality, especially so late in the dev cycle, so perhaps we open an tech debt issue for test coverage around this area and ensure it's prioritized before new work.) |
07d255d
to
0782085
Compare
0782085
to
bb1d9c5
Compare
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
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.
Checked out, tested locally, and looked over code. Looks like there's one minor bug left (no refresh after adding an exception via Alert Details -> Take Action
menu), but approving now as most issues have been resolved and these existing fixes are critical to have in the next BC. If not a super simple low-risk 1-liner, can you please open a follow-up issue to track this remaining work? Great simple fixes here @dplumlee -- thanks for seeing this one through! 👍 🚀 🙂
Testing update from past comments:
✅ Verified adding exception refreshes all views (#111042 (comment))
❌ Verified adding exception from the Alert Details -> Take Action
menu does not refresh all views (#111042 (comment))
✅ Verified changing the workflow status from alert details refreshes all views #111042 (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!!
…elastic#111042) Co-authored-by: Xavier Mouligneau <[email protected]>
…elastic#111042) Co-authored-by: Xavier Mouligneau <[email protected]>
…#111042) (#112039) Co-authored-by: Xavier Mouligneau <[email protected]> Co-authored-by: Davis Plumlee <[email protected]> Co-authored-by: Xavier Mouligneau <[email protected]>
…#111042) (#112038) Co-authored-by: Xavier Mouligneau <[email protected]> Co-authored-by: Davis Plumlee <[email protected]> Co-authored-by: Xavier Mouligneau <[email protected]>
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @dplumlee |
Summary
Addresses #108244
Fixes refresh problems on the Alert and rule details page when alerts status is updated and the table updates but the histogram and count table don't unless a hard refresh is run.
Checklist
Delete any items that are not applicable to this PR.
For maintainers