From ad6543afae2cb714c4ad2bb1745501efded4bd93 Mon Sep 17 00:00:00 2001 From: Ran Byron Date: Thu, 17 Oct 2019 14:08:30 +0300 Subject: [PATCH 1/2] Fix: Alert condition rendering --- client/app/pages/alert/Alert.jsx | 6 ++- .../app/pages/alert/components/Criteria.jsx | 48 +++++++++---------- .../alert/components/NotificationTemplate.jsx | 3 +- redash/models/__init__.py | 17 ++++++- 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/client/app/pages/alert/Alert.jsx b/client/app/pages/alert/Alert.jsx index efe8f810b1..a9c221a2ce 100644 --- a/client/app/pages/alert/Alert.jsx +++ b/client/app/pages/alert/Alert.jsx @@ -13,6 +13,7 @@ import LoadingState from '@/components/items-list/components/LoadingState'; import AlertView from './AlertView'; import AlertEdit from './AlertEdit'; import AlertNew from './AlertNew'; +import { getConditionText } from './components/Criteria'; import Modal from 'antd/lib/modal'; @@ -25,13 +26,14 @@ const MODES = { EDIT: 2, }; -const defaultNameBuilder = template('<%= query.name %>: <%= options.column %> <%= options.op %> <%= options.value %>'); +// eslint-disable-next-line no-template-curly-in-string +const defaultNameBuilder = template('${query.name}: ${options.column} ${conditionText} ${options.value}'); export function getDefaultName(alert) { if (!alert.query) { return 'New Alert'; } - return defaultNameBuilder(alert); + return defaultNameBuilder({ ...alert, conditionText: getConditionText(alert.options.op) }); } class AlertPage extends React.Component { diff --git a/client/app/pages/alert/components/Criteria.jsx b/client/app/pages/alert/components/Criteria.jsx index 57516fd899..f10b47bf45 100644 --- a/client/app/pages/alert/components/Criteria.jsx +++ b/client/app/pages/alert/components/Criteria.jsx @@ -12,14 +12,24 @@ import { AlertOptions as AlertOptionsType } from '@/components/proptypes'; import './Criteria.less'; const CONDITIONS = { - '>': '\u003e', - '>=': '\u2265', - '<': '\u003c', - '<=': '\u2264', - '==': '\u003d', - '!=': '\u2260', + '>': ['\u003e', 'greater than'], + '>=': ['\u2265', 'greater than or equals'], + '<': ['\u003c', 'less than'], + '<=': ['\u2264', 'less than or equals'], + '==': ['\u003d', 'equals'], + '!=': ['\u2260', 'not equal to'], }; +function getConditionOption(key) { + const [char, text] = CONDITIONS[key]; + return {char} {text}; +} + +export function getConditionText(key) { + const [, text] = CONDITIONS[key]; + return text; +} + const VALID_STRING_CONDITIONS = ['==', '!=']; function DisabledInput({ children, minWidth }) { @@ -87,33 +97,21 @@ export default function Criteria({ columnNames, resultValues, alertOptions, onCh dropdownMatchSelectWidth={false} style={{ width: 55 }} > - ']}> - {CONDITIONS['>']} greater than - - =']}> - {CONDITIONS['>=']} greater than or equals - + {getConditionOption('>')} + {getConditionOption('>=')} - - {CONDITIONS['<']} less than - - - {CONDITIONS['<=']} less than or equals - + {getConditionOption('<')} + {getConditionOption('<=')} - - {CONDITIONS['==']} equals - - - {CONDITIONS['!=']} not equal to - + {getConditionOption('==')} + {getConditionOption('!=')} ) : ( - {CONDITIONS[alertOptions.op]} + {head(CONDITIONS[alertOptions.op])} )}
diff --git a/client/app/pages/alert/components/NotificationTemplate.jsx b/client/app/pages/alert/components/NotificationTemplate.jsx index 31e10bdc12..6d971aeeb5 100644 --- a/client/app/pages/alert/components/NotificationTemplate.jsx +++ b/client/app/pages/alert/components/NotificationTemplate.jsx @@ -10,6 +10,7 @@ import Input from 'antd/lib/input'; import Select from 'antd/lib/select'; import Modal from 'antd/lib/modal'; import Switch from 'antd/lib/switch'; +import { getConditionText } from './Criteria'; import './NotificationTemplate.less'; @@ -19,7 +20,7 @@ function normalizeCustomTemplateData(alert, query, columnNames, resultValues) { return { ALERT_STATUS: 'TRIGGERED', - ALERT_CONDITION: alert.options.op, + ALERT_CONDITION: getConditionText(alert.options.op), ALERT_THRESHOLD: alert.options.value, ALERT_NAME: alert.name, ALERT_URL: `${window.location.origin}/alerts/${alert.id}`, diff --git a/redash/models/__init__.py b/redash/models/__init__.py index 25744d264f..3337b59ba7 100644 --- a/redash/models/__init__.py +++ b/redash/models/__init__.py @@ -785,6 +785,15 @@ class Alert(TimestampMixin, BelongsToOrgMixin, db.Model): OK_STATE = 'ok' TRIGGERED_STATE = 'triggered' + CONDITION_TEXTS = { + '>': 'greater than', + '>=': 'greater than or equals', + '<': 'less than', + '<=': 'less than or equals', + '==': 'equals', + '!=': 'not equal to', + } + id = Column(db.Integer, primary_key=True) name = Column(db.String(255)) query_id = Column(db.Integer, db.ForeignKey("queries.id")) @@ -870,7 +879,7 @@ def render_template(self, template): 'ALERT_NAME': self.name, 'ALERT_URL': '{host}/alerts/{alert_id}'.format(host=host, alert_id=self.id), 'ALERT_STATUS': self.state.upper(), - 'ALERT_CONDITION': self.options['op'], + 'ALERT_CONDITION': self.conditionText, 'ALERT_THRESHOLD': self.options['value'], 'QUERY_NAME': self.query_rel.name, 'QUERY_URL': '{host}/queries/{query_id}'.format(host=host, query_id=self.query_rel.id), @@ -894,6 +903,12 @@ def custom_subject(self): def groups(self): return self.query_rel.groups + @property + def conditionText(self): + condition = self.options['op']; + default = condition # backwards compatibility + return self.CONDITION_TEXTS.get(condition, default); + def generate_slug(ctx): slug = utils.slugify(ctx.current_parameters['name']) From 2389447613c709a9d77bf61799e6892919fff4d6 Mon Sep 17 00:00:00 2001 From: Ran Byron Date: Thu, 17 Oct 2019 15:44:55 +0300 Subject: [PATCH 2/2] Fixed template render test Made getConditionText safer --- client/app/pages/alert/components/Criteria.jsx | 5 ++--- client/cypress/integration/alert/edit_alert_spec.js | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/client/app/pages/alert/components/Criteria.jsx b/client/app/pages/alert/components/Criteria.jsx index f10b47bf45..aecb1ca7c4 100644 --- a/client/app/pages/alert/components/Criteria.jsx +++ b/client/app/pages/alert/components/Criteria.jsx @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { head, includes, toString, isEmpty } from 'lodash'; +import { head, includes, toString, isEmpty, get } from 'lodash'; import Input from 'antd/lib/input'; import Icon from 'antd/lib/icon'; @@ -26,8 +26,7 @@ function getConditionOption(key) { } export function getConditionText(key) { - const [, text] = CONDITIONS[key]; - return text; + return get(CONDITIONS, [key, 1], key); } const VALID_STRING_CONDITIONS = ['==', '!=']; diff --git a/client/cypress/integration/alert/edit_alert_spec.js b/client/cypress/integration/alert/edit_alert_spec.js index 503c0d508c..a7be3ab61c 100644 --- a/client/cypress/integration/alert/edit_alert_spec.js +++ b/client/cypress/integration/alert/edit_alert_spec.js @@ -28,7 +28,7 @@ describe('Edit Alert', () => { it('previews rendered template correctly', () => { const options = { value: '123', - op: '=', + op: '==', custom_subject: '{{ ALERT_CONDITION }}', custom_body: '{{ ALERT_THRESHOLD }}', }; @@ -38,7 +38,7 @@ describe('Edit Alert', () => { .then(({ id: alertId }) => { cy.visit(`/alerts/${alertId}/edit`); cy.get('.alert-template-preview').click(); - cy.getByTestId('CustomSubject').should('have.value', options.op); + cy.getByTestId('CustomSubject').should('have.value', 'equals'); cy.getByTestId('CustomBody').should('have.value', options.value); }); });