Skip to content

Commit

Permalink
feat(notifications): remove notification double write feature flag (#…
Browse files Browse the repository at this point in the history
…57863)

Now that we have double writes enabled for everyone, we can remove the
feature flag entirely
  • Loading branch information
Stephen Cefali authored Oct 16, 2023
1 parent 6cc2769 commit 268280e
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 207 deletions.
26 changes: 11 additions & 15 deletions src/sentry/api/endpoints/user_notification_fine_tuning.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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():
Expand All @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1792,8 +1792,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
Expand Down
1 change: 0 additions & 1 deletion src/sentry/features/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,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)
Expand Down
32 changes: 1 addition & 31 deletions src/sentry/notifications/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
38 changes: 12 additions & 26 deletions src/sentry/notifications/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions src/sentry/services/hybrid_cloud/notifications/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
33 changes: 0 additions & 33 deletions tests/sentry/api/endpoints/test_user_notification_details.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions tests/sentry/api/endpoints/test_user_notification_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion tests/sentry/api/serializers/test_organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 4 additions & 6 deletions tests/sentry/models/test_notificationsetting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
Loading

0 comments on commit 268280e

Please sign in to comment.