Skip to content
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.

Sign up for GitHub

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

[Endpoint] Host Details Policy Response Panel #63518

Merged
merged 17 commits into from
Apr 16, 2020
Merged

[Endpoint] Host Details Policy Response Panel #63518

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9950f9b
File/code restructure + policy response component placeholder
paul-tavares Apr 14, 2020
a79bc42
Added link to Policy status that updates URL and show details panel
paul-tavares Apr 14, 2020
a6ea3fa
Policy Response panel title + back to details link
paul-tavares Apr 14, 2020
9e6c3f5
Move sub-panel title to EuiFlyoutHeader
paul-tavares Apr 14, 2020
ed9f2c5
label change
paul-tavares Apr 14, 2020
8b5182d
Custom Styled Flyout Panel sub-header component
paul-tavares Apr 14, 2020
72e1c5e
Fix type
paul-tavares Apr 14, 2020
0c9df52
Move Middleware spy utils under `store/` for re-use
paul-tavares Apr 15, 2020
f00bcbb
Changed `appStoreFactory()` to accept optional `additionalMiddleware`…
paul-tavares Apr 15, 2020
9b45f61
`waitForAction` middleware test utility now return Action on Promise …
paul-tavares Apr 15, 2020
11e6f0c
Test cases for Host Details Policy Response panel
paul-tavares Apr 15, 2020
135e014
Fix Functional test case
paul-tavares Apr 15, 2020
469655f
Merge remote-tracking branch 'upstream/master' into task/EMT-163-poli…
paul-tavares Apr 15, 2020
a29a22f
Fixes post-master merge
paul-tavares Apr 15, 2020
9069a40
Simplified the types for `waitForAction`
paul-tavares Apr 16, 2020
9bfbb22
Updated PageView component to remove bottom margin
paul-tavares Apr 16, 2020
c99b360
Merge remote-tracking branch 'upstream/master' into task/EMT-163-poli…
paul-tavares Apr 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion x-pack/plugins/endpoint/public/applications/endpoint/store/hosts/selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const uiQueryParams: (
// Removes the `?` from the beginning of query string if it exists
const query = querystring.parse(location.search.slice(1));

const keys: Array<keyof HostIndexUIQueryParams> = ['selected_host'];
const keys: Array<keyof HostIndexUIQueryParams> = ['selected_host', 'show'];

for (const key of keys) {
const value = query[key];
Expand All @@ -58,3 +58,11 @@ export const hasSelectedHost: (state: HostListState) => boolean = createSelector
return selectedHost !== undefined;
}
);

/** What policy details panel view to show */
export const showView: (state: HostListState) => 'policy_response' | 'details' = createSelector(
uiQueryParams,
searchParams => {
return searchParams.show === 'policy_response' ? 'policy_response' : 'details';
}
);
1 change: 1 addition & 0 deletions x-pack/plugins/endpoint/public/applications/endpoint/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export interface HostListPagination {
}
export interface HostIndexUIQueryParams {
selected_host?: string;
show?: string;
}

export interface ServerApiError {
Expand Down
72 changes: 72 additions & 0 deletions ...endpoint/public/applications/endpoint/view/hosts/details/components/flyout_sub_header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { memo } from 'react';
import { EuiFlyoutHeader, CommonProps, EuiButtonEmpty } from '@elastic/eui';
import styled from 'styled-components';

export type FlyoutSubHeaderProps = CommonProps & {
children: React.ReactNode;
backButton?: {
title: string;
onClick: (event: React.MouseEvent) => void;
href?: string;
};
};

const StyledEuiFlyoutHeader = styled(EuiFlyoutHeader)`
padding: ${props => props.theme.eui.paddingSizes.s};

&.hasButtons {
.buttons {
padding-bottom: ${props => props.theme.eui.paddingSizes.s};
}

.back-button-content {
padding-left: 0;
&-text {
margin-left: 0;
}
}
}

.flyout-content {
padding-left: ${props => props.theme.eui.paddingSizes.m};
}
`;

const BUTTON_CONTENT_PROPS = Object.freeze({ className: 'back-button-content' });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question about why you have to object.freeze this object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am defining this constant at the module level and want to ensure it is never mutated accidentally, since that would impact future instantiations of the component.
Maybe I should have also used a type to set the value as Immutable.

const BUTTON_TEXT_PROPS = Object.freeze({ className: 'back-button-content-text' });

/**
* A Eui Flyout Header component that has its styles adjusted to display a panel sub-header.
* Component also provides a way to display a "back" button above the header title.
*/
export const FlyoutSubHeader = memo<FlyoutSubHeaderProps>(
({ children, backButton, ...otherProps }) => {
return (
<StyledEuiFlyoutHeader hasBorder {...otherProps} className={backButton && `hasButtons`}>
{backButton && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of having a second back button?

Copy link
Contributor Author

@paul-tavares paul-tavares Apr 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is only 1 back button - if you look at the screen capture above, you can see the link in the sub-panel. perhaps if I rename the prop to leftLink it would be clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also - FYI: I made this a separate component because I think we'll have more use cases we will need it - at least that seems to be true for Host details. Once we have that need, we should likely move the component to a higher directory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was thinking 'second' as in addition to the browser back button. does this do the same thing? or is it different

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oatkiller

does this do the same thing? or is it different

Yes and No :)
The link (in the current usage by Host Policy Response panel) will always take you to the Host Details panel. If user navigated to the Policy Response panel from the host details, then yes, the behaviour would be the same as the browser back button. But since this panel is driven by a URL param, the user might have also gotten there by using the URL directly - in that case, this link would (possibly) not match the behavior of the browser's back button.

<div className="buttons">
{/* eslint-disable-next-line @elastic/eui/href-or-on-click */}
<EuiButtonEmpty
data-test-subj="flyoutSubHeaderBackButton"
iconType="arrowLeft"
contentProps={BUTTON_CONTENT_PROPS}
textProps={BUTTON_TEXT_PROPS}
size="xs"
href={backButton?.href ?? ''}
onClick={backButton?.onClick}
>
{backButton?.title}
</EuiButtonEmpty>
</div>
)}
<div className={'flyout-content'}>{children}</div>
</StyledEuiFlyoutHeader>
);
}
);
137 changes: 50 additions & 87 deletions ...lications/endpoint/view/hosts/details.tsx → ...point/view/hosts/details/host_details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,25 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useCallback, useMemo, memo, useEffect } from 'react';
import styled from 'styled-components';
import {
EuiFlyout,
EuiFlyoutBody,
EuiFlyoutHeader,
EuiTitle,
EuiDescriptionList,
EuiLoadingContent,
EuiHorizontalRule,
EuiHealth,
EuiSpacer,
EuiHorizontalRule,
EuiLink,
EuiListGroup,
EuiListGroupItem,
} from '@elastic/eui';
import { useHistory } from 'react-router-dom';
import styled from 'styled-components';
import { i18n } from '@kbn/i18n';
import React, { memo, useMemo } from 'react';
import { FormattedMessage } from '@kbn/i18n/react';
import { useKibana } from '../../../../../../../../src/plugins/kibana_react/public';
import { HostMetadata } from '../../../../../common/types';
import { useHostListSelector } from './hooks';
import { urlFromQueryParams } from './url_from_query_params';
import { FormattedDateAndTime } from '../formatted_date_time';
import { uiQueryParams, detailsData, detailsError } from './../../store/hosts/selectors';
import { LinkToApp } from '../components/link_to_app';
import { i18n } from '@kbn/i18n';
import { useHistory } from 'react-router-dom';
import { HostMetadata } from '../../../../../../common/types';
import { FormattedDateAndTime } from '../../formatted_date_time';
import { LinkToApp } from '../../components/link_to_app';
import { useHostListSelector, useHostLogsUrl } from '../hooks';
import { urlFromQueryParams } from '../url_from_query_params';
import { uiQueryParams } from '../../../store/hosts/selectors';

const HostIds = styled(EuiListGroupItem)`
margin-top: 0;
Expand All @@ -37,8 +31,10 @@ const HostIds = styled(EuiListGroupItem)`
}
`;

const HostDetails = memo(({ details }: { details: HostMetadata }) => {
export const HostDetails = memo(({ details }: { details: HostMetadata }) => {
const { appId, appPath, url } = useHostLogsUrl(details.host.id);
const queryParams = useHostListSelector(uiQueryParams);
const history = useHistory();
const detailsResultsUpper = useMemo(() => {
return [
{
Expand All @@ -62,6 +58,14 @@ const HostDetails = memo(({ details }: { details: HostMetadata }) => {
];
}, [details]);

const policyResponseUri = useMemo(() => {
return urlFromQueryParams({
...queryParams,
selected_host: details.host.id,
show: 'policy_response',
});
}, [details.host.id, queryParams]);

const detailsResultsLower = useMemo(() => {
return [
{
Expand All @@ -74,7 +78,24 @@ const HostDetails = memo(({ details }: { details: HostMetadata }) => {
title: i18n.translate('xpack.endpoint.host.details.policyStatus', {
defaultMessage: 'Policy Status',
}),
description: <EuiHealth color="success">active</EuiHealth>,
description: (
<EuiHealth color="success">
{/* eslint-disable-next-line @elastic/eui/href-or-on-click */}
<EuiLink
data-test-subj="hostnameCellLink"
href={'?' + policyResponseUri.search}
onClick={(ev: React.MouseEvent) => {
ev.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this use the same logic as we do here: https://github.com/elastic/kibana/blob/master/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_to_app_event_handler.ts#L42

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - its different here. In this case, its not a navigate to app, but rather a direct router history update. @kevinlog is assigned to issue 230 which will (I assume) add an additional hook similar to the one you referenced above, but that will use react-router's history.push.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paul-tavares gotcha.

re: using the same logic, I meant should ev.preventDefault() here be replaced with:

 try {
        if (onClick) {
          onClick(ev);
        }
      } catch (error) {
        ev.preventDefault();
        throw error;
      }

      if (ev.defaultPrevented) {
        return;
      }

      if (ev.button !== 0) {
        return;
      }

      if (
        ev.currentTarget instanceof HTMLAnchorElement &&
        ev.currentTarget.target !== '' &&
        ev.currentTarget.target !== '_self'
      ) {
        return;
      }

      if (ev.metaKey || ev.altKey || ev.ctrlKey || ev.shiftKey) {
        return;
      }

      ev.preventDefault();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about that. I'm hoping we will have a true hook implemented soon so that we don't keep duplicating this type of code (increase tech. debt). There are a few links already doing this through out our codebase

@kevinlog you ok if I just go ahead and implement the hook for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paul-tavares yes, sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. If @oatkiller is ok with it, then I will work on that Issue next and will search/replace usages in our code base with the new callback.

history.push(policyResponseUri);
}}
>
<FormattedMessage
id="xpack.endpoint.host.details.policyStatus.success"
defaultMessage="Successful"
/>
</EuiLink>
</EuiHealth>
),
},
{
title: i18n.translate('xpack.endpoint.host.details.ipAddress', {
Expand All @@ -101,7 +122,15 @@ const HostDetails = memo(({ details }: { details: HostMetadata }) => {
description: details.agent.version,
},
];
}, [details.agent.version, details.endpoint.policy.id, details.host.hostname, details.host.ip]);
}, [
details.agent.version,
details.endpoint.policy.id,
details.host.hostname,
details.host.ip,
history,
policyResponseUri,
]);

return (
<>
<EuiDescriptionList
Expand Down Expand Up @@ -132,69 +161,3 @@ const HostDetails = memo(({ details }: { details: HostMetadata }) => {
</>
);
});

export const HostDetailsFlyout = () => {
const history = useHistory();
const { notifications } = useKibana();
const queryParams = useHostListSelector(uiQueryParams);
const { selected_host: selectedHost, ...queryParamsWithoutSelectedHost } = queryParams;
const details = useHostListSelector(detailsData);
const error = useHostListSelector(detailsError);

const handleFlyoutClose = useCallback(() => {
history.push(urlFromQueryParams(queryParamsWithoutSelectedHost));
}, [history, queryParamsWithoutSelectedHost]);

useEffect(() => {
if (error !== undefined) {
notifications.toasts.danger({
title: (
<FormattedMessage
id="xpack.endpoint.host.details.errorTitle"
defaultMessage="Could not find host"
/>
),
body: (
<FormattedMessage
id="xpack.endpoint.host.details.errorBody"
defaultMessage="Please exit the flyout and select an available host."
/>
),
toastLifeTimeMs: 10000,
});
}
}, [error, notifications.toasts]);

return (
<EuiFlyout onClose={handleFlyoutClose} data-test-subj="hostDetailsFlyout">
<EuiFlyoutHeader hasBorder>
<EuiTitle size="s">
<h2 data-test-subj="hostDetailsFlyoutTitle">
{details === undefined ? <EuiLoadingContent lines={1} /> : details.host.hostname}
</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
{details === undefined ? (
<>
<EuiLoadingContent lines={3} /> <EuiSpacer size="l" /> <EuiLoadingContent lines={3} />
</>
) : (
<HostDetails details={details} />
)}
</EuiFlyoutBody>
</EuiFlyout>
);
};

const useHostLogsUrl = (hostId: string): { url: string; appId: string; appPath: string } => {
const { services } = useKibana();
return useMemo(() => {
const appPath = `/stream?logFilter=(expression:'host.id:${hostId}',kind:kuery)`;
return {
url: `${services.application.getUrlForApp('logs')}${appPath}`,
appId: 'logs',
appPath,
};
}, [hostId, services.application]);
};
Loading