-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Security Solution] New Add Data Page #112142
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
Changes from all commits
dd77671
7922134
57c8667
c29c673
b6f22aa
566b111
c6243e4
6b587c5
99dd66a
5937a32
b128771
ded53c8
6503edd
d10b407
f000481
b3b827c
be141b9
e798745
545955b
5474b72
1e08ded
10c2ba2
7e172a6
ae18471
d6f4b3e
c5a7194
dc541c7
6d0eb4e
d2b2c1a
71ef616
f63620f
89bbb93
fdda9f2
e6741bf
0d27d45
13ce501
34394f7
d76cc52
7dd1a6e
7de5044
70dcec1
572b5f2
ae1aadf
aeed2c3
74475f7
f4f8f9b
708a2c8
2450e2b
27c39b7
c2577cf
67c1cc7
5e55a1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,10 @@ import { | |
| } from './bottom_bar'; | ||
| import { useShowTimeline } from '../../../common/utils/timeline/use_show_timeline'; | ||
| import { gutterTimeline } from '../../../common/lib/helpers'; | ||
| import { useSourcererScope } from '../../../common/containers/sourcerer'; | ||
| import { OverviewEmpty } from '../../../overview/components/overview_empty'; | ||
| import { ENDPOINT_METADATA_INDEX } from '../../../../common/constants'; | ||
| import { useFetchIndex } from '../../../common/containers/source'; | ||
|
|
||
| /* eslint-disable react/display-name */ | ||
|
|
||
|
|
@@ -73,11 +77,16 @@ export const SecuritySolutionTemplateWrapper: React.FC<SecuritySolutionPageWrapp | |
| const { show: isShowingTimelineOverlay } = useDeepEqualSelector((state) => | ||
| getTimelineShowStatus(state, TimelineId.active) | ||
| ); | ||
| const endpointMetadataIndex = useMemo<string[]>(() => { | ||
| return [ENDPOINT_METADATA_INDEX]; | ||
| }, []); | ||
| const [, { indexExists: metadataIndexExists }] = useFetchIndex(endpointMetadataIndex, true); | ||
| const { indicesExist } = useSourcererScope(); | ||
| const securityIndicesExist = indicesExist || metadataIndexExists; | ||
|
|
||
| /* StyledKibanaPageTemplate is a styled EuiPageTemplate. Security solution currently passes the header and page content as the children of StyledKibanaPageTemplate, as opposed to using the pageHeader prop, which may account for any style discrepancies, such as the bottom border not extending the full width of the page, between EuiPageTemplate and the security solution pages. | ||
| */ | ||
| // StyledKibanaPageTemplate is a styled EuiPageTemplate. Security solution currently passes the header and page content as the children of StyledKibanaPageTemplate, as opposed to using the pageHeader prop, which may account for any style discrepancies, such as the bottom border not extending the full width of the page, between EuiPageTemplate and the security solution pages. | ||
|
|
||
| return ( | ||
| return securityIndicesExist ? ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we're doing this check all the way up here, should we perhaps remove it from downstream? Ex: These are just a few spots. Also, what about the rules page? currently, we will show it even if no index exists
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephmilovic I'm happy to remove the checks downstream, however note that this check does include one more index with the metadata index, so show/hiding this Add Data screen has one additional check. Thoughts? Should we still remove all downstream checks?
We also have a couple pages that displayed without having data such as Trusted Apps and Event Filters. The intention here is to have a consistent "Empty State" across all apps. Based on this conversation with @cchaos I think the UX/Design team is open to more granular "Empty State" views in apps in the next iteration.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, the next step will the the in-page empty states. We're working our way through the user's journey. 😄
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, so if your check is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephmilovic yes, I see your point on things not being consistent. The condition should still be You can see it in this commit : 7de5044 let me know if this looks OK
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephmilovic and I spoke offline. We'd like the same empty state to still be available for the other pages in the event that we have Commit: 27c39b7 |
||
| <StyledKibanaPageTemplate | ||
| $isTimelineBottomBarVisible={isTimelineBottomBarVisible} | ||
| $isShowingTimelineOverlay={isShowingTimelineOverlay} | ||
|
|
@@ -98,5 +107,7 @@ export const SecuritySolutionTemplateWrapper: React.FC<SecuritySolutionPageWrapp | |
| {children} | ||
| </EuiPanel> | ||
| </StyledKibanaPageTemplate> | ||
| ) : ( | ||
| <OverviewEmpty /> | ||
| ); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,8 @@ | |
|
|
||
| import { i18n } from '@kbn/i18n'; | ||
|
|
||
| export const EMPTY_TITLE = i18n.translate('xpack.securitySolution.pages.common.emptyTitle', { | ||
| defaultMessage: 'Welcome to Elastic Security. Let’s get you started.', | ||
| export const SOLUTION_NAME = i18n.translate('xpack.securitySolution.pages.common.solutionName', { | ||
| defaultMessage: 'Security', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good suggestion here, I consolidated some of the translations where I thought it made sense semantically, for instance any translation that was using "Security" as an app or solution name. In some other areas I left it alone because it seemed more in context with a specific feature. |
||
| }); | ||
|
|
||
| export const EMPTY_ACTION_ELASTIC_AGENT = i18n.translate( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,30 +41,17 @@ describe('OverviewEmpty', () => { | |
| (useUserPrivileges as jest.Mock).mockReset(); | ||
| }); | ||
|
|
||
| test('render with correct actions ', () => { | ||
| expect(wrapper.find('[data-test-subj="empty-page"]').prop('actions')).toEqual({ | ||
| beats: { | ||
| description: | ||
| 'Lightweight Beats can send data from hundreds or thousands of machines and systems', | ||
| fill: false, | ||
| label: 'Add data with Beats', | ||
| url: '/app/home#/tutorial_directory/security', | ||
| }, | ||
| elasticAgent: { | ||
| description: | ||
| 'The Elastic Agent provides a simple, unified way to add monitoring to your hosts.', | ||
| fill: false, | ||
| label: 'Add data with Elastic Agent', | ||
| url: 'ingestUrl', | ||
| }, | ||
| endpoint: { | ||
| description: | ||
| 'Protect your hosts with threat prevention, detection, and deep security data visibility.', | ||
| fill: false, | ||
| label: 'Add Endpoint Security', | ||
| onClick: undefined, | ||
| url: `/integrations/endpoint-${endpointPackageVersion}/add-integration`, | ||
| it('render with correct actions ', () => { | ||
| expect(wrapper.find('[data-test-subj="empty-page"]').prop('noDataConfig')).toEqual({ | ||
| actions: { | ||
| elasticAgent: { | ||
| description: | ||
| 'Use Elastic Agent to collect security events and protect your endpoints from threats. Manage your agents in Fleet and add integrations with a single click.', | ||
| href: '/app/integrations/browse/security', | ||
| }, | ||
| }, | ||
| docsLink: 'https://www.elastic.co/guide/en/security/mocked-test-branch/index.html', | ||
| solution: 'Security', | ||
| }); | ||
| }); | ||
| }); | ||
|
|
@@ -78,15 +65,15 @@ describe('OverviewEmpty', () => { | |
| wrapper = shallow(<OverviewEmpty />); | ||
| }); | ||
|
|
||
| test('render with correct actions ', () => { | ||
| expect(wrapper.find('[data-test-subj="empty-page"]').prop('actions')).toEqual({ | ||
| beats: { | ||
| description: | ||
| 'Lightweight Beats can send data from hundreds or thousands of machines and systems', | ||
| fill: false, | ||
| label: 'Add data with Beats', | ||
| url: '/app/home#/tutorial_directory/security', | ||
| it('render with correct actions ', () => { | ||
| expect(wrapper.find('[data-test-subj="empty-page"]').prop('noDataConfig')).toEqual({ | ||
| actions: { | ||
| beats: { | ||
| href: '/app/home#/tutorial_directory/security', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Thanks for the tests |
||
| }, | ||
| }, | ||
| docsLink: 'https://www.elastic.co/guide/en/security/mocked-test-branch/index.html', | ||
| solution: 'Security', | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,105 +6,57 @@ | |
| */ | ||
|
|
||
| import React, { useMemo } from 'react'; | ||
| import { omit } from 'lodash/fp'; | ||
| import { createStructuredSelector } from 'reselect'; | ||
|
|
||
| import { FormattedMessage } from '@kbn/i18n/react'; | ||
| import { EuiLink } from '@elastic/eui'; | ||
| import * as i18nCommon from '../../../common/translations'; | ||
| import { EmptyPage, EmptyPageActionsProps } from '../../../common/components/empty_page'; | ||
| import { i18n } from '@kbn/i18n'; | ||
| import { useKibana } from '../../../common/lib/kibana'; | ||
| import { ADD_DATA_PATH } from '../../../../common/constants'; | ||
| import { | ||
| useEndpointSelector, | ||
| useIngestUrl, | ||
| } from '../../../management/pages/endpoint_hosts/view/hooks'; | ||
| import { useNavigateToAppEventHandler } from '../../../common/hooks/endpoint/use_navigate_to_app_event_handler'; | ||
| import { CreateStructuredSelector } from '../../../common/store'; | ||
| import { endpointPackageVersion as useEndpointPackageVersion } from '../../../management/pages/endpoint_hosts/store/selectors'; | ||
| import { pagePathGetters } from '../../../../../fleet/public'; | ||
| import { SOLUTION_NAME } from '../../../../public/common/translations'; | ||
| import { useUserPrivileges } from '../../../common/components/user_privileges'; | ||
|
|
||
| import { | ||
| KibanaPageTemplate, | ||
| NoDataPageActionsProps, | ||
| } from '../../../../../../../src/plugins/kibana_react/public'; | ||
|
|
||
| const OverviewEmptyComponent: React.FC = () => { | ||
| const { http, docLinks } = useKibana().services; | ||
| const basePath = http.basePath.get(); | ||
| const selector = (createStructuredSelector as CreateStructuredSelector)({ | ||
| endpointPackageVersion: useEndpointPackageVersion, | ||
| }); | ||
| const { endpointPackageVersion } = useEndpointSelector(selector); | ||
| const { url: ingestUrl } = useIngestUrl(''); | ||
|
|
||
| const endpointIntegrationUrlPath = endpointPackageVersion | ||
| ? `/endpoint-${endpointPackageVersion}/add-integration` | ||
| : ''; | ||
| const endpointIntegrationUrl = `/integrations${endpointIntegrationUrlPath}`; | ||
| const handleEndpointClick = useNavigateToAppEventHandler('fleet', { | ||
| path: endpointIntegrationUrl, | ||
| }); | ||
| const canAccessFleet = useUserPrivileges().endpointPrivileges.canAccessFleet; | ||
| const integrationsPathComponents = pagePathGetters.integrations_all({ category: 'security' }); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm using this to get the integrations + category link for use with this "Add Data" screen. The URL we're picking up is This navigates us to the Integrations page, however, it doesn't seem the The result I see is that it navigates to the integrations page, but it drops the If I selected Security, I do see After refresh, Security is dropped: Is there a bug here? Should I be able to navigate to the Integrations page with a Category selected?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is almost certainly a bug, you should definitely be able to browse to a specific category given the URL path. could you file a separate issue? thanks in advance!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @jen-huang - I filed the above in this ticket: #112871 @cchaos note on the above - as it stands, this navigation won't sort the integrations by Security until the above bug is fixed. The navigation still works, but the user won't see the Security integrations sorted. We could merge this as is and it should work after the bug is fixed. Thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. Let's not block this PR because of the routing bug.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI this bug was fixed so this should work now. let me know if you still encounter issues
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks @jen-huang ! Just tested it and it works great! |
||
|
|
||
| const emptyPageActions: EmptyPageActionsProps = useMemo( | ||
| const agentAction: NoDataPageActionsProps = useMemo( | ||
| () => ({ | ||
| elasticAgent: { | ||
| label: i18nCommon.EMPTY_ACTION_ELASTIC_AGENT, | ||
| url: ingestUrl, | ||
| description: i18nCommon.EMPTY_ACTION_ELASTIC_AGENT_DESCRIPTION, | ||
| fill: false, | ||
| }, | ||
| beats: { | ||
| label: i18nCommon.EMPTY_ACTION_BEATS, | ||
| url: `${basePath}${ADD_DATA_PATH}`, | ||
| description: i18nCommon.EMPTY_ACTION_BEATS_DESCRIPTION, | ||
| fill: false, | ||
| }, | ||
| endpoint: { | ||
| label: i18nCommon.EMPTY_ACTION_ENDPOINT, | ||
| url: endpointIntegrationUrl, | ||
| description: i18nCommon.EMPTY_ACTION_ENDPOINT_DESCRIPTION, | ||
| onClick: handleEndpointClick, | ||
| fill: false, | ||
| href: `${basePath}${integrationsPathComponents[0]}${integrationsPathComponents[1]}`, | ||
| description: i18n.translate( | ||
| 'xpack.securitySolution.pages.emptyPage.beatsCard.description', | ||
| { | ||
| defaultMessage: | ||
| 'Use Elastic Agent to collect security events and protect your endpoints from threats. Manage your agents in Fleet and add integrations with a single click.', | ||
| } | ||
| ), | ||
| }, | ||
| }), | ||
| [basePath, ingestUrl, endpointIntegrationUrl, handleEndpointClick] | ||
| [basePath, integrationsPathComponents] | ||
| ); | ||
|
|
||
| const emptyPageIngestDisabledActions = useMemo( | ||
| () => omit(['elasticAgent', 'endpoint'], emptyPageActions), | ||
| [emptyPageActions] | ||
| const beatsAction: NoDataPageActionsProps = useMemo( | ||
| () => ({ | ||
| beats: { | ||
| href: `${basePath}${ADD_DATA_PATH}`, | ||
| }, | ||
| }), | ||
| [basePath] | ||
| ); | ||
|
|
||
| return canAccessFleet === true ? ( | ||
| <EmptyPage | ||
| actions={emptyPageActions} | ||
| data-test-subj="empty-page" | ||
| message={ | ||
| <> | ||
| <FormattedMessage | ||
| id="xpack.securitySolution.emptyMessage" | ||
| defaultMessage="Elastic Security integrates the free and open Elastic SIEM with Endpoint Security to prevent, detect, and respond to threats. To begin, you’ll need to add security solution related data to the Elastic Stack. For additional information, you can view our " | ||
| /> | ||
| <EuiLink href={docLinks.links.siem.gettingStarted} target="_blank"> | ||
| {i18nCommon.EMPTY_ACTION_SECONDARY} | ||
| </EuiLink> | ||
| </> | ||
| } | ||
| title={i18nCommon.EMPTY_TITLE} | ||
| /> | ||
| ) : ( | ||
| <EmptyPage | ||
| actions={emptyPageIngestDisabledActions} | ||
| return ( | ||
| <KibanaPageTemplate | ||
| data-test-subj="empty-page" | ||
| message={ | ||
| <> | ||
| <FormattedMessage | ||
| id="xpack.securitySolution.emptyMessage" | ||
| defaultMessage="Elastic Security integrates the free and open Elastic SIEM with Endpoint Security to prevent, detect, and respond to threats. To begin, you’ll need to add security solution related data to the Elastic Stack. For additional information, you can view our " | ||
| /> | ||
| <EuiLink href={docLinks.links.siem.gettingStarted} target="_blank"> | ||
| {i18nCommon.EMPTY_ACTION_SECONDARY} | ||
| </EuiLink> | ||
| </> | ||
| } | ||
| title={i18nCommon.EMPTY_TITLE} | ||
| noDataConfig={{ | ||
| solution: SOLUTION_NAME, | ||
| actions: canAccessFleet ? agentAction : beatsAction, | ||
| docsLink: docLinks.links.siem.gettingStarted, | ||
| }} | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||





Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw,
react/display-namewas originally added because we weren't setting the react display name and we added a linter rule for it and disabled it as a fast way to get it into the codebase. However, this linter rule where possible should be removed. You don't have to do it here, but just mentioning it. Information is here: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/display-name.mdBasically it helps us debug React applications. (Again background info, you don't have to do anything in this PR).
...But with this enabled at the file level it makes it easy for devs to continue to create things without the name being set...