[Security Solution] New Add Data Page #108763
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 tasks
|
The google doc shows this text for the Security app, but I wonder if the second line is too much. Title: Add Elastic Agent |
|
Oi, yep, I put it in the wrong place. 🤦 Updated. |
12 tasks
|
@elasticmachine merge upstream |
src/plugins/kibana_react/public/page_template/no_data_page/no_data_card/elastic_beats_card.tsx
Outdated
Show resolved
Hide resolved
| const beatsAction: NoDataPageActionsProps = useMemo( | ||
| () => ({ | ||
| beats: { | ||
| href: `${basePath}${ADD_DATA_PATH}`, |
There was a problem hiding this comment.
Did we want to have a description for the beats link like we do for elasticAgent?
There was a problem hiding this comment.
There is a default message for both of these links/cards. So only if you want to differentiate from that which is:
Use Beats to add data from various systems to Elasticsearch.
| @@ -19,6 +19,7 @@ export type ElasticBeatsCardProps = NoDataPageActions & { | |||
|
|
|||
| export const ElasticBeatsCard: FunctionComponent<ElasticBeatsCardProps> = ({ | |||
There was a problem hiding this comment.
Also can we add a data-test-subj property to the card so we can reference the card in the tests?
There was a problem hiding this comment.
It should inherit all the same props from EuiCard which does support this. Have you tried adding it?
…data_card/elastic_beats_card.tsx
|
Also note: I'm handing this PR over to the Security Solutions team. Please feel free to edit/push to directly. Or grab it and restart a new PR. |
|
@elasticmachine merge upstream |
21 tasks
epixa
added
the
Team:Defend Workflows
|
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
kevinlog
reviewed
Sep 9, 2021
| @@ -75,10 +77,12 @@ export const SecuritySolutionTemplateWrapper: React.FC<SecuritySolutionPageWrapp | |||
| getTimelineShowStatus(state, TimelineId.active) | |||
| ); | |||
|
|
|||
| const { indicesExist } = useSourcererScope(); | |||
There was a problem hiding this comment.
@michaelolo24 @jonathan-buttner - to get the tests to pass, I believe this indicesExist needs to resolve to true when the metrics-endpoint.metadata index exists in addition to any of the other indices that this already tracks.
The issue is the new empty state covers the entire app, including the Endpoint management page which tracks just Endpoint metadata.
I'm happy to help make the change, just wanted to point this out. I can work with you offline.
There was a problem hiding this comment.
@kevinlog yea, that might be a good idea. I know @stephmilovic is making updates to a lot of the sourcerer stuff now, so might be good to sync up with her as well
There was a problem hiding this comment.
I spoke with @stephmilovic offline - a potential solution is to use the useFetchIndex hook in combination with this check in order to check for the existence of metrics-endpoint.metadata. I can take a closer look at this a bit later.
[Security Solution] Fix tests for new data screen and update index check
|
@elasticmachine merge upstream |
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / general / task-queue-process-11 / X-Pack Endpoint Functional Tests.x-pack/test/security_solution_endpoint/apps/endpoint/trusted_apps_list·ts.endpoint When on the Trusted Apps list should show page titleStandard OutStack TraceKibana Pipeline / general / task-queue-process-11 / X-Pack Endpoint Functional Tests.x-pack/test/security_solution_endpoint/apps/endpoint/trusted_apps_list·ts.endpoint When on the Trusted Apps list should show page titleStandard OutStack TraceKibana Pipeline / general / "before each" hook for "Closes and opens alerts".Closing alerts "before each" hook for "Closes and opens alerts"Stack Traceand 12 more failures, only showing the first 3. Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Some comments (maybe for @kevinlog who I think is making changes to endpoint related files)
| @@ -16,6 +16,7 @@ export const APP_ICON = 'securityAnalyticsApp'; | |||
| export const APP_ICON_SOLUTION = 'logoSecurity'; | |||
| export const APP_PATH = `/app/security`; | |||
| export const ADD_DATA_PATH = `/app/home#/tutorial_directory/security`; | |||
| export const ADD_INTEGRATION_PATH = `/app/integrations/browse?category=security`; | |||
There was a problem hiding this comment.
This path seems to be invalid.
Suggestion: instead of hardcoding the path, use Fleet's path getter method which is exported out of the public folder:
import {pagePathGetters} from '../path/to/fleet/plugin/public'if wanting to land on the all integration page and search/filter by security:
Suggested change
| export const ADD_INTEGRATION_PATH = `/app/integrations/browse?category=security`; | |
| export const ADD_INTEGRATION_PATH = pagePathGetters.integration_all({searchTerm: 'security'}; |
If wanting to land on the all integration page showing only the security category integrations:
Suggested change
| export const ADD_INTEGRATION_PATH = `/app/integrations/browse?category=security`; | |
| export const ADD_INTEGRATION_PATH = pagePathGetters.integration_all({ category: 'security'}; |
and finally, if wanting to show only specific types of integrations on the Security category, you can combine the above:
Suggested change
| export const ADD_INTEGRATION_PATH = `/app/integrations/browse?category=security`; | |
| export const ADD_INTEGRATION_PATH = pagePathGetters.integration_all({searchTerm: 'security', searchTerm: 'endpoint'}; |
There was a problem hiding this comment.
Thanks @paul-tavares , looks like the category version is what we want based on the decisions made in #107682
There was a problem hiding this comment.
I've moved this suggestion to the new PR: #112142 (comment)
|
|
||
| /* 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 ? ( |
There was a problem hiding this comment.
🤔 I'm wondering if this is the right place for this change to occur, which impact all areas of security solution.
For endpoint, one might (maybe?) want to start configuring policies (ex. trusted applications, event filters, etc.) before any data is available in the Endpoint metadata index. This change makes it impossible for the user to do that until at least one of the checked indexes has data in it.
Do I have that right?
I'm thinking that each section of security solution should problaby have its won way to determine if the onboarding page should be displayed or not.
cc/ @kevinlog
There was a problem hiding this comment.
In general I agree, but for now we just need something up. This suggestion would require a lot more coordination that I"m not sure we have time for in 7.16.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In support of #107682 and pulled out of original PR #107709
Replaces the current OverviewEmpty contents with the
KibanaPageTemplate.noDataConfig.It also adds logic to the main TemplateWrapper where it checks for
indicesExistfromuseSourcererScope()to show the OverviewEmpty page over any other page contents.The links are:
learn more: https://www.elastic.co/guide/en/security/master/index.htmlAdd Elastic Agent: /app/integrations/browse?category=securityChecklist
Delete any items that are not applicable to this PR.