[SIEM] Fix Detections page breadcrumbs #55173

patrykkopycinski · 2020-01-17T13:07:24Z

Summary

#55127 has to go first

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

…d-alerts

elasticmachine · 2020-01-17T13:07:27Z

Pinging @elastic/siem (Team:SIEM)

…s-breadcrumbs # Conflicts: # x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_detection_engine.tsx # x-pack/legacy/plugins/siem/public/pages/detection_engine/detection_engine.tsx # x-pack/legacy/plugins/siem/public/pages/detection_engine/index.tsx # x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/details/index.tsx

…s-breadcrumbs

XavierM · 2020-01-21T16:01:13Z

I think that we need to add create's route in the breadcrumb too

…s-breadcrumbs

XavierM · 2020-01-21T16:09:54Z

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/utils.ts

+  if (tabPath === 'alerts') {
+    return {
+      text: i18nDetections.ALERT,
+      href: `${getDetectionEngineTabUrl(tabPath)}${search && search[0] ? search[0] : ''}`,


nit we can use isEmpty(search)? '' : search[0]

XavierM · 2020-01-21T16:10:54Z

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/utils.ts

+    ];
+  }
+
+  if (isRuleEditPage(params.pathName) && params.detailName && params.state?.ruleName) {


and isRuleCreatePage ;)

XavierM · 2020-01-21T16:11:23Z

x-pack/legacy/plugins/siem/public/utils/route/spy_routes.tsx

@@ -19,6 +20,7 @@ export const SpyRouteComponent = memo<SpyRouteProps & { location: H.Location }>(
    match: {
      params: { pageName, detailName, tabName, flowTarget },
    },
+    state,


good idea to add a state here

XavierM · 2020-01-21T16:14:26Z

x-pack/legacy/plugins/siem/public/utils/route/types.ts

@@ -55,4 +59,6 @@ export type SpyRouteProps = RouteComponentProps<{
  tabName: HostsTableType | undefined;
  search: string;
  flowTarget: FlowTarget | undefined;
-}>;
+}> & {
+  state?: Record<string, string | undefined>;


is it a good idea to allow a Record with key string with an undefined value. I feel like if the value is undefined we should not add the key in the object.

https://github.com/elastic/kibana/pull/55173/files/9ac7b15d9bd942c3ddf040ece037cd980b9ea000#diff-f0513e4cfb90441e54375f10a9f0157aR350
in case we didn't allow for undefined then in a case like this we would have to have additional logic to don't pass anything or if only one of the values would be undefined then we would need to filter those keys out, so I think it will be easier to work with to allow undefined values

or we can just do that <SpyRoute state={{ ruleName: rule?.name ?? 'unknown' }} /> || <SpyRoute state={{ ruleName: rule?.name ?? '' }} />

yeah, but then we need to have additional logic to handle empty strings or 'unknown' string case

XavierM

LGTM, thank you for adding the breadcrumbs for detections.

kibanamachine · 2020-01-21T17:52:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5f359c8

History

💚 Build #21514 succeeded 9ac7b15
💔 Build #21433 failed 8c0969d
💔 Build #21412 failed 9a6066d
💔 Build #21377 failed f199549
💔 Build #21358 failed b523b30

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

XavierM added 4 commits January 16, 2020 16:28

add alert on detections

7660edb

Merge branch 'master' of github.com:elastic/kibana into detections-ad…

12395be

…d-alerts

Merge branch 'master' of github.com:elastic/kibana into detections-ad…

edc92d9

…d-alerts

review I + fix unit test

b3e3929

patrykkopycinski added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v7.6.0 labels Jan 17, 2020

patrykkopycinski self-assigned this Jan 17, 2020

XavierM and others added 3 commits January 17, 2020 09:42

review II

de3af29

review III

50b760a

[SIEM] Add Detections breadcrumbs

4a78bb8

patrykkopycinski force-pushed the fix/detections-breadcrumbs branch from 33460a7 to 4a78bb8 Compare January 17, 2020 18:15

patrykkopycinski changed the title ~~[SIEM[ Fix Detections page breadcrumbs~~ [SIEM Fix Detections page breadcrumbs Jan 17, 2020

patrykkopycinski changed the title ~~[SIEM Fix Detections page breadcrumbs~~ [SIEM] Fix Detections page breadcrumbs Jan 17, 2020

patrykkopycinski added 6 commits January 20, 2020 12:27

fix types

f199549

fix tests

9a6066d

Merge branch 'master' of github.com:elastic/kibana into fix/detection…

8c0969d

…s-breadcrumbs

Merge branch 'master' of github.com:elastic/kibana into fix/detection…

5a4c1e8

…s-breadcrumbs

yarn.lock

9ac7b15

Merge branch 'master' of github.com:elastic/kibana into fix/detection…

59e5f3d

…s-breadcrumbs

XavierM reviewed Jan 21, 2020

View reviewed changes

add create page breadcrumb

5f359c8

XavierM approved these changes Jan 21, 2020

View reviewed changes

patrykkopycinski merged commit fc21c49 into elastic:master Jan 21, 2020

patrykkopycinski deleted the fix/detections-breadcrumbs branch January 21, 2020 18:20

patrykkopycinski mentioned this pull request Jan 21, 2020

[7.x] [SIEM] Fix Detections page breadcrumbs (#55173) #55448

Merged

patrykkopycinski added a commit that referenced this pull request Jan 21, 2020

[SIEM] Fix Detections page breadcrumbs (#55173) (#55448)

ce34962

patrykkopycinski mentioned this pull request Jan 21, 2020

[7.6] [SIEM] Fix Detections page breadcrumbs (#55173) #55478

Merged

XavierM pushed a commit that referenced this pull request Jan 22, 2020

[SIEM] Fix Detections page breadcrumbs (#55173) (#55478)

e48317f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIEM] Fix Detections page breadcrumbs #55173

[SIEM] Fix Detections page breadcrumbs #55173

patrykkopycinski commented Jan 17, 2020 •

edited

Loading

elasticmachine commented Jan 17, 2020

XavierM commented Jan 21, 2020

XavierM Jan 21, 2020

XavierM Jan 21, 2020

patrykkopycinski Jan 21, 2020

XavierM Jan 21, 2020

XavierM Jan 21, 2020

patrykkopycinski Jan 21, 2020

XavierM Jan 21, 2020

patrykkopycinski Jan 21, 2020

XavierM left a comment

kibanamachine commented Jan 21, 2020

[SIEM] Fix Detections page breadcrumbs #55173

[SIEM] Fix Detections page breadcrumbs #55173

Conversation

patrykkopycinski commented Jan 17, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jan 17, 2020

XavierM commented Jan 21, 2020

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

Choose a reason for hiding this comment

XavierM left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 21, 2020

💚 Build Succeeded

History

patrykkopycinski commented Jan 17, 2020 •

edited

Loading