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.

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

[SECURITY_SOLUTION] Task/hostname policy response ux updates #76444

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,24 @@ export const PreferenceFormattedDate = React.memo<{ dateFormat?: string; value:

PreferenceFormattedDate.displayName = 'PreferenceFormattedDate';

export const PreferenceFormattedDateFromPrimitive = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a new component under .../public/common, I would suggest adding some jsdocs here

Also - wondering why this was needed - <FormattedDate/> (below) did not handle this use case?

value,
}: {
value?: string | number | null;
}) => {
if (value == null) {
return getOrEmptyTagFromValue(value);
}
const maybeDate = getMaybeDate(value);
if (!maybeDate.isValid()) {
return getOrEmptyTagFromValue(value);
}
const date = maybeDate.toDate();
return <PreferenceFormattedDate value={date} />;
};

PreferenceFormattedDateFromPrimitive.displayName = 'PreferenceFormattedDateFromPrimitive';

/**
* This function may be passed to `Array.find()` to locate the `P1DT`
* configuration (sub) setting, a string array that contains two entries
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ export const patternsError = (state: Immutable<EndpointState>) => state.patterns
const detailsPolicyAppliedResponse = (state: Immutable<EndpointState>) =>
state.policyResponse && state.policyResponse.Endpoint.policy.applied;

/**
* Returns the policy response timestamp from the endpoint after a user modifies a policy.
*/
export const policyResponseTimestamp = (state: Immutable<EndpointState>) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a response type to this function? Looks like its either the value of @timestamp or false/undefined? Probably should be something like: Immutable<EndpointState>['policyResponse]['@timestamp'] ??

state.policyResponse && state.policyResponse['@timestamp'];

/**
* Returns the response configurations from the endpoint after a user modifies a policy.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const EndpointDetails = memo(({ details }: { details: HostMetadata }) =>
return [
{
title: i18n.translate('xpack.securitySolution.endpoint.details.policy', {
defaultMessage: 'Integration',
defaultMessage: 'Integration Policy',
}),
description: (
<>
Expand All @@ -140,7 +140,7 @@ export const EndpointDetails = memo(({ details }: { details: HostMetadata }) =>
},
{
title: i18n.translate('xpack.securitySolution.endpoint.details.policyStatus', {
defaultMessage: 'Configuration response',
defaultMessage: 'Policy Response',
}),
description: (
<EuiHealth
Expand Down Expand Up @@ -219,7 +219,7 @@ export const EndpointDetails = memo(({ details }: { details: HostMetadata }) =>
<EuiIcon type="savedObjectsApp" className="linkToAppIcon" />
<FormattedMessage
id="xpack.securitySolution.endpoint.details.linkToIngestTitle"
defaultMessage="Reassign Configuration"
defaultMessage="Reassign Policy"
/>
<EuiIcon type="popout" className="linkToAppPopoutIcon" />
</LinkToApp>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
policyResponseFailedOrWarningActionCount,
policyResponseError,
policyResponseLoading,
policyResponseTimestamp,
} from '../../store/selectors';
import { EndpointDetails } from './endpoint_details';
import { PolicyResponse } from './policy_response';
Expand All @@ -41,6 +42,7 @@ import { useNavigateByRouterEventHandler } from '../../../../../common/hooks/end
import { getEndpointListPath } from '../../../../common/routing';
import { SecurityPageName } from '../../../../../app/types';
import { useFormatUrl } from '../../../../../common/components/link_to';
import { PreferenceFormattedDateFromPrimitive } from '../../../../../common/components/formatted_date';

export const EndpointDetailsFlyout = memo(() => {
const history = useHistory();
Expand Down Expand Up @@ -122,6 +124,7 @@ const PolicyResponseFlyoutPanel = memo<{
const loading = useEndpointSelector(policyResponseLoading);
const error = useEndpointSelector(policyResponseError);
const { formatUrl } = useFormatUrl(SecurityPageName.administration);
const responseTimestamp = useEndpointSelector(policyResponseTimestamp);
const [detailsUri, detailsRoutePath] = useMemo(
() => [
formatUrl(
Expand Down Expand Up @@ -161,16 +164,21 @@ const PolicyResponseFlyoutPanel = memo<{
<h4>
<FormattedMessage
id="xpack.securitySolution.endpoint.policyResponse.title"
defaultMessage="Configuration Response"
defaultMessage="Policy Response"
/>
</h4>
</EuiText>
<EuiSpacer size="s" />
<EuiText size="xs" color="subdued" data-test-subj="endpointDetailsPolicyResponseTimestamp">
<PreferenceFormattedDateFromPrimitive value={responseTimestamp} />
</EuiText>
<EuiSpacer size="s" />
{error && (
<EuiEmptyPrompt
title={
<FormattedMessage
id="xpack.securitySolution.endpoint.details.noPolicyResponse"
defaultMessage="No configuration response available"
defaultMessage="No policy response available"
/>
}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ describe('when on the list page', () => {
const renderResult = await renderAndWaitForData();
const linkToReassign = await renderResult.findByTestId('endpointDetailsLinkToIngest');
expect(linkToReassign).not.toBeNull();
expect(linkToReassign.textContent).toEqual('Reassign Configuration');
expect(linkToReassign.textContent).toEqual('Reassign Policy');
expect(linkToReassign.getAttribute('href')).toEqual(
`/app/ingestManager#/fleet/agents/${agentId}/activity?openReassignFlyout=true`
);
Expand Down Expand Up @@ -572,7 +572,7 @@ describe('when on the list page', () => {
it('should include the sub-panel title', async () => {
expect(
(await renderResult.findByTestId('endpointDetailsPolicyResponseFlyoutTitle')).textContent
).toBe('Configuration Response');
).toBe('Policy Response');
});

it('should show a configuration section for each protection', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,17 +270,20 @@ export const EndpointList = () => {
);
const toRouteUrl = formatUrl(toRoutePath);
return (
<EndpointListNavLink
name={hostname}
href={toRouteUrl}
route={toRoutePath}
dataTestSubj="hostnameCellLink"
/>
<EuiToolTip content={hostname} anchorClassName="eui-fullWidth">
<EndpointListNavLink
name={hostname}
href={toRouteUrl}
route={toRoutePath}
dataTestSubj="hostnameCellLink"
/>
</EuiToolTip>
);
},
},
{
field: 'host_status',
width: '9%',
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm guessing these hardcoded widths is to make sure the values truncate with ellipsis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's do restrict some columns that we can predict won't be very long such as the Statuses since they can only have a handful of values. this frees up room for longer columns like hostname, timestamp, etc

name: i18n.translate('xpack.securitySolution.endpoint.list.hostStatus', {
defaultMessage: 'Agent Status',
}),
Expand All @@ -303,27 +306,31 @@ export const EndpointList = () => {
},
{
field: 'metadata.Endpoint.policy.applied',
width: '15%',
Copy link
Contributor

Choose a reason for hiding this comment

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

+100 points for use of % :)

name: i18n.translate('xpack.securitySolution.endpoint.list.policy', {
defaultMessage: 'Integration',
defaultMessage: 'Integration Policy',
}),
truncateText: true,
// eslint-disable-next-line react/display-name
render: (policy: HostInfo['metadata']['Endpoint']['policy']['applied']) => {
return (
<EndpointPolicyLink
policyId={policy.id}
className="eui-textTruncate"
data-test-subj="policyNameCellLink"
>
{policy.name}
</EndpointPolicyLink>
<EuiToolTip content={policy.name} anchorClassName="eui-fullWidth">
<EndpointPolicyLink
policyId={policy.id}
className="eui-textTruncate"
data-test-subj="policyNameCellLink"
>
{policy.name}
</EndpointPolicyLink>
</EuiToolTip>
);
},
},
{
field: 'metadata.Endpoint.policy.applied',
width: '9%',
name: i18n.translate('xpack.securitySolution.endpoint.list.policyStatus', {
defaultMessage: 'Configuration Status',
defaultMessage: 'Policy Status',
}),
render: (policy: HostInfo['metadata']['Endpoint']['policy']['applied'], item: HostInfo) => {
const toRoutePath = getEndpointDetailsPath({
Expand All @@ -350,6 +357,7 @@ export const EndpointList = () => {
},
{
field: 'metadata.host.os.name',
width: '10%',
name: i18n.translate('xpack.securitySolution.endpoint.list.os', {
defaultMessage: 'Operating System',
}),
Expand All @@ -375,6 +383,7 @@ export const EndpointList = () => {
},
{
field: 'metadata.agent.version',
width: '5%',
name: i18n.translate('xpack.securitySolution.endpoint.list.endpointVersion', {
defaultMessage: 'Version',
}),
Expand All @@ -394,6 +403,7 @@ export const EndpointList = () => {
},
{
field: '',
width: '5%',
name: i18n.translate('xpack.securitySolution.endpoint.list.actions', {
defaultMessage: 'Actions',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
[
'Hostname',
'Agent Status',
'Integration',
'Configuration Status',
'Integration Policy',
'Policy Status',
'Operating System',
'IP Address',
'Version',
Expand Down Expand Up @@ -188,8 +188,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
'OS',
'Last Seen',
'Alerts',
'Integration',
'Configuration Status',
'Integration Policy',
'Policy Status',
'IP Address',
'Hostname',
'Sensor Version',
Expand Down Expand Up @@ -236,8 +236,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
[
'Hostname',
'Agent Status',
'Integration',
'Configuration Status',
'Integration Policy',
'Policy Status',
'Operating System',
'IP Address',
'Version',
Expand Down Expand Up @@ -267,8 +267,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
[
'Hostname',
'Agent Status',
'Integration',
'Configuration Status',
'Integration Policy',
'Policy Status',
'Operating System',
'IP Address',
'Version',
Expand Down