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

[RAC][Security Solution] Add to case actions in detail flyout #108057

Merged

Conversation

kqualters-elastic
Copy link
Contributor

@kqualters-elastic kqualters-elastic commented Aug 10, 2021

Summary

This pr breaks the existing add to case "action," the button, popover to select new or existing case, select existing case modal, and create new case flyout into 4 separate components. One for the existing case icon and popover, one for the button to launch the add to new case popover, one button to launch the add to existing case modal, and a component to render said modal or popover. State is shared between these 4 components via redux and the timelines plugin store. Note these changes are just to add the new functionality to the flyout, using in observability and the ux changes to the security solution table to be done in a forthcoming pr.
flyout_add_to_case

Checklist

  • Unit or functional tests were updated or added to match the most common scenarios
  • Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

@monina-n
Copy link

We will keep the flyout open when a modal appears on top of to keep it consistent with the add exception actions. However, when a user completes the action, the flyout should close and the confirmation toast should be standalone

  • User clicks take action
  • User selects add to existing case
  • Modal opens on top of flyout
  • User selects existing case
  • Modal closes, alert flyout closes, toast confirmation appears
Screen.Recording.2021-08-10.at.11.37.11.AM.mov

@angorayc
Copy link
Contributor

Hey @kqualters-elastic , thanks for the PR. I pulled and played around locally. Here are some observation I found:

  1. At the beginning, I set tGridEnabled to false, and the security solution wasn't working.
  2. After clicking on add to new case, are we opening the CreateCaseFlyout from timeline/body/actions/index.tsx line 188? Are we able to check if Observability would work? Would it be better to create a specific export for CreateCaseFlyout from timeline plugin, so Observability can use if from there if they haven't had one on the page?

@kqualters-elastic
Copy link
Contributor Author

Hey @kqualters-elastic , thanks for the PR. I pulled and played around locally. Here are some observation I found:

  1. At the beginning, I set tGridEnabled to false, and the security solution wasn't working.
  2. After clicking on add to new case, are we opening the CreateCaseFlyout from timeline/body/actions/index.tsx line 188? Are we able to check if Observability would work? Would it be better to create a specific export for CreateCaseFlyout from timeline plugin, so Observability can use if from there if they haven't had one on the page?

Thanks for checking it out. Yes, for #1 we just need to add a conditional check like in x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx#L137 . For #2, I'm working on how best to do this, the modal will have the same problem.

@kqualters-elastic kqualters-elastic added Feature:RAC label obsolete release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Theme: rac label obsolete v7.15.0 v8.0.0 labels Aug 12, 2021
@kqualters-elastic kqualters-elastic changed the title WIP add to case action in flyout [RAC][Security Solution] Add to case actions in detail flyout Aug 12, 2021
@kqualters-elastic kqualters-elastic marked this pull request as ready for review August 12, 2021 04:12
@kqualters-elastic kqualters-elastic requested review from a team as code owners August 12, 2021 04:12
@elasticmachine
Copy link
Contributor

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

useInsertTimeline: insertTimelineHook,
casePermissions,
appId: 'securitySolution',
onClose: onCaseSelection,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe naming: onCaseSelection: closePopoverHandler or afterCaseSelection

savedObjects: {
client: {},
},
timelines: {
Copy link
Contributor

Choose a reason for hiding this comment

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

we can put this in a common timelines mock file so this and events_viewer can share it

createCaseUrl,
isAllCaseModalOpen,
isCreateCaseFlyoutOpen,
} = useAddToCase({ ecsRowData, useInsertTimeline, casePermissions, appId, onClose });
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! 🎉

@@ -263,6 +71,8 @@ const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({
updateCase: onCaseSuccess,
userCanCrud: casePermissions?.crud ?? false,
owner: [appId],
onClose: () =>
dispatch(tGridActions.setOpenAddToExistingCase({ id: eventId, isOpen: false })),
Copy link
Contributor

Choose a reason for hiding this comment

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

Any issue with moving closeCaseFlyoutOpen up and using it here? And maybe we can keep the name a bit simpler with closeCaseFlyout?

Copy link
Contributor Author

@kqualters-elastic kqualters-elastic Aug 12, 2021

Choose a reason for hiding this comment

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

these are 2 different actions, setOpenAddToExistingCase here and setOpenAddToNewCase below

import { AddToCaseActionProps } from './add_to_case_action';
import * as i18n from './translations';

const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({
Copy link
Contributor

@michaelolo24 michaelolo24 Aug 12, 2021

Choose a reason for hiding this comment

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

I think you meant this to be AddtoExistingCaseActionComponent Ignore me, I see the exports at the bottom. 👍🏾

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2360 2361 +1
timelines 242 246 +4
total +5

Async chunks

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

id before after diff
cases 579.7KB 579.7KB +44.0B
securitySolution 6.5MB 6.5MB +1.2KB
timelines 267.6KB 308.6KB +41.0KB
total +42.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
timelines 306.8KB 309.5KB +2.8KB
Unknown metric groups

API count

id before after diff
cases 451 452 +1
timelines 930 936 +6
total +7

API count missing comments

id before after diff
cases 409 410 +1
timelines 810 816 +6
total +7

async chunk count

id before after diff
timelines 12 15 +3

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kqualters-elastic kqualters-elastic merged commit 1b88880 into elastic:master Aug 12, 2021
@kqualters-elastic kqualters-elastic deleted the timelines-actions-hooks branch August 12, 2021 17:45
kqualters-elastic added a commit to kqualters-elastic/kibana that referenced this pull request Aug 12, 2021
…c#108057)

* add to case action in flyout

* Fix most type errors

* Use context menu item instead of empty button for popover items

* Remove unused import

* Fire action on case modal close

* Update tests to use both components and remove console.log

* Update mocks in unit tests

* Use an onClose prop instead of closeCallbacks

* Pr feedback, create shared mock and rename handler

* Make app usable when timelines is not enabled

* Remove unused translations
kqualters-elastic added a commit that referenced this pull request Aug 12, 2021
… (#108422)

* add to case action in flyout

* Fix most type errors

* Use context menu item instead of empty button for popover items

* Remove unused import

* Fire action on case modal close

* Update tests to use both components and remove console.log

* Update mocks in unit tests

* Use an onClose prop instead of closeCallbacks

* Pr feedback, create shared mock and rename handler

* Make app usable when timelines is not enabled

* Remove unused translations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:RAC label obsolete release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Theme: rac label obsolete v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants