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

Fixed bugs associated with alerts table, and addressed UX review feedback. #222

Merged
merged 8 commits into from
Apr 21, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ import { DEFAULT_PAGE_SIZE_OPTIONS } from '../../../../pages/Monitors/containers
import queryString from 'query-string';
import { MAX_ALERT_COUNT } from '../../../../pages/Dashboard/utils/constants';
import { SEVERITY_OPTIONS } from '../../../../pages/CreateTrigger/utils/constants';
import { TABLE_TAB_IDS } from '../../../../pages/Dashboard/components/FindingsDashboard/utils';
import {
ALERTS_FINDING_COLUMN,
TABLE_TAB_IDS,
} from '../../../../pages/Dashboard/components/FindingsDashboard/utils';
import FindingsDashboard from '../../../../pages/Dashboard/containers/FindingsDashboard';

export const DEFAULT_NUM_FLYOUT_ROWS = 10;
Expand Down Expand Up @@ -147,8 +150,8 @@ export default class AlertsDashboardFlyoutComponent extends Component {

getBucketLevelGraphConditions = (trigger) => {
let conditions = _.get(trigger, 'condition.script.source', '-');
conditions = _.replace(conditions, ' && ', '&AND&');
conditions = _.replace(conditions, ' || ', '&OR&');
conditions = conditions.replaceAll(' && ', '&AND&');
conditions = conditions.replaceAll(' || ', '&OR&');
conditions = conditions.split(/&/);
return conditions.join('\n');
};
Expand All @@ -167,7 +170,7 @@ export default class AlertsDashboardFlyoutComponent extends Component {
};

getAlerts = async () => {
this.setState({ loading: true });
this.setState({ loading: true, tabContent: undefined });
const {
from,
search,
Expand Down Expand Up @@ -263,8 +266,7 @@ export default class AlertsDashboardFlyoutComponent extends Component {
alertState,
monitorIds
);
this.setState({ selectedItems: [], tabContent: undefined });
this.setState({ tabContent: this.renderAlertsTable() });
this.setState({ selectedItems: [] });
this.props.refreshDashboard();
};

Expand Down Expand Up @@ -349,6 +351,10 @@ export default class AlertsDashboardFlyoutComponent extends Component {
case MONITOR_TYPE.BUCKET_LEVEL:
columns = insertGroupByColumn(groupBy);
break;
case MONITOR_TYPE.DOC_LEVEL:
columns = _.cloneDeep(queryColumns);
columns.splice(0, 0, ALERTS_FINDING_COLUMN);
break;
default:
columns = queryColumns;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const documentLevelDescription = ( // TODO DRAFT: confirm wording
);

const MonitorType = ({ values }) => (
<EuiFlexGrid alignItems={'flexStart'} gutterSize={'m'}>
<EuiFlexGrid gutterSize={'m'}>
<EuiFlexItem grow={false}>
<FormikCheckableCard
name="monitorTypeQueryLevel"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import ContentPanel from '../../../../components/ContentPanel';
const QueryPerformance = ({ response, actions }) => (
<Fragment>
<ContentPanel
title="Query performance"
title="Monitor performance"
titleSize="s"
panelStyles={{ paddingLeft: '10px', paddingRight: '10px' }}
description={
Expand All @@ -31,7 +31,7 @@ const QueryPerformance = ({ response, actions }) => (
<EuiFlexGroup alignItems="flexStart" gutterSize="xl">
<EuiFlexItem grow={false}>
<EuiText size="xs">
<strong>Query duration</strong>
<strong>Monitor duration</strong>
<span style={{ display: 'block' }}>
{`${_.get(response, 'took', DEFAULT_EMPTY_DATA)} ms`}
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import React, { Component, Fragment } from 'react';
import moment from 'moment';
import _ from 'lodash';
import PropTypes from 'prop-types';
import { EuiSpacer, EuiButton, EuiCallOut, EuiAccordion } from '@elastic/eui';
import { EuiSpacer, EuiButton, EuiCallOut, EuiAccordion, EuiLoadingSpinner } from '@elastic/eui';
import ContentPanel from '../../../../components/ContentPanel';
import VisualGraph from '../../components/VisualGraph';
import ExtractionQuery from '../../components/ExtractionQuery';
Expand All @@ -29,14 +29,15 @@ import { FORMIK_INITIAL_VALUES } from '../CreateMonitor/utils/constants';
import { API_TYPES } from '../../components/ClusterMetricsMonitor/utils/clusterMetricsMonitorConstants';
import ConfigureDocumentLevelQueries from '../../components/DocumentLevelMonitorQueries/ConfigureDocumentLevelQueries';
import FindingsDashboard from '../../../Dashboard/containers/FindingsDashboard';
import { validDocLevelGraphQueries } from '../../../Dashboard/components/FindingsDashboard/utils';

function renderEmptyMessage(message) {
return (
<div style={{ padding: '20px', border: '1px solid #D9D9D9', borderRadius: '5px' }}>
<div
style={{ display: 'flex', alignItems: 'center', justifyContent: 'center', height: '450px' }}
>
<div>{message}</div>
<div>{_.isEmpty(message) ? <EuiLoadingSpinner size="xl" /> : message}</div>
</div>
</div>
);
Expand Down Expand Up @@ -236,14 +237,18 @@ class DefineMonitor extends Component {
}
};

let accordionTitle = 'Preview query and performance';
const previewContent = () => {
switch (values.monitor_type) {
case MONITOR_TYPE.BUCKET_LEVEL:
return this.getBucketMonitorGraphs(aggregations, formikSnapshot, response);
case MONITOR_TYPE.DOC_LEVEL:
const { index, queries } = values;
accordionTitle = 'Preview findings and performance';
return _.isEmpty(response) ? (
renderEmptyMessage('Loading findings...')
renderEmptyMessage(
validDocLevelGraphQueries(queries) ? '' : 'You must define at least one query.'
)
) : (
<FindingsDashboard
isPreview={true}
Expand All @@ -264,10 +269,7 @@ class DefineMonitor extends Component {
{monitorExpressions()}
<EuiSpacer size="xl" />

<EuiAccordion
id="preview-query-performance-accordion"
buttonContent="Preview query and performance"
>
<EuiAccordion id="preview-query-performance-accordion" buttonContent={accordionTitle}>
<EuiSpacer size="s" />
<QueryPerformance response={performanceResponse} />
<EuiSpacer size="m" />
Expand All @@ -286,9 +288,16 @@ class DefineMonitor extends Component {
async onRunQuery() {
this.setState({ loadingResponse: true });
const { httpClient, values, notifications } = this.props;
const formikSnapshot = _.cloneDeep(values);
const { monitor_type, searchType } = values;

// Cancel execution criteria
switch (monitor_type) {
case MONITOR_TYPE.DOC_LEVEL:
const { queries } = values;
if (SEARCH_TYPE.GRAPH && !validDocLevelGraphQueries(queries)) return;
}

const searchType = values.searchType;
const formikSnapshot = _.cloneDeep(values);
let requests;
switch (searchType) {
case SEARCH_TYPE.QUERY:
Expand Down Expand Up @@ -341,20 +350,29 @@ class DefineMonitor extends Component {
const endTime = moment();
const duration = moment.duration(endTime.diff(startTime)).milliseconds();
const response = _.get(queryResponse.resp, 'input_results.results[0]');

// If there is an optionalResponse use it's results, otherwise use the original response
const performanceResponse = optionalResponse
? _.get(optionalResponse, 'resp.input_results.results[0]', null)
: response;
this.setState({
response,
formikSnapshot,
// TODO FIXME: Doc level backend monitor run results don't include duration metric. Using this for now.
// This returns a much longer duration than other monitors, though.
performanceResponse:
values.monitor_type === MONITOR_TYPE.DOC_LEVEL
? { ...performanceResponse, took: duration }
: performanceResponse,
});
this.setState({ response, formikSnapshot, performanceResponse });

// TODO FIXME: Doc level backend monitor run results don't include duration metrics. Using this for now.
// This returns a much longer duration than other monitors, though.
if (monitor_type === MONITOR_TYPE.DOC_LEVEL) {
let hitsCount = 0;
_.keys(response).forEach(
(resultKey) => (hitsCount += _.values(performanceResponse[resultKey]).length)
);
this.setState({
performanceResponse: {
...performanceResponse,
took: duration,
invalid: { path: duration },
hits: { total: { value: hitsCount } },
},
});
}
} else {
console.error('There was an error running the query', queryResponse.resp);
backendErrorNotification(notifications, 'run', 'query', queryResponse.resp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export function getDocumentLevelScriptSource(conditions) {
if (!_.isEmpty(query) && !_.isEmpty(query.queryName)) {
const queryExpression = _.get(query, 'expression');
const operator = query.operator === '!=' ? '!' : '';
scriptSourceContents.push(`(${operator}query[${queryExpression}])`);
scriptSourceContents.push(`${operator}query[${queryExpression}]`);
}
});
return scriptSourceContents.join(' ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { backendErrorNotification, inputLimitText } from '../../../../utils/help
import monitorToFormik from '../../../CreateMonitor/containers/CreateMonitor/utils/monitorToFormik';
import { buildRequest } from '../../../CreateMonitor/containers/DefineMonitor/utils/searchRequests';

const MAX_TRIGGER_CONDITIONS = 5; // TODO DRAFT: Placeholder limit
const MAX_TRIGGER_CONDITIONS = 10;

const defaultRowProps = {
label: 'Trigger name',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import React from 'react';
import PropTypes from 'prop-types';
import { EuiButton, EuiEmptyPrompt, EuiText } from '@elastic/eui';
import { EuiButton, EuiButtonEmpty, EuiEmptyPrompt, EuiText } from '@elastic/eui';

import { APP_PATH } from '../../../../utils/constants';
import { PLUGIN_NAME } from '../../../../../utils/constants';
Expand All @@ -23,9 +23,7 @@ const createMonitorButton = (
</EuiButton>
);
const editMonitorButton = (onCreateTrigger) => (
<EuiButton fill onClick={onCreateTrigger}>
Edit monitor
</EuiButton>
<EuiButtonEmpty onClick={onCreateTrigger}>Edit monitor</EuiButtonEmpty>
);

const DashboardEmptyPrompt = ({ onCreateTrigger, isModal = false }) => {
Expand All @@ -35,6 +33,11 @@ const DashboardEmptyPrompt = ({ onCreateTrigger, isModal = false }) => {
: inMonitorDetails
? createTriggerText
: createMonitorText;
const actions = inMonitorDetails
? undefined
: isModal
? editMonitorButton(onCreateTrigger)
: createMonitorButton;
return (
<EuiEmptyPrompt
style={{ maxWidth: '45em' }}
Expand All @@ -43,9 +46,7 @@ const DashboardEmptyPrompt = ({ onCreateTrigger, isModal = false }) => {
<p>{displayText}</p>
</EuiText>
}
actions={
inMonitorDetails || isModal ? editMonitorButton(onCreateTrigger) : createMonitorButton
}
actions={actions}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ export default class FindingFlyout extends Component {
onClose={this.closeFlyout}
ownFocus={false}
hideCloseButton={true}
side={isAlertsFlyout ? 'left' : 'right'}
size={'s'}
side={'right'}
size={'m'}
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size={'m'}>
Expand Down Expand Up @@ -119,13 +119,20 @@ export default class FindingFlyout extends Component {
overflowHeight={600}
inline={false}
isCopyable
style={{ height: '400px' }}
>
{JSON.stringify(documentDisplay, null, 3)}
</EuiCodeBlock>
</EuiFlyoutBody>

<EuiFlyoutFooter>
<EuiButtonEmpty onClick={this.closeFlyout}>Close</EuiButtonEmpty>
<EuiButtonEmpty
iconType={'cross'}
onClick={this.closeFlyout}
style={{ paddingLeft: '0px', marginLeft: '0px' }}
>
Close
</EuiButtonEmpty>
</EuiFlyoutFooter>
</EuiFlyout>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,49 @@
*/

import React, { useState } from 'react';
import _ from 'lodash';

import { EuiLink, EuiPopover, EuiSpacer, EuiText } from '@elastic/eui';

export default function QueryPopover(queries) {
export default function FindingsPopover({ docIds = [], queries = [] }) {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const onButtonClick = () => setIsPopoverOpen(!isPopoverOpen);
const closePopover = () => setIsPopoverOpen(false);

const popoverContent = queries.queries.map((query, index) => {
return (
let header;
let popoverContent;
let count;

if (!_.isEmpty(docIds)) {
header = 'Documents';
popoverContent = docIds.map((docId, index) => (
<div key={`${docId}${index}`}>
{index > 0 && <EuiSpacer size={'s'} />}
<EuiText size={'s'}>
<p>{docId}</p>
</EuiText>
</div>
));
count = popoverContent.length;
} else {
header = 'Queries';
popoverContent = queries.map((query, index) => (
<div key={`${query.name}${index}`}>
{index > 0 && <EuiSpacer size={'s'} />}
<EuiText size={'s'}>
<strong>{query.name}</strong>
<p>{query.query}</p>
</EuiText>
</div>
);
});
));
count = popoverContent.length;
}

const buttonContent = `${count} ${header}`;

return (
<EuiPopover
button={<EuiLink onClick={onButtonClick}>{`${queries.queries.length} Queries`}</EuiLink>}
button={<EuiLink onClick={onButtonClick}>{buttonContent}</EuiLink>}
isOpen={isPopoverOpen}
closePopover={closePopover}
>
Expand Down
Loading