diff --git a/src/sentry/notifications/helpers.py b/src/sentry/notifications/helpers.py index aa6efd356cafdd..542941581b9ac8 100644 --- a/src/sentry/notifications/helpers.py +++ b/src/sentry/notifications/helpers.py @@ -57,27 +57,31 @@ def _get_setting_mapping_from_mapping( type: NotificationSettingTypes, should_use_slack_automatic: bool = False, ) -> Mapping[ExternalProviders, NotificationSettingOptionValues]: - # XXX(CEO): may not respect granularity of a setting for Slack a setting for email - # but we'll worry about that later since we don't have a FE for it yet + """ + XXX(CEO): may not respect granularity of a setting for Slack a setting for + email but we'll worry about that later since we don't have a FE for it yet. + """ from sentry.notifications.notify import notification_providers - specific_scope = get_scope_type(type) + # Fill in with the fallback values. + notification_setting_option = { + provider: _get_notification_setting_default( + provider, type, should_use_slack_automatic=should_use_slack_automatic + ) + for provider in notification_providers() + } + notification_settings_mapping = notification_settings_by_recipient.get(recipient) if notification_settings_mapping: - notification_setting_option = ( + specific_scope = get_scope_type(type) + notification_setting_option.update( notification_settings_mapping.get(specific_scope) or notification_settings_mapping.get(NotificationScopeType.USER) or notification_settings_mapping.get(NotificationScopeType.TEAM) + or {} ) - if notification_setting_option: - return notification_setting_option - return { - provider: _get_notification_setting_default( - provider, type, should_use_slack_automatic=should_use_slack_automatic - ) - for provider in notification_providers() - } + return notification_setting_option def where_should_recipient_be_notified( diff --git a/src/sentry/testutils/helpers/slack.py b/src/sentry/testutils/helpers/slack.py index d40911c5171040..2ca893435e563c 100644 --- a/src/sentry/testutils/helpers/slack.py +++ b/src/sentry/testutils/helpers/slack.py @@ -92,8 +92,9 @@ def link_team(team: Team, integration: Integration, channel_name: str, channel_i def send_notification(*args): - args_list = list(args)[1:] - send_notification_as_slack(*args_list, {}) + provider, *args_list = args + if provider == ExternalProviders.SLACK: + send_notification_as_slack(*args_list, {}) def get_attachment(): diff --git a/tests/sentry/notifications/notifications/test_organization_request.py b/tests/sentry/notifications/notifications/test_organization_request.py index 582e2381e9591c..e57e34dbb9e337 100644 --- a/tests/sentry/notifications/notifications/test_organization_request.py +++ b/tests/sentry/notifications/notifications/test_organization_request.py @@ -34,7 +34,6 @@ def test_default_to_slack(self): @with_feature("organizations:slack-requests") def test_turn_off_settings(self): - NotificationSetting.objects.update_settings( ExternalProviders.SLACK, NotificationSettingTypes.APPROVAL, @@ -53,6 +52,6 @@ def test_turn_off_settings(self): notification = DummyRequestNotification(self.organization, self.user, member_ids) assert notification.get_participants() == { - ExternalProviders.EMAIL: {self.user2}, - ExternalProviders.SLACK: {self.user1}, + ExternalProviders.EMAIL: {self.user1, self.user2}, + ExternalProviders.SLACK: {self.user1, self.user2}, } diff --git a/tests/sentry/notifications/test_utils.py b/tests/sentry/notifications/test_utils.py index ebf31b35eefcbe..a79080ce647509 100644 --- a/tests/sentry/notifications/test_utils.py +++ b/tests/sentry/notifications/test_utils.py @@ -1,6 +1,5 @@ from sentry.models import NotificationSetting from sentry.notifications.helpers import ( - _get_setting_mapping_from_mapping, collect_groups_by_project, get_scope_type, get_settings_by_provider, @@ -42,51 +41,6 @@ def setUp(self): user=self.user, ) - def test_get_setting_mapping_from_mapping_issue_alerts(self): - notification_settings = { - self.user: { - NotificationScopeType.USER: { - ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS - } - } - } - mapping = _get_setting_mapping_from_mapping( - notification_settings, - self.user, - NotificationSettingTypes.ISSUE_ALERTS, - ) - assert mapping == {ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS} - - def test_get_setting_mapping_from_mapping_deploy(self): - notification_settings = { - self.user: { - NotificationScopeType.USER: { - ExternalProviders.EMAIL: NotificationSettingOptionValues.COMMITTED_ONLY - } - } - } - mapping = _get_setting_mapping_from_mapping( - notification_settings, - self.user, - NotificationSettingTypes.DEPLOY, - ) - assert mapping == {ExternalProviders.EMAIL: NotificationSettingOptionValues.COMMITTED_ONLY} - - def test_get_setting_mapping_from_mapping_workflow(self): - notification_settings = { - self.user: { - NotificationScopeType.USER: { - ExternalProviders.EMAIL: NotificationSettingOptionValues.SUBSCRIBE_ONLY - } - } - } - mapping = _get_setting_mapping_from_mapping( - notification_settings, - self.user, - NotificationSettingTypes.WORKFLOW, - ) - assert mapping == {ExternalProviders.EMAIL: NotificationSettingOptionValues.SUBSCRIBE_ONLY} - def test_get_deploy_values_by_provider_empty_settings(self): values_by_provider = get_values_by_provider_by_type( {}, diff --git a/tests/sentry/notifications/utils/test_get_setting_mapping_from_mapping.py b/tests/sentry/notifications/utils/test_get_setting_mapping_from_mapping.py new file mode 100644 index 00000000000000..4ccb45e3098d2b --- /dev/null +++ b/tests/sentry/notifications/utils/test_get_setting_mapping_from_mapping.py @@ -0,0 +1,116 @@ +from unittest import TestCase + +from sentry.models import User +from sentry.notifications.helpers import _get_setting_mapping_from_mapping +from sentry.notifications.types import ( + NotificationScopeType, + NotificationSettingOptionValues, + NotificationSettingTypes, +) +from sentry.types.integrations import ExternalProviders + + +class GetSettingMappingFromMappingTest(TestCase): + def setUp(self): + self.user = User(id=1) + + def test_get_setting_mapping_from_mapping_issue_alerts(self): + notification_settings = { + self.user: { + NotificationScopeType.USER: { + ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS + } + } + } + mapping = _get_setting_mapping_from_mapping( + notification_settings, + self.user, + NotificationSettingTypes.ISSUE_ALERTS, + ) + assert mapping == { + ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS, + ExternalProviders.SLACK: NotificationSettingOptionValues.NEVER, + } + + def test_get_setting_mapping_from_mapping_deploy(self): + notification_settings = { + self.user: { + NotificationScopeType.USER: { + ExternalProviders.EMAIL: NotificationSettingOptionValues.COMMITTED_ONLY + } + } + } + mapping = _get_setting_mapping_from_mapping( + notification_settings, + self.user, + NotificationSettingTypes.DEPLOY, + ) + assert mapping == { + ExternalProviders.EMAIL: NotificationSettingOptionValues.COMMITTED_ONLY, + ExternalProviders.SLACK: NotificationSettingOptionValues.NEVER, + } + + def test_get_setting_mapping_from_mapping_workflow(self): + notification_settings = { + self.user: { + NotificationScopeType.USER: { + ExternalProviders.EMAIL: NotificationSettingOptionValues.SUBSCRIBE_ONLY + } + } + } + mapping = _get_setting_mapping_from_mapping( + notification_settings, + self.user, + NotificationSettingTypes.WORKFLOW, + ) + assert mapping == { + ExternalProviders.EMAIL: NotificationSettingOptionValues.SUBSCRIBE_ONLY, + ExternalProviders.SLACK: NotificationSettingOptionValues.NEVER, + } + + def test_get_setting_mapping_from_mapping_empty(self): + mapping = _get_setting_mapping_from_mapping( + {}, self.user, NotificationSettingTypes.ISSUE_ALERTS + ) + assert mapping == { + ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS, + ExternalProviders.SLACK: NotificationSettingOptionValues.NEVER, + } + + def test_get_setting_mapping_from_mapping_slack_never(self): + notification_settings = { + self.user: { + NotificationScopeType.USER: { + ExternalProviders.SLACK: NotificationSettingOptionValues.NEVER + } + } + } + + mapping = _get_setting_mapping_from_mapping( + notification_settings, + self.user, + NotificationSettingTypes.ISSUE_ALERTS, + ) + assert mapping == { + ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS, + ExternalProviders.SLACK: NotificationSettingOptionValues.NEVER, + } + + def test_get_setting_mapping_from_mapping_slack_always(self): + notification_settings = { + self.user: { + NotificationScopeType.USER: { + ExternalProviders.SLACK: NotificationSettingOptionValues.ALWAYS + } + } + } + + mapping = _get_setting_mapping_from_mapping( + notification_settings, + self.user, + NotificationSettingTypes.ISSUE_ALERTS, + ) + assert mapping == { + ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS, + ExternalProviders.SLACK: NotificationSettingOptionValues.ALWAYS, + }