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

[michaelolo24]

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]

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

[elasticmachine]

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

[stephmilovic]

alertIds does not appear to be used?

[michaelolo24]

removed, gracias

[stephmilovic]

ohh i see weird

[cnasikas]

@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]

@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]

@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. 👍🏾

[stephmilovic]

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?

[stephmilovic]

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

[michaelolo24]

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

[stephmilovic]

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

[stephmilovic]

wondering if we should use useMemo here or not

[michaelolo24]

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.

[stephmilovic]

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]

💛 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