[Security Solution][Cases] Re-enable timeline functionality #96496

michaelolo24 · 2021-04-07T20:04:32Z

Summary

This PR reintroduces the functionality necessary for inserting timeline into cases via comments as well as encapsulating any UI timeline components. The views that utilize this functionality Create_Case and Case_Details now take in the below optional configuration. This logic will be able to be removed once timeline is moved into it's own plugin and we can import directly from there.

timelineIntegration?: {
	editor_plugins: {
		parsingPlugin,
		processingPluginRenderer,
		uiPlugin,
	},
	hooks: {
		useInsertTimeline,
	},
	ui?: {
		renderInvestigateInTimelineActionComponent,
		renderTimelineDetailsPanel,
	}

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-04-07T20:04:34Z

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

elasticmachine · 2021-04-07T20:04:34Z

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

stephmilovic · 2021-04-08T19:29:37Z

x-pack/plugins/cases/public/components/__mock__/timeline.tsx

+    useInsertTimeline: jest.fn(),
+  },
+  ui: {
+    renderInvestigateInTimelineActionComponent: (alertIds: string[]) =>


alertIds does not appear to be used?

removed, gracias

x-pack/plugins/cases/public/components/insert_timeline/index.tsx

x-pack/plugins/cases/public/components/markdown_editor/editor.tsx

stephmilovic · 2021-04-08T19:39:29Z

x-pack/plugins/cases/public/components/markdown_editor/types.ts

 export interface CursorPosition {
  start: number;
  end: number;
 }
+
+export type TemporaryProcessingPluginsType = [


ohh i see weird

cnasikas · 2021-04-12T08:57:12Z

@michaelolo24 @stephmilovic @XavierM I am wondering why we need to pass the UI markdown plugins as a prop in this PR? Shouldn't cases handle all the markdown plugins internally? If we need other plugins to define their own markdown plugins in cases, then I think we need to provide a markdown plugin registry as the actions plugin does with their connectors. An advantage of this approach is that cases do not have to define any plugin dependency (such as lens) in kibana.json and in the StartPlugins inside plugin.ts A disadvantaged of allowing other plugins to register their own markdown plugins is SDH support handling. Who gonna own the markdown plugin and who gonna support it? Cases or the plugin that registered it?

XavierM · 2021-04-12T13:00:10Z

@michaelolo24 @stephmilovic @XavierM I am wondering why we need to pass the UI markdown plugins as a prop in this PR? Shouldn't cases handle all the markdown plugins internally? If we need other plugins to define their own markdown plugins in cases, then I think we need to provide a markdown plugin registry as the actions plugin does with their connectors. An advantage of this approach is that cases do not have to define any plugin dependency (such as lens) in kibana.json and in the StartPlugins inside plugin.ts A disadvantaged of allowing other plugins to register their own markdown plugins is SDH support handling. Who gonna own the markdown plugin and who gonna support it? Cases or the plugin that registered it?

Let's discuss it, Also see another drawback with the registry that each time a plugin want to use case they will have to register an action for the EUI markdown, I think that will be annoying. I think for now I agree that cases should handle the markdown internally and maybe we should only pass props from security solution to have the functionality to attach a timeline.

michaelolo24 · 2021-04-12T13:31:11Z

@michaelolo24 @stephmilovic @XavierM I am wondering why we need to pass the UI markdown plugins as a prop in this PR? Shouldn't cases handle all the markdown plugins internally? If we need other plugins to define their own markdown plugins in cases, then I think we need to provide a markdown plugin registry as the actions plugin does with their connectors. An advantage of this approach is that cases do not have to define any plugin dependency (such as lens) in kibana.json and in the StartPlugins inside plugin.ts A disadvantaged of allowing other plugins to register their own markdown plugins is SDH support handling. Who gonna own the markdown plugin and who gonna support it? Cases or the plugin that registered it?

Let's discuss it, Also see another drawback with the registry that each time a plugin want to use case they will have to register an action for the EUI markdown, I think that will be annoying. I think for now I agree that cases should handle the markdown internally and maybe we should only pass props from security solution to have the functionality to attach a timeline.

Thanks @cnasikas & @XavierM, I'll update it to just pass down the timeline specific parts, rather than the whole plugins. Definitely cleaner that way. 👍🏾

x-pack/plugins/cases/public/components/all_cases/index.test.tsx

stephmilovic · 2021-04-13T14:21:30Z

x-pack/plugins/cases/public/components/case_view/index.tsx

-          useFetchAlertData={useFetchAlertData}
-          userCanCrud={userCanCrud}
-        />
+        <CasesTimelineIntegrationProvider timelineIntegration={timelineIntegration}>


I'm wondering if we should only render the CasesTimelineIntegrationProvider if timelineIntegration exists? I know you have the logic further down the line, but why go further down the line if we know timelineIntegration does not exist?

Because InsertTimeline always needs it? should we have a boolean where InsertTimeline is to render it or not depending on the timelineIntegration props? or is it simpler just to keep it as is

No it would still work without the provider since all the useTimelineContext()'s can be the default value of null. So I could do something like below and it would still work, but nothing would really be functionally different. The only difference would be CasesTimelineIntegrationContext not showing up in the tree anymore. It may be simpler to just keep it as is for now?

CasesTimelineIntegrationProvider = ({ children, timelineIntegration }) => { const [activeTimelineIntegration] = useState(timelineIntegration ?? null); return activeTimelineIntegration ? ( <CasesTimelineIntegrationContext.Provider value={activeTimelineIntegration}> {children} </CasesTimelineIntegrationContext.Provider> ) : ( <>{children}</> ); };

useTimelineContext is used for InsertTimeline, but also all the editor plugin stuff, and the renderInvestigateInTimeline and renderTimelineDetailsPanel logic. Ideally once the timeline plugin is done we can just nuke all of that stuff. You can test it out by just setting the useState here to null:

https://github.com/elastic/kibana/pull/96496/files/94a75a11d4240237d5225db9ea8d1fb81591996d#diff-9610a1614eed5937c6929340eb693370c84e283a021bf51217b49bc477e1ffecR58

yeah i went in an passed null from create to see. im good w this

stephmilovic · 2021-04-13T15:48:29Z

x-pack/plugins/cases/public/components/markdown_editor/use_plugins.ts

+export const usePlugins = () => {
+  const timelinePlugins = useTimelineContext()?.editor_plugins;
+
+  return useMemo(() => {


wondering if we should use useMemo here or not

I can get rid of the useMemo, I'd just need to move all the getDefault* references out of the hook so a new reference isn't created every single time the hook is called because that caused an infinite loop to happen before 😂. I initially wanted to avoid mutating them as singleton's to guard against any weird bugs that may come up. For example, if someone was to navigate from the cases in one app instance that does have timeline enabled directly to cases in another app instance that doesn't have it enabled. New plugin instances would probably be created in that scenario depending on how navigation happens, but wanted to avoid potential bugs there.

then leave it, i dont want a bug! lol

stephmilovic

LGTM! Thanks for your work on this one, I was definitely avoiding it 😂 rock n roll woohoo 🎸 🥁 🎺

kibanamachine · 2021-04-13T18:10:13Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 31b7f5b
Storybooks Preview
Flaky suites:
- xpack-securitySolutionCypressChrome

Test Failures

Kibana Pipeline / general / Creates and activates a new EQL rule with a sequence.Detection rules, sequence EQL Creates and activates a new EQL rule with a sequence

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has failed 7 times on tracked branches: https://github.com/elastic/kibana/issues/85691

AssertionError: Timed out retrying after 60000ms: Expected <span.euiLoadingContent> not to exist in the DOM, but it was continuously found.
    at Object.waitForAlertsPanelToBeLoaded (http://localhost:6121/__cypress/tests?p=cypress/integration/detection_rules/event_correlation_rule.spec.ts:21254:43)
    at Context.eval (http://localhost:6121/__cypress/tests?p=cypress/integration/detection_rules/event_correlation_rule.spec.ts:20314:18)

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [d2eaf3b]

History

💚 Build #119258 succeeded 3339aad
💚 Build #119155 succeeded 94a75a1
💔 Build #119002 failed fb6d973
💔 Build #118972 failed e205b8b050d4b614f5334223ffb84110ddfde0db
💔 Build #118953 failed f7423eb28231d13c380ec0111c1b2eed5e559a43

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

michaelolo24 requested a review from a team as a code owner April 7, 2021 20:04

michaelolo24 changed the title ~~[Security Solution][Cases]add timeline functionality~~ [Security Solution][Cases] Re-enable timeline functionality Apr 7, 2021

michaelolo24 requested a review from stephmilovic April 7, 2021 20:12

stephmilovic reviewed Apr 8, 2021

View reviewed changes

x-pack/plugins/cases/public/components/insert_timeline/index.tsx Outdated Show resolved Hide resolved

stephmilovic reviewed Apr 8, 2021

View reviewed changes

x-pack/plugins/cases/public/components/markdown_editor/editor.tsx Show resolved Hide resolved

stephmilovic reviewed Apr 8, 2021

View reviewed changes

x-pack/plugins/cases/public/components/markdown_editor/editor.tsx Outdated Show resolved Hide resolved

stephmilovic reviewed Apr 8, 2021

View reviewed changes

michaelolo24 force-pushed the finish_timeline_integration branch from b9a9959 to a029589 Compare April 12, 2021 17:30

michaelolo24 mentioned this pull request Apr 12, 2021

[SECURITY SOLUTIONS] [Case everywhere] Move Insert Timeline functionality to Cases #94697

Closed

michaelolo24 force-pushed the finish_timeline_integration branch from a029589 to f7423eb Compare April 12, 2021 19:31

michaelolo24 commented Apr 12, 2021

View reviewed changes

x-pack/plugins/cases/public/components/all_cases/index.test.tsx Outdated Show resolved Hide resolved

michaelolo24 added 5 commits April 12, 2021 16:53

add timeline functionality

06ce43b

remove redundant import

b83bbcc

hype type

2cbee21

pr updates

71d0652

reverted sort updated at

fb6d973

michaelolo24 force-pushed the finish_timeline_integration branch from e205b8b to fb6d973 Compare April 12, 2021 20:53

change updatedAt to createdAt in test

94a75a1

stephmilovic reviewed Apr 13, 2021

View reviewed changes

michaelolo24 added 2 commits April 13, 2021 10:37

Merge branch 'cases_rac_ui' into finish_timeline_integration

3339aad

re-add accidentally removed prop

31b7f5b

stephmilovic reviewed Apr 13, 2021

View reviewed changes

stephmilovic mentioned this pull request Apr 13, 2021

[SECURITY SOLUTIONS] [Case everywhere] Remaining cases removal work #96074

Closed

12 tasks

stephmilovic approved these changes Apr 13, 2021

View reviewed changes

michaelolo24 merged commit 636524a into elastic:cases_rac_ui Apr 13, 2021

michaelolo24 deleted the finish_timeline_integration branch April 13, 2021 18:11

michaelolo24 mentioned this pull request Apr 14, 2021

[SECURITY SOLUTIONS] Meta to move cases everywhere #90905

Closed

15 tasks

stephmilovic mentioned this pull request Apr 21, 2021

[Cases] Add Lens markdown plugin #96703

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Cases] Re-enable timeline functionality #96496

[Security Solution][Cases] Re-enable timeline functionality #96496

michaelolo24 commented Apr 7, 2021 •

edited

Loading

elasticmachine commented Apr 7, 2021

elasticmachine commented Apr 7, 2021

stephmilovic Apr 8, 2021

michaelolo24 Apr 12, 2021

stephmilovic Apr 8, 2021

cnasikas commented Apr 12, 2021 •

edited

Loading

XavierM commented Apr 12, 2021

michaelolo24 commented Apr 12, 2021

stephmilovic Apr 13, 2021

stephmilovic Apr 13, 2021

michaelolo24 Apr 13, 2021

stephmilovic Apr 13, 2021

stephmilovic Apr 13, 2021

michaelolo24 Apr 13, 2021

stephmilovic Apr 13, 2021

stephmilovic left a comment

kibanamachine commented Apr 13, 2021

Stack Trace

[Security Solution][Cases] Re-enable timeline functionality #96496

[Security Solution][Cases] Re-enable timeline functionality #96496

Conversation

michaelolo24 commented Apr 7, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Apr 7, 2021

elasticmachine commented Apr 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas commented Apr 12, 2021 • edited Loading

XavierM commented Apr 12, 2021

michaelolo24 commented Apr 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephmilovic left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 13, 2021

💛 Build succeeded, but was flaky

Stack Trace

Metrics [docs]

History

michaelolo24 commented Apr 7, 2021 •

edited

Loading

cnasikas commented Apr 12, 2021 •

edited

Loading