diff --git a/Makefile b/Makefile index 64f6630b78..8efb8e266e 100644 --- a/Makefile +++ b/Makefile @@ -181,7 +181,7 @@ ensure-yarn: # Yarn ensures the correct version of yarn is installed yarn: corepack enable - corepack prepare yarn@stable --activate + corepack prepare yarn@$(YARN_VERSION) --activate check-js: generate $(NODE_DEPS) $(MAKE) ensure-yarn diff --git a/notification/slack/usergrouperror.go b/notification/slack/usergrouperror.go index a6c87c253f..f6825fc792 100644 --- a/notification/slack/usergrouperror.go +++ b/notification/slack/usergrouperror.go @@ -19,8 +19,6 @@ type userGroupError struct { ScheduleName string Missing []notification.User - OnCall string - callbackFunc func(string, ...url.Values) string } @@ -56,23 +54,17 @@ func slackLink(url, label string) string { } // userGroupErrorMissing is a template for when a user-group update fails because one or more users are missing from Slack. -var userGroupErrorMissing = template.Must(template.New("userGroupErrorMissing").Parse(`{{.OnCall}} - -Hey everyone! I couldn't update {{.GroupRef}} because I couldn't find the following user(s) in Slack: {{.MissingUserRefs}} +var userGroupErrorMissing = template.Must(template.New("userGroupErrorMissing").Parse(`Hey everyone! I couldn't update {{.GroupRef}} because I couldn't find the following user(s) in Slack: {{.MissingUserRefs}} If you could have them add a SLACK_DM contact method from their respective GoAlert profile page(s), that would be great! Hopefully I'll be able to update the user-group next time.`)) // userGroupErrorEmpty is a template for when a user-group update fails because there are no users on-call. -var userGroupErrorEmpty = template.Must(template.New("userGroupErrorEmpty").Parse(`{{.OnCall}} - -Hey everyone! I couldn't update {{.GroupRef}} because there is nobody on-call for {{.ScheduleRef}}. +var userGroupErrorEmpty = template.Must(template.New("userGroupErrorEmpty").Parse(`Hey everyone! I couldn't update {{.GroupRef}} because there is nobody on-call for {{.ScheduleRef}}. Since a Slack user-group cannot be empty, I'm going to leave it as-is for now.`)) // userGroupErrorUpdate is a template for when a user-group update fails for any reason other than missing users. -var userGroupErrorUpdate = template.Must(template.New("userGroupErrorUpdate").Parse(`{{.OnCall}} - -Hey everyone! I couldn't update {{.GroupRef}} because I ran into a problem. Maybe touch base with the GoAlert admin(s) to see if they can help? I'm sorry for the inconvenience! +var userGroupErrorUpdate = template.Must(template.New("userGroupErrorUpdate").Parse(`Hey everyone! I couldn't update {{.GroupRef}} because I ran into a problem. Maybe touch base with the GoAlert admin(s) to see if they can help? I'm sorry for the inconvenience! Here's the ID I left with the error in my logs so they can find it: {{.ErrorRef}}`)) diff --git a/notification/slack/usergroupsender.go b/notification/slack/usergroupsender.go index 663a72cf45..bae2bc0e2e 100644 --- a/notification/slack/usergroupsender.go +++ b/notification/slack/usergroupsender.go @@ -78,8 +78,7 @@ func (s *UserGroupSender) Send(ctx context.Context, msg notification.Message) (* ugID, chanID, _ := strings.Cut(t.Dest.Value, ":") cfg := config.FromContext(ctx) - onCallMsg := renderOnCallNotificationMessage(t, userSlackIDs) - var stateDetails string + var errorMsg, stateDetails string // If any users are missing, we need to abort and let the channel know. switch { @@ -90,12 +89,11 @@ func (s *UserGroupSender) Send(ctx context.Context, msg notification.Message) (* GroupID: ugID, Missing: missing, callbackFunc: cfg.CallbackURL, - OnCall: onCallMsg, }) if err != nil { return nil, fmt.Errorf("execute template: %w", err) } - onCallMsg = buf.String() + errorMsg = buf.String() stateDetails = "missing users, sent error to channel" // If no users are on-call, we need to abort and let the channel know. @@ -108,12 +106,11 @@ func (s *UserGroupSender) Send(ctx context.Context, msg notification.Message) (* ScheduleID: t.ScheduleID, ScheduleName: t.ScheduleName, callbackFunc: cfg.CallbackURL, - OnCall: onCallMsg, }) if err != nil { return nil, fmt.Errorf("execute template: %w", err) } - onCallMsg = buf.String() + errorMsg = buf.String() stateDetails = "empty user-group, sent error to channel" default: err = s.withClient(ctx, func(c *slack.Client) error { @@ -134,25 +131,26 @@ func (s *UserGroupSender) Send(ctx context.Context, msg notification.Message) (* err := userGroupErrorUpdate.Execute(&buf, userGroupError{ ErrorID: errID, GroupID: ugID, - OnCall: onCallMsg, }) if err != nil { return nil, fmt.Errorf("execute template: %w", err) } - onCallMsg = buf.String() + errorMsg = buf.String() stateDetails = "failed to update user-group, sent error to channel and log" } + // Only send to the channel if an error occurred + if stateDetails == "" { + return ¬ification.SentMessage{State: notification.StateDelivered}, nil + } + var ts string err = s.withClient(ctx, func(c *slack.Client) error { - _, ts, err = c.PostMessageContext(ctx, chanID, slack.MsgOptionText(onCallMsg, false)) - if err != nil { - return fmt.Errorf("post message to channel '%s': %w", chanID, err) - } - return nil + _, ts, err = c.PostMessageContext(ctx, chanID, slack.MsgOptionText(errorMsg, false)) + return err }) if err != nil { - return nil, err + return nil, fmt.Errorf("post message to channel '%s': %w", chanID, err) } return ¬ification.SentMessage{State: notification.StateDelivered, ExternalID: ts, StateDetails: stateDetails}, nil diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.tsx index 18e3f66024..48edd22917 100644 --- a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.tsx +++ b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.tsx @@ -1,35 +1,33 @@ import React, { useState } from 'react' -import { mapOnCallErrors, NO_DAY, Value } from './util' import FormDialog from '../../dialogs/FormDialog' import ScheduleOnCallNotificationsForm from './ScheduleOnCallNotificationsForm' import { useOnCallRulesData, useSetOnCallRulesSubmit } from './hooks' +import { NO_DAY, Value, mapOnCallErrors } from './util' interface ScheduleOnCallNotificationsCreateDialogProps { onClose: () => void scheduleID: string } +const defaultValue: Value = { + time: null, + weekdayFilter: NO_DAY, + type: 'slackChannel', + targetID: null, +} + export default function ScheduleOnCallNotificationsCreateDialog( props: ScheduleOnCallNotificationsCreateDialogProps, ): JSX.Element { const { onClose, scheduleID } = props - const [value, setValue] = useState(null) - const [slackType, setSlackType] = useState('channel') + const [value, setValue] = useState(defaultValue) const { q, zone, rules } = useOnCallRulesData(scheduleID) - const newValue: Value = value || { - time: null, - weekdayFilter: NO_DAY, - slackChannelID: null, - slackUserGroup: null, - } - if (!newValue.slackChannelID) delete newValue.slackChannelID - if (!newValue.slackUserGroup) delete newValue.slackUserGroup const { m, submit } = useSetOnCallRulesSubmit( scheduleID, zone, - newValue, + value, ...rules, ) @@ -47,10 +45,8 @@ export default function ScheduleOnCallNotificationsCreateDialog( setValue(value)} - slackType={slackType} - setSlackType={setSlackType} + value={value} + onChange={setValue} /> } /> diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.tsx index 75feb927b0..05543d4ba9 100644 --- a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.tsx +++ b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.tsx @@ -1,10 +1,10 @@ import React, { useState } from 'react' -import { EVERY_DAY, mapOnCallErrors, NO_DAY, Value } from './util' -import { useOnCallRulesData, useSetOnCallRulesSubmit } from './hooks' +import { DateTime } from 'luxon' import FormDialog from '../../dialogs/FormDialog' +import { useOnCallRulesData, useSetOnCallRulesSubmit } from './hooks' import ScheduleOnCallNotificationsForm from './ScheduleOnCallNotificationsForm' -import { DateTime } from 'luxon' +import { EVERY_DAY, mapOnCallErrors, NO_DAY, Value } from './util' interface ScheduleOnCallNotificationsEditDialogProps { onClose: () => void @@ -17,7 +17,6 @@ export default function ScheduleOnCallNotificationsEditDialog( p: ScheduleOnCallNotificationsEditDialogProps, ): JSX.Element { const [value, setValue] = useState(null) - const [slackType, setSlackType] = useState('channel') const { q, zone, rules } = useOnCallRulesData(p.scheduleID) @@ -27,14 +26,8 @@ export default function ScheduleOnCallNotificationsEditDialog( ? DateTime.fromFormat(rule.time, 'HH:mm', { zone }).toISO() : null, weekdayFilter: rule?.time ? rule.weekdayFilter || EVERY_DAY : NO_DAY, - slackChannelID: - rule?.target.type === 'slackChannel' - ? rule?.target.id - : rule?.target.id.split(':')[1], - slackUserGroup: - rule?.target.type === 'slackUserGroup' - ? rule?.target.id.split(':')[0] - : null, + type: rule?.target?.type ?? 'slackChannel', + targetID: rule?.target?.id ?? null, } const { m, submit } = useSetOnCallRulesSubmit( p.scheduleID, @@ -57,9 +50,7 @@ export default function ScheduleOnCallNotificationsEditDialog( scheduleID={p.scheduleID} errors={fieldErrors} value={newValue} - onChange={(value) => setValue(value)} - slackType={slackType} - setSlackType={setSlackType} + onChange={setValue} /> } /> diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.tsx index 1699f432db..1148e62667 100644 --- a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.tsx +++ b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.tsx @@ -1,20 +1,26 @@ -import React, { useState } from 'react' -import FormControlLabel from '@mui/material/FormControlLabel' -import Grid from '@mui/material/Grid' -import RadioGroup from '@mui/material/RadioGroup' -import Radio from '@mui/material/Radio' -import { DateTime } from 'luxon' -import { Checkbox, Tooltip, Typography } from '@mui/material' -import InfoIcon from '@mui/icons-material/Info' +import { + Checkbox, + FormControlLabel, + Grid, + MenuItem, + Radio, + RadioGroup, + TextField, + Typography, +} from '@mui/material' import makeStyles from '@mui/styles/makeStyles' +import { DateTime } from 'luxon' +import React, { useMemo } from 'react' import { FormContainer, FormField } from '../../forms' import { SlackChannelSelect, SlackUserGroupSelect } from '../../selection' import { ISOTimePicker } from '../../util/ISOPickers' -import { Value, NO_DAY, EVERY_DAY, RuleFieldError } from './util' +import { useConfigValue } from '../../util/RequireConfig' import { Time } from '../../util/Time' -import { useScheduleTZ } from '../useScheduleTZ' import { useExpFlag } from '../../util/useExpFlag' +import { useScheduleTZ } from '../useScheduleTZ' +import { EVERY_DAY, NO_DAY, RuleFieldError, Value } from './util' +import { TargetType } from '../../../schema' const days = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'] @@ -23,31 +29,27 @@ interface ScheduleOnCallNotificationsFormProps { value: Value errors: RuleFieldError[] onChange: (val: Value) => void - slackType: string - setSlackType: (slackType: string) => void } const useStyles = makeStyles({ margin0: { margin: 0 }, tzNote: { fontStyle: 'italic' }, }) + export default function ScheduleOnCallNotificationsForm( props: ScheduleOnCallNotificationsFormProps, ): JSX.Element { - const { scheduleID, slackType, setSlackType, ...formProps } = props + const { scheduleID, ...formProps } = props const classes = useStyles() + const [slackEnabled] = useConfigValue('Slack.Enable') const slackUGEnabled = useExpFlag('slack-ug') const { zone } = useScheduleTZ(scheduleID) - const [slackUGChecked, setSlackUGChecked] = useState( - !!formProps.value.slackUserGroup, - ) const handleRuleChange = (e: React.ChangeEvent): void => { if (e.target.value === 'on-change') { props.onChange({ ...formProps.value, time: null, weekdayFilter: NO_DAY }) return } - props.onChange({ ...props.value, weekdayFilter: EVERY_DAY, @@ -55,6 +57,72 @@ export default function ScheduleOnCallNotificationsForm( }) } + const handleTypeChange = (e: React.ChangeEvent): void => { + const newType = e.target.value as TargetType + if (props.value.type !== newType) { + props.onChange({ + ...props.value, + type: newType, + targetID: null, + }) + } + } + + const channelTypeItems = useMemo( + () => [ + + SLACK CHANNEL + , + ...(slackUGEnabled + ? [ + + SLACK USER GROUP + , + ] + : []), + ], + [slackEnabled, slackUGEnabled], + ) + + function renderTypeFields(type: TargetType): JSX.Element { + switch (type) { + case 'slackUserGroup': + return ( + + + + ) + case 'slackChannel': + return ( + + + + ) + default: + // unsupported type + return + } + } + return ( @@ -126,65 +194,19 @@ export default function ScheduleOnCallNotificationsForm( - { - setSlackType('channel') - return v - }} - /> + label='Type' + select + onChange={handleTypeChange} + disabled={channelTypeItems.length <= 1} + > + {channelTypeItems} + - {slackUGEnabled && ( - - { - const newVal = !slackUGChecked - setSlackUGChecked(newVal) - setSlackType(newVal ? 'usergroup' : 'channel') - if (!newVal) { - props.onChange({ - ...props.value, - slackUserGroup: null, - }) - } - }} - /> - } - label={ - - Also set the members of a Slack user group? - - - - - } - /> - { - setSlackType('usergroup') - return v - }} - /> - - )} + {renderTypeFields(formProps.value.type)} ) diff --git a/web/src/app/schedules/on-call-notifications/hooks.ts b/web/src/app/schedules/on-call-notifications/hooks.ts index cf85204c71..79331e148b 100644 --- a/web/src/app/schedules/on-call-notifications/hooks.ts +++ b/web/src/app/schedules/on-call-notifications/hooks.ts @@ -8,15 +8,15 @@ import { import { DateTime } from 'luxon' import { OnCallNotificationRule, Schedule } from '../../../schema' import { - Value, - onCallValueToRuleInput, - RuleFieldError, - onCallRuleSummary, - NO_DAY, - mapOnCallErrors, EVERY_DAY, formatLocalClockTime, + mapOnCallErrors, + NO_DAY, + onCallRuleSummary, onCallRuleToInput, + onCallValueToRuleInput, + RuleFieldError, + Value, } from './util' const schedTZQuery = gql` @@ -101,7 +101,7 @@ export function useSetOnCallRulesSubmit( rules: rules .map((r) => { if (r === null) return null - if ('slackChannelID' in r || 'slackUserGroup' in r) { + if ('targetID' in r) { return onCallValueToRuleInput(zone, r) } @@ -144,8 +144,8 @@ export function useEditOnCallRule( ? DateTime.fromFormat(rule.time, 'HH:mm', { zone }).toISO() : null, weekdayFilter: rule?.time ? rule.weekdayFilter || EVERY_DAY : NO_DAY, - slackChannelID: rule?.target.id || null, - slackUserGroup: rule?.target.id || null, + type: rule?.target?.type ?? 'slackChannel', + targetID: rule?.target?.id ?? null, } const { m, submit } = useSetOnCallRulesSubmit( scheduleID, diff --git a/web/src/app/schedules/on-call-notifications/util.ts b/web/src/app/schedules/on-call-notifications/util.ts index 62da2bbe02..f8efc535d7 100644 --- a/web/src/app/schedules/on-call-notifications/util.ts +++ b/web/src/app/schedules/on-call-notifications/util.ts @@ -4,20 +4,21 @@ import { DateTime } from 'luxon' import { OnCallNotificationRule, OnCallNotificationRuleInput, + TargetType, WeekdayFilter, } from '../../../schema' import { allErrors, fieldErrors, nonFieldErrors } from '../../util/errutil' import { weekdaySummary } from '../util' export type Value = { - slackChannelID?: string | null - slackUserGroup?: string | null time: string | null weekdayFilter: WeekdayFilter + type: TargetType + targetID: string | null } export type RuleFieldError = { - field: 'time' | 'weekdayFilter' | 'slackChannelID' | 'slackUserGroup' + field: 'time' | 'weekdayFilter' | 'type' | 'slackChannelID' | 'slackUserGroup' message: string } @@ -71,12 +72,7 @@ export const onCallValueToRuleInput = ( ? DateTime.fromISO(v.time).setZone(zone).toFormat('HH:mm') : undefined, weekdayFilter: v.time ? v.weekdayFilter : undefined, - target: { - type: v?.slackUserGroup ? 'slackUserGroup' : 'slackChannel', - id: v?.slackUserGroup - ? `${v.slackUserGroup}:${v.slackChannelID}` - : v?.slackChannelID ?? '', - }, + target: { id: v.targetID || '', type: v.type }, }) export const onCallRuleToInput = ( @@ -103,7 +99,6 @@ export function mapOnCallErrors( } dialogErrs = dialogErrs.concat(nonFieldErrors(mErr)) - const fieldErrs = fieldErrors(mErr) .map((e) => { switch (e.field) { diff --git a/web/src/app/selection/SlackUserGroupSelect.tsx b/web/src/app/selection/SlackUserGroupSelect.tsx index e529160256..280259d103 100644 --- a/web/src/app/selection/SlackUserGroupSelect.tsx +++ b/web/src/app/selection/SlackUserGroupSelect.tsx @@ -1,5 +1,8 @@ +import React, { useEffect, useState } from 'react' import { gql } from 'urql' import { makeQuerySelect } from './QuerySelect' +import { SlackChannelSelect } from './SlackChannelSelect' +import { FormControl, FormHelperText } from '@mui/material' const query = gql` query ($input: SlackUserGroupSearchOptions) { @@ -21,7 +24,70 @@ const valueQuery = gql` } ` -export const SlackUserGroupSelect = makeQuerySelect('SlackUserGroupSelect', { +const SlackUserGroupQuerySelect = makeQuerySelect('SlackUserGroupSelect', { query, valueQuery, }) + +export type SlackUserGroupSelectProps = { + value: string | null + onChange: (newValue: string | null) => void + error?: { message: string } + label?: string +} + +export const SlackUserGroupSelect: React.FC = ( + props, +) => { + const [groupID, setGroupID] = useState(null) + const [channelID, setChannelID] = useState(null) + + useEffect(() => { + if (!props.value) return + const [groupID, channelID] = props.value?.split(':') || [null, null] + setGroupID(groupID) + setChannelID(channelID) + }, [props.value]) + + function handleGroupChange(newGroupID: string | null): void { + setGroupID(newGroupID) + if (newGroupID && channelID) { + props.onChange(`${newGroupID}:${channelID}`) + } + } + function handleChannelChange(newChannelID: string | null): void { + setChannelID(newChannelID) + if (newChannelID && groupID) { + props.onChange(`${groupID}:${newChannelID}`) + } + } + + return ( + + + + + The selected group's membership will be replaced/set to the schedule's + on-call user(s). + + + + + + + Any problems updating the user group will be sent to this channel. + + + + ) +} + +export default SlackUserGroupSelect diff --git a/web/src/cypress/e2e/schedules.cy.ts b/web/src/cypress/e2e/schedules.cy.ts index ffda60e635..e611b0be95 100644 --- a/web/src/cypress/e2e/schedules.cy.ts +++ b/web/src/cypress/e2e/schedules.cy.ts @@ -488,7 +488,7 @@ function testSchedules(screen: ScreenFormat): void { cy.dialogTitle('Create Notification Rule') cy.dialogForm({ ruleType: 'on-change', - slackChannelID: 'general', + targetID: 'general', }) cy.dialogFinish('Submit') cy.get('body').should('contain', '#general') @@ -502,7 +502,7 @@ function testSchedules(screen: ScreenFormat): void { } cy.dialogTitle('Create Notification Rule') cy.dialogForm({ - slackChannelID: 'foobar', + targetID: 'foobar', ruleType: 'time-of-day', time: '00:00', 'weekdayFilter[0]': false, @@ -600,7 +600,7 @@ function testSchedules(screen: ScreenFormat): void { cy.dialogTitle('Edit Notification Rule') cy.dialogForm({ ruleType: 'time-of-day', - slackChannelID: 'foobar', + targetID: 'foobar', time: '07:00', 'weekdayFilter[0]': false, 'weekdayFilter[1]': true,