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

[8.0][RAC] 117686 replace alert workflow status in alerts view #118723

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
69091b4
Add AlertStatus types
fkanout Nov 16, 2021
38f1829
Add alert status filter component
fkanout Nov 16, 2021
f4cb043
Remove Filter in action from the t grid table
fkanout Nov 16, 2021
7880781
Update group buttons to applied Alert status filter instead of Workfl…
fkanout Nov 16, 2021
b17d8db
Keep the Alert status button in sync when typing and first page load
fkanout Nov 16, 2021
ec3be8c
Fix data test object name and translation keys label
fkanout Nov 17, 2021
391945b
Add possibility to hide the bulk actions
fkanout Nov 17, 2021
3918b8d
Update how hide the bulk actions
fkanout Nov 18, 2021
2fb0b01
Fix showCheckboxes hardcoded "true". Instead use the leadingControlCo…
fkanout Nov 18, 2021
b4f8df4
Hide the leading checkboxes in the T Grid with the bulk actions
fkanout Nov 18, 2021
80132a3
Update showCheckboxes to false
fkanout Nov 19, 2021
9b8ef26
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
kibanamachine Nov 19, 2021
67bf04d
Fix test as the leading checkboxes are hidden
fkanout Nov 19, 2021
1b82d80
Update tests
fkanout Nov 22, 2021
edca181
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
fkanout Nov 22, 2021
338da2d
Get back disabledCellActions as it's required by T Grid
fkanout Nov 22, 2021
b1562d6
Update tests to skip test related to Workflow action buttons
fkanout Nov 22, 2021
3e6c8c6
Skip workflow tests
fkanout Nov 22, 2021
caab870
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
kibanamachine Nov 22, 2021
3ae465d
Revert fix showCheckboxes
fkanout Nov 23, 2021
f79bcff
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
fkanout Nov 23, 2021
762ba4d
Remove unused imports
fkanout Nov 23, 2021
963b34e
Revert the o11y tests as the checkBoxes fix is reverted
fkanout Nov 23, 2021
3ade606
Reactive the tests effected by checkBoxes
fkanout Nov 23, 2021
1682878
Skip alert workflow status
fkanout Nov 23, 2021
5b55600
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
kibanamachine Nov 23, 2021
8c8f114
[Code review] use predefined types
fkanout Nov 24, 2021
2a0774f
Remove unused prop
fkanout Nov 24, 2021
6ebbb68
Use the alert-data index name in the RegEx
fkanout Nov 25, 2021
029bbea
Detect * in KQL as "show al"l alert filter
fkanout Nov 25, 2021
8ada6ae
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
kibanamachine Nov 25, 2021
01caf34
Merge branch 'main' into 117686-replace-alert-workflow-status-in-aler…
kibanamachine Nov 29, 2021
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
13 changes: 13 additions & 0 deletions x-pack/plugins/observability/common/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
import * as t from 'io-ts';

export type Maybe<T> = T | null | undefined;
import {
ALERT_STATUS_ACTIVE,
ALERT_STATUS_RECOVERED,
} from '@kbn/rule-data-utils/alerts_as_data_status';

export const alertWorkflowStatusRt = t.keyof({
open: null,
Expand All @@ -25,3 +29,12 @@ export interface ApmIndicesConfig {
apmAgentConfigurationIndex: string;
apmCustomLinkIndex: string;
}
export type AlertStatusFilterButton =
| typeof ALERT_STATUS_ACTIVE
| typeof ALERT_STATUS_RECOVERED
| '';
export interface AlertStatusFilter {
status: AlertStatusFilterButton;
query: string;
label: string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { EuiButtonGroup, EuiButtonGroupOptionProps } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import {
ALERT_STATUS_ACTIVE,
ALERT_STATUS_RECOVERED,
} from '@kbn/rule-data-utils/alerts_as_data_status';
import { ALERT_STATUS } from '@kbn/rule-data-utils/technical_field_names';
import { AlertStatusFilterButton } from '../../../common/typings';
import { AlertStatusFilter } from '../../../common/typings';

export interface AlertStatusFilterProps {
status: AlertStatusFilterButton;
onChange: (id: string, value: string) => void;
}

export const allAlerts: AlertStatusFilter = {
status: '',
query: '',
label: i18n.translate('xpack.observability.alerts.alertStatusFilter.showAll', {
defaultMessage: 'Show all',
}),
};

export const activeAlerts: AlertStatusFilter = {
status: ALERT_STATUS_ACTIVE,
query: `${ALERT_STATUS}: "${ALERT_STATUS_ACTIVE}"`,
label: i18n.translate('xpack.observability.alerts.alertStatusFilter.active', {
defaultMessage: 'Active',
}),
};

export const recoveredAlerts: AlertStatusFilter = {
status: ALERT_STATUS_RECOVERED,
query: `${ALERT_STATUS}: "${ALERT_STATUS_RECOVERED}"`,
label: i18n.translate('xpack.observability.alerts.alertStatusFilter.recovered', {
defaultMessage: 'Recovered',
}),
};

const options: EuiButtonGroupOptionProps[] = [
{
id: allAlerts.status,
label: allAlerts.label,
value: allAlerts.query,
'data-test-subj': 'alert-status-filter-show-all-button',
},
{
id: activeAlerts.status,
label: activeAlerts.label,
value: activeAlerts.query,
'data-test-subj': 'alert-status-filter-active-button',
},
{
id: recoveredAlerts.status,
label: recoveredAlerts.label,
value: recoveredAlerts.query,
'data-test-subj': 'alert-status-filter-recovered-button',
},
];

export function AlertsStatusFilter({ status, onChange }: AlertStatusFilterProps) {
return (
<EuiButtonGroup
legend="Filter by"
color="primary"
options={options}
idSelected={status}
onChange={onChange}
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
import {
ALERT_DURATION,
ALERT_REASON,
ALERT_RULE_CONSUMER,
ALERT_RULE_PRODUCER,
ALERT_STATUS,
ALERT_WORKFLOW_STATUS,
TIMESTAMP,
Expand All @@ -34,33 +32,30 @@ import {
import styled from 'styled-components';
import React, { Suspense, useMemo, useState, useCallback, useEffect } from 'react';
import usePrevious from 'react-use/lib/usePrevious';
import { get, pick } from 'lodash';
import {
getAlertsPermissions,
useGetUserAlertsPermissions,
} from '../../hooks/use_alert_permission';
import { pick } from 'lodash';
import { getAlertsPermissions } from '../../hooks/use_alert_permission';
import type {
TimelinesUIStart,
TGridType,
TGridState,
TGridModel,
SortDirection,
} from '../../../../timelines/public';
import { useStatusBulkActionItems } from '../../../../timelines/public';

import type { TopAlert } from './';
import { useKibana } from '../../../../../../src/plugins/kibana_react/public';
import type {
ActionProps,
AlertWorkflowStatus,
ColumnHeaderOptions,
ControlColumnProps,
RowRenderer,
} from '../../../../timelines/common';

import { getRenderCellValue } from './render_cell_value';
import { observabilityAppId, observabilityFeatureId } from '../../../common';
import { useGetUserCasesPermissions } from '../../hooks/use_get_user_cases_permissions';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { getDefaultCellActions } from './default_cell_actions';
import { LazyAlertsFlyout } from '../..';
import { parseAlert } from './parse_alert';
import { CoreStart } from '../../../../../../src/core/public';
Expand All @@ -75,7 +70,6 @@ interface AlertsTableTGridProps {
kuery: string;
workflowStatus: AlertWorkflowStatus;
setRefetch: (ref: () => void) => void;
addToQuery: (value: string) => void;
}

interface ObservabilityActionsProps extends ActionProps {
Expand Down Expand Up @@ -154,21 +148,21 @@ function ObservabilityActions({
const [openActionsPopoverId, setActionsPopover] = useState(null);
const {
timelines,
application: { capabilities },
application: {},
} = useKibana<CoreStart & { timelines: TimelinesUIStart }>().services;

const parseObservabilityAlert = useMemo(
() => parseAlert(observabilityRuleTypeRegistry),
[observabilityRuleTypeRegistry]
);
const alertDataConsumer = useMemo<string>(
() => get(dataFieldEs, ALERT_RULE_CONSUMER, [''])[0],
[dataFieldEs]
);
const alertDataProducer = useMemo<string>(
() => get(dataFieldEs, ALERT_RULE_PRODUCER, [''])[0],
[dataFieldEs]
);
// const alertDataConsumer = useMemo<string>(
// () => get(dataFieldEs, ALERT_RULE_CONSUMER, [''])[0],
// [dataFieldEs]
// );
// const alertDataProducer = useMemo<string>(
// () => get(dataFieldEs, ALERT_RULE_PRODUCER, [''])[0],
// [dataFieldEs]
// );

const alert = parseObservabilityAlert(dataFieldEs);
const { prepend } = core.http.basePath;
Expand All @@ -194,27 +188,29 @@ function ObservabilityActions({
};
}, [data, eventId, ecsData]);

const onAlertStatusUpdated = useCallback(() => {
setActionsPopover(null);
if (refetch) {
refetch();
}
}, [setActionsPopover, refetch]);

const alertPermissions = useGetUserAlertsPermissions(
capabilities,
alertDataConsumer === 'alerts' ? alertDataProducer : alertDataConsumer
);

const statusActionItems = useStatusBulkActionItems({
eventIds: [eventId],
currentStatus,
indexName: ecsData._index ?? '',
setEventsLoading,
setEventsDeleted,
onUpdateSuccess: onAlertStatusUpdated,
onUpdateFailure: onAlertStatusUpdated,
});
// Hide the WorkFlow filter, but keep its code as required in https://github.com/elastic/kibana/issues/117686

// const onAlertStatusUpdated = useCallback(() => {
// setActionsPopover(null);
// if (refetch) {
// refetch();
// }
// }, [setActionsPopover, refetch]);

// const alertPermissions = useGetUserAlertsPermissions(
// capabilities,
// alertDataConsumer === 'alerts' ? alertDataProducer : alertDataConsumer
// );

// const statusActionItems = useStatusBulkActionItems({
// eventIds: [eventId],
// currentStatus,
// indexName: ecsData._index ?? '',
// setEventsLoading,
// setEventsDeleted,
// onUpdateSuccess: onAlertStatusUpdated,
// onUpdateFailure: onAlertStatusUpdated,
// });

const ruleId = alert.fields['kibana.alert.rule.uuid'] ?? null;
const linkToRule = ruleId ? prepend(paths.management.ruleDetails(ruleId)) : null;
Expand All @@ -239,7 +235,8 @@ function ObservabilityActions({
}),
]
: []),
...(alertPermissions.crud ? statusActionItems : []),
// Hide the WorkFlow filter, but keep its code as required in https://github.com/elastic/kibana/issues/117686
// ...(alertPermissions.crud ? statusActionItems : []),
...(!!linkToRule
? [
<EuiContextMenuItem
Expand All @@ -252,15 +249,7 @@ function ObservabilityActions({
]
: []),
];
}, [
afterCaseSelection,
casePermissions,
timelines,
event,
statusActionItems,
alertPermissions,
linkToRule,
]);
}, [afterCaseSelection, casePermissions, timelines, event, linkToRule]);

const actionsToolTip =
actionsMenuItems.length <= 0
Expand Down Expand Up @@ -320,6 +309,7 @@ function ObservabilityActions({
</>
);
}
// Hide the WorkFlow filter, but keep its code as required in https://github.com/elastic/kibana/issues/117686

const FIELDS_WITHOUT_CELL_ACTIONS = [
'@timestamp',
Expand All @@ -330,7 +320,7 @@ const FIELDS_WITHOUT_CELL_ACTIONS = [
];

export function AlertsTableTGrid(props: AlertsTableTGridProps) {
const { indexNames, rangeFrom, rangeTo, kuery, workflowStatus, setRefetch, addToQuery } = props;
const { indexNames, rangeFrom, rangeTo, kuery, workflowStatus, setRefetch } = props;
Copy link
Contributor

Choose a reason for hiding this comment

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

@fkanout We should delete workflowStatus from here. There are probably a few more places that workflowStatus might need to be deleted as well. This is not needed, since we are not filtering by workflowStatus anymore in the UI. That's why we still see workflowStatus:open in the urlbar when we first load the page. This is not needed anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as here:
#118723 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing workflowStatus It will blocked by TGridStandaloneCompProps requires filterStatus. Because the value of filterStatus is workflowStatus

Copy link
Contributor

@mgiota mgiota Nov 29, 2021

Choose a reason for hiding this comment

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

@fkanout filterStatus at the moment has value workflowStatus, but nothing prevents us by changing it back to alert status. This change was introduced as part of this PR #109817, so we could pair together to manually revert some workflows status leftover that is not needed

Let's get this PR merged for now. I summarized later on in a comment what we discussed during our sync up.

const prevWorkflowStatus = usePrevious(workflowStatus);
const {
timelines,
Expand Down Expand Up @@ -382,7 +372,7 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) {
}
}, []);

const leadingControlColumns = useMemo(() => {
const leadingControlColumns: ControlColumnProps[] = useMemo(() => {
fkanout marked this conversation as resolved.
Show resolved Hide resolved
return [
{
id: 'expand',
Expand Down Expand Up @@ -428,7 +418,8 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) {
type,
columns: tGridState?.columns ?? columns,
deletedEventIds,
defaultCellActions: getDefaultCellActions({ addToQuery }),
// Hide the WorkFlow filter, but keep its code as required in https://github.com/elastic/kibana/issues/117686
// defaultCellActions: getDefaultCellActions({ addToQuery }),
disabledCellActions: FIELDS_WITHOUT_CELL_ACTIONS,
end: rangeTo,
filters: [],
Expand Down Expand Up @@ -462,7 +453,6 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) {
};
}, [
casePermissions,
addToQuery,
rangeTo,
hasAlertsCrudPermissions,
indexNames,
Expand Down
Loading