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 a bug that displayed all alerts for a monitor on individual tri… #130

Merged
merged 1 commit into from
Oct 12, 2021
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
8 changes: 5 additions & 3 deletions public/components/Flyout/flyouts/alertsDashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const getBucketLevelGraphFilter = (trigger) => {

const alertsDashboard = (payload) => {
const {
alerts,
history,
httpClient,
last_notification_time,
Expand All @@ -90,9 +91,8 @@ const alertsDashboard = (payload) => {
monitor_name,
notifications,
setFlyout,
severity,
start_time,
triggerId,
triggerID,
trigger_name,
} = payload;
const monitor = _.get(_.find(monitors, { _id: monitor_id }), '_source');
Expand All @@ -104,10 +104,11 @@ const alertsDashboard = (payload) => {
monitorType === MONITOR_TYPE.QUERY_LEVEL ? TRIGGER_TYPE.QUERY_LEVEL : TRIGGER_TYPE.BUCKET_LEVEL;

let trigger = _.get(monitor, 'triggers', []).find((trigger) => {
return trigger[triggerType].triggerId === triggerId;
return trigger[triggerType].id === triggerID;
});
trigger = _.get(trigger, triggerType);

const severity = _.get(trigger, 'severity');
const groupBy = _.get(monitor, MONITOR_GROUP_BY);

const condition =
Expand Down Expand Up @@ -260,6 +261,7 @@ const alertsDashboard = (payload) => {
monitorType={monitorType}
perAlertView={true}
groupBy={groupBy}
flyoutAlerts={alerts}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { EuiText, EuiButtonEmpty, EuiSpacer } from '@elastic/eui';
import { getIndexFields } from './utils/dataTypes';
import { getGroupByExpressionAllowedTypes } from './utils/helpers';
import GroupByItem from './GroupByItem';
import { GROUP_BY_ERROR } from './utils/constants';
import { GROUP_BY_ERROR, QUERY_TYPE_GROUP_BY_ERROR } from './utils/constants';
import { MONITOR_TYPE } from '../../../../../utils/constants';
import { inputLimitText } from '../../../../../utils/helpers';
import {
Expand Down Expand Up @@ -86,6 +86,8 @@ class GroupByExpression extends Component {
const isBucketLevelMonitor = values.monitor_type === MONITOR_TYPE.BUCKET_LEVEL;
if (!values.groupBy.length && isBucketLevelMonitor) {
errors.groupBy = GROUP_BY_ERROR;
} else if (!isBucketLevelMonitor && values.groupBy.length > MAX_NUM_QUERY_LEVEL_GROUP_BYS) {
errors.groupBy = QUERY_TYPE_GROUP_BY_ERROR;
} else {
delete errors.groupBy;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import React, { useState } from 'react';
import _ from 'lodash';
import { EuiPopover, EuiBadge, EuiPopoverTitle } from '@elastic/eui';
import { GroupByPopover } from './index';

Expand All @@ -18,6 +19,7 @@ export default function GroupByItem(
) {
const [isPopoverOpen, setIsPopoverOpen] = useState(groupByItem === '');
const closePopover = () => {
if (_.isEmpty(groupByItem)) arrayHelpers.remove(index);
setIsPopoverOpen(false);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
import React, { Component } from 'react';
import { connect } from 'formik';
import { EuiText, EuiButtonEmpty, EuiSpacer, EuiBadge, EuiToolTip, EuiIcon } from '@elastic/eui';
import _ from 'lodash';
import { getIndexFields } from './utils/dataTypes';
import { getMetricExpressionAllowedTypes, validateAggregationsDuplicates } from './utils/helpers';
import _ from 'lodash';
import {
FORMIK_INITIAL_AGG_VALUES,
METRIC_TOOLTIP_TEXT,
Expand All @@ -38,6 +38,7 @@ import { MetricItem } from './index';
import { MONITOR_TYPE } from '../../../../../utils/constants';
import { inputLimitText } from '../../../../../utils/helpers';
import IconToolTip from '../../../../../components/IconToolTip';
import { QUERY_TYPE_METRIC_ERROR } from './utils/constants';

export const MAX_NUM_QUERY_LEVEL_METRICS = 1;
export const MAX_NUM_BUCKET_LEVEL_METRICS = 5;
Expand Down Expand Up @@ -107,6 +108,11 @@ class MetricExpression extends Component {

if (validateAggregationsDuplicates(aggregations)) {
errors.aggregations = `You have defined duplicated metrics.`;
} else if (
MONITOR_TYPE.QUERY_LEVEL === monitorType &&
aggregations.length > MAX_NUM_QUERY_LEVEL_METRICS
) {
errors.aggregations = QUERY_TYPE_METRIC_ERROR;
} else {
delete errors.aggregations;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import React, { useState } from 'react';
import _ from 'lodash';
import { EuiPopover, EuiBadge, EuiPopoverTitle } from '@elastic/eui';
import MetricPopover from './MetricPopover';

Expand All @@ -18,6 +19,7 @@ export default function MetricItem(
) {
const [isPopoverOpen, setIsPopoverOpen] = useState(aggregation.fieldName === '');
const closePopover = () => {
if (_.isEmpty(aggregation.fieldName)) arrayHelpers.remove(index);
setIsPopoverOpen(false);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,7 @@ import {
isNullOperator,
isRangeOperator,
} from './utils/whereHelpers';
import {
hasError,
isInvalid,
required,
validateRequiredNumber,
} from '../../../../../utils/validate';
import { hasError, isInvalid } from '../../../../../utils/validate';
import {
FormikComboBox,
FormikSelect,
Expand All @@ -66,7 +61,6 @@ import { getFilteredIndexFields, getIndexFields } from './utils/dataTypes';
import {
FILTERS_TOOLTIP_TEXT,
FORMIK_INITIAL_VALUES,
TIME_RANGE_TOOLTIP_TEXT,
} from '../../../containers/CreateMonitor/utils/constants';
import { DATA_TYPES } from '../../../../../utils/constants';
import {
Expand Down Expand Up @@ -105,9 +99,10 @@ class WhereExpression extends Component {
}
};

handleOperatorChange = (e, field) => {
handleOperatorChange = (e, field, form) => {
this.props.onMadeChanges();
field.onChange(e);
form.setFieldError('where', undefined);
};

handleChangeWrapper = (e, field) => {
Expand All @@ -123,9 +118,16 @@ class WhereExpression extends Component {
} = this.props;
// Explicitly invoking validation, this component unmount after it closes.
const fieldName = _.get(values, `${fieldPath}where.fieldName`, '');
const fieldOperator = _.get(values, `${fieldPath}where.operator`, 'is');
const fieldValue = _.get(values, `${fieldPath}where.fieldValue`, '');
if (fieldName > 0) {
await this.props.formik.validateForm();
}
if (
_.isEmpty(fieldName) ||
(!isNullOperator(fieldOperator) && _.isEmpty(fieldValue.toString()))
)
this.resetValues();
closeExpression(Expressions.WHERE);
};

Expand Down Expand Up @@ -182,7 +184,6 @@ class WhereExpression extends Component {
) : (
<FormikFieldNumber
name={`${fieldPath}where.fieldValue`}
fieldProps={{ validate: validateRequiredNumber }}
inputProps={{ onChange: this.handleChangeWrapper }}
formRow
rowProps={{ isInvalid, error: hasError }}
Expand All @@ -192,7 +193,6 @@ class WhereExpression extends Component {
return (
<FormikSelect
name={`${fieldPath}where.fieldValue`}
fieldProps={{ validate: required }}
inputProps={{
onChange: this.handleChangeWrapper,
options: WHERE_BOOLEAN_FILTERS,
Expand All @@ -204,7 +204,6 @@ class WhereExpression extends Component {
return (
<FormikFieldText
name={`${fieldPath}where.fieldValue`}
fieldProps={{ validate: required }}
inputProps={{ onChange: this.handleChangeWrapper, isInvalid }}
/>
);
Expand Down Expand Up @@ -267,7 +266,7 @@ class WhereExpression extends Component {
iconSide="right"
iconType="cross"
iconOnClick={() => this.resetValues()}
iconOnClickAriaLabel="Remove where filter"
iconOnClickAriaLabel="Remove filter"
onClick={() => {
openExpression(Expressions.WHERE);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,6 @@ export const AGGREGATION_TYPES = [
];

export const GROUP_BY_ERROR = 'Must specify at least 1 group by expression.';
export const QUERY_TYPE_GROUP_BY_ERROR = 'Can have a maximum of 1 group by selections.';

export const QUERY_TYPE_METRIC_ERROR = 'Can have a maximum of 1 metric selections.';
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export default class VisualGraph extends Component {
renderAggregationXYPlot = (data, groupedData) => {
const { annotation, thresholdValue, values, fieldName, aggregationType } = this.props;
const { hint } = this.state;
const xDomain = getBufferedXDomain(data);
const xDomain = getBufferedXDomain(data, values);
const yDomain = getYDomain(data);
const annotations = getAnnotationData(xDomain, yDomain, thresholdValue);
const xTitle = values.timeField;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
*/

import _ from 'lodash';
import moment from 'moment';

import { selectOptionValueToText } from '../../MonitorExpressions/expressions/utils/helpers';
import {
Expand Down Expand Up @@ -61,10 +62,14 @@ export function getXDomain(data) {
return [minDate.x, maxDate.x];
}

export function getBufferedXDomain(data) {
export function getBufferedXDomain(data, values) {
const { bucketValue, bucketUnitOfTime } = values;
const minDate = data[0].x;
const maxDate = data[data.length - 1].x;
const timeRange = maxDate - minDate;
// If minDate equals to maxDate, then use bucketValue and bucketUnitOfTime as timeRange.
let timeRange = maxDate - minDate;
if (!timeRange) timeRange = moment.duration(bucketValue, bucketUnitOfTime);

const minDateBuffer = minDate - timeRange * X_DOMAIN_BUFFER;
const maxDateBuffer = maxDate.getTime() + timeRange * X_DOMAIN_BUFFER;
return [minDateBuffer, maxDateBuffer];
Expand Down Expand Up @@ -111,13 +116,13 @@ export function getDataFromResponse(response, fieldName, monitorType) {
}
}

// Function for aggregation type monitors to get Map of data.
// The current response gives a large number of data aggregated in buckets, and this function returns the top n results with highest count of data points.
// The number n is based on the constant BAY_KEY_COUNT.
// Function for aggregation type monitors to get Map of data.
// The current response gives a large number of data aggregated in buckets, and this function returns the top n results with highest count of data points.
// The number n is based on the constant BAY_KEY_COUNT.
export function getMapDataFromResponse(response, fieldName, groupByFields) {
if (!response) return [];
const buckets = _.get(response, 'aggregations.composite_agg.buckets', []);
let allData = new Map();
const allData = new Map();
buckets.map((bucket) => {
const dataPoint = getXYValuesByFieldName(bucket, fieldName);
// Key of object is the string concat by group by field values
Expand All @@ -126,7 +131,7 @@ export function getMapDataFromResponse(response, fieldName, groupByFields) {
? allData.set(key, [dataPoint, ...allData.get(key)])
: allData.set(key, [dataPoint]);
});
let entryLength = [];
const entryLength = [];
for (const [key, value] of allData.entries()) {
allData.set(key, _.filter(value, filterInvalidYValues));
entryLength.push({ key, length: value.length });
Expand All @@ -141,7 +146,9 @@ export function getMapDataFromResponse(response, fieldName, groupByFields) {

export function getXYValuesByFieldName(bucket, fieldName) {
const x = new Date(bucket.key.date);
const path = bucket[fieldName] ? `${fieldName}.value` : 'doc_count';
// Parse the fieldName containing "." to "_"
const parsedFieldName = fieldName.replace(/\./g, '_');
const path = bucket[parsedFieldName] ? `${parsedFieldName}.value` : 'doc_count';
const y = _.get(bucket, path, null);
return { x, y };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class DefineMonitor extends Component {
// Default `count of documents` graph when using Bucket-level monitor
let graphs = [
<Fragment key={`multi-visual-graph-0`}>
<VisualGraph values={formikSnapshot} fieldName="doc_count" response={response} />,
<VisualGraph values={formikSnapshot} fieldName="doc_count" response={response} />
</Fragment>,
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import _ from 'lodash';
import { FORMIK_INITIAL_TRIGGER_VALUES, TRIGGER_TYPE } from '../../CreateTrigger/utils/constants';

export const validateTriggerName = (triggers, triggerToEdit, fieldPath) => (value) => {
if (!value) return 'Required';
if (!value) return 'Required.';
const nameExists = triggers.filter((trigger) => {
const triggerId = _.get(
trigger,
Expand All @@ -44,7 +44,7 @@ export const validateTriggerName = (triggers, triggerToEdit, fieldPath) => (valu
return triggerToEditId !== triggerId && triggerName.toLowerCase() === value.toLowerCase();
});
if (nameExists.length > 0) {
return 'Trigger name already used';
return 'Trigger name already used.';
}
// TODO: character restrictions
// TODO: character limits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ describe('validateTriggerName', () => {
});

test('returns Required string if falsy value', () => {
expect(validateTriggerName([], {})()).toBe('Required');
expect(validateTriggerName([], {})('')).toBe('Required');
expect(validateTriggerName([], {})()).toBe('Required.');
expect(validateTriggerName([], {})('')).toBe('Required.');
});
test('returns false if name already exists in monitor while creates new trigger', () => {
const triggers = [{ [TRIGGER_TYPE.QUERY_LEVEL]: { id: '123', name: 'Test' } }];
expect(validateTriggerName(triggers, { [TRIGGER_TYPE.QUERY_LEVEL]: {} })('Test')).toBe(
'Trigger name already used'
'Trigger name already used.'
);
});

Expand Down
Loading