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

New feature - Alert muting #4276

Merged
merged 7 commits into from
Nov 2, 2019
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
39 changes: 38 additions & 1 deletion client/app/pages/alert/Alert.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class AlertPage extends React.Component {
options: {
op: '>',
value: 1,
muted: false,
},
}),
pendingRearm: 0,
Expand Down Expand Up @@ -159,6 +160,30 @@ class AlertPage extends React.Component {
});
};

mute = () => {
const { alert } = this.state;
return alert.$mute()
.then(() => {
this.setAlertOptions({ muted: true });
notification.warn('Notifications have been muted.');
})
.catch(() => {
notification.error('Failed muting notifications.');
});
}

unmute = () => {
const { alert } = this.state;
return alert.$unmute()
.then(() => {
this.setAlertOptions({ muted: false });
notification.success('Notifications have been restored.');
})
.catch(() => {
notification.error('Failed restoring notifications.');
});
}

edit = () => {
const { id } = this.state.alert;
navigateTo(`/alerts/${id}/edit`, true, false);
Expand All @@ -177,11 +202,15 @@ class AlertPage extends React.Component {
return <LoadingState className="m-t-30" />;
}

const muted = !!alert.options.muted;
const { queryResult, mode, canEdit, pendingRearm } = this.state;

const menuButton = (
<MenuButton
doDelete={this.delete}
muted={muted}
mute={this.mute}
unmute={this.unmute}
canEdit={canEdit}
/>
);
Expand All @@ -202,7 +231,15 @@ class AlertPage extends React.Component {
return (
<div className="container alert-page">
{mode === MODES.NEW && <AlertNew {...commonProps} />}
{mode === MODES.VIEW && <AlertView canEdit={canEdit} onEdit={this.edit} {...commonProps} />}
{mode === MODES.VIEW && (
<AlertView
canEdit={canEdit}
onEdit={this.edit}
muted={muted}
unmute={this.unmute}
{...commonProps}
/>
)}
{mode === MODES.EDIT && <AlertEdit cancel={this.cancel} {...commonProps} />}
</div>
);
Expand Down
32 changes: 32 additions & 0 deletions client/app/pages/alert/AlertView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Alert as AlertType } from '@/components/proptypes';
import Form from 'antd/lib/form';
import Button from 'antd/lib/button';
import Tooltip from 'antd/lib/tooltip';
import AntAlert from 'antd/lib/alert';

import Title from './components/Title';
import Criteria from './components/Criteria';
Expand Down Expand Up @@ -47,6 +48,17 @@ AlertState.defaultProps = {

// eslint-disable-next-line react/prefer-stateless-function
export default class AlertView extends React.Component {
state = {
unmuting: false,
}

unmute = () => {
this.setState({ unmuting: true });
this.props.unmute().finally(() => {
this.setState({ unmuting: false });
});
}

render() {
const { alert, queryResult, canEdit, onEdit, menuButton } = this.props;
const { query, name, options, rearm } = alert;
Expand Down Expand Up @@ -87,6 +99,24 @@ export default class AlertView extends React.Component {
</Form>
</div>
<div className="col-md-4">
{options.muted && (
<AntAlert
className="m-b-20"
message={<><i className="fa fa-bell-slash-o" /> Notifications are muted</>}
description={(
<>
Notifications for this alert will not be sent.<br />
{canEdit && (
<>
To restore notifications click
<Button size="small" type="primary" onClick={this.unmute} loading={this.state.unmuting} className="m-t-5 m-l-5">Unmute</Button>
</>
)}
</>
)}
type="warning"
/>
)}
<h4>Destinations{' '}
<Tooltip title="Open Alert Destinations page in a new tab.">
<a href="destinations" target="_blank">
Expand All @@ -108,8 +138,10 @@ AlertView.propTypes = {
canEdit: PropTypes.bool.isRequired,
onEdit: PropTypes.func.isRequired,
menuButton: PropTypes.node.isRequired,
unmute: PropTypes.func,
};

AlertView.defaultProps = {
queryResult: null,
unmute: null,
};
8 changes: 5 additions & 3 deletions client/app/pages/alert/components/AlertDestinations.less
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
.alert-destinations {
ul {
position: relative;

ul {
list-style: none;
padding: 0;
margin-top: 15px;
Expand Down Expand Up @@ -34,8 +36,8 @@

.add-button {
position: absolute;
right: 14px;
top: 9px;
right: 0;
top:-33px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just locks the "+Add" button to its parent instead of the whole wrapper.
Needed cause this section gets pushed down by the "Notifications are muted" block.

}
}

Expand Down
23 changes: 22 additions & 1 deletion client/app/pages/alert/components/MenuButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,16 @@ import Button from 'antd/lib/button';
import Icon from 'antd/lib/icon';


export default function MenuButton({ doDelete, canEdit }) {
export default function MenuButton({ doDelete, canEdit, mute, unmute, muted }) {
const [loading, setLoading] = useState(false);

const execute = useCallback((action) => {
setLoading(true);
action().finally(() => {
setLoading(false);
});
}, []);

const confirmDelete = useCallback(() => {
Modal.confirm({
title: 'Delete Alert',
Expand All @@ -36,6 +43,13 @@ export default function MenuButton({ doDelete, canEdit }) {
placement="bottomRight"
overlay={(
<Menu>
<Menu.Item>
{muted ? (
<a onClick={() => execute(unmute)}>Unmute Notifications</a>
) : (
<a onClick={() => execute(mute)}>Mute Notifications</a>
)}
</Menu.Item>
<Menu.Item>
<a onClick={confirmDelete}>Delete Alert</a>
</Menu.Item>
Expand All @@ -52,4 +66,11 @@ export default function MenuButton({ doDelete, canEdit }) {
MenuButton.propTypes = {
doDelete: PropTypes.func.isRequired,
canEdit: PropTypes.bool.isRequired,
mute: PropTypes.func.isRequired,
unmute: PropTypes.func.isRequired,
muted: PropTypes.bool,
};

MenuButton.defaultProps = {
muted: false,
};
7 changes: 7 additions & 0 deletions client/app/pages/alerts/AlertsList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ class AlertsList extends React.Component {
};

listColumns = [
Columns.custom.sortable((text, alert) => (
<i className={`fa fa-bell-${alert.options.muted ? 'slash' : 'o'} p-r-0`} />
), {
title: <i className="fa fa-bell p-r-0" />,
field: 'muted',
width: '1%',
}),
Columns.custom.sortable((text, alert) => (
<div>
<a className="table-main-title" href={'alerts/' + alert.id}>{alert.name}</a>
Expand Down
2 changes: 2 additions & 0 deletions client/app/services/alert.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ function AlertService($resource, $http) {
return newData;
}].concat($http.defaults.transformRequest),
},
mute: { method: 'POST', url: 'api/alerts/:id/mute' },
unmute: { method: 'DELETE', url: 'api/alerts/:id/mute' },
};
return $resource('api/alerts/:id', { id: '@id' }, actions);
}
Expand Down
28 changes: 28 additions & 0 deletions redash/handlers/alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,34 @@ def delete(self, alert_id):
models.db.session.commit()


class AlertMuteResource(BaseResource):
def post(self, alert_id):
alert = get_object_or_404(models.Alert.get_by_id_and_org, alert_id, self.current_org)
require_admin_or_owner(alert.user.id)

alert.options['muted'] = True
models.db.session.commit()
kravets-levko marked this conversation as resolved.
Show resolved Hide resolved

self.record_event({
'action': 'mute',
'object_id': alert.id,
'object_type': 'alert'
})

def delete(self, alert_id):
alert = get_object_or_404(models.Alert.get_by_id_and_org, alert_id, self.current_org)
require_admin_or_owner(alert.user.id)

alert.options['muted'] = False
models.db.session.commit()

self.record_event({
'action': 'unmute',
'object_id': alert.id,
'object_type': 'alert'
})


class AlertListResource(BaseResource):
def post(self):
req = request.get_json(True)
Expand Down
4 changes: 3 additions & 1 deletion redash/handlers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
from flask_restful import Api
from werkzeug.wrappers import Response

from redash.handlers.alerts import (AlertListResource, AlertResource,
from redash.handlers.alerts import (AlertListResource,
AlertResource, AlertMuteResource,
AlertSubscriptionListResource,
AlertSubscriptionResource)
from redash.handlers.base import org_scoped_rule
Expand Down Expand Up @@ -75,6 +76,7 @@ def json_representation(data, code, headers=None):


api.add_org_resource(AlertResource, '/api/alerts/<alert_id>', endpoint='alert')
api.add_org_resource(AlertMuteResource, '/api/alerts/<alert_id>/mute', endpoint='alert_mute')
api.add_org_resource(AlertSubscriptionListResource, '/api/alerts/<alert_id>/subscriptions', endpoint='alert_subscriptions')
api.add_org_resource(AlertSubscriptionResource, '/api/alerts/<alert_id>/subscriptions/<subscriber_id>', endpoint='alert_subscription')
api.add_org_resource(AlertListResource, '/api/alerts', endpoint='alerts')
Expand Down
4 changes: 4 additions & 0 deletions redash/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,10 @@ def custom_subject(self):
def groups(self):
return self.query_rel.groups

@property
def muted(self):
return self.options.get('muted', False)


def generate_slug(ctx):
slug = utils.slugify(ctx.current_parameters['name'])
Expand Down
4 changes: 4 additions & 0 deletions redash/tasks/alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,8 @@ def check_alerts_for_query(query_id):
logger.debug("Skipping notification (previous state was unknown and now it's ok).")
continue

if alert.muted:
logger.debug("Skipping notification (alert muted).")
continue

notify_subscriptions(alert, new_state)
9 changes: 9 additions & 0 deletions tests/tasks/test_alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ def test_doesnt_notify_when_nothing_changed(self):

self.assertFalse(redash.tasks.alerts.notify_subscriptions.called)

def test_doesnt_notify_when_muted(self):
redash.tasks.alerts.notify_subscriptions = MagicMock()
Alert.evaluate = MagicMock(return_value=Alert.TRIGGERED_STATE)

alert = self.factory.create_alert(options={"muted": True})
check_alerts_for_query(alert.query_id)

self.assertFalse(redash.tasks.alerts.notify_subscriptions.called)


class TestNotifySubscriptions(BaseTestCase):
def test_calls_notify_for_subscribers(self):
Expand Down