diff --git a/src/sentry/api/endpoints/user_notification_fine_tuning.py b/src/sentry/api/endpoints/user_notification_fine_tuning.py index ff696d9509b0c9..9d17e829d8d4b9 100644 --- a/src/sentry/api/endpoints/user_notification_fine_tuning.py +++ b/src/sentry/api/endpoints/user_notification_fine_tuning.py @@ -14,7 +14,6 @@ from sentry.models.notificationsettingoption import NotificationSettingOption from sentry.models.options.user_option import UserOption from sentry.models.useremail import UserEmail -from sentry.notifications.helpers import is_double_write_enabled from sentry.notifications.types import ( FineTuningAPIKey, NotificationScopeEnum, @@ -123,8 +122,6 @@ def _handle_put_reports(user, data): value = set(user_option.value or []) - double_write_enabled = is_double_write_enabled(user_id=user.id) - # The set of IDs of the organizations of which the user is a member. org_ids = {o.id for o in user_service.get_organizations(user_id=user.id, only_visible=True)} for org_id, enabled in data.items(): @@ -146,18 +143,17 @@ def _handle_put_reports(user, data): elif not enabled: value.add(org_id) - if double_write_enabled: - NotificationSettingOption.objects.create_or_update( - scope_type=NotificationScopeEnum.ORGANIZATION.value, - scope_identifier=org_id, - user_id=user.id, - type=NotificationSettingEnum.REPORTS.value, - values={ - "value": NotificationSettingsOptionEnum.ALWAYS.value - if enabled - else NotificationSettingsOptionEnum.NEVER.value, - }, - ) + NotificationSettingOption.objects.create_or_update( + scope_type=NotificationScopeEnum.ORGANIZATION.value, + scope_identifier=org_id, + user_id=user.id, + type=NotificationSettingEnum.REPORTS.value, + values={ + "value": NotificationSettingsOptionEnum.ALWAYS.value + if enabled + else NotificationSettingsOptionEnum.NEVER.value, + }, + ) user_option.update(value=list(value)) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 44a87d0ce4c3cf..4ace7c8bb69fa4 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -1790,8 +1790,6 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: "organizations:sourcemaps-upload-release-as-artifact-bundle": False, # Signals that the organization supports the on demand metrics prefill. "organizations:on-demand-metrics-prefill": False, - # Enable writing to the new notification system when updating the old system - "organizations:notifications-double-write": True, # Enable source maps debugger "organizations:source-maps-debugger-blue-thunder-edition": False, # Enable the new suspect commits calculation that uses all frames in the stack trace diff --git a/src/sentry/features/__init__.py b/src/sentry/features/__init__.py index b5cb579471e1f9..2e0e8cb124e956 100644 --- a/src/sentry/features/__init__.py +++ b/src/sentry/features/__init__.py @@ -246,7 +246,6 @@ default_manager.add("organizations:sourcemaps-upload-release-as-artifact-bundle", OrganizationFeature, FeatureHandlerStrategy.REMOTE) default_manager.add("organizations:recap-server", OrganizationFeature, FeatureHandlerStrategy.INTERNAL) default_manager.add("organizations:notification-settings-v2", OrganizationFeature, FeatureHandlerStrategy.INTERNAL) -default_manager.add("organizations:notifications-double-write", OrganizationFeature, FeatureHandlerStrategy.REMOTE) default_manager.add("organizations:release-ui-v2", OrganizationFeature, FeatureHandlerStrategy.REMOTE) default_manager.add("organizations:source-maps-debugger-blue-thunder-edition", OrganizationFeature, FeatureHandlerStrategy.REMOTE) default_manager.add("organizations:suspect-commits-all-frames", OrganizationFeature, FeatureHandlerStrategy.INTERNAL) diff --git a/src/sentry/notifications/helpers.py b/src/sentry/notifications/helpers.py index eaf1667a8d8db3..56a3dc25b89542 100644 --- a/src/sentry/notifications/helpers.py +++ b/src/sentry/notifications/helpers.py @@ -2,13 +2,11 @@ import logging from collections import defaultdict -from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping, Optional +from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping from django.contrib.auth.models import AnonymousUser from sentry import features -from sentry.models.organizationmapping import OrganizationMapping -from sentry.models.organizationmembermapping import OrganizationMemberMapping from sentry.notifications.defaults import ( NOTIFICATION_SETTING_DEFAULTS, NOTIFICATION_SETTINGS_ALL_SOMETIMES, @@ -717,34 +715,6 @@ def get_recipient_from_team_or_user(user_id: int | None, team_id: int | None) -> return recipient -def is_double_write_enabled( - user_id: Optional[int] = None, organization_id_for_team: Optional[int] = None -): - from sentry.services.hybrid_cloud.organization_mapping.serial import ( - serialize_organization_mapping, - ) - - # all operations are expected to happen in the control siolo - if organization_id_for_team is not None: - org_ids = [organization_id_for_team] - elif user_id is not None: - org_ids = OrganizationMemberMapping.objects.filter(user_id=user_id).values_list( - "organization_id", flat=True - ) - else: - raise ValueError("Need organization_id_for_team or user_id") - orgs = list( - map( - serialize_organization_mapping, - OrganizationMapping.objects.filter(organization_id__in=list(org_ids)), - ) - ) - # if no orgs, then automatically enable the feature - if not orgs: - return True - return any(features.has("organizations:notifications-double-write", org) for org in orgs) - - PROVIDER_DEFAULTS: list[ExternalProviderEnum] = get_provider_defaults() TYPE_DEFAULTS: Mapping[ NotificationSettingEnum, NotificationSettingsOptionEnum diff --git a/src/sentry/notifications/manager.py b/src/sentry/notifications/manager.py index a3337d7aa46ba8..516d6697d4975c 100644 --- a/src/sentry/notifications/manager.py +++ b/src/sentry/notifications/manager.py @@ -28,7 +28,6 @@ from sentry.notifications.helpers import ( get_scope, get_scope_type, - is_double_write_enabled, should_use_notifications_v2, transform_to_notification_settings_by_recipient, validate, @@ -210,11 +209,6 @@ def update_settings( **{id_key: actor_id}, ) - if not is_double_write_enabled( - user_id=user_id, organization_id_for_team=organization_id_for_team - ): - return - # implement the double write now query_args = { "type": NOTIFICATION_SETTING_TYPES[type], @@ -255,23 +249,19 @@ def remove_settings( user_id = user.id # get the actor type and actor id - use_double_write = is_double_write_enabled( - user_id=user_id, organization_id_for_team=organization_id_for_team + scope_type, scope_identifier = get_scope( + team=team_id, user=user_id, project=project, organization=organization ) - if use_double_write: - scope_type, scope_identifier = get_scope( - team=team_id, user=user_id, project=project, organization=organization - ) - scope_type_str = NOTIFICATION_SCOPE_TYPE[scope_type] - # remove the option setting - NotificationSettingOption.objects.filter( - scope_type=scope_type_str, - scope_identifier=scope_identifier, - team_id=team_id, - user_id=user_id, - type=type, - ).delete() - # the provider setting is updated elsewhere + scope_type_str = NOTIFICATION_SCOPE_TYPE[scope_type] + # remove the option setting + NotificationSettingOption.objects.filter( + scope_type=scope_type_str, + scope_identifier=scope_identifier, + team_id=team_id, + user_id=user_id, + type=type, + ).delete() + # the provider setting is updated elsewhere self.find_settings( provider, @@ -614,10 +604,6 @@ def update_settings_bulk( user_id = user.id if user else None team_id = team.id if team else None - if not is_double_write_enabled( - user_id=user_id, organization_id_for_team=organization_id_for_team - ): - return enabled_providers = defaultdict(set) disabled_providers = defaultdict(set) diff --git a/src/sentry/services/hybrid_cloud/notifications/impl.py b/src/sentry/services/hybrid_cloud/notifications/impl.py index c8c2f280053b2b..91c2c33f758ab7 100644 --- a/src/sentry/services/hybrid_cloud/notifications/impl.py +++ b/src/sentry/services/hybrid_cloud/notifications/impl.py @@ -11,7 +11,7 @@ from sentry.models.notificationsettingoption import NotificationSettingOption from sentry.models.notificationsettingprovider import NotificationSettingProvider from sentry.models.user import User -from sentry.notifications.helpers import get_scope_type, is_double_write_enabled +from sentry.notifications.helpers import get_scope_type from sentry.notifications.notificationcontroller import NotificationController from sentry.notifications.types import ( NotificationScopeEnum, @@ -88,8 +88,7 @@ def bulk_update_settings( skip_provider_updates=True, ) # update the providers at the end - if is_double_write_enabled(user_id=user_id): - NotificationSetting.objects.update_provider_settings(user_id, None) + NotificationSetting.objects.update_provider_settings(user_id, None) # TODO(snigdha): This can be removed in V2. def get_settings_for_users( diff --git a/tests/sentry/api/endpoints/test_user_notification_details.py b/tests/sentry/api/endpoints/test_user_notification_details.py index ebf3465ac53e77..a4655661564620 100644 --- a/tests/sentry/api/endpoints/test_user_notification_details.py +++ b/tests/sentry/api/endpoints/test_user_notification_details.py @@ -1,10 +1,8 @@ from sentry.models.notificationsetting import NotificationSetting from sentry.models.notificationsettingoption import NotificationSettingOption from sentry.models.notificationsettingprovider import NotificationSettingProvider -from sentry.models.organizationmembermapping import OrganizationMemberMapping from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes from sentry.testutils.cases import APITestCase -from sentry.testutils.helpers.features import with_feature from sentry.testutils.silo import control_silo_test from sentry.types.integrations import ExternalProviders @@ -83,38 +81,7 @@ def test_subscribe_by_default(self): class UserNotificationDetailsPutTest(UserNotificationDetailsTestBase): method = "put" - @with_feature({"organizations:notifications-double-write": False}) def test_saves_and_returns_values(self): - data = { - "deployNotifications": 2, - "personalActivityNotifications": True, - "selfAssignOnResolve": True, - } - # make an org mapping for the user - OrganizationMemberMapping.objects.create( - organization_id=self.organization.id, user_id=self.user.id - ) - - response = self.get_success_response("me", **data) - - assert response.data.get("deployNotifications") == 2 - assert response.data.get("personalActivityNotifications") is True - assert response.data.get("selfAssignOnResolve") is True - assert response.data.get("subscribeByDefault") is True - assert response.data.get("workflowNotifications") == 1 - - value = NotificationSetting.objects.get_settings( - ExternalProviders.EMAIL, - NotificationSettingTypes.DEPLOY, - user_id=self.user.id, - ) - assert value == NotificationSettingOptionValues.ALWAYS - # ensure double write does not happen - assert not NotificationSettingOption.objects.filter(user_id=self.user.id).exists() - assert not NotificationSettingProvider.objects.filter(user_id=self.user.id).exists() - - @with_feature("organizations:notifications-double-write") - def test_double_write(self): org = self.create_organization() self.create_member(user=self.user, organization=org) data = { diff --git a/tests/sentry/api/endpoints/test_user_notification_fine_tuning.py b/tests/sentry/api/endpoints/test_user_notification_fine_tuning.py index 238f75aa338465..e8b10754461b16 100644 --- a/tests/sentry/api/endpoints/test_user_notification_fine_tuning.py +++ b/tests/sentry/api/endpoints/test_user_notification_fine_tuning.py @@ -4,7 +4,6 @@ from sentry.models.useremail import UserEmail from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes from sentry.testutils.cases import APITestCase -from sentry.testutils.helpers.features import with_feature from sentry.testutils.silo import control_silo_test from sentry.types.integrations import ExternalProviders @@ -152,7 +151,6 @@ def test_saves_and_returns_workflow(self): assert value1 == NotificationSettingOptionValues.DEFAULT assert value2 == NotificationSettingOptionValues.NEVER - @with_feature("organizations:notifications-double-write") def test_double_write(self): data = {str(self.project.id): 1, str(self.project2.id): 2} self.get_success_response("me", "workflow", status_code=204, **data) @@ -333,7 +331,6 @@ def test_enable_weekly_reports_from_default_setting(self): == set() ) - @with_feature("organizations:notifications-double-write") def test_double_write_weekly_report(self): data = {str(self.organization.id): 1, str(self.organization2.id): "1"} self.get_success_response("me", "reports", status_code=204, **data) diff --git a/tests/sentry/api/endpoints/test_user_notification_settings.py b/tests/sentry/api/endpoints/test_user_notification_settings.py index 2b0b2922a5a11b..203ca06a4f2599 100644 --- a/tests/sentry/api/endpoints/test_user_notification_settings.py +++ b/tests/sentry/api/endpoints/test_user_notification_settings.py @@ -5,7 +5,6 @@ from sentry.models.notificationsettingprovider import NotificationSettingProvider from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes from sentry.testutils.cases import APITestCase -from sentry.testutils.helpers.features import with_feature from sentry.testutils.silo import control_silo_test from sentry.types.integrations import ExternalProviders @@ -125,7 +124,6 @@ def test_simple(self): == NotificationSettingOptionValues.ALWAYS ) - @with_feature("organizations:notifications-double-write") def test_double_write(self): org = self.create_organization() self.create_member(user=self.user, organization=org) @@ -180,7 +178,6 @@ def test_double_write(self): **query_args, value="never", provider="slack" ) - @with_feature("organizations:notifications-double-write") def test_double_write_with_email_off(self): org = self.create_organization() self.create_member(user=self.user, organization=org) diff --git a/tests/sentry/api/serializers/test_organization.py b/tests/sentry/api/serializers/test_organization.py index 62b0b6998b0df6..ef2404df071642 100644 --- a/tests/sentry/api/serializers/test_organization.py +++ b/tests/sentry/api/serializers/test_organization.py @@ -88,7 +88,6 @@ def test_simple(self): "invite-members", "invite-members-rate-limits", "minute-resolution-sessions", - "notifications-double-write", "open-membership", "project-stats", "relay", diff --git a/tests/sentry/models/test_notificationsetting.py b/tests/sentry/models/test_notificationsetting.py index 94d5fa7b070d24..718f48d03d7280 100644 --- a/tests/sentry/models/test_notificationsetting.py +++ b/tests/sentry/models/test_notificationsetting.py @@ -10,7 +10,6 @@ from sentry.silo import SiloMode from sentry.tasks.deletion.hybrid_cloud import schedule_hybrid_cloud_foreign_key_jobs_control from sentry.testutils.cases import TestCase -from sentry.testutils.helpers import Feature from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, control_silo_test from sentry.types.integrations import ExternalProviders @@ -29,11 +28,10 @@ def assert_no_notification_settings(): def create_setting(**kwargs): - with Feature({"organizations:notifications-double-write": True}): - NotificationSetting.objects.update_settings( - value=NotificationSettingOptionValues.ALWAYS, - **_get_kwargs(kwargs), - ) + NotificationSetting.objects.update_settings( + value=NotificationSettingOptionValues.ALWAYS, + **_get_kwargs(kwargs), + ) @control_silo_test(stable=True) diff --git a/tests/sentry/notifications/test_helpers.py b/tests/sentry/notifications/test_helpers.py index d5591abe58fe66..8aeb196511558d 100644 --- a/tests/sentry/notifications/test_helpers.py +++ b/tests/sentry/notifications/test_helpers.py @@ -1,11 +1,7 @@ import types -from unittest import mock from urllib.parse import parse_qs, urlparse -import pytest - from sentry.models.notificationsetting import NotificationSetting -from sentry.models.organizationmapping import OrganizationMapping from sentry.models.rule import Rule from sentry.notifications.helpers import ( collect_groups_by_project, @@ -13,7 +9,6 @@ get_settings_by_provider, get_subscription_from_attributes, get_values_by_provider_by_type, - is_double_write_enabled, validate, ) from sentry.notifications.notify import notification_providers @@ -27,7 +22,6 @@ get_group_settings_link, get_rules, ) -from sentry.services.hybrid_cloud.organization_mapping.serial import serialize_organization_mapping from sentry.testutils.cases import TestCase from sentry.types.integrations import ExternalProviders @@ -36,81 +30,6 @@ def mock_event(*, transaction, data=None): return types.SimpleNamespace(data=data or {}, transaction=transaction) -class DoubleWriteTests(TestCase): - @mock.patch("sentry.notifications.helpers.features.has", return_value=False) - def test_is_double_write_enabled_user(self, mock_has): - # Create dummy users and organizations - user1 = self.create_user() - user2 = self.create_user() - org1 = self.create_organization() - org2 = self.create_organization() - org3 = self.create_organization() - - # Add users to organizations - self.create_member(user=user1, organization=org1) - self.create_member(user=user2, organization=org2) - self.create_member(user=user1, organization=org3) - - is_double_write_enabled(user_id=user1.id) - - mapped_org1 = OrganizationMapping.objects.get(organization_id=org1.id) - mapped_org2 = OrganizationMapping.objects.get(organization_id=org2.id) - mapped_org3 = OrganizationMapping.objects.get(organization_id=org3.id) - # Ensure mock_has is called on the right organizations - mock_has.assert_any_call( - "organizations:notifications-double-write", serialize_organization_mapping(mapped_org1) - ) - mock_has.assert_any_call( - "organizations:notifications-double-write", serialize_organization_mapping(mapped_org3) - ) - for call in mock_has.call_args_list: - self.assertNotEqual( - call[0], - ( - "organizations:notifications-double-write", - serialize_organization_mapping(mapped_org2), - ), - ) - - @mock.patch("sentry.notifications.helpers.features.has", return_value=False) - def test_is_double_write_enabled_team(self, mock_has): - # Create dummy users and organizations - org1 = self.create_organization() - org2 = self.create_organization() - - team1 = self.create_team(organization=org1) - self.create_team(organization=org2) - - is_double_write_enabled(organization_id_for_team=team1.organization_id) - - mapped_org1 = OrganizationMapping.objects.get(organization_id=org1.id) - mapped_org2 = OrganizationMapping.objects.get(organization_id=org2.id) - - # Ensure mock_has is called on the right organizations - mock_has.assert_any_call( - "organizations:notifications-double-write", serialize_organization_mapping(mapped_org1) - ) - for call in mock_has.call_args_list: - self.assertNotEqual( - call[0], - ( - "organizations:notifications-double-write", - serialize_organization_mapping(mapped_org2), - ), - ) - - def test_test_is_double_write_invalid_input(self): - with pytest.raises(ValueError): - is_double_write_enabled() - - @mock.patch("sentry.notifications.helpers.features.has", return_value=False) - def test_no_orgs(self, mock_has): - user1 = self.create_user() - - assert is_double_write_enabled(user_id=user1.id) - mock_has.assert_not_called() - - class NotificationHelpersTest(TestCase): def setUp(self): super().setUp() diff --git a/tests/sentry/notifications/utils/test_participants.py b/tests/sentry/notifications/utils/test_participants.py index 623668e87c5141..b7141f5de23dfa 100644 --- a/tests/sentry/notifications/utils/test_participants.py +++ b/tests/sentry/notifications/utils/test_participants.py @@ -1186,7 +1186,6 @@ def get_send_to_owners(self, event: Event) -> Mapping[ExternalProviders, Set[Rpc def store_event_owners(self, filename: str) -> Event: return super().store_event(data=make_event_data(filename), project_id=self.project.id) - @with_feature("organizations:notifications-double-write") def setUp(self): super().setUp() @@ -1243,7 +1242,6 @@ def test_team_owners(self): slack=[self.user.id], ) - @with_feature("organizations:notifications-double-write") @with_feature("organizations:notification-settings-v2") def test_disable_alerts_multiple_scopes(self): super().test_disable_alerts_multiple_scopes()