-
Notifications
You must be signed in to change notification settings - Fork 48
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
Updating Preview Message functionality while setting notifications in detector alerts #1241
Changes from 3 commits
beb1a8b
6c24361
00d2e7f
4da8746
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import { | |
EuiCompressedSwitch, | ||
EuiText, | ||
EuiCompressedTextArea, | ||
EuiCompressedCheckbox, | ||
} from '@elastic/eui'; | ||
import React, { useState } from 'react'; | ||
import { NOTIFICATIONS_HREF } from '../../utils/constants'; | ||
|
@@ -24,8 +25,10 @@ import { | |
NotificationChannelOption, | ||
NotificationChannelTypeOptions, | ||
TriggerAction, | ||
TriggerContext, | ||
} from '../../../types'; | ||
import { getIsNotificationPluginInstalled } from '../../utils/helpers'; | ||
import Mustache from 'mustache'; | ||
|
||
export interface NotificationFormProps { | ||
allNotificationChannels: NotificationChannelTypeOptions[]; | ||
|
@@ -37,10 +40,12 @@ export interface NotificationFormProps { | |
onMessageBodyChange: (message: string) => void; | ||
onMessageSubjectChange: (subject: string) => void; | ||
onNotificationToggle?: (enabled: boolean) => void; | ||
context: TriggerContext; | ||
} | ||
|
||
export const NotificationForm: React.FC<NotificationFormProps> = ({ | ||
action, | ||
context, | ||
allNotificationChannels, | ||
loadingNotifications, | ||
prepareMessage, | ||
|
@@ -53,6 +58,8 @@ export const NotificationForm: React.FC<NotificationFormProps> = ({ | |
const hasNotificationPlugin = getIsNotificationPluginInstalled(); | ||
const [shouldSendNotification, setShouldSendNotification] = useState(!!action?.destination_id); | ||
const selectedNotificationChannelOption: NotificationChannelOption[] = []; | ||
const onDisplayPreviewChange = (e) => setDisplayPreview(e.target.checked); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. memoize this using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
const [displayPreview, setDisplayPreview] = useState(false); | ||
if (shouldSendNotification && action?.destination_id) { | ||
allNotificationChannels.forEach((typeOption) => { | ||
const matchingChannel = typeOption.options.find( | ||
|
@@ -61,7 +68,17 @@ export const NotificationForm: React.FC<NotificationFormProps> = ({ | |
if (matchingChannel) selectedNotificationChannelOption.push(matchingChannel); | ||
}); | ||
} | ||
|
||
let preview = ''; | ||
try { | ||
preview = `${Mustache.render(action?.subject_template.source, context)}\n\n${Mustache.render( | ||
action?.message_template.source, | ||
context | ||
)}`; | ||
} catch (err) { | ||
preview = `There was an error rendering message preview: ${err.message}`; | ||
console.error('There was an error rendering mustache template', err); | ||
amsiglan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
``; | ||
return ( | ||
<> | ||
<EuiCompressedSwitch | ||
|
@@ -168,18 +185,27 @@ export const NotificationForm: React.FC<NotificationFormProps> = ({ | |
/> | ||
</EuiCompressedFormRow> | ||
</EuiFlexItem> | ||
{prepareMessage && ( | ||
|
||
<EuiFlexItem> | ||
<EuiCompressedCheckbox | ||
id="notification-message-preview-checkbox" | ||
label="Preview message" | ||
checked={displayPreview} | ||
onChange={onDisplayPreviewChange} | ||
/> | ||
</EuiFlexItem> | ||
{displayPreview ? ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: We can also do `displayPreview && () instead of the ternary to keep it short There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
<EuiFlexItem> | ||
<EuiCompressedFormRow> | ||
<EuiSmallButton | ||
fullWidth={false} | ||
onClick={() => prepareMessage(true /* updateMessage */)} | ||
> | ||
Generate message | ||
</EuiSmallButton> | ||
<EuiCompressedFormRow label="Message preview" fullWidth> | ||
<EuiCompressedTextArea | ||
placeholder="Preview of mustache template" | ||
fullWidth | ||
value={preview} | ||
readOnly | ||
/> | ||
</EuiCompressedFormRow> | ||
</EuiFlexItem> | ||
)} | ||
) : null} | ||
</EuiFlexGroup> | ||
</EuiAccordion> | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ import { | |
NotificationChannelTypeOptions, | ||
} from '../../../../../../../types'; | ||
import { NotificationForm } from '../../../../../../components/Notifications/NotificationForm'; | ||
import { ALERT_SEVERITY_OPTIONS } from '../../../../../../utils/constants'; | ||
import { ALERT_SEVERITY_OPTIONS, DEFAULT_MESSAGE_SOURCE } from '../../../../../../utils/constants'; | ||
|
||
interface AlertConditionPanelProps extends RouteComponentProps { | ||
alertCondition: AlertCondition; | ||
|
@@ -99,6 +99,27 @@ export default class AlertConditionPanel extends Component< | |
}); | ||
} | ||
|
||
getTriggerContext = () => { | ||
const lineBreakAndTab = '\n\t'; | ||
const { alertCondition, detector } = this.props; | ||
const detectorInput = detector.inputs[0].detector_input; | ||
const detectorIndices = `${lineBreakAndTab}${detectorInput.indices.join( | ||
`,${lineBreakAndTab}` | ||
)}`; | ||
return { | ||
trigger: { | ||
name: alertCondition.name, | ||
severity: | ||
parseAlertSeverityToOption(alertCondition.severity)?.label || alertCondition.severity, | ||
}, | ||
detector: { | ||
name: detector.name, | ||
description: detectorInput.description, | ||
datasources: detectorIndices, | ||
}, | ||
}; | ||
}; | ||
|
||
// When component mounts, we prepare message but at this point we don't want to emit the | ||
// trigger changed metric since it is not user initiated. So we use the onMount flag to determine that | ||
// and pass it downstream accordingly. | ||
|
@@ -113,7 +134,7 @@ export default class AlertConditionPanel extends Component< | |
parseAlertSeverityToOption(alertCondition.severity)?.label || alertCondition.severity | ||
}`; | ||
const detectorName = `Threat detector: ${detector.name}`; | ||
const defaultSubject = [alertConditionName, alertConditionSeverity, detectorName].join(' - '); | ||
const defaultSubject = DEFAULT_MESSAGE_SOURCE.MESSAGE_SUBJECT; | ||
|
||
if (updateMessage || !alertCondition.actions[0]?.subject_template.source) | ||
this.onMessageSubjectChange(defaultSubject, !onMount); | ||
|
@@ -157,7 +178,7 @@ export default class AlertConditionPanel extends Component< | |
if (alertConditionSelections.length) | ||
defaultMessageBody = | ||
defaultMessageBody + lineBreak + lineBreak + alertConditionSelections.join(lineBreak); | ||
this.onMessageBodyChange(defaultMessageBody, !onMount); | ||
this.onMessageBodyChange(DEFAULT_MESSAGE_SOURCE.MESSAGE_BODY, !onMount); | ||
} | ||
}; | ||
|
||
|
@@ -286,6 +307,7 @@ export default class AlertConditionPanel extends Component< | |
}; | ||
|
||
render() { | ||
const context = this.getTriggerContext(); | ||
const { | ||
alertCondition = getEmptyAlertCondition(), | ||
allNotificationChannels, | ||
|
@@ -537,6 +559,9 @@ export default class AlertConditionPanel extends Component< | |
|
||
<NotificationForm | ||
action={alertCondition.actions[0]} | ||
context={{ | ||
ctx: context, | ||
}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem to match the expected type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
allNotificationChannels={allNotificationChannels} | ||
loadingNotifications={loadingNotifications} | ||
prepareMessage={this.prepareMessage} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use named imports here and other places too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done