Skip to content
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] Enable actions in document details preview footer #203691

Merged
merged 3 commits into from
Dec 16, 2024

Conversation

christineweng
Copy link
Contributor

@christineweng christineweng commented Dec 10, 2024

Summary

Updated document details preview footer to also show actions

Before

image

After

  • Users can perform alert/event actions in a preview
    image

  • In analyzer, when examining an event, event actions are also available
    image

  • No change to flyout in rule creation workflow, action is not available in preview nor non-preview

Checklist

@christineweng christineweng self-assigned this Dec 10, 2024
@christineweng christineweng force-pushed the viz-ga-preview-actions branch 2 times, most recently from f22ab8f to d51af8d Compare December 12, 2024 00:40
@christineweng christineweng marked this pull request as ready for review December 12, 2024 18:00
@christineweng christineweng requested a review from a team as a code owner December 12, 2024 18:00
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

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.

a couple of comments we can discuss on Monday:

  • do we want to have the action left aligned? Have you tried centered or right aligned (next to the Take action button)
Screenshot 2024-12-13 at 4 14 29 PM
  • I'm not a fan of using the footer.tsx file from the right folder inside the preview folder. What I had in mind for this feature was to duplicate some code and have the preview footer do everything itself, or move some of the logic to the shared folder (either the entire footer code or some of its content

  • while I understand why you named the property additionalActions (I'm guessing to be more flexible in the future?) I feel like it's also a bit vague and we could name it a bit more specifically (related to navigating to the full flyout). Granted that we want to keep this strucutre (see my second point above).

@christineweng
Copy link
Contributor Author

christineweng commented Dec 13, 2024

@PhilippeOberti thanks for the review!

a couple of comments we can discuss on Monday:

  • do we want to have the action left aligned? Have you tried centered or right aligned (next to the Take action button)

Frankly I can do any design, but I didn't get specific directions from UIUX. I can try a couple alignments

  • I'm not a fan of using the footer.tsx file from the right folder inside the preview folder. What I had in mind for this feature was to duplicate some code and have the preview footer do everything itself, or move some of the logic to the shared folder (either the entire footer code or some of its content

Me neither, I think the biggest thing is the right footer has all the take actions hooks, so direct copy and paste feels redundant to me. Ideally we should extract all the hooks into a separate component and call it takeActions?

Another consideration is how there is stuff outside of the EuiFlyoutFooter component (do you know why??) I didn't want to make this PR more complicated than it has to be, hence landing on re-using the right footer

<>
  <EuiFlyoutFooter data-test-subj={FLYOUT_FOOTER_TEST_ID}>
     <EuiPanel color="transparent">
          ...
      </EuiPanel>
   </EuiFlyoutFooter>
   
  {openAddExceptionFlyout &&
        ....
        <AddExceptionFlyoutWrapper
         ....
        />
  )}
</>

@PhilippeOberti
Copy link
Contributor

Briefly summarizing a Zoom meeting discussion:

  • do we want to have the action left aligned? Have you tried centered or right aligned (next to the Take action button)

Frankly I can do any design, but I didn't get specific directions from UIUX. I can try a couple alignments

let's see how having the action right aligned looks!

  • I'm not a fan of using the footer.tsx file from the right folder inside the preview folder. What I had in mind for this feature was to duplicate some code and have the preview footer do everything itself, or move some of the logic to the shared folder (either the entire footer code or some of its content

Me neither, I think the biggest thing is the right footer has all the take actions hooks, so direct copy and paste feels redundant to me. Ideally we should extract all the hooks into a separate component and call it takeActions?

Let's see if extracting all the Take Action logic in its own component would work. This way we would have the Take Action logic self contained and we would use that component in both footers (the one in preview and the one in right). The one in preview would have some extra logic, a simple button to navigate to the full flyout.

If this does not work, happens to be too complex or too much code change, what you have no works @christineweng. We can just move the footer.tsx file from right to shared and we should be good!

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.

thank you for implementing the suggested change related to the Take Action button, I really like this implementation.

Code LGTM and I desk tested everything looks good!

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #4 / EQL Tab rendering should render the timeline table

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6410 6411 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 14.7MB 14.7MB +899.0B

History

cc @christineweng

@christineweng christineweng merged commit 7aac71c into elastic:main Dec 16, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12363189364

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 16, 2024
…elastic#203691)

## Summary

Updated document details preview footer to also show actions
### Before
<img width="1481" alt="image"
src="https://github.com/user-attachments/assets/aace12aa-f3d1-4fe2-a3b7-392878207f39"
/>

### After
- Users can perform alert/event actions in a preview

![image](https://github.com/user-attachments/assets/d42be26d-9d4a-4701-bc88-92549ebfb65c)

- In analyzer, when examining an event, event actions are also available

![image](https://github.com/user-attachments/assets/d30515d9-b428-4112-86f8-9bb872eaf921)

- No change to flyout in rule creation workflow, action is not available
in preview nor non-preview

### 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 7aac71c)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 17, 2024
…footer (#203691) (#204504)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Enable actions in document details preview footer
(#203691)](#203691)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"christineweng","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-16T23:27:47Z","message":"[Security
Solution] Enable actions in document details preview footer
(#203691)\n\n## Summary\r\n\r\nUpdated document details preview footer
to also show actions\r\n### Before\r\n<img width=\"1481\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/aace12aa-f3d1-4fe2-a3b7-392878207f39\"\r\n/>\r\n\r\n\r\n###
After \r\n- Users can perform alert/event actions in a
preview\r\n\r\n![image](https://github.com/user-attachments/assets/d42be26d-9d4a-4701-bc88-92549ebfb65c)\r\n\r\n-
In analyzer, when examining an event, event actions are also
available\r\n\r\n![image](https://github.com/user-attachments/assets/d30515d9-b428-4112-86f8-9bb872eaf921)\r\n\r\n-
No change to flyout in rule creation workflow, action is not
available\r\nin preview nor non-preview\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"7aac71c2aa5eb78f57bfe7bbb047104766b55a30","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport","v9.0.0","Team:Threat
Hunting","Team:Threat
Hunting:Investigations","v8.18.0"],"title":"[Security Solution] Enable
actions in document details preview
footer","number":203691,"url":"https://github.com/elastic/kibana/pull/203691","mergeCommit":{"message":"[Security
Solution] Enable actions in document details preview footer
(#203691)\n\n## Summary\r\n\r\nUpdated document details preview footer
to also show actions\r\n### Before\r\n<img width=\"1481\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/aace12aa-f3d1-4fe2-a3b7-392878207f39\"\r\n/>\r\n\r\n\r\n###
After \r\n- Users can perform alert/event actions in a
preview\r\n\r\n![image](https://github.com/user-attachments/assets/d42be26d-9d4a-4701-bc88-92549ebfb65c)\r\n\r\n-
In analyzer, when examining an event, event actions are also
available\r\n\r\n![image](https://github.com/user-attachments/assets/d30515d9-b428-4112-86f8-9bb872eaf921)\r\n\r\n-
No change to flyout in rule creation workflow, action is not
available\r\nin preview nor non-preview\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"7aac71c2aa5eb78f57bfe7bbb047104766b55a30"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203691","number":203691,"mergeCommit":{"message":"[Security
Solution] Enable actions in document details preview footer
(#203691)\n\n## Summary\r\n\r\nUpdated document details preview footer
to also show actions\r\n### Before\r\n<img width=\"1481\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/aace12aa-f3d1-4fe2-a3b7-392878207f39\"\r\n/>\r\n\r\n\r\n###
After \r\n- Users can perform alert/event actions in a
preview\r\n\r\n![image](https://github.com/user-attachments/assets/d42be26d-9d4a-4701-bc88-92549ebfb65c)\r\n\r\n-
In analyzer, when examining an event, event actions are also
available\r\n\r\n![image](https://github.com/user-attachments/assets/d30515d9-b428-4112-86f8-9bb872eaf921)\r\n\r\n-
No change to flyout in rule creation workflow, action is not
available\r\nin preview nor non-preview\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"7aac71c2aa5eb78f57bfe7bbb047104766b55a30"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: christineweng <[email protected]>
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
…elastic#203691)

## Summary

Updated document details preview footer to also show actions
### Before
<img width="1481" alt="image"
src="https://github.com/user-attachments/assets/aace12aa-f3d1-4fe2-a3b7-392878207f39"
/>


### After 
- Users can perform alert/event actions in a preview

![image](https://github.com/user-attachments/assets/d42be26d-9d4a-4701-bc88-92549ebfb65c)

- In analyzer, when examining an event, event actions are also available

![image](https://github.com/user-attachments/assets/d30515d9-b428-4112-86f8-9bb872eaf921)

- No change to flyout in rule creation workflow, action is not available
in preview nor non-preview



### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants