Skip to content

Commit

Permalink
feat(notifications): Add notification settings option for msteams (ge…
Browse files Browse the repository at this point in the history
…tsentry#36167)

Change the dropdown to a multi-select to select from options - Email, Slack and Microsoft Teams!
  • Loading branch information
97varun authored Aug 2, 2022
1 parent ee322ec commit 6595cb1
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 49 deletions.
44 changes: 26 additions & 18 deletions static/app/components/forms/selectField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,27 +102,35 @@ export default class SelectField<OptionType extends SelectValue<any>> extends Co
clearable={allowClear}
multiple={multiple}
onChange={val => {
if (!confirm) {
this.handleChange(onBlur, onChange, val);
return;
}
try {
if (!confirm) {
this.handleChange(onBlur, onChange, val);
return;
}

// Support 'confirming' selections. This only works with
// `val` objects that use the new-style options format
const previousValue = props.value?.toString();
// `val` may be null if clearing the select for an optional field
const newValue = val?.value?.toString();
// Support 'confirming' selections. This only works with
// `val` objects that use the new-style options format
const previousValue = props.value?.toString();
// `val` may be null if clearing the select for an optional field
const newValue = val?.value?.toString();

// Value not marked for confirmation, or hasn't changed
if (!confirm[newValue] || previousValue === newValue) {
this.handleChange(onBlur, onChange, val);
return;
}
// Value not marked for confirmation, or hasn't changed
if (!confirm[newValue] || previousValue === newValue) {
this.handleChange(onBlur, onChange, val);
return;
}

openConfirmModal({
onConfirm: () => this.handleChange(onBlur, onChange, val),
message: confirm[val?.value] ?? t('Continue with these changes?'),
});
openConfirmModal({
onConfirm: () => this.handleChange(onBlur, onChange, val),
message: confirm[val?.value] ?? t('Continue with these changes?'),
});
} catch (e) {
// Swallow expected error to prevent bubbling up.
if (e.message === 'Invalid selection. Field cannot be empty.') {
return;
}
throw e;
}
}}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {t} from 'sentry/locale';
export const ALL_PROVIDERS = {
email: 'default',
slack: 'never',
msteams: 'never',
};
export const ALL_PROVIDER_NAMES = Object.keys(ALL_PROVIDERS);

Expand Down
14 changes: 11 additions & 3 deletions static/app/views/settings/account/notifications/fields2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,18 @@ export const NOTIFICATION_SETTING_FIELDS: Record<string, Field> = {
type: 'select',
label: t('Delivery Method'),
choices: [
['email', t('Send to Email')],
['slack', t('Send to Slack')],
['email+slack', t('Send to Email and Slack')],
['email', t('Email')],
['slack', t('Slack')],
['msteams', t('Microsoft Teams')],
],
multiple: true,
onChange: val => {
// This is a little hack to prevent this field from being empty.
// TODO(nisanthan): need to prevent showing the clearable on. the multi-select when its only 1 value.
if (!val || val.length === 0) {
throw Error('Invalid selection. Field cannot be empty.');
}
},
},
approval: {
name: 'approval',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
isGroupedByProject,
isSufficientlyComplex,
mergeNotificationSettings,
providerListToString,
} from 'sentry/views/settings/account/notifications/utils';
import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader';
import TextBlock from 'sentry/views/settings/components/text/textBlock';
Expand Down Expand Up @@ -211,23 +210,28 @@ class NotificationSettingsByType extends AsyncComponent<Props, State> {

/* Methods responsible for rendering the page. */

getInitialData(): {[key: string]: string} {
getInitialData(): {[key: string]: string | string[]} {
const {notificationType} = this.props;
const {notificationSettings} = this.state;

const initialData = {
// NOTE: Update this when we want to enable Slack by default.
const provider = !isEverythingDisabled(notificationType, notificationSettings)
? getCurrentProviders(notificationType, notificationSettings)
: ['email'];

const childTypes: string[] = typeMappedChildren[notificationType] || [];
const childTypesDefaults = Object.fromEntries(
childTypes.map(childType => [
childType,
getCurrentDefault(childType, notificationSettings),
])
);

return {
[notificationType]: getCurrentDefault(notificationType, notificationSettings),
provider,
...childTypesDefaults,
};
if (!isEverythingDisabled(notificationType, notificationSettings)) {
initialData.provider = providerListToString(
getCurrentProviders(notificationType, notificationSettings)
);
}
const childTypes: string[] = typeMappedChildren[notificationType] || [];
childTypes.forEach(childType => {
initialData[childType] = getCurrentDefault(childType, notificationSettings);
});
return initialData;
}

getFields(): Field[] {
Expand Down
4 changes: 3 additions & 1 deletion static/app/views/settings/account/notifications/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,9 @@ export const getStateToPutForProvider = (
notificationSettings: NotificationSettingsObject,
changedData: NotificationSettingsByProviderObject
): NotificationSettingsObject => {
const providerList: string[] = changedData.provider?.split('+') || [];
const providerList: string[] = changedData.provider
? Object.values(changedData.provider)
: [];
const fallbackValue = getFallBackValue(notificationType);

// If the user has no settings, we need to create them.
Expand Down
26 changes: 13 additions & 13 deletions tests/js/spec/utils/settings/notifications/backfill.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,52 +5,52 @@ describe('backfillMissingProvidersWithFallback', () => {
it('should add missing provider with the fallback value', () => {
expect(
backfillMissingProvidersWithFallback({}, ['email'], 'sometimes', 'user')
).toEqual({email: 'sometimes', slack: 'never'});
).toEqual({email: 'sometimes', slack: 'never', msteams: 'never'});
});

it('should turn on both providers with the fallback value', () => {
it('should turn on all providers with the fallback value', () => {
expect(
backfillMissingProvidersWithFallback(
{email: 'never', slack: 'never'},
['email', 'slack'],
{email: 'never', slack: 'never', msteams: 'never'},
['email', 'slack', 'msteams'],
'sometimes',
'user'
)
).toEqual({email: 'sometimes', slack: 'sometimes'});
).toEqual({email: 'sometimes', slack: 'sometimes', msteams: 'sometimes'});
});

it('should move the existing setting when providers are swapped', () => {
expect(
backfillMissingProvidersWithFallback(
{email: 'always', slack: 'never'},
['slack'],
{email: 'always', slack: 'never', msteams: 'never'},
['slack', 'msteams'],
'',
'user'
)
).toEqual({email: 'never', slack: 'always'});
).toEqual({email: 'never', slack: 'always', msteams: 'always'});
});

it('should turn off both providers when providers is empty', () => {
it('should turn off all providers when providers is empty', () => {
expect(
backfillMissingProvidersWithFallback(
{email: 'always', slack: 'always'},
{email: 'always', slack: 'always', msteams: 'always'},
[],
'',
'user'
)
).toEqual({email: 'never', slack: 'never'});
).toEqual({email: 'never', slack: 'never', msteams: 'never'});
});
});
describe('when scopeType is organization', () => {
it('should retain OFF organization scope preference when provider list changes', () => {
expect(
backfillMissingProvidersWithFallback(
{email: 'never', slack: 'never'},
{email: 'never', slack: 'never', msteams: 'never'},
['slack'],
'sometimes',
'organization'
)
).toEqual({email: 'never', slack: 'never'});
).toEqual({email: 'never', slack: 'never', msteams: 'never'});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe('Notification Settings Utils', () => {
expect(getUserDefaultValues('deploy', {})).toEqual({
email: 'committed_only',
slack: 'never',
msteams: 'never',
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,30 @@ describe('NotificationSettingsByType', function () {
expect(fields.at(0).find('FieldLabel').text()).toEqual('Issue Alerts');
expect(fields.at(0).find('Select').text()).toEqual('On');
expect(fields.at(1).find('FieldLabel').text()).toEqual('Delivery Method');
expect(fields.at(1).find('Select').text()).toEqual('Send to Email and Slack');
expect(fields.at(1).find('Select').prop('options')).toEqual([
expect.objectContaining({
value: 'email',
label: 'Email',
}),
expect.objectContaining({
value: 'slack',
label: 'Slack',
}),
expect.objectContaining({
value: 'msteams',
label: 'Microsoft Teams',
}),
]);
expect(fields.at(1).find('Select').prop('value')).toEqual([
expect.objectContaining({
value: 'email',
label: 'Email',
}),
expect.objectContaining({
value: 'slack',
label: 'Slack',
}),
]);
});

it('should render warning modal when identity not linked', function () {
Expand Down

0 comments on commit 6595cb1

Please sign in to comment.