Skip to content

Commit

Permalink
fix(watch): Add WatchErrors to capture invalid watches (elastic#23887) (
Browse files Browse the repository at this point in the history
elastic#24755)

Instead of immediately throwing an error if a watch Action is invalid (an email for example), there is now an `option` object that we can pass to the `fromUpstreamJson()` method and receive back any error that might exist in a Watch.
The Watch has a new "watchErrors" property to display configuration error in the UI.

fixes elastic#20305
fixes elastic#20970
  • Loading branch information
sebelga authored Oct 29, 2018
1 parent 4329778 commit 500da50
Show file tree
Hide file tree
Showing 42 changed files with 825 additions and 248 deletions.
5 changes: 5 additions & 0 deletions x-pack/plugins/watcher/common/constants/action_states.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,9 @@ export const ACTION_STATES = {
defaultMessage: 'Error'
}),

// Action has a configuration error
CONFIG_ERROR: i18n.translate('xpack.watcher.constants.actionStates.configErrorStateText', {
defaultMessage: 'Config error'
}),

};
11 changes: 11 additions & 0 deletions x-pack/plugins/watcher/common/constants/error_codes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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.
*/

export const ERROR_CODES = {

// Property missing on object
ERR_PROP_MISSING: 'ERR_PROP_MISSING',
};
1 change: 1 addition & 0 deletions x-pack/plugins/watcher/common/constants/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ export { WATCH_STATE_COMMENTS } from './watch_state_comments';
export { WATCH_HISTORY } from './watch_history';
export { WATCH_STATES } from './watch_states';
export { WATCH_TYPES } from './watch_types';
export { ERROR_CODES } from './error_codes';
4 changes: 4 additions & 0 deletions x-pack/plugins/watcher/common/constants/watch_states.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,8 @@ export const WATCH_STATES = {
defaultMessage: 'Error!'
}),

CONFIG_ERROR: i18n.translate('xpack.watcher.constants.watchStates.configErrorStateText', {
defaultMessage: 'Config error'
}),

};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
></div>
<div
class="kuiIcon kuiIcon--error fa-exclamation-triangle"
ng-if="actionStateIcon.actionStatus.state === actionStateIcon.ACTION_STATES.ERROR"
ng-if="actionStateIcon.actionStatus.state === actionStateIcon.ACTION_STATES.ERROR || actionStateIcon.actionStatus.state === actionStateIcon.ACTION_STATES.CONFIG_ERROR"
></div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<div class="kuiModal">
<div class="kuiModalHeader">
<h1 class="kuiModalHeader__title" id="watcher__error-display-modal-title">{{ vm.title }}</h1>
</div>
<div class="kuiModalBody">
<ul>
<li class="kuiVerticalRhythm" ng-repeat="error in vm.errors">{{ error.message }}</li>
</ul>
</div>
<div class="kuiModalFooter">
<button
ng-click="vm.close()"
class="kuiButton kuiButton--primary"
aria-label="close">
Close
</button>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* 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 { uiModules } from 'ui/modules';

const app = uiModules.get('xpack/watcher');

app.controller('WatcherErrorsDisplayController', function WatcherErrorsDisplayController($scope, $modalInstance, params) {
this.title = params.title;
this.errors = params.errors;

this.close = function close() {
$modalInstance.close();
};
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* 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 './errors_display_modal';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@
></div>
<div
class="kuiIcon kuiIcon--error fa-exclamation-triangle"
ng-if="watchStateIcon.watchStatus.state === watchStateIcon.WATCH_STATES.ERROR"
ng-if="watchStateIcon.watchStatus.state === watchStateIcon.WATCH_STATES.ERROR || watchStateIcon.watchStatus.state === watchStateIcon.WATCH_STATES.CONFIG_ERROR"
></div>
</div>
5 changes: 4 additions & 1 deletion x-pack/plugins/watcher/public/models/action/email_action.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ export class EmailAction extends BaseAction {
Object.assign(result, {
to: this.to,
subject: this.subject,
body: this.body
body: this.body,
email: {
to: this.to.length ? this.to : undefined,
}
});

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ export class LoggingAction extends BaseAction {

get upstreamJson() {
const result = super.upstreamJson;
const text = !!this.text.trim() ? this.text : undefined;

Object.assign(result, {
text: this.text
text,
logging: {
text,
}
});

return result;
Expand Down
27 changes: 25 additions & 2 deletions x-pack/plugins/watcher/public/models/action/slack_action.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,35 @@ export class SlackAction extends BaseAction {
this.text = props.text;
}

validate() {
const errors = [];

if (!this.to.length) {
errors.push({
message: i18n.translate('xpack.watcher.sections.watchEdit.json.warningPossibleInvalidSlackAction.description', {
// eslint-disable-next-line max-len
defaultMessage: 'This watch has a Slack action without a "to" property. This watch will only be valid if you specified the "to" property in the Slack "message_default" setting in Elasticsearch.'
})
});
}

return { errors: errors.length ? errors : null };
}

get upstreamJson() {
const result = super.upstreamJson;

const message = this.text || this.to.length
? {
text: this.text,
to: this.to.length ? this.to : undefined
}
: undefined;
Object.assign(result, {
to: this.to,
text: this.text
text: this.text,
slack: {
message
}
});

return result;
Expand Down
44 changes: 44 additions & 0 deletions x-pack/plugins/watcher/public/models/watch/base_watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getSearchValue } from 'plugins/watcher/lib/get_search_value';
import { get, isEqual, remove, map, merge } from 'lodash';
import { Action } from '../action';
import { WatchStatus } from '../watch_status';
import { WatchErrors } from '../watch_errors';
import { createActionId } from './lib/create_action_id';
import { checkActionIdCollision } from './lib/check_action_id_collision';
import { i18n } from '@kbn/i18n';
Expand All @@ -31,6 +32,7 @@ export class BaseWatch {
this.name = get(props, 'name', '');
this.isSystemWatch = Boolean(get(props, 'isSystemWatch'));
this.watchStatus = WatchStatus.fromUpstreamJson(get(props, 'watchStatus'));
this.watchErrors = WatchErrors.fromUpstreamJson(get(props, 'watchErrors'));

const actions = get(props, 'actions', []);
this.actions = actions.map(Action.fromUpstreamJson);
Expand Down Expand Up @@ -76,6 +78,10 @@ export class BaseWatch {
remove(this.actions, action);
}

resetActions = () => {
this.actions = [];
};

get displayName() {
if (this.isNew) {
return i18n.translate('xpack.watcher.models.baseWatch.displayName', {
Expand Down Expand Up @@ -130,6 +136,44 @@ export class BaseWatch {
return isEqual(cleanWatch, cleanOtherWatch);
}

/**
* Client validation of the Watch.
* Currently we are *only* validating the Watch "Actions"
*/
validate() {

// Get the errors from each watch action
const actionsErrors = this.actions.reduce((actionsErrors, action) => {
if (action.validate) {
const { errors } = action.validate();
if (!errors) {
return actionsErrors;
}
return [...actionsErrors, ...errors];
}
return actionsErrors;
}, []);

if (!actionsErrors.length) {
return { warning: null };
}

// Concatenate their message
const warningMessage = actionsErrors.reduce((message, error) => (
!!message
? `${message}, ${error.message}`
: error.message
), '');

// We are not doing any *blocking* validation in the client,
// so we return the errors as a _warning_
return {
warning: {
message: warningMessage,
}
};
}

static typeName = i18n.translate('xpack.watcher.models.baseWatch.typeName', {
defaultMessage: 'Watch',
});
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/watcher/public/models/watch_errors/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* 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.
*/

export { WatchErrors } from './watch_errors';
17 changes: 17 additions & 0 deletions x-pack/plugins/watcher/public/models/watch_errors/watch_errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* 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 { get } from 'lodash';

export class WatchErrors {
constructor(props = {}) {
this.actionErrors = get(props, 'actions');
}

static fromUpstreamJson(upstreamWatchErrors) {
return new WatchErrors(upstreamWatchErrors);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
{{ 'xpack.watcher.sections.watchDetail.actionStatusTable.stateColumnLabel' | i18n: { defaultMessage: 'State' } }}
</sortable-column>
</th>
<th scope="col" class="kuiTableHeaderCell" ng-if="actionStatusTable.actionErrors">
<span class="kuiTableHeaderCell__liner">Errors</span>
</th>
<th scope="col" class="kuiTableHeaderCell">
</th>
</tr>
Expand All @@ -39,6 +42,14 @@
{{ actionStatus.state }}
</div>
</td>
<td class="kuiTableRowCell" ng-if="actionStatusTable.actionErrors">
<div class="kuiTableRowCell__liner">
<a
role="button"
ng-if="actionStatusTable.actionErrors[actionStatus.id]"
ng-click="actionStatusTable.showErrors(actionStatus.id, actionStatusTable.actionErrors[actionStatus.id])">{{actionStatusTable.getLabelErrors(actionStatus.id)}}</a>
</div>
</td>
<td class="kuiTableRowCell">
<div class="kuiTableRowCell__liner">
<div class="kuiMenuButtonGroup kuiMenuButtonGroup--alignRight">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,36 @@ import template from './action_status_table.html';

const app = uiModules.get('xpack/watcher');

app.directive('actionStatusTable', function () {
app.directive('actionStatusTable', function ($injector, i18n) {
return {
restrict: 'E',
replace: true,
template: template,
scope: {
actionStatuses: '=',
actionErrors: '=',
sortField: '=',
sortReverse: '=',
onSortChange: '=',
onActionAcknowledge: '=',
showErrors: '='
},
bindToController: true,
controllerAs: 'actionStatusTable',
controller: class ActionStatusTableController {}
controller: class ActionStatusTableController {
getLabelErrors(actionId) {
const errors = this.actionErrors[actionId];
const total = errors.length;

const label = i18n('xpack.watcher.sections.watchDetail.actionStatusTotalErrors', {
defaultMessage: '{total, number} {total, plural, one {error} other {errors}}',
values: {
total,
}
});

return label;
}
}
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@
<!-- Action status table -->
<action-status-table
action-statuses="watchDetail.sortedActionStatuses"
action-errors="watchDetail.actionErrors"
on-action-acknowledge="watchDetail.onActionAcknowledge"
sort-field="watchDetail.actionStatusTableSortField"
sort-reverse="watchDetail.actionStatusTableSortReverse"
on-sort-change="watchDetail.onActionSortChange"
show-errors="watchDetail.showErrors"
></action-status-table>
<table-info ng-show="!watchDetail.hasActionStatusTableActions">
{{ 'xpack.watcher.sections.watchDetail.noActionsFoundText' | i18n: { defaultMessage: 'No actions found.' } }}
Expand Down
Loading

0 comments on commit 500da50

Please sign in to comment.