Skip to content

[Security Solution][Detections] Add assignees UI into alert's details flyout component (#7662)#168467

Closed
e40pud wants to merge 23 commits intoelastic:security/feature/alert-user-assignmentfrom
e40pud:security/feature/alert-user-assignment-7662
Closed

[Security Solution][Detections] Add assignees UI into alert's details flyout component (#7662)#168467
e40pud wants to merge 23 commits intoelastic:security/feature/alert-user-assignmentfrom
e40pud:security/feature/alert-user-assignment-7662

Conversation

@e40pud
Copy link
Contributor

@e40pud e40pud commented Oct 10, 2023

Summary

Closes https://github.com/elastic/security-team/issues/7662

This PR adds Alert user assignment UI within alert's details flyout component.

Screen.Recording.2023-10-10.at.14.48.10.mov

@e40pud e40pud added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. release_note:feature Makes this part of the condensed release notes ci:cloud-deploy Create or update a Cloud deployment Team:Detection Engine Security Solution Detection Engine Area labels Oct 10, 2023
@e40pud e40pud self-assigned this Oct 10, 2023
@e40pud e40pud requested review from a team as code owners October 10, 2023 12:50
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

thanks for implementing this @e40pud!

First question is how am I supposed to test this locally? Are there any easy steps I can do to get data to show in the assignee popover dropdown?

I see that the feature was only added to the new alert details flyout (the one using the expandable flyout). Users can still revert to the old flyout using the advanced settings. Are we ok with not adding this new feature to the old flyout as well?

I don't know if UIUX was involved here, but on smaller screens (like Macbook Pro), the header gets even more crowded than it was, and it's easy to get in a state where things aren't really readable anymore
Screenshot 2023-10-11 at 8 27 49 AM

@e40pud
Copy link
Contributor Author

e40pud commented Oct 12, 2023

Thanks for the review @PhilippeOberti !!

First question is how am I supposed to test this locally? Are there any easy steps I can do to get data to show in the assignee popover dropdown?

You need to add users (and login with those users at least once) to be able to see those in the User Profiles dropdown menu.

Screenshot 2023-10-12 at 18 11 13

I see that the feature was only added to the new alert details flyout (the one using the expandable flyout). Users can still revert to the old flyout using the advanced settings. Are we ok with not adding this new feature to the old flyout as well?

I will double check with design and product about this.

I don't know if UIUX was involved here, but on smaller screens (like Macbook Pro), the header gets even more crowded than it was, and it's easy to get in a state where things aren't really readable anymore

Good point! I will check this with design team.

@PhilippeOberti
Copy link
Contributor

First question is how am I supposed to test this locally? Are there any easy steps I can do to get data to show in the assignee popover dropdown?

You need to add users (and login with those users at least once) to be able to see those in the User Profiles dropdown menu.

@e40pud is there anything else that I need to do? As you can see in the video below, I've added 4 users with some roles, logged in with each of them (and verify I can see data), but still can't see them in the Assignee dropdown. I'm getting an error finding users

Screen.Recording.2023-10-13.at.10.38.59.AM.mov

…eature/alert-user-assignment-7662

# Conflicts:
#	x-pack/plugins/security_solution/public/flyout/document_details/right/components/assignees.tsx
#	x-pack/plugins/security_solution/public/flyout/document_details/right/components/assignees_popover.tsx
#	x-pack/plugins/security_solution/public/flyout/document_details/right/components/header_title.test.tsx
#	x-pack/plugins/security_solution/public/flyout/document_details/right/components/header_title.tsx
@PhilippeOberti
Copy link
Contributor

First question is how am I supposed to test this locally? Are there any easy steps I can do to get data to show in the assignee popover dropdown?

You need to add users (and login with those users at least once) to be able to see those in the User Profiles dropdown menu.

@e40pud is there anything else that I need to do? As you can see in the video below, I've added 4 users with some roles, logged in with each of them (and verify I can see data), but still can't see them in the Assignee dropdown. I'm getting an error finding users

Ok with a fresh install of ES and Kibana I got it to work, not sure what I did last week. Will resume reviewing the PR shortly :)

@PhilippeOberti
Copy link
Contributor

Looking at the UI a bit more specifically now that I can test locally, here are a few comments:

I see a slight difference in the badge itself, in the mocks I see a white border that I'm not seeing in the code here

Code Mock
Screenshot 2023-10-16 at 2 05 35 PM Screenshot 2023-10-16 at 2 04 40 PM

The spacing between the badges when show 2 of them isn't what the mocks are showing

Code Mocks
Screenshot 2023-10-16 at 1 58 48 PM Screenshot 2023-10-16 at 2 04 43 PM

The UI when displaying 3+ assignees is also very different from the mocks

Code Mocks
Screenshot 2023-10-16 at 2 01 17 PM Screenshot 2023-10-16 at 2 04 46 PM

One more thing, and that one might be tricky to handle but with small screens the UI gets messed up a bit. You might be able to ignore this last point as UIUX is discussing setting a min-width of the flyout that could prevent this from happening...
Screenshot 2023-10-16 at 2 14 58 PM
Screenshot 2023-10-16 at 2 14 33 PM

@e40pud
Copy link
Contributor Author

e40pud commented Oct 17, 2023

@PhilippeOberti thanks for testing! You are right about misalignment with the mockups. Those we discussed and are ok to go with as of right now. I will be looking into those little improvements in a separate PR (borders and overlaps).

Also, which link do you use for mockups? It looks like the one you are looking at are slightly different from what I'm using :-)

Here is what I see for the 3+ users in the mockups here:

Screenshot 2023-10-17 at 18 09 51

@e40pud e40pud requested a review from PhilippeOberti October 17, 2023 16:14
…assignment-7662"

This reverts commit 27a29e1, reversing
changes made to 3d708b1.
@e40pud e40pud removed request for a team, parkiino and patrykkopycinski October 23, 2023 11:12
@e40pud e40pud closed this Oct 23, 2023
@e40pud e40pud deleted the security/feature/alert-user-assignment-7662 branch October 23, 2023 11:22
@e40pud
Copy link
Contributor Author

e40pud commented Oct 23, 2023

Replaced by #169508

e40pud added a commit that referenced this pull request Oct 23, 2023
… flyout component (#7662) (#169508)

## Summary

Closes elastic/security-team#7662

This PR adds Alert user assignment UI within alert's details flyout
component.


https://github.com/elastic/kibana/assets/2700761/b84299d7-5d65-4e9a-8836-807f51c0bbc7


This PR is a replacement to
#168467 since I broke that one
with wrong merges from main.

cc @PhilippeOberti
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:cloud-deploy Create or update a Cloud deployment release_note:feature Makes this part of the condensed release notes Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants