From 7ade6c8b8f9f8336a8dea45f956f8ec393472e38 Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Mon, 13 Feb 2023 10:40:21 -0800 Subject: [PATCH 01/11] WIP --- migrations_lockfile.txt | 2 +- src/sentry/data_export/models.py | 3 +- src/sentry/discover/models.py | 3 +- src/sentry/incidents/models.py | 7 +- src/sentry/migrations/0355_donot_merge_me.py | 163 +++++++++++++++++++ tests/sentry/db/test_silo_models.py | 3 + 6 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 src/sentry/migrations/0355_donot_merge_me.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index a0622214bcf01a..d19e71ba0da708 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi will then be regenerated, and you should be able to merge without conflicts. nodestore: 0002_nodestore_no_dictfield -sentry: 0354_break_saved_search_foreign_key +sentry: 0355_donot_merge_me social_auth: 0001_initial diff --git a/src/sentry/data_export/models.py b/src/sentry/data_export/models.py index 1d1caeefeb2306..52d24ecbf08700 100644 --- a/src/sentry/data_export/models.py +++ b/src/sentry/data_export/models.py @@ -17,6 +17,7 @@ ) from sentry.utils import json +from ..db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from .base import DEFAULT_EXPIRATION, ExportQueryType, ExportStatus logger = logging.getLogger(__name__) @@ -31,7 +32,7 @@ class ExportedData(Model): __include_in_export__ = False organization = FlexibleForeignKey("sentry.Organization") - user = FlexibleForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete=models.SET_NULL) + user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete="SET_NULL") file_id = BoundedBigIntegerField(null=True) date_added = models.DateTimeField(default=timezone.now) date_finished = models.DateTimeField(null=True) diff --git a/src/sentry/discover/models.py b/src/sentry/discover/models.py index d0abddbb743f47..dbaeaa34cdb130 100644 --- a/src/sentry/discover/models.py +++ b/src/sentry/discover/models.py @@ -12,6 +12,7 @@ ) from sentry.db.models.fields import JSONField from sentry.db.models.fields.bounded import BoundedBigIntegerField +from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.models.projectteam import ProjectTeam from sentry.tasks.relay import schedule_invalidate_project_config @@ -42,7 +43,7 @@ class DiscoverSavedQuery(Model): projects = models.ManyToManyField("sentry.Project", through=DiscoverSavedQueryProject) organization = FlexibleForeignKey("sentry.Organization") - created_by = FlexibleForeignKey("sentry.User", null=True, on_delete=models.SET_NULL) + created_by_id = HybridCloudForeignKey("sentry.User", null=True, on_delete="SET_NULL") name = models.CharField(max_length=255) query = JSONField() version = models.IntegerField(null=True) diff --git a/src/sentry/incidents/models.py b/src/sentry/incidents/models.py index d123d302a564c2..3cc174f1c812f8 100644 --- a/src/sentry/incidents/models.py +++ b/src/sentry/incidents/models.py @@ -18,6 +18,7 @@ region_silo_only_model, sane_repr, ) +from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.db.models.manager import BaseManager from sentry.models import Team, User from sentry.snuba.models import QuerySubscription @@ -268,13 +269,13 @@ class IncidentSubscription(Model): __include_in_export__ = True incident = FlexibleForeignKey("sentry.Incident", db_index=False) - user = FlexibleForeignKey(settings.AUTH_USER_MODEL) + user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, on_delete="CASCADE") date_added = models.DateTimeField(default=timezone.now) class Meta: app_label = "sentry" db_table = "sentry_incidentsubscription" - unique_together = (("incident", "user"),) + unique_together = (("incident", "user_id"),) __repr__ = sane_repr("incident_id", "user_id") @@ -658,7 +659,7 @@ class AlertRuleActivity(Model): previous_alert_rule = FlexibleForeignKey( "sentry.AlertRule", null=True, related_name="previous_alert_rule" ) - user = FlexibleForeignKey("sentry.User", null=True, on_delete=models.SET_NULL) + user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete="SET_NULL") type = models.IntegerField() date_added = models.DateTimeField(default=timezone.now) diff --git a/src/sentry/migrations/0355_donot_merge_me.py b/src/sentry/migrations/0355_donot_merge_me.py new file mode 100644 index 00000000000000..2559e07f79403c --- /dev/null +++ b/src/sentry/migrations/0355_donot_merge_me.py @@ -0,0 +1,163 @@ +# Generated by Django 2.2.28 on 2023-02-13 17:12 + +from django.db import migrations + +import sentry.db.models.fields.hybrid_cloud_foreign_key +from sentry.new_migrations.migrations import CheckedMigration + +# DO NOT MERGE THIS MIGRATION. It is intended as a staging ground for the aggregate of all migrations. Instead, break +# off each individual domain migration into a smaller migration in a separate PR and work that into master alongside +# the necessary query changes in this PR. + + +def exported_data_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="exporteddata", + name="user", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True, null=True + ), + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="exporteddata", + name="user", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", db_index=True, null=True, on_delete="SET_NULL" + ), + ), + migrations.RenameField( + model_name="exporteddata", + old_name="user", + new_name="user_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + +def alertruleactivity_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="alertruleactivity", + name="user", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True, null=True + ), + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="alertruleactivity", + name="user", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", db_index=True, null=True, on_delete="SET_NULL" + ), + ), + migrations.RenameField( + model_name="alertruleactivity", + old_name="user", + new_name="user_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + +def discover_saved_query_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="discoversavedquery", + name="created_by", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True, null=True + ), + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="discoversavedquery", + name="created_by", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", db_index=True, null=True, on_delete="SET_NULL" + ), + ), + migrations.RenameField( + model_name="discoversavedquery", + old_name="created_by", + new_name="created_by_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + +def incidentsubscription_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="incidentsubscription", + name="user", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True + ), + ), + migrations.AlterUniqueTogether( + name="incidentsubscription", + unique_together={("incident", "user_id")}, + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="incidentsubscription", + name="user", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", db_index=True, on_delete="CASCADE" + ), + ), + migrations.RenameField( + model_name="incidentsubscription", + old_name="user", + new_name="user_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. For + # the most part, this should only be used for operations where it's safe to run the migration + # after your code has deployed. So this should not be used for most operations that alter the + # schema of a table. + # Here are some things that make sense to mark as dangerous: + # - Large data migrations. Typically we want these to be run manually by ops so that they can + # be monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # have ops run this and not block the deploy. Note that while adding an index is a schema + # change, it's completely safe to run the operation after the code has deployed. + is_dangerous = False + + dependencies = [ + ("sentry", "0354_break_saved_search_foreign_key"), + ] + + operations = ( + exported_data_user_migrations() + + discover_saved_query_user_migrations() + + alertruleactivity_user_migrations() + + incidentsubscription_user_migrations() + ) diff --git a/tests/sentry/db/test_silo_models.py b/tests/sentry/db/test_silo_models.py index 69809464aefe68..99845e380a4b73 100644 --- a/tests/sentry/db/test_silo_models.py +++ b/tests/sentry/db/test_silo_models.py @@ -141,8 +141,11 @@ def __hash__(self): (IncidentSeen, User), (IncidentSnapshot, TimeSeriesSnapshot), (IncidentActivity, User), + # (IncidentSubscription, User), + # (AlertRuleTriggerAction, SentryApp), + # (AlertRuleActivity, User), (DiscoverSavedQuery, User), (ExportedData, User), From 549856c7cead592a2b83bf6c00a4c983054e672d Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Tue, 14 Feb 2023 08:45:03 -0800 Subject: [PATCH 02/11] More broken keys --- src/sentry/incidents/models.py | 7 +- src/sentry/migrations/0358_donot_merge_me.py | 100 +++++++++++++++++++ src/sentry/models/transaction_threshold.py | 5 +- tests/sentry/db/test_silo_models.py | 7 +- 4 files changed, 107 insertions(+), 12 deletions(-) diff --git a/src/sentry/incidents/models.py b/src/sentry/incidents/models.py index 3d790c7def4cbe..eb84570b2cb0d9 100644 --- a/src/sentry/incidents/models.py +++ b/src/sentry/incidents/models.py @@ -14,7 +14,6 @@ Model, OneToOneCascadeDeletes, UUIDField, - control_silo_only_model, region_silo_only_model, sane_repr, ) @@ -44,13 +43,13 @@ class IncidentSeen(Model): __include_in_export__ = False incident = FlexibleForeignKey("sentry.Incident") - user = FlexibleForeignKey(settings.AUTH_USER_MODEL, db_index=False) + user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, on_delete="CASCADE", db_index=False) last_seen = models.DateTimeField(default=timezone.now) class Meta: app_label = "sentry" db_table = "sentry_incidentseen" - unique_together = (("user", "incident"),) + unique_together = (("user_id", "incident"),) class IncidentManager(BaseManager): @@ -225,7 +224,7 @@ class Meta: db_table = "sentry_incidentsnapshot" -@control_silo_only_model +@region_silo_only_model class TimeSeriesSnapshot(Model): __include_in_export__ = True diff --git a/src/sentry/migrations/0358_donot_merge_me.py b/src/sentry/migrations/0358_donot_merge_me.py index a8c735923e0189..0209de0401102e 100644 --- a/src/sentry/migrations/0358_donot_merge_me.py +++ b/src/sentry/migrations/0358_donot_merge_me.py @@ -138,6 +138,103 @@ def incidentsubscription_user_migrations(): ] +def incidentactivity_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="incidentactivity", + name="user", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True, null=True + ), + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="incidentactivity", + name="user", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", db_index=True, on_delete="CASCADE", null=True + ), + ), + migrations.RenameField( + model_name="incidentactivity", + old_name="user", + new_name="user_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + +def incidentseen_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="incidentseen", + name="user", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=False + ), + ), + migrations.AlterUniqueTogether( + name="incidentseen", + unique_together={("user_id", "incident")}, + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="incidentseen", + name="user", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", db_index=False, on_delete="CASCADE" + ), + ), + migrations.RenameField( + model_name="incidentseen", + old_name="user", + new_name="user_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + +def project_transaction_threshold_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="projecttransactionthreshold", + name="edited_by", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True + ), + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="projecttransactionthreshold", + name="edited_by", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", null=True, on_delete="SET_NULL" + ), + ), + migrations.RenameField( + model_name="projecttransactionthreshold", + old_name="edited_by", + new_name="edited_by_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + class Migration(CheckedMigration): # This flag is used to mark that a migration shouldn't be automatically run in production. For # the most part, this should only be used for operations where it's safe to run the migration @@ -160,4 +257,7 @@ class Migration(CheckedMigration): + discover_saved_query_user_migrations() + alertruleactivity_user_migrations() + incidentsubscription_user_migrations() + + incidentactivity_user_migrations() + + incidentseen_user_migrations() + + project_transaction_threshold_user_migrations() ) diff --git a/src/sentry/models/transaction_threshold.py b/src/sentry/models/transaction_threshold.py index 5ef276b7cedfa8..5c5a8604e8ed5e 100644 --- a/src/sentry/models/transaction_threshold.py +++ b/src/sentry/models/transaction_threshold.py @@ -3,6 +3,7 @@ from django.db import models from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_only_model +from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.utils.cache import cache from sentry.utils.hashlib import md5_text @@ -89,9 +90,7 @@ class ProjectTransactionThreshold(DefaultFieldsModel): organization = FlexibleForeignKey("sentry.Organization") threshold = models.IntegerField() metric = models.PositiveSmallIntegerField(default=TransactionMetric.DURATION.value) - edited_by = FlexibleForeignKey( - "sentry.User", null=True, on_delete=models.SET_NULL, db_constraint=False - ) + edited_by_id = HybridCloudForeignKey("sentry.User", null=True, on_delete="SET_NULL") class Meta: app_label = "sentry" diff --git a/tests/sentry/db/test_silo_models.py b/tests/sentry/db/test_silo_models.py index b561f6b5aa95c1..8bfa70583980a1 100644 --- a/tests/sentry/db/test_silo_models.py +++ b/tests/sentry/db/test_silo_models.py @@ -6,9 +6,7 @@ AlertRuleTriggerAction, IncidentActivity, IncidentSeen, - IncidentSnapshot, IncidentSubscription, - TimeSeriesSnapshot, ) from sentry.models import ( Activity, @@ -140,11 +138,10 @@ def __hash__(self): (SavedSearch, User), (ServiceHook, ApiApplication), (ProjectTransactionThresholdOverride, User), - (ProjectTransactionThreshold, User), (User, Actor), - (IncidentSeen, User), - (IncidentSnapshot, TimeSeriesSnapshot), # + (ProjectTransactionThreshold, User), + (IncidentSeen, User), (IncidentActivity, User), (IncidentSubscription, User), # From 34405bda9d0a85ba5ae01d89d2db65e06fe4673e Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Tue, 14 Feb 2023 09:50:31 -0800 Subject: [PATCH 03/11] WIP --- src/sentry/discover/models.py | 2 +- src/sentry/migrations/0358_donot_merge_me.py | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/sentry/discover/models.py b/src/sentry/discover/models.py index dbaeaa34cdb130..c504fa8bcf43d0 100644 --- a/src/sentry/discover/models.py +++ b/src/sentry/discover/models.py @@ -58,7 +58,7 @@ class Meta: db_table = "sentry_discoversavedquery" constraints = [ UniqueConstraint( - fields=["organization", "created_by", "is_homepage"], + fields=["organization", "created_by_id", "is_homepage"], condition=Q(is_homepage=True), name="unique_user_homepage_query", ) diff --git a/src/sentry/migrations/0358_donot_merge_me.py b/src/sentry/migrations/0358_donot_merge_me.py index 0209de0401102e..860edb21ff3b70 100644 --- a/src/sentry/migrations/0358_donot_merge_me.py +++ b/src/sentry/migrations/0358_donot_merge_me.py @@ -1,6 +1,6 @@ # Generated by Django 2.2.28 on 2023-02-13 17:12 -from django.db import migrations +from django.db import migrations, models import sentry.db.models.fields.hybrid_cloud_foreign_key from sentry.new_migrations.migrations import CheckedMigration @@ -84,6 +84,17 @@ def discover_saved_query_user_migrations(): ] state_operations = [ + migrations.RemoveConstraint( + model_name="discoversavedquery", name="unique_user_homepage_query" + ), + migrations.AddConstraint( + model_name="discoversavedquery", + constraint=models.UniqueConstraint( + condition=models.Q(is_homepage=True), + fields=("organization", "created_by_id", "is_homepage"), + name="unique_user_homepage_query", + ), + ), migrations.AlterField( model_name="discoversavedquery", name="created_by", From 84016c3fb8697331652e3c75c9e5e62f0230375a Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Tue, 14 Feb 2023 16:55:16 -0800 Subject: [PATCH 04/11] Test fixes for breaking keys. --- .../api/serializers/models/alert_rule.py | 51 ++++++++++++------- .../serializers/models/discoversavedquery.py | 30 +++++------ src/sentry/api/serializers/models/incident.py | 4 +- .../api/serializers/models/incidentseen.py | 13 +++-- .../endpoints/discover_key_transactions.py | 6 +-- .../endpoints/discover_saved_queries.py | 4 +- .../endpoints/discover_saved_query_detail.py | 6 +-- src/sentry/incidents/logic.py | 14 +++-- src/sentry/incidents/models.py | 8 ++- src/sentry/migrations/0358_donot_merge_me.py | 35 ++++++++++++- src/sentry/models/transaction_threshold.py | 4 +- .../endpoints/test_data_export_details.py | 5 +- tests/sentry/db/test_silo_models.py | 2 +- tests/sentry/discover/test_models.py | 12 ++--- ...rganization_incident_subscription_index.py | 8 +-- .../sentry/integrations/slack/test_unfurl.py | 9 ++-- .../test_unsubscribe_notifications.py | 2 +- .../test_discover_saved_query_detail.py | 17 ++++--- 18 files changed, 141 insertions(+), 89 deletions(-) diff --git a/src/sentry/api/serializers/models/alert_rule.py b/src/sentry/api/serializers/models/alert_rule.py index 5e3d3f108028f7..d5ec3be8723b5a 100644 --- a/src/sentry/api/serializers/models/alert_rule.py +++ b/src/sentry/api/serializers/models/alert_rule.py @@ -22,6 +22,7 @@ actor_type_to_class, actor_type_to_string, ) +from sentry.services.hybrid_cloud.user import user_service from sentry.snuba.models import SnubaQueryEventType @@ -73,31 +74,47 @@ def get_attrs(self, item_list, user, **kwargs): rule_result = result[alert_rules[alert_rule_id]].setdefault("projects", []) rule_result.append(project_slug) - for rule_activity in AlertRuleActivity.objects.filter( - alert_rule__in=item_list, type=AlertRuleActivityType.CREATED.value - ).select_related("alert_rule", "user"): - if rule_activity.user: - user = { - "id": rule_activity.user.id, - "name": rule_activity.user.get_display_name(), - "email": rule_activity.user.email, - } - else: - user = None + rule_activities = list( + AlertRuleActivity.objects.filter( + alert_rule__in=item_list, type=AlertRuleActivityType.CREATED.value + ) + ) - result[alert_rules[rule_activity.alert_rule.id]].update({"created_by": user}) + rule_activities_by_user_id = {r.user_id: r for r in rule_activities} + for user in user_service.get_many( + filter={"user_ids": list(rule_activities_by_user_id.keys())} + ): + rule_activity = rule_activities_by_user_id.get(user.id) + if not rule_activity: + continue + + result[alert_rules[rule_activity.alert_rule_id]]["created_by"] = dict( + id=user.id, name=user.get_display_name(), email=user.email + ) + + # Fill in any created_by in the rule_activities query that didn't have a user. + for rule_activity in rule_activities: + item = alert_rules[rule_activity.alert_rule_id] + if "created_by" not in result[item]: + result[item]["created_by"] = None - resolved_actors = {} owners_by_type = defaultdict(list) for item in item_list: if item.owner_id is not None: owners_by_type[actor_type_to_string(item.owner.type)].append(item.owner_id) + resolved_actors = {} for k, v in ACTOR_TYPES.items(): - resolved_actors[k] = { - a.actor_id: a.id - for a in actor_type_to_class(v).objects.filter(actor_id__in=owners_by_type[k]) - } + if k == ACTOR_TYPES["user"]: + resolved_actors[k] = { + u.actor_id: u.id + for u in user_service.get_by_actor_ids(actor_ids=owners_by_type[k]) + } + else: + resolved_actors[k] = { + a.actor_id: a.id + for a in actor_type_to_class(v).objects.filter(actor_id__in=owners_by_type[k]) + } for alert_rule in alert_rules.values(): if alert_rule.owner_id: diff --git a/src/sentry/api/serializers/models/discoversavedquery.py b/src/sentry/api/serializers/models/discoversavedquery.py index 855f5e71457d5b..a90d4aa8a81e47 100644 --- a/src/sentry/api/serializers/models/discoversavedquery.py +++ b/src/sentry/api/serializers/models/discoversavedquery.py @@ -1,34 +1,28 @@ from collections import defaultdict -from django.db.models.query import prefetch_related_objects - -from sentry.api.serializers import Serializer, register, serialize -from sentry.api.serializers.models.user import UserSerializer +from sentry.api.serializers import Serializer, register from sentry.constants import ALL_ACCESS_PROJECTS from sentry.discover.models import DiscoverSavedQuery +from sentry.services.hybrid_cloud.user import user_service from sentry.utils.dates import outside_retention_with_modified_start, parse_timestamp @register(DiscoverSavedQuery) class DiscoverSavedQuerySerializer(Serializer): def get_attrs(self, item_list, user): - prefetch_related_objects(item_list, "created_by") - result = defaultdict(lambda: {"created_by": {}}) - user_serializer = UserSerializer() - serialized_users = { - user["id"]: user - for user in serialize( - [ - discover_saved_query.created_by + service_serialized = user_service.serialize_many( + filter={ + "user_ids": [ + discover_saved_query.created_by_id for discover_saved_query in item_list - if discover_saved_query.created_by - ], - user=user, - serializer=user_serializer, - ) - } + if discover_saved_query.created_by_id + ] + }, + as_user=user, + ) + serialized_users = {user["id"]: user for user in service_serialized} for discover_saved_query in item_list: result[discover_saved_query]["created_by"] = serialized_users.get( diff --git a/src/sentry/api/serializers/models/incident.py b/src/sentry/api/serializers/models/incident.py index bbe16e9ced681f..348a11e873f5ce 100644 --- a/src/sentry/api/serializers/models/incident.py +++ b/src/sentry/api/serializers/models/incident.py @@ -44,9 +44,7 @@ def get_attrs(self, item_list, user, **kwargs): if "seen_by" in self.expand: incident_seen_list = list( - IncidentSeen.objects.filter(incident__in=item_list) - .select_related("user") - .order_by("-last_seen") + IncidentSeen.objects.filter(incident__in=item_list).order_by("-last_seen") ) incident_seen_dict = defaultdict(list) for incident_seen, serialized_seen_by in zip( diff --git a/src/sentry/api/serializers/models/incidentseen.py b/src/sentry/api/serializers/models/incidentseen.py index fefcbc916a5fe8..aabc58d8226bd2 100644 --- a/src/sentry/api/serializers/models/incidentseen.py +++ b/src/sentry/api/serializers/models/incidentseen.py @@ -1,14 +1,17 @@ -from django.db.models import prefetch_related_objects - -from sentry.api.serializers import Serializer, register, serialize +from sentry.api.serializers import Serializer, register from sentry.incidents.models import IncidentSeen +from sentry.services.hybrid_cloud.user import user_service @register(IncidentSeen) class IncidentSeenSerializer(Serializer): def get_attrs(self, item_list, user): - prefetch_related_objects(item_list, "user") - user_map = {d["id"]: d for d in serialize({i.user for i in item_list}, user)} + user_map = { + d["id"]: d + for d in user_service.serialize_many( + filter=dict(user_ids={i.user_id for i in item_list}), as_user=user + ) + } result = {} for item in item_list: diff --git a/src/sentry/discover/endpoints/discover_key_transactions.py b/src/sentry/discover/endpoints/discover_key_transactions.py index 73b5bc7c4b8de5..11db24d807eb83 100644 --- a/src/sentry/discover/endpoints/discover_key_transactions.py +++ b/src/sentry/discover/endpoints/discover_key_transactions.py @@ -5,7 +5,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry.api.base import pending_silo_endpoint +from sentry.api.base import region_silo_endpoint from sentry.api.bases import KeyTransactionBase from sentry.api.bases.organization import OrganizationPermission from sentry.api.helpers.teams import get_teams @@ -26,7 +26,7 @@ class KeyTransactionPermission(OrganizationPermission): } -@pending_silo_endpoint +@region_silo_endpoint class KeyTransactionEndpoint(KeyTransactionBase): permission_classes = (KeyTransactionPermission,) @@ -133,7 +133,7 @@ def delete(self, request: Request, organization) -> Response: return Response(serializer.errors, status=400) -@pending_silo_endpoint +@region_silo_endpoint class KeyTransactionListEndpoint(KeyTransactionBase): permission_classes = (KeyTransactionPermission,) diff --git a/src/sentry/discover/endpoints/discover_saved_queries.py b/src/sentry/discover/endpoints/discover_saved_queries.py index b0a16140e268ed..fe0a6d3038d228 100644 --- a/src/sentry/discover/endpoints/discover_saved_queries.py +++ b/src/sentry/discover/endpoints/discover_saved_queries.py @@ -4,7 +4,7 @@ from rest_framework.response import Response from sentry import features -from sentry.api.base import pending_silo_endpoint +from sentry.api.base import region_silo_endpoint from sentry.api.bases import NoProjects, OrganizationEndpoint from sentry.api.paginator import GenericOffsetPaginator from sentry.api.serializers import serialize @@ -14,7 +14,7 @@ from sentry.search.utils import tokenize_query -@pending_silo_endpoint +@region_silo_endpoint class DiscoverSavedQueriesEndpoint(OrganizationEndpoint): permission_classes = (DiscoverSavedQueryPermission,) diff --git a/src/sentry/discover/endpoints/discover_saved_query_detail.py b/src/sentry/discover/endpoints/discover_saved_query_detail.py index 498f36759ff9c5..19f7ab90638895 100644 --- a/src/sentry/discover/endpoints/discover_saved_query_detail.py +++ b/src/sentry/discover/endpoints/discover_saved_query_detail.py @@ -5,7 +5,7 @@ from rest_framework.response import Response from sentry import features -from sentry.api.base import pending_silo_endpoint +from sentry.api.base import region_silo_endpoint from sentry.api.bases import NoProjects, OrganizationEndpoint from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.serializers import serialize @@ -14,7 +14,7 @@ from sentry.discover.models import DiscoverSavedQuery -@pending_silo_endpoint +@region_silo_endpoint class DiscoverSavedQueryDetailEndpoint(OrganizationEndpoint): permission_classes = (DiscoverSavedQueryPermission,) @@ -108,7 +108,7 @@ def delete(self, request: Request, organization, query_id) -> Response: from rest_framework.response import Response -@pending_silo_endpoint +@region_silo_endpoint class DiscoverSavedQueryVisitEndpoint(OrganizationEndpoint): permission_classes = (DiscoverSavedQueryPermission,) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 27a8d421d12fa0..d82ed977e4c7c8 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -542,7 +542,9 @@ def create_alert_rule( subscribe_projects_to_alert_rule(alert_rule, projects) AlertRuleActivity.objects.create( - alert_rule=alert_rule, user=user, type=AlertRuleActivityType.CREATED.value + alert_rule=alert_rule, + user_id=user.id if user else None, + type=AlertRuleActivityType.CREATED.value, ) return alert_rule @@ -565,7 +567,7 @@ def snapshot_alert_rule(alert_rule, user=None): AlertRuleActivity.objects.create( alert_rule=alert_rule_snapshot, previous_alert_rule=alert_rule, - user=user, + user_id=user.id if user else None, type=AlertRuleActivityType.SNAPSHOT.value, ) @@ -685,7 +687,9 @@ def update_alert_rule( snapshot_alert_rule(alert_rule, user) alert_rule.update(**updated_fields) AlertRuleActivity.objects.create( - alert_rule=alert_rule, user=user, type=AlertRuleActivityType.UPDATED.value + alert_rule=alert_rule, + user_id=user.id if user else None, + type=AlertRuleActivityType.UPDATED.value, ) if updated_query_fields or environment != alert_rule.snuba_query.environment: @@ -831,7 +835,9 @@ def delete_alert_rule(alert_rule, user=None, ip_address=None): if incidents.exists(): alert_rule.update(status=AlertRuleStatus.SNAPSHOT.value) AlertRuleActivity.objects.create( - alert_rule=alert_rule, user=user, type=AlertRuleActivityType.DELETED.value + alert_rule=alert_rule, + user_id=user.id if user else None, + type=AlertRuleActivityType.DELETED.value, ) else: alert_rule.delete() diff --git a/src/sentry/incidents/models.py b/src/sentry/incidents/models.py index eb84570b2cb0d9..004381dbea599b 100644 --- a/src/sentry/incidents/models.py +++ b/src/sentry/incidents/models.py @@ -19,7 +19,8 @@ ) from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.db.models.manager import BaseManager -from sentry.models import Team, User +from sentry.models import Team +from sentry.services.hybrid_cloud.user import user_service from sentry.snuba.models import QuerySubscription from sentry.utils import metrics from sentry.utils.retries import TimedRetryPolicy @@ -578,10 +579,7 @@ class Meta: @property def target(self): if self.target_type == self.TargetType.USER.value: - try: - return User.objects.get(id=int(self.target_identifier)) - except User.DoesNotExist: - pass + return user_service.get_user(user_id=int(self.target_identifier)) elif self.target_type == self.TargetType.TEAM.value: try: return Team.objects.get(id=int(self.target_identifier)) diff --git a/src/sentry/migrations/0358_donot_merge_me.py b/src/sentry/migrations/0358_donot_merge_me.py index 860edb21ff3b70..351937aba4bb46 100644 --- a/src/sentry/migrations/0358_donot_merge_me.py +++ b/src/sentry/migrations/0358_donot_merge_me.py @@ -215,13 +215,13 @@ def incidentseen_user_migrations(): ] -def project_transaction_threshold_user_migrations(): +def projecttransactionthresholdoverride_user_migrations(): database_operations = [ migrations.AlterField( model_name="projecttransactionthreshold", name="edited_by", field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( - to="sentry.User", db_constraint=False, db_index=True + to="sentry.User", db_constraint=False, db_index=True, null=True ), ), ] @@ -246,6 +246,37 @@ def project_transaction_threshold_user_migrations(): ] +def project_transaction_threshold_user_migrations(): + database_operations = [ + migrations.AlterField( + model_name="projecttransactionthresholdoverride", + name="edited_by", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + to="sentry.User", db_constraint=False, db_index=True, null=True + ), + ), + ] + + state_operations = [ + migrations.AlterField( + model_name="projecttransactionthresholdoverride", + name="edited_by", + field=sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey( + "sentry.User", null=True, on_delete="SET_NULL" + ), + ), + migrations.RenameField( + model_name="projecttransactionthresholdoverride", + old_name="edited_by", + new_name="edited_by_id", + ), + ] + + return database_operations + [ + migrations.SeparateDatabaseAndState(state_operations=state_operations) + ] + + class Migration(CheckedMigration): # This flag is used to mark that a migration shouldn't be automatically run in production. For # the most part, this should only be used for operations where it's safe to run the migration diff --git a/src/sentry/models/transaction_threshold.py b/src/sentry/models/transaction_threshold.py index 5c5a8604e8ed5e..753361a0226db0 100644 --- a/src/sentry/models/transaction_threshold.py +++ b/src/sentry/models/transaction_threshold.py @@ -60,9 +60,7 @@ class ProjectTransactionThresholdOverride(DefaultFieldsModel): organization = FlexibleForeignKey("sentry.Organization") threshold = models.IntegerField() metric = models.PositiveSmallIntegerField(default=TransactionMetric.DURATION.value) - edited_by = FlexibleForeignKey( - "sentry.User", null=True, on_delete=models.SET_NULL, db_constraint=False - ) + edited_by_id = HybridCloudForeignKey("sentry.User", null=True, on_delete="SET_NULL") class Meta: app_label = "sentry" diff --git a/tests/sentry/data_export/endpoints/test_data_export_details.py b/tests/sentry/data_export/endpoints/test_data_export_details.py index b3d788ac341794..b28b33b2d27ad6 100644 --- a/tests/sentry/data_export/endpoints/test_data_export_details.py +++ b/tests/sentry/data_export/endpoints/test_data_export_details.py @@ -19,7 +19,10 @@ def setUp(self): self.organization = self.create_organization(owner=self.user) self.login_as(user=self.user) self.data_export = ExportedData.objects.create( - user=self.user, organization=self.organization, query_type=0, query_info={"env": "test"} + user_id=self.user.id, + organization=self.organization, + query_type=0, + query_info={"env": "test"}, ) def test_content(self): diff --git a/tests/sentry/db/test_silo_models.py b/tests/sentry/db/test_silo_models.py index 8bfa70583980a1..403f0c3cd13aaf 100644 --- a/tests/sentry/db/test_silo_models.py +++ b/tests/sentry/db/test_silo_models.py @@ -137,9 +137,9 @@ def __hash__(self): (RuleActivity, User), (SavedSearch, User), (ServiceHook, ApiApplication), - (ProjectTransactionThresholdOverride, User), (User, Actor), # + (ProjectTransactionThresholdOverride, User), (ProjectTransactionThreshold, User), (IncidentSeen, User), (IncidentActivity, User), diff --git a/tests/sentry/discover/test_models.py b/tests/sentry/discover/test_models.py index 5e412bf607931e..6d5736a054d0d0 100644 --- a/tests/sentry/discover/test_models.py +++ b/tests/sentry/discover/test_models.py @@ -54,7 +54,7 @@ def test_can_only_create_single_homepage_query_for_user(self): organization=self.org, name="Test query", query=self.query, - created_by=self.user, + created_by_id=self.user.id, is_homepage=True, ) @@ -63,7 +63,7 @@ def test_can_only_create_single_homepage_query_for_user(self): organization=self.org, name="Test query 2", query=self.query, - created_by=self.user, + created_by_id=self.user.id, is_homepage=True, ) @@ -72,14 +72,14 @@ def test_can_only_have_single_homepage_query_for_user_on_update(self): organization=self.org, name="Test query", query=self.query, - created_by=self.user, + created_by_id=self.user.id, is_homepage=True, ) new_query = DiscoverSavedQuery.objects.create( organization=self.org, name="Test query 2", query=self.query, - created_by=self.user, + created_by_id=self.user.id, ) with pytest.raises(IntegrityError), transaction.atomic(): @@ -98,14 +98,14 @@ def test_user_can_have_homepage_query_in_multiple_orgs(self): organization=self.org, name="Test query", query=self.query, - created_by=self.user, + created_by_id=self.user.id, is_homepage=True, ) new_query = DiscoverSavedQuery.objects.create( organization=other_org, name="Test query 2", query=self.query, - created_by=self.user, + created_by_id=self.user.id, ) # Does not error since the query is in another org diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_subscription_index.py b/tests/sentry/incidents/endpoints/test_organization_incident_subscription_index.py index 754cad9ab6b38c..eb70485deb1ee0 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_subscription_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_subscription_index.py @@ -49,9 +49,9 @@ def test_simple(self): incident = self.create_incident() with self.feature("organizations:incidents"): self.get_success_response(self.organization.slug, incident.identifier, status_code=201) - sub = IncidentSubscription.objects.filter(incident=incident, user=self.user).get() + sub = IncidentSubscription.objects.filter(incident=incident, user_id=self.user.id).get() assert sub.incident == incident - assert sub.user == self.user + assert sub.user_id == self.user.id @region_silo_test @@ -69,4 +69,6 @@ def test_simple(self): subscribe_to_incident(incident, self.user.id) with self.feature("organizations:incidents"): self.get_success_response(self.organization.slug, incident.identifier, status_code=200) - assert not IncidentSubscription.objects.filter(incident=incident, user=self.user).exists() + assert not IncidentSubscription.objects.filter( + incident=incident, user_id=self.user.id + ).exists() diff --git a/tests/sentry/integrations/slack/test_unfurl.py b/tests/sentry/integrations/slack/test_unfurl.py index ee1d7a25eaaaaa..97e2cdc41a02de 100644 --- a/tests/sentry/integrations/slack/test_unfurl.py +++ b/tests/sentry/integrations/slack/test_unfurl.py @@ -168,6 +168,7 @@ def test_match_link(url, expected): assert match_link(url) == expected +# @region_silo_test(stable=True) class UnfurlTest(TestCase): def setUp(self): super().setUp() @@ -585,7 +586,7 @@ def test_unfurl_discover_short_url(self, mock_generate_chart, _): } saved_query = DiscoverSavedQuery.objects.create( organization=self.organization, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=query, version=2, @@ -649,7 +650,7 @@ def test_unfurl_correct_y_axis_for_saved_query(self, mock_generate_chart, _): } saved_query = DiscoverSavedQuery.objects.create( organization=self.organization, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=query, version=2, @@ -826,7 +827,7 @@ def test_unfurl_discover_short_url_without_project_ids(self, mock_generate_chart } saved_query = DiscoverSavedQuery.objects.create( organization=self.organization, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=query, version=2, @@ -1092,7 +1093,7 @@ def test_saved_query_with_interval(self, mock_generate_chart, api_mock): } saved_query = DiscoverSavedQuery.objects.create( organization=self.organization, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=query, version=2, diff --git a/tests/sentry/web/frontend/test_unsubscribe_notifications.py b/tests/sentry/web/frontend/test_unsubscribe_notifications.py index 6343c81575c19d..f26e352f1acbd3 100644 --- a/tests/sentry/web/frontend/test_unsubscribe_notifications.py +++ b/tests/sentry/web/frontend/test_unsubscribe_notifications.py @@ -70,4 +70,4 @@ def create_instance(self): return self.create_incident() def assert_unsubscribed(self, instance, user): - assert not IncidentSubscription.objects.filter(incident=instance, user=user).exists() + assert not IncidentSubscription.objects.filter(incident=instance, user_id=user.id).exists() diff --git a/tests/snuba/api/endpoints/test_discover_saved_query_detail.py b/tests/snuba/api/endpoints/test_discover_saved_query_detail.py index 426da087318880..c270bd158f7512 100644 --- a/tests/snuba/api/endpoints/test_discover_saved_query_detail.py +++ b/tests/snuba/api/endpoints/test_discover_saved_query_detail.py @@ -6,6 +6,7 @@ from sentry.testutils.silo import region_silo_test +@region_silo_test(stable=True) class DiscoverSavedQueryDetailTest(APITestCase, SnubaTestCase): feature_name = "organizations:discover" @@ -21,7 +22,7 @@ def setUp(self): query = {"fields": ["test"], "conditions": [], "limit": 10} model = DiscoverSavedQuery.objects.create( - organization=self.org, created_by=self.user, name="Test query", query=query + organization=self.org, created_by_id=self.user.id, name="Test query", query=query ) model.set_projects(self.project_ids) @@ -70,7 +71,7 @@ def test_get_discover_query_flag(self): def test_get_version(self): query = {"fields": ["event_id"], "query": "event.type:error", "limit": 10, "version": 2} model = DiscoverSavedQuery.objects.create( - organization=self.org, created_by=self.user, name="v2 query", query=query + organization=self.org, created_by_id=self.user.id, name="v2 query", query=query ) model.set_projects(self.project_ids) @@ -102,7 +103,7 @@ def test_get_homepage_query(self): query = {"fields": ["event_id"], "query": "event.type:error", "limit": 10, "version": 2} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="v2 query", query=query, is_homepage=True, @@ -185,7 +186,7 @@ def test_put_query_with_team(self): project = self.create_project(organization=self.org, teams=[team]) query = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query={"fields": ["test"], "conditions": [], "limit": 10}, ) @@ -206,7 +207,7 @@ def test_put_query_without_team(self): project = self.create_project(organization=self.org, teams=[team]) query = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query={"fields": ["test"], "conditions": [], "limit": 10}, ) @@ -227,7 +228,7 @@ def test_put_homepage_query(self): query = {"fields": ["event_id"], "query": "event.type:error", "limit": 10, "version": 2} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="v2 query", query=query, is_homepage=True, @@ -308,7 +309,7 @@ def test_delete_homepage_query(self): query = {"fields": ["event_id"], "query": "event.type:error", "limit": 10, "version": 2} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="v2 query", query=query, is_homepage=True, @@ -339,7 +340,7 @@ def setUp(self): q = {"fields": ["test"], "conditions": [], "limit": 10} self.query = DiscoverSavedQuery.objects.create( - organization=self.org, created_by=self.user, name="Test query", query=q + organization=self.org, created_by_id=self.user.id, name="Test query", query=q ) self.query.set_projects(self.project_ids) From 42fa93c29ff00f9b6a9ad8c9ae17de3b1772257a Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Tue, 14 Feb 2023 23:49:41 -0800 Subject: [PATCH 05/11] Fixing migration some. --- src/sentry/migrations/0359_donot_merge_me.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/sentry/migrations/0359_donot_merge_me.py b/src/sentry/migrations/0359_donot_merge_me.py index 2c47882ec5f678..0a786503d81bfe 100644 --- a/src/sentry/migrations/0359_donot_merge_me.py +++ b/src/sentry/migrations/0359_donot_merge_me.py @@ -123,10 +123,6 @@ def incidentsubscription_user_migrations(): to="sentry.User", db_constraint=False, db_index=True ), ), - migrations.AlterUniqueTogether( - name="incidentsubscription", - unique_together={("incident", "user_id")}, - ), ] state_operations = [ @@ -142,6 +138,10 @@ def incidentsubscription_user_migrations(): old_name="user", new_name="user_id", ), + migrations.AlterUniqueTogether( + name="incidentsubscription", + unique_together={("incident", "user_id")}, + ), ] return database_operations + [ @@ -189,10 +189,6 @@ def incidentseen_user_migrations(): to="sentry.User", db_constraint=False, db_index=False ), ), - migrations.AlterUniqueTogether( - name="incidentseen", - unique_together={("user_id", "incident")}, - ), ] state_operations = [ @@ -208,6 +204,10 @@ def incidentseen_user_migrations(): old_name="user", new_name="user_id", ), + migrations.AlterUniqueTogether( + name="incidentseen", + unique_together={("user_id", "incident")}, + ), ] return database_operations + [ @@ -302,4 +302,5 @@ class Migration(CheckedMigration): + incidentactivity_user_migrations() + incidentseen_user_migrations() + project_transaction_threshold_user_migrations() + + projecttransactionthresholdoverride_user_migrations() ) From e616cd6fd91578c04ad55503bc3974a9a1c4d869 Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Wed, 15 Feb 2023 00:39:25 -0800 Subject: [PATCH 06/11] Fixing more tests. --- .../project_transaction_threshold.py | 2 +- .../project_transaction_threshold_override.py | 2 +- .../serializers/models/incidentactivity.py | 1 - src/sentry/data_export/models.py | 13 ++++++-- src/sentry/incidents/models.py | 2 +- src/sentry/incidents/tasks.py | 33 ++++++++++++------- .../test_project_transaction_threshold.py | 2 +- ..._project_transaction_threshold_override.py | 2 +- .../api/serializers/test_incident_activity.py | 13 ++++++-- tests/sentry/data_export/test_models.py | 5 ++- .../test_organization_incident_seen.py | 6 ++-- tests/sentry/incidents/test_logic.py | 24 ++++++++------ tests/sentry/incidents/test_tasks.py | 16 ++++++--- 13 files changed, 81 insertions(+), 40 deletions(-) diff --git a/src/sentry/api/endpoints/project_transaction_threshold.py b/src/sentry/api/endpoints/project_transaction_threshold.py index 963b7d4c82030d..3a352c162dd12e 100644 --- a/src/sentry/api/endpoints/project_transaction_threshold.py +++ b/src/sentry/api/endpoints/project_transaction_threshold.py @@ -101,7 +101,7 @@ def post(self, request: Request, project) -> Response: organization=project.organization, threshold=data.get("threshold", 300), metric=data.get("metric", TransactionMetric.DURATION.value), - edited_by=request.user, + edited_by_id=request.user.id, ) created = True diff --git a/src/sentry/api/endpoints/project_transaction_threshold_override.py b/src/sentry/api/endpoints/project_transaction_threshold_override.py index c223a5c5f9f8a1..ab92d6971a1ab3 100644 --- a/src/sentry/api/endpoints/project_transaction_threshold_override.py +++ b/src/sentry/api/endpoints/project_transaction_threshold_override.py @@ -117,7 +117,7 @@ def post(self, request: Request, organization) -> Response: defaults={ "threshold": data["threshold"], "metric": data["metric"], - "edited_by": request.user, + "edited_by_id": request.user.id, }, ) diff --git a/src/sentry/api/serializers/models/incidentactivity.py b/src/sentry/api/serializers/models/incidentactivity.py index 286c27dd1a2d05..f83e4e3da6125a 100644 --- a/src/sentry/api/serializers/models/incidentactivity.py +++ b/src/sentry/api/serializers/models/incidentactivity.py @@ -9,7 +9,6 @@ class IncidentActivitySerializer(Serializer): def get_attrs(self, item_list, user, **kwargs): prefetch_related_objects(item_list, "incident__organization") - prefetch_related_objects(item_list, "user") serialized_users = user_service.serialize_many( filter={"user_ids": [i.user_id for i in item_list if i.user_id]}, as_user=user ) diff --git a/src/sentry/data_export/models.py b/src/sentry/data_export/models.py index 52d24ecbf08700..b6899d72c1cf6b 100644 --- a/src/sentry/data_export/models.py +++ b/src/sentry/data_export/models.py @@ -18,6 +18,7 @@ from sentry.utils import json from ..db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey +from ..services.hybrid_cloud.user import user_service from .base import DEFAULT_EXPIRATION, ExportQueryType, ExportStatus logger = logging.getLogger(__name__) @@ -86,6 +87,10 @@ def finalize_upload(self, file, expiration=DEFAULT_EXPIRATION): def email_success(self): from sentry.utils.email import MessageBuilder + user = user_service.get_user(user_id=self.user_id) + if user is None: + return + # The following condition should never be true, but it's a safeguard in case someone manually calls this method if self.date_finished is None or self.date_expired is None or self._get_file() is None: logger.warning( @@ -103,11 +108,15 @@ def email_success(self): template="sentry/emails/data-export-success.txt", html_template="sentry/emails/data-export-success.html", ) - msg.send_async([self.user.email]) + msg.send_async([user.email]) def email_failure(self, message): from sentry.utils.email import MessageBuilder + user = user_service.get_user(user_id=self.user_id) + if user is None: + return + msg = MessageBuilder( subject="We couldn't export your data.", context={ @@ -119,7 +128,7 @@ def email_failure(self, message): template="sentry/emails/data-export-failure.txt", html_template="sentry/emails/data-export-failure.html", ) - msg.send_async([self.user.email]) + msg.send_async([user.email]) self.delete() def _get_file(self): diff --git a/src/sentry/incidents/models.py b/src/sentry/incidents/models.py index f81252fba0b160..b047bc7d4d5a09 100644 --- a/src/sentry/incidents/models.py +++ b/src/sentry/incidents/models.py @@ -398,7 +398,7 @@ def created_by(self): created_activity = AlertRuleActivity.objects.get( alert_rule=self, type=AlertRuleActivityType.CREATED.value ) - return created_activity.user + return user_service.get_user(user_id=created_activity.user_id) except AlertRuleActivity.DoesNotExist: pass return None diff --git a/src/sentry/incidents/tasks.py b/src/sentry/incidents/tasks.py index b2110eefaf2b56..1d34df01d57fd4 100644 --- a/src/sentry/incidents/tasks.py +++ b/src/sentry/incidents/tasks.py @@ -16,6 +16,7 @@ IncidentStatusMethod, ) from sentry.models import Project +from sentry.services.hybrid_cloud.user import user_service from sentry.snuba.dataset import Dataset from sentry.snuba.query_subscription_consumer import register_subscriber from sentry.tasks.base import instrumented_task @@ -36,11 +37,13 @@ def send_subscriber_notifications(activity_id): try: activity = IncidentActivity.objects.select_related( - "incident", "user", "incident__organization" + "incident", "incident__organization" ).get(id=activity_id) except IncidentActivity.DoesNotExist: return + activity_user = user_service.get_user(user_id=activity.user_id) + # Only send notifications for specific activity types. if activity.type not in ( IncidentActivityType.COMMENT.value, @@ -51,28 +54,34 @@ def send_subscriber_notifications(activity_id): # Check that the user still has access to at least one of the projects # related to the incident. If not then unsubscribe them. projects = list(activity.incident.projects.all()) - for subscriber in get_incident_subscribers(activity.incident).select_related("user"): - user = subscriber.user - access = from_user(user, activity.incident.organization) + for subscriber in get_incident_subscribers(activity.incident): + subscriber_user = user_service.get_user(user_id=subscriber.user_id) + if subscriber_user is None: + continue + + access = from_user(subscriber_user, activity.incident.organization) if not any(project for project in projects if access.has_project_access(project)): - unsubscribe_from_incident(activity.incident, user.id) - elif user != activity.user: - msg = generate_incident_activity_email(activity, user) - msg.send_async([user.email]) + unsubscribe_from_incident(activity.incident, subscriber_user.id) + elif subscriber_user.id != activity.user_id: + msg = generate_incident_activity_email(activity, subscriber_user, activity_user) + msg.send_async([subscriber_user.email]) -def generate_incident_activity_email(activity, user): +def generate_incident_activity_email(activity, user, activity_user=None): incident = activity.incident return MessageBuilder( subject=f"Activity on Alert {incident.title} (#{incident.identifier})", template="sentry/emails/incidents/activity.txt", html_template="sentry/emails/incidents/activity.html", type="incident.activity", - context=build_activity_context(activity, user), + context=build_activity_context(activity, user, activity_user), ) -def build_activity_context(activity, user): +def build_activity_context(activity, user, activity_user=None): + if activity_user is None: + activity_user = user_service.get_user(user_id=activity.user_id) + if activity.type == IncidentActivityType.COMMENT.value: action = "left a comment" else: @@ -85,7 +94,7 @@ def build_activity_context(activity, user): action = f"{action} on alert {incident.title} (#{incident.identifier})" return { - "user_name": activity.user.name if activity.user else "Sentry", + "user_name": activity_user.name if activity_user else "Sentry", "action": action, "link": absolute_uri( reverse( diff --git a/tests/sentry/api/endpoints/test_project_transaction_threshold.py b/tests/sentry/api/endpoints/test_project_transaction_threshold.py index 3b2d1d372aa378..ba14e27e038c90 100644 --- a/tests/sentry/api/endpoints/test_project_transaction_threshold.py +++ b/tests/sentry/api/endpoints/test_project_transaction_threshold.py @@ -5,7 +5,7 @@ from sentry.testutils.silo import region_silo_test -@region_silo_test +@region_silo_test(stable=True) class ProjectTransactionThresholdTest(APITestCase): feature_name = "organizations:performance-view" diff --git a/tests/sentry/api/endpoints/test_project_transaction_threshold_override.py b/tests/sentry/api/endpoints/test_project_transaction_threshold_override.py index 70f5844c385df1..eff2ad6b605e2f 100644 --- a/tests/sentry/api/endpoints/test_project_transaction_threshold_override.py +++ b/tests/sentry/api/endpoints/test_project_transaction_threshold_override.py @@ -12,7 +12,7 @@ from sentry.utils.samples import load_data -@region_silo_test +@region_silo_test(stable=True) class ProjectTransactionThresholdOverrideTest(APITestCase): feature_name = "organizations:performance-view" diff --git a/tests/sentry/api/serializers/test_incident_activity.py b/tests/sentry/api/serializers/test_incident_activity.py index a390e8803deb09..ff7d1e2630e41d 100644 --- a/tests/sentry/api/serializers/test_incident_activity.py +++ b/tests/sentry/api/serializers/test_incident_activity.py @@ -7,10 +7,13 @@ from sentry.api.serializers import serialize from sentry.incidents.logic import create_incident_activity from sentry.incidents.models import IncidentActivityType +from sentry.services.hybrid_cloud.user import user_service from sentry.testutils import SnubaTestCase, TestCase from sentry.testutils.helpers.datetime import before_now, iso_format +from sentry.testutils.silo import region_silo_test +@region_silo_test(stable=True) class IncidentActivitySerializerTest(TestCase, SnubaTestCase): def test_simple(self): activity = create_incident_activity( @@ -23,7 +26,10 @@ def test_simple(self): assert result["id"] == str(activity.id) assert result["incidentIdentifier"] == str(activity.incident.identifier) - assert result["user"] == serialize(activity.user) + assert ( + result["user"] + == user_service.serialize_many(filter=dict(user_ids=[activity.user_id]))[0] + ) assert result["type"] == activity.type assert result["value"] is None assert result["previousValue"] is None @@ -73,7 +79,10 @@ def test_event_stats(self): assert result["id"] == str(activity.id) assert result["incidentIdentifier"] == str(activity.incident.identifier) - assert result["user"] == serialize(activity.user) + assert ( + result["user"] + == user_service.serialize_many(filter=dict(user_ids=[activity.user_id]))[0] + ) assert result["type"] == activity.type assert result["value"] is None assert result["previousValue"] is None diff --git a/tests/sentry/data_export/test_models.py b/tests/sentry/data_export/test_models.py index aa8ff96021c152..02960499a4f009 100644 --- a/tests/sentry/data_export/test_models.py +++ b/tests/sentry/data_export/test_models.py @@ -23,7 +23,10 @@ def setUp(self): self.user = self.create_user() self.organization = self.create_organization() self.data_export = ExportedData.objects.create( - user=self.user, organization=self.organization, query_type=0, query_info={"env": "test"} + user_id=self.user.id, + organization=self.organization, + query_type=0, + query_info={"env": "test"}, ) self.file1 = File.objects.create( name="tempfile-data-export", type="export.csv", headers={"Content-Type": "text/csv"} diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_seen.py b/tests/sentry/incidents/endpoints/test_organization_incident_seen.py index 6b3253ec308b18..8450664000faa7 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_seen.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_seen.py @@ -42,7 +42,7 @@ def test_has_user_seen(self): seen_incidents = IncidentSeen.objects.filter(incident=incident) assert len(seen_incidents) == 1 - assert seen_incidents[0].user == self.user + assert seen_incidents[0].user_id == self.user.id # mark set as seen by new_user resp = self.get_response(incident.organization.slug, incident.identifier) @@ -50,8 +50,8 @@ def test_has_user_seen(self): seen_incidents = IncidentSeen.objects.filter(incident=incident) assert len(seen_incidents) == 2 - assert seen_incidents[0].user == self.user - assert seen_incidents[1].user == new_user + assert seen_incidents[0].user_id == self.user.id + assert seen_incidents[1].user_id == new_user.id url = reverse( "sentry-api-0-organization-incident-details", diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py index b42924f90713a7..586175f82617bf 100644 --- a/tests/sentry/incidents/test_logic.py +++ b/tests/sentry/incidents/test_logic.py @@ -170,9 +170,9 @@ def run_test( assert incident.date_closed == expected_date_closed activity = self.get_most_recent_incident_activity(incident) assert activity.type == IncidentActivityType.STATUS_CHANGE.value - assert activity.user == user + assert activity.user_id == (user.id if user else None) if user: - assert IncidentSubscription.objects.filter(incident=incident, user=user).exists() + assert IncidentSubscription.objects.filter(incident=incident, user_id=user.id).exists() assert activity.value == str(status.value) assert activity.previous_value == str(prev_status) assert activity.comment == comment @@ -339,7 +339,7 @@ def test_no_snapshot(self): ) assert activity.incident == incident assert activity.type == IncidentActivityType.STATUS_CHANGE.value - assert activity.user == self.user + assert activity.user_id == self.user.id assert activity.value == str(IncidentStatus.CLOSED.value) assert activity.previous_value == str(IncidentStatus.WARNING.value) self.assert_notifications_sent(activity) @@ -349,16 +349,18 @@ def test_comment(self): incident = self.create_incident() comment = "hello" - assert not IncidentSubscription.objects.filter(incident=incident, user=self.user).exists() + assert not IncidentSubscription.objects.filter( + incident=incident, user_id=self.user.id + ).exists() self.record_event.reset_mock() activity = create_incident_activity( incident, IncidentActivityType.COMMENT, user=self.user, comment=comment ) - assert IncidentSubscription.objects.filter(incident=incident, user=self.user).exists() + assert IncidentSubscription.objects.filter(incident=incident, user_id=self.user.id).exists() assert activity.incident == incident assert activity.type == IncidentActivityType.COMMENT.value - assert activity.user == self.user + assert activity.user_id == self.user.id assert activity.comment == comment assert activity.value is None assert activity.previous_value is None @@ -378,11 +380,13 @@ def test_mentioned_user_ids(self): incident = self.create_incident() mentioned_member = self.create_user() subscribed_mentioned_member = self.create_user() - IncidentSubscription.objects.create(incident=incident, user=subscribed_mentioned_member) + IncidentSubscription.objects.create( + incident=incident, user_id=subscribed_mentioned_member.id + ) comment = f"hello **@{mentioned_member.username}** and **@{subscribed_mentioned_member.username}**" assert not IncidentSubscription.objects.filter( - incident=incident, user=mentioned_member + incident=incident, user_id=mentioned_member.id ).exists() self.record_event.reset_mock() activity = create_incident_activity( @@ -393,12 +397,12 @@ def test_mentioned_user_ids(self): mentioned_user_ids=[mentioned_member.id, subscribed_mentioned_member.id], ) assert IncidentSubscription.objects.filter( - incident=incident, user=mentioned_member + incident=incident, user_id=mentioned_member.id ).exists() assert activity.incident == incident assert activity.type == IncidentActivityType.COMMENT.value - assert activity.user == self.user + assert activity.user_id == self.user.id assert activity.comment == comment assert activity.value is None assert activity.previous_value is None diff --git a/tests/sentry/incidents/test_tasks.py b/tests/sentry/incidents/test_tasks.py index a98e7b5c108bbc..ed9e2b62173fc2 100644 --- a/tests/sentry/incidents/test_tasks.py +++ b/tests/sentry/incidents/test_tasks.py @@ -33,10 +33,12 @@ ) from sentry.sentry_metrics.configuration import UseCaseKey from sentry.sentry_metrics.utils import resolve_tag_key, resolve_tag_value +from sentry.services.hybrid_cloud.user import user_service from sentry.snuba.dataset import Dataset from sentry.snuba.models import SnubaQuery from sentry.snuba.subscriptions import create_snuba_query, create_snuba_subscription from sentry.testutils import TestCase +from sentry.testutils.silo import region_silo_test from sentry.utils.http import absolute_uri pytestmark = pytest.mark.sentry_metrics @@ -48,6 +50,7 @@ def incident(self): return self.create_incident(title="hello") +@region_silo_test(stable=True) class TestSendSubscriberNotifications(BaseIncidentActivityTest, TestCase): @pytest.fixture(autouse=True) def _setup_send_async_patch(self): @@ -72,10 +75,10 @@ def test_simple(self): send_subscriber_notifications(activity.id) self.send_async.assert_called_once_with([member_user.email]) assert not IncidentSubscription.objects.filter( - incident=activity.incident, user=non_member_user + incident=activity.incident, user_id=non_member_user.id ).exists() assert IncidentSubscription.objects.filter( - incident=activity.incident, user=member_user + incident=activity.incident, user_id=member_user.id ).exists() def test_invalid_types(self): @@ -86,6 +89,7 @@ def test_invalid_types(self): self.send_async.reset_mock() +@region_silo_test(stable=True) class TestGenerateIncidentActivityEmail(BaseIncidentActivityTest, TestCase): @freeze_time() def test_simple(self): @@ -100,6 +104,7 @@ def test_simple(self): assert message.context == build_activity_context(activity, recipient) +@region_silo_test(stable=True) class TestBuildActivityContext(BaseIncidentActivityTest, TestCase): def run_test( self, activity, expected_username, expected_action, expected_comment, expected_recipient @@ -134,7 +139,7 @@ def test_simple(self): recipient = self.create_user() self.run_test( activity, - expected_username=activity.user.name, + expected_username=user_service.get_user(user_id=activity.user_id).name, expected_action="left a comment", expected_comment=activity.comment, expected_recipient=recipient, @@ -144,7 +149,7 @@ def test_simple(self): activity.previous_value = str(IncidentStatus.WARNING.value) self.run_test( activity, - expected_username=activity.user.name, + expected_username=user_service.get_user(user_id=activity.user_id).name, expected_action="changed status from %s to %s" % (INCIDENT_STATUS[IncidentStatus.WARNING], INCIDENT_STATUS[IncidentStatus.CLOSED]), expected_comment=activity.comment, @@ -152,6 +157,7 @@ def test_simple(self): ) +@region_silo_test(stable=True) class HandleTriggerActionTest(TestCase): @pytest.fixture(autouse=True) def _setup_metric_patch(self): @@ -212,6 +218,7 @@ def test(self): ) +@region_silo_test(stable=True) class TestHandleSubscriptionMetricsLogger(TestCase): @cached_property def subscription(self): @@ -260,6 +267,7 @@ def test(self): ] +@region_silo_test(stable=True) class TestHandleSubscriptionMetricsLoggerV1(TestHandleSubscriptionMetricsLogger): """Repeat TestHandleSubscriptionMetricsLogger with old (v1) subscription updates. From b021324ac3985f3eb0b8e97f019e816f87332136 Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Wed, 15 Feb 2023 12:15:18 -0800 Subject: [PATCH 07/11] More test fixes. --- .../endpoints/discover_saved_queries.py | 3 +-- .../endpoints/test_discover_homepage_query.py | 14 ++++++------ .../endpoints/test_discover_saved_queries.py | 22 +++++++++++-------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/sentry/discover/endpoints/discover_saved_queries.py b/src/sentry/discover/endpoints/discover_saved_queries.py index fe0a6d3038d228..5635aba0a46fd9 100644 --- a/src/sentry/discover/endpoints/discover_saved_queries.py +++ b/src/sentry/discover/endpoints/discover_saved_queries.py @@ -32,7 +32,6 @@ def get(self, request: Request, organization) -> Response: queryset = ( DiscoverSavedQuery.objects.filter(organization=organization) - .select_related("created_by") .prefetch_related("projects") .extra(select={"lower_name": "lower(name)"}) ).exclude(is_homepage=True) @@ -133,7 +132,7 @@ def post(self, request: Request, organization) -> Response: name=data["name"], query=data["query"], version=data["version"], - created_by=request.user if request.user.is_authenticated else None, + created_by_id=request.user.id if request.user.is_authenticated else None, ) model.set_projects(data["project_ids"]) diff --git a/tests/snuba/api/endpoints/test_discover_homepage_query.py b/tests/snuba/api/endpoints/test_discover_homepage_query.py index 71f3bc1e624c3e..4f500079dd582b 100644 --- a/tests/snuba/api/endpoints/test_discover_homepage_query.py +++ b/tests/snuba/api/endpoints/test_discover_homepage_query.py @@ -27,7 +27,7 @@ def test_returns_no_response_if_no_homepage_query_for_user(self): def test_returns_serialized_saved_query_if_homepage_is_set(self): saved_query = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=self.query, is_homepage=True, @@ -41,7 +41,7 @@ def test_returns_serialized_saved_query_if_homepage_is_set(self): def test_put_updates_existing_homepage_query_to_reflect_new_data(self): saved_query = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=self.query, is_homepage=True, @@ -79,7 +79,7 @@ def test_put_creates_new_discover_saved_query_if_none_exists(self): assert response.status_code == 201, response.content new_query = DiscoverSavedQuery.objects.get( - created_by=self.user, organization=self.org, is_homepage=True + created_by_id=self.user.id, organization=self.org, is_homepage=True ) assert response.data == serialize(new_query) assert new_query.query["fields"] == homepage_query_payload["fields"] @@ -102,7 +102,7 @@ def test_put_responds_with_saved_empty_name_field(self): assert response.status_code == 201, response.content new_query = DiscoverSavedQuery.objects.get( - created_by=self.user, organization=self.org, is_homepage=True + created_by_id=self.user.id, organization=self.org, is_homepage=True ) assert new_query.name == "" assert response.data["name"] == "" @@ -123,7 +123,7 @@ def test_put_with_no_name(self): assert response.status_code == 201, response.content new_query = DiscoverSavedQuery.objects.get( - created_by=self.user, organization=self.org, is_homepage=True + created_by_id=self.user.id, organization=self.org, is_homepage=True ) assert new_query.name == "" assert response.data["name"] == "" @@ -146,7 +146,7 @@ def test_post_not_allowed(self): def test_delete_resets_saved_query(self): DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="Test query", query=self.query, is_homepage=True, @@ -156,5 +156,5 @@ def test_delete_resets_saved_query(self): assert response.status_code == 204 assert not DiscoverSavedQuery.objects.filter( - created_by=self.user, organization=self.org, is_homepage=True + created_by_id=self.user.id, organization=self.org, is_homepage=True ).exists() diff --git a/tests/snuba/api/endpoints/test_discover_saved_queries.py b/tests/snuba/api/endpoints/test_discover_saved_queries.py index bf61b319e82202..2b1bf1ac1629ba 100644 --- a/tests/snuba/api/endpoints/test_discover_saved_queries.py +++ b/tests/snuba/api/endpoints/test_discover_saved_queries.py @@ -19,7 +19,11 @@ def setUp(self): query = {"fields": ["test"], "conditions": [], "limit": 10} model = DiscoverSavedQuery.objects.create( - organization=self.org, created_by=self.user, name="Test query", query=query, version=1 + organization=self.org, + created_by_id=self.user.id, + name="Test query", + query=query, + version=1, ) model.set_projects(self.project_ids) @@ -89,7 +93,7 @@ def test_get_all_paginated(self): query = {"fields": ["test"], "conditions": [], "limit": 10} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name=f"My query {i}", query=query, version=1, @@ -111,7 +115,7 @@ def test_get_sortby(self): query = {"fields": ["message"], "query": "", "limit": 10} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="My query", query=query, version=2, @@ -142,7 +146,7 @@ def test_get_sortby_most_popular(self): query = {"fields": ["message"], "query": "", "limit": 10} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="My query", query=query, version=2, @@ -171,7 +175,7 @@ def test_get_sortby_recently_viewed(self): query = {"fields": ["message"], "query": "", "limit": 10} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="My query", query=query, version=2, @@ -206,7 +210,7 @@ def test_get_sortby_myqueries(self): query = {"fields": ["message"], "query": "", "limit": 10} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=uhoh_user, + created_by_id=uhoh_user.id, name="a query for uhoh", query=query, version=2, @@ -217,7 +221,7 @@ def test_get_sortby_myqueries(self): model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=whoops_user, + created_by_id=whoops_user.id, name="a query for whoops", query=query, version=2, @@ -239,7 +243,7 @@ def test_get_expired_query(self): } DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="My expired query", query=query, version=2, @@ -256,7 +260,7 @@ def test_get_ignores_homepage_queries(self): query = {"fields": ["test"], "conditions": [], "limit": 10} model = DiscoverSavedQuery.objects.create( organization=self.org, - created_by=self.user, + created_by_id=self.user.id, name="Homepage Test Query", query=query, version=2, From 0c9b517431641fc40c7c521b6ee608db5200ef37 Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Wed, 22 Feb 2023 11:00:20 -0800 Subject: [PATCH 08/11] Test fixes --- src/sentry/api/serializers/models/incident.py | 6 +-- src/sentry/api/serializers/models/user.py | 2 +- src/sentry/data_export/tasks.py | 20 ++------- .../endpoints/discover_homepage_query.py | 4 +- src/sentry/incidents/logic.py | 2 +- .../slack/find_channel_id_for_rule.py | 14 +++--- .../sentry/api/serializers/test_alert_rule.py | 6 ++- tests/sentry/data_export/test_tasks.py | 43 ++++++++++--------- ...st_organization_incident_activity_index.py | 2 +- ...est_organization_incident_comment_index.py | 8 ++-- .../test_organization_incident_seen.py | 2 +- tests/sentry/integrations/slack/test_tasks.py | 4 +- 12 files changed, 56 insertions(+), 57 deletions(-) diff --git a/src/sentry/api/serializers/models/incident.py b/src/sentry/api/serializers/models/incident.py index 348a11e873f5ce..29e3c48a89c779 100644 --- a/src/sentry/api/serializers/models/incident.py +++ b/src/sentry/api/serializers/models/incident.py @@ -106,9 +106,9 @@ def get_attrs(self, item_list, user, **kwargs): subscribed_incidents = set() if user.is_authenticated: subscribed_incidents = set( - IncidentSubscription.objects.filter(incident__in=item_list, user=user).values_list( - "incident_id", flat=True - ) + IncidentSubscription.objects.filter( + incident__in=item_list, user_id=user.id + ).values_list("incident_id", flat=True) ) for item in item_list: diff --git a/src/sentry/api/serializers/models/user.py b/src/sentry/api/serializers/models/user.py index 87660a44717d25..169fced8bcd9fa 100644 --- a/src/sentry/api/serializers/models/user.py +++ b/src/sentry/api/serializers/models/user.py @@ -130,7 +130,7 @@ def _get_identities( self, item_list: Sequence[User], user: User ) -> Dict[int, List[AuthIdentity]]: if not (env.request and is_active_superuser(env.request)): - item_list = [x for x in item_list if x == user] + item_list = [x for x in item_list if x.id == user.id] queryset = AuthIdentity.objects.filter( user_id__in=[i.id for i in item_list] diff --git a/src/sentry/data_export/tasks.py b/src/sentry/data_export/tasks.py index 952424478cb348..63033af6cef9c7 100644 --- a/src/sentry/data_export/tasks.py +++ b/src/sentry/data_export/tasks.py @@ -84,14 +84,8 @@ def assemble_download( return with sentry_sdk.configure_scope() as scope: - if data_export.user: - user = {} - if data_export.user.id: - user["id"] = data_export.user.id - if data_export.user.username: - user["username"] = data_export.user.username - if data_export.user.email: - user["email"] = data_export.user.email + if data_export.user_id: + user = dict(id=data_export.user_id) scope.user = user scope.set_tag("organization.slug", data_export.organization.slug) scope.set_tag("export.type", ExportQueryType.as_str(data_export.query_type)) @@ -313,14 +307,8 @@ def merge_export_blobs(data_export_id, **kwargs): return with sentry_sdk.configure_scope() as scope: - if data_export.user: - user = {} - if data_export.user.id: - user["id"] = data_export.user.id - if data_export.user.username: - user["username"] = data_export.user.username - if data_export.user.email: - user["email"] = data_export.user.email + if data_export.user_id: + user = dict(id=data_export.user_id) scope.user = user scope.set_tag("organization.slug", data_export.organization.slug) scope.set_tag("export.type", ExportQueryType.as_str(data_export.query_type)) diff --git a/src/sentry/discover/endpoints/discover_homepage_query.py b/src/sentry/discover/endpoints/discover_homepage_query.py index c43d903c6996f9..74f8c4b2eba643 100644 --- a/src/sentry/discover/endpoints/discover_homepage_query.py +++ b/src/sentry/discover/endpoints/discover_homepage_query.py @@ -16,7 +16,7 @@ def get_homepage_query(organization, user): return DiscoverSavedQuery.objects.get( - organization=organization, is_homepage=True, created_by=user + organization=organization, is_homepage=True, created_by_id=user.id ) @@ -85,7 +85,7 @@ def put(self, request: Request, organization) -> Response: name="", query=data["query"], version=data["version"], - created_by=request.user, + created_by_id=request.user.id, is_homepage=True, ) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 90dd0fdba54e18..9d045095b4636b 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -413,7 +413,7 @@ def get_incident_subscribers(incident): def get_incident_activity(incident): - return IncidentActivity.objects.filter(incident=incident).select_related("user", "incident") + return IncidentActivity.objects.filter(incident=incident).select_related("incident") class AlertRuleNameAlreadyUsedError(Exception): diff --git a/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py b/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py index 1d0546372450f4..ee76753e6e7200 100644 --- a/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py +++ b/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py @@ -9,7 +9,8 @@ strip_channel_name, ) from sentry.mediators import project_rules -from sentry.models import Integration, Project, Rule, RuleActivity, RuleActivityType, User +from sentry.models import Project, Rule, RuleActivity, RuleActivityType, User +from sentry.services.hybrid_cloud.integration import RpcIntegration, integration_service from sentry.shared_integrations.exceptions import ApiRateLimitedError, DuplicateDisplayNameError from sentry.tasks.base import instrumented_task @@ -55,13 +56,14 @@ def find_channel_id_for_rule( channel_name = strip_channel_name(action["channel"]) break - try: - integration = Integration.objects.get( - provider="slack", organizations=organization, id=integration_id - ) - except Integration.DoesNotExist: + integration: RpcIntegration + integrations = integration_service.get_integrations( + organization_id=organization.id, providers=["slack"], integration_ids=[integration_id] + ) + if not integrations: redis_rule_status.set_value("failed") return + integration = integrations[0] # We do not know exactly how long it will take to paginate through all of the Slack # endpoints but need some time limit imposed. 3 minutes should be more than enough time, diff --git a/tests/sentry/api/serializers/test_alert_rule.py b/tests/sentry/api/serializers/test_alert_rule.py index 07ac2ed0493409..2bba3c2a28b896 100644 --- a/tests/sentry/api/serializers/test_alert_rule.py +++ b/tests/sentry/api/serializers/test_alert_rule.py @@ -8,6 +8,7 @@ from sentry.models import Rule from sentry.snuba.models import SnubaQueryEventType from sentry.testutils import APITestCase, TestCase +from sentry.testutils.silo import region_silo_test NOT_SET = object() @@ -99,6 +100,7 @@ def create_issue_alert_rule(self, data): return rule +@region_silo_test(stable=True) class AlertRuleSerializerTest(BaseAlertRuleSerializerTest, TestCase): def test_simple(self): alert_rule = self.create_alert_rule() @@ -130,7 +132,7 @@ def test_created_by(self): alert_rule = self.create_alert_rule(environment=self.environment, user=user) result = serialize(alert_rule) self.assert_alert_rule_serialized(alert_rule, result) - assert alert_rule.created_by == user + assert alert_rule.created_by.id == user.id def test_owner(self): user = self.create_user("foo@example.com") @@ -154,6 +156,7 @@ def test_comparison_delta_below(self): self.assert_alert_rule_serialized(alert_rule, result, resolve_threshold=10) +@region_silo_test(stable=True) class DetailedAlertRuleSerializerTest(BaseAlertRuleSerializerTest, TestCase): def test_simple(self): projects = [self.project, self.create_project()] @@ -192,6 +195,7 @@ def test_triggers(self): assert result[1]["triggers"] == [] +@region_silo_test(stable=True) class CombinedRuleSerializerTest(BaseAlertRuleSerializerTest, APITestCase, TestCase): def test_combined_serializer(self): projects = [self.project, self.create_project()] diff --git a/tests/sentry/data_export/test_tasks.py b/tests/sentry/data_export/test_tasks.py index cb794bc46452f5..2446ee64660971 100644 --- a/tests/sentry/data_export/test_tasks.py +++ b/tests/sentry/data_export/test_tasks.py @@ -10,6 +10,7 @@ from sentry.search.events.constants import TIMEOUT_ERROR_MESSAGE from sentry.testutils import SnubaTestCase, TestCase from sentry.testutils.helpers.datetime import before_now, iso_format +from sentry.testutils.silo import region_silo_test from sentry.utils.samples import load_data from sentry.utils.snuba import ( DatasetSelectionError, @@ -28,6 +29,7 @@ ) +@region_silo_test(stable=True) class AssembleDownloadTest(TestCase, SnubaTestCase): def setUp(self): super().setUp() @@ -68,7 +70,7 @@ def test_task_persistent_name(self): @patch("sentry.data_export.models.ExportedData.email_success") def test_issue_by_tag_batched(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.ISSUES_BY_TAG, query_info={"project": [self.project.id], "group": self.event.group_id, "key": "foo"}, @@ -98,7 +100,7 @@ def test_issue_by_tag_batched(self, emailer): @patch("sentry.data_export.models.ExportedData.email_success") def test_no_error_on_retry(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.ISSUES_BY_TAG, query_info={"project": [self.project.id], "group": self.event.group_id, "key": "foo"}, @@ -131,7 +133,7 @@ def test_no_error_on_retry(self, emailer): @patch("sentry.data_export.models.ExportedData.email_failure") def test_issue_by_tag_missing_key(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.ISSUES_BY_TAG, query_info={"project": [self.project.id], "group": self.event.group_id, "key": "bar"}, @@ -144,7 +146,7 @@ def test_issue_by_tag_missing_key(self, emailer): @patch("sentry.data_export.models.ExportedData.email_failure") def test_issue_by_tag_missing_project(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.ISSUES_BY_TAG, query_info={"project": [-1], "group": self.event.group_id, "key": "user"}, @@ -157,7 +159,7 @@ def test_issue_by_tag_missing_project(self, emailer): @patch("sentry.data_export.models.ExportedData.email_failure") def test_issue_by_tag_missing_issue(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.ISSUES_BY_TAG, query_info={"project": [self.project.id], "group": -1, "key": "user"}, @@ -176,7 +178,7 @@ def test_issue_by_tag_outside_retention(self, emailer, mock_query, mock_get_tag_ This gives us an empty CSV with just the headers. """ de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.ISSUES_BY_TAG, query_info={"project": [self.project.id], "group": self.event.group_id, "key": "foo"}, @@ -202,7 +204,7 @@ def test_issue_by_tag_outside_retention(self, emailer, mock_query, mock_get_tag_ @patch("sentry.data_export.models.ExportedData.email_success") def test_discover_batched(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -232,7 +234,7 @@ def test_discover_batched(self, emailer): @patch("sentry.data_export.models.ExportedData.email_success") def test_discover_respects_selected_environment(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={ @@ -266,7 +268,7 @@ def test_discover_respects_selected_environment(self, emailer): @patch("sentry.data_export.models.ExportedData.email_success") def test_discover_respects_selected_environment_multiple(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={ @@ -301,7 +303,7 @@ def test_discover_respects_selected_environment_multiple(self, emailer): @patch("sentry.data_export.models.ExportedData.email_failure") def test_discover_missing_environment(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={ @@ -319,7 +321,7 @@ def test_discover_missing_environment(self, emailer): @patch("sentry.data_export.models.ExportedData.email_failure") def test_discover_missing_project(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [-1], "group": self.event.group_id, "key": "user"}, @@ -334,7 +336,7 @@ def test_discover_missing_project(self, emailer): @patch("sentry.data_export.models.ExportedData.email_success") def test_discover_export_file_too_large(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -364,7 +366,7 @@ def test_discover_export_file_too_large(self, emailer): @patch("sentry.data_export.models.ExportedData.email_success") def test_discover_export_too_many_rows(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -399,7 +401,7 @@ def test_discover_outside_retention(self, emailer, mock_query): use a more recent date range. """ de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -422,7 +424,7 @@ def test_discover_outside_retention(self, emailer, mock_query): @patch("sentry.data_export.models.ExportedData.email_failure") def test_discover_invalid_search_query(self, emailer, mock_query): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -444,7 +446,7 @@ def test_discover_invalid_search_query(self, emailer, mock_query): @patch("sentry.search.events.builder.discover.raw_snql_query") def test_retries_on_recoverable_snuba_errors(self, mock_query): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -474,7 +476,7 @@ def test_retries_on_recoverable_snuba_errors(self, mock_query): @patch("sentry.data_export.models.ExportedData.email_failure") def test_discover_snuba_error(self, emailer, mock_query): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -570,7 +572,7 @@ def test_discover_snuba_error(self, emailer, mock_query): @patch("sentry.data_export.models.ExportedData.email_failure") def test_discover_integrity_error(self, emailer, finalize_upload): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, @@ -584,7 +586,7 @@ def test_discover_integrity_error(self, emailer, finalize_upload): @patch("sentry.data_export.models.ExportedData.email_success") def test_discover_sort(self, emailer): de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={ @@ -609,6 +611,7 @@ def test_discover_sort(self, emailer): assert emailer.called +@region_silo_test(stable=True) class AssembleDownloadLargeTest(TestCase, SnubaTestCase): def setUp(self): super().setUp() @@ -638,7 +641,7 @@ def test_discover_large_batch(self, emailer): during the 3rd batch, it will finish exporting all 50 rows. """ de = ExportedData.objects.create( - user=self.user, + user_id=self.user.id, organization=self.org, query_type=ExportQueryType.DISCOVER, query_info={"project": [self.project.id], "field": ["title"], "query": ""}, diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_activity_index.py b/tests/sentry/incidents/endpoints/test_organization_incident_activity_index.py index fa0ad4aad0a82d..a2f8f904b5a073 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_activity_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_activity_index.py @@ -10,7 +10,7 @@ from sentry.testutils.silo import region_silo_test -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentActivityIndexTest(APITestCase): endpoint = "sentry-api-0-organization-incident-activity" diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_comment_index.py b/tests/sentry/incidents/endpoints/test_organization_incident_comment_index.py index 1c8cb1028c289e..b300f0e33f1f56 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_comment_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_comment_index.py @@ -6,7 +6,7 @@ from sentry.testutils.silo import region_silo_test -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentCommentCreateEndpointTest(APITestCase): endpoint = "sentry-api-0-organization-incident-comments" method = "post" @@ -36,7 +36,7 @@ def test_simple(self): ) activity = IncidentActivity.objects.get(id=resp.data["id"]) assert activity.type == IncidentActivityType.COMMENT.value - assert activity.user == self.user + assert activity.user_id == self.user.id assert activity.comment == comment assert resp.data == serialize([activity], self.user)[0] @@ -61,11 +61,11 @@ def test_mentions(self): ) activity = IncidentActivity.objects.get(id=resp.data["id"]) assert activity.type == IncidentActivityType.COMMENT.value - assert activity.user == self.user + assert activity.user_id == self.user.id assert activity.comment == comment assert resp.data == serialize([activity], self.user)[0] assert IncidentSubscription.objects.filter( - user=mentioned_member, incident=incident + user_id=mentioned_member.id, incident=incident ).exists() def test_access(self): diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_seen.py b/tests/sentry/incidents/endpoints/test_organization_incident_seen.py index bea2d7f182f5fc..8450664000faa7 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_seen.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_seen.py @@ -7,7 +7,7 @@ from sentry.testutils.silo import region_silo_test -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentSeenTest(APITestCase): method = "post" endpoint = "sentry-api-0-organization-incident-seen" diff --git a/tests/sentry/integrations/slack/test_tasks.py b/tests/sentry/integrations/slack/test_tasks.py index 26bfbcaf3138bd..7e7b83c1617602 100644 --- a/tests/sentry/integrations/slack/test_tasks.py +++ b/tests/sentry/integrations/slack/test_tasks.py @@ -17,9 +17,11 @@ ) from sentry.testutils.cases import TestCase from sentry.testutils.helpers import install_slack +from sentry.testutils.silo import region_silo_test from sentry.utils import json +@region_silo_test(stable=True) class SlackTasksTest(TestCase): def setUp(self): self.integration = install_slack(self.organization) @@ -250,7 +252,7 @@ def test_task_new_alert_rule(self, mock_get_channel_id, mock_set_value): find_channel_id_for_alert_rule(**data) rule = AlertRule.objects.get(name="New Rule") - assert rule.created_by == self.user + assert rule.created_by.id == self.user.id mock_set_value.assert_called_with("success", rule.id) mock_get_channel_id.assert_called_with( integration_service._serialize_integration(self.integration), "my-channel", 180 From 12d1c320e76cbc06a62094eab583ab9e67d335bd Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Wed, 22 Feb 2023 16:41:40 -0800 Subject: [PATCH 09/11] More fixes --- src/sentry/api/helpers/group_index/update.py | 30 ++++++++++--- .../api/serializers/models/exporteddata.py | 32 ++++++++------ src/sentry/api/serializers/models/group.py | 34 +++++++-------- .../data_export/endpoints/data_export.py | 6 +-- .../organization_incident_comment_details.py | 2 +- src/sentry/receivers/sentry_apps.py | 5 ++- src/sentry/testutils/factories.py | 4 +- .../api/serializers/test_auditlogentry.py | 6 +-- .../data_export/endpoints/test_data_export.py | 2 + ...t_organization_incident_comment_details.py | 23 +++++----- tests/sentry/receivers/test_sentry_apps.py | 42 ++++++++++++------- 11 files changed, 115 insertions(+), 71 deletions(-) diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index 58701e69228dec..bf92ec0c96eb84 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/src/sentry/api/helpers/group_index/update.py @@ -49,7 +49,7 @@ from sentry.models.grouphistory import record_group_history_from_activity_type from sentry.models.groupinbox import GroupInboxRemoveAction, add_group_to_inbox from sentry.notifications.types import SUBSCRIPTION_REASON_MAP, GroupSubscriptionReason -from sentry.services.hybrid_cloud.user import RpcUser +from sentry.services.hybrid_cloud.user import RpcUser, user_service from sentry.signals import ( issue_ignored, issue_mark_reviewed, @@ -61,7 +61,6 @@ from sentry.tasks.merge import merge_groups from sentry.types.activity import ActivityType from sentry.utils import metrics -from sentry.utils.functional import extract_lazy_object from . import ACTIVITIES_COUNT, BULK_MUTATION_LIMIT, SearchFunction, delete_group_list from .validators import GroupValidator, ValidationError @@ -280,10 +279,15 @@ def update_groups( # no version yet "version": "" } + + serialized_user = user_service.serialize_many( + filter=dict(user_ids=[user.id]), as_user=user + ) status_details = { "inNextRelease": True, - "actor": serialize(extract_lazy_object(user), user), } + if serialized_user: + status_details["actor"] = serialized_user[0] res_type = GroupResolution.Type.in_next_release res_type_str = "in_next_release" res_status = GroupResolution.Status.pending @@ -301,10 +305,15 @@ def update_groups( # no version yet "version": release.version } + + serialized_user = user_service.serialize_many( + filter=dict(user_ids=[user.id]), as_user=user + ) status_details = { "inRelease": release.version, - "actor": serialize(extract_lazy_object(user), user), } + if serialized_user: + status_details["actor"] = serialized_user[0] res_type = GroupResolution.Type.in_release res_type_str = "in_release" res_status = GroupResolution.Status.resolved @@ -318,10 +327,15 @@ def update_groups( commit = statusDetails["inCommit"] activity_type = ActivityType.SET_RESOLVED_IN_COMMIT.value activity_data = {"commit": commit.id} + serialized_user = user_service.serialize_many( + filter=dict(user_ids=[user.id]), as_user=user + ) + status_details = { "inCommit": serialize(commit, user), - "actor": serialize(extract_lazy_object(user), user), } + if serialized_user: + status_details["actor"] = serialized_user[0] res_type_str = "in_commit" else: res_type_str = "now" @@ -572,14 +586,18 @@ def update_groups( "actor_id": user.id if user.is_authenticated else None, }, ) + serialized_user = user_service.serialize_many( + filter=dict(user_ids=[user.id]), as_user=user + ) result["statusDetails"] = { "ignoreCount": ignore_count, "ignoreUntil": ignore_until, "ignoreUserCount": ignore_user_count, "ignoreUserWindow": ignore_user_window, "ignoreWindow": ignore_window, - "actor": serialize(extract_lazy_object(user), user), } + if serialized_user: + result["statusDetails"]["actor"] = serialized_user[0] else: GroupSnooze.objects.filter(group__in=group_ids).delete() ignore_until = None diff --git a/src/sentry/api/serializers/models/exporteddata.py b/src/sentry/api/serializers/models/exporteddata.py index b2652d0dbd22ed..b756661b67f3e7 100644 --- a/src/sentry/api/serializers/models/exporteddata.py +++ b/src/sentry/api/serializers/models/exporteddata.py @@ -1,25 +1,31 @@ -from sentry.api.serializers import Serializer, register, serialize +from sentry.api.serializers import Serializer, register from sentry.data_export.base import ExportQueryType from sentry.data_export.models import ExportedData -from sentry.models import User +from sentry.services.hybrid_cloud.user import user_service @register(ExportedData) class ExportedDataSerializer(Serializer): def get_attrs(self, item_list, user, **kwargs): attrs = {} - users = User.objects.filter(id__in={item.user_id for item in item_list}) - user_lookup = {user.id: user for user in users} + serialized_users = { + u["id"]: u + for u in user_service.serialize_many( + filter=dict(user_ids=[item.user_id for item in item_list]) + ) + } for item in item_list: - user = user_lookup[item.user_id] - serialized_user = serialize(user) - attrs[item] = { - "user": { - "id": serialized_user["id"], - "email": serialized_user["email"], - "username": serialized_user["username"], + if str(item.user_id) in serialized_users: + serialized_user = serialized_users[str(item.user_id)] + attrs[item] = { + "user": { + "id": serialized_user["id"], + "email": serialized_user["email"], + "username": serialized_user["username"], + } } - } + else: + attrs[item] = {} return attrs def serialize(self, obj, attrs, user, **kwargs): @@ -33,7 +39,7 @@ def serialize(self, obj, attrs, user, **kwargs): return { "id": obj.id, - "user": attrs["user"], + "user": attrs.get("user"), "dateCreated": obj.date_added, "dateFinished": obj.date_finished, "dateExpired": obj.date_expired, diff --git a/src/sentry/api/serializers/models/group.py b/src/sentry/api/serializers/models/group.py index cba720b1cde20f..772bbd439b40fc 100644 --- a/src/sentry/api/serializers/models/group.py +++ b/src/sentry/api/serializers/models/group.py @@ -35,7 +35,6 @@ from sentry.constants import LOG_LEVELS from sentry.issues.grouptype import GroupCategory from sentry.models import ( - ActorTuple, Commit, Environment, Group, @@ -183,19 +182,24 @@ def __init__( self.collapse = collapse self.expand = expand - def _serialize_assigness( - self, actor_dict: Mapping[int, ActorTuple] - ) -> Mapping[int, Union[Team, Any]]: - actors_by_type: MutableMapping[Any, List[ActorTuple]] = defaultdict(list) - for actor in actor_dict.values(): - actors_by_type[actor.type].append(actor) + def _serialize_assignees(self, item_list: Sequence[Group]) -> Mapping[int, Union[Team, Any]]: + gas = GroupAssignee.objects.filter(group__in=item_list) + result: MutableMapping[int, Union[Team, Any]] = {} + all_team_ids: MutableMapping[int, int] = {} + all_user_ids: MutableMapping[int, int] = {} - resolved_actors = {} - for t, actors in actors_by_type.items(): - serializable = ActorTuple.resolve_many(actors) - resolved_actors[t] = {actor.id: actor for actor in serializable} + for g in gas: + if g.team_id: + all_team_ids[g.team_id] = g.group_id + if g.user_id: + all_user_ids[g.user_id] = g.group_id - return {key: resolved_actors[value.type][value.id] for key, value in actor_dict.items()} + for team in Team.objects.filter(id__in=all_team_ids.keys()): + result[all_team_ids[team.id]] = team + for user in user_service.serialize_many(filter=dict(user_ids=list(all_user_ids.keys()))): + result[all_user_ids[user["id"]]] = user + + return result def get_attrs( self, item_list: Sequence[Group], user: Any, **kwargs: Any @@ -223,11 +227,7 @@ def get_attrs( seen_groups = {} subscriptions = defaultdict(lambda: (False, False, None)) - assignees: Mapping[int, ActorTuple] = { - a.group_id: a.assigned_actor() - for a in GroupAssignee.objects.filter(group__in=item_list) - } - resolved_assignees = self._serialize_assigness(assignees) + resolved_assignees = self._serialize_assignees(item_list) ignore_items = {g.group_id: g for g in GroupSnooze.objects.filter(group__in=item_list)} diff --git a/src/sentry/data_export/endpoints/data_export.py b/src/sentry/data_export/endpoints/data_export.py index f7f98e2d6c1efb..6b79babc08b47f 100644 --- a/src/sentry/data_export/endpoints/data_export.py +++ b/src/sentry/data_export/endpoints/data_export.py @@ -5,7 +5,7 @@ from rest_framework.response import Response from sentry import features -from sentry.api.base import EnvironmentMixin, pending_silo_endpoint +from sentry.api.base import EnvironmentMixin, region_silo_endpoint from sentry.api.bases.organization import OrganizationDataExportPermission, OrganizationEndpoint from sentry.api.serializers import serialize from sentry.api.utils import InvalidParams, get_date_range_from_params @@ -106,7 +106,7 @@ def validate(self, data): return data -@pending_silo_endpoint +@region_silo_endpoint class DataExportEndpoint(OrganizationEndpoint, EnvironmentMixin): permission_classes = (OrganizationDataExportPermission,) @@ -148,7 +148,7 @@ def post(self, request: Request, organization) -> Response: query_type = ExportQueryType.from_str(data["query_type"]) data_export, created = ExportedData.objects.get_or_create( organization=organization, - user=request.user, + user_id=request.user.id, query_type=query_type, query_info=data["query_info"], date_finished=None, diff --git a/src/sentry/incidents/endpoints/organization_incident_comment_details.py b/src/sentry/incidents/endpoints/organization_incident_comment_details.py index c1e2da70c25041..a31bb8e16f28da 100644 --- a/src/sentry/incidents/endpoints/organization_incident_comment_details.py +++ b/src/sentry/incidents/endpoints/organization_incident_comment_details.py @@ -29,7 +29,7 @@ def convert_args(self, request: Request, activity_id, *args, **kwargs): try: # Superusers may mutate any comment - user_filter = {} if request.user.is_superuser else {"user": request.user} + user_filter = {} if request.user.is_superuser else {"user_id": request.user.id} kwargs["activity"] = IncidentActivity.objects.get( id=activity_id, diff --git a/src/sentry/receivers/sentry_apps.py b/src/sentry/receivers/sentry_apps.py index 6e3585273ec9d2..21b9f3577f9f2d 100644 --- a/src/sentry/receivers/sentry_apps.py +++ b/src/sentry/receivers/sentry_apps.py @@ -3,7 +3,6 @@ from typing import Any, List, Mapping from sentry import features -from sentry.api.serializers import serialize from sentry.api.serializers.models.user import UserSerializer from sentry.models import Group, GroupAssignee, Organization, SentryFunction, Team, User from sentry.services.hybrid_cloud.app import RpcSentryAppInstallation, app_service @@ -83,7 +82,9 @@ def send_comment_webhooks(organization, issue, user, event, data=None): ) if features.has("organizations:sentry-functions", organization, actor=user): if user: - data["user"] = serialize(user, serializer=UserSerializer()) + serialized = user_service.serialize_many(filter=dict(user_ids=[user.id])) + if serialized: + data["user"] = serialized[0] for fn in SentryFunction.objects.get_sentry_functions(organization, "comment"): send_sentry_function_webhook.delay(fn.external_id, event, issue.id, data) diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 57503cd76336c9..5fc03900d6d78e 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -1146,9 +1146,9 @@ def create_incident( @staticmethod @exempt_from_silo_limits() - def create_incident_activity(incident, type, comment=None, user=None): + def create_incident_activity(incident, type, comment=None, user_id=None): return IncidentActivity.objects.create( - incident=incident, type=type, comment=comment, user=user + incident=incident, type=type, comment=comment, user_id=user_id ) @staticmethod diff --git a/tests/sentry/api/serializers/test_auditlogentry.py b/tests/sentry/api/serializers/test_auditlogentry.py index 330e38a5a5c5ef..d740f5f2b0d3c5 100644 --- a/tests/sentry/api/serializers/test_auditlogentry.py +++ b/tests/sentry/api/serializers/test_auditlogentry.py @@ -18,7 +18,7 @@ def test_simple(self): ) serializer = AuditLogEntrySerializer() - result = serialize(log, serializer) + result = serialize(log, serializer=serializer) assert result["event"] == "team.create" assert result["actor"]["username"] == self.user.username @@ -39,7 +39,7 @@ def test_scim_logname(self): ) serializer = AuditLogEntrySerializer() - result = serialize(log, serializer) + result = serialize(log, serializer=serializer) assert result["actor"]["name"] == "SCIM Internal Integration (" + uuid_prefix + ")" @@ -53,5 +53,5 @@ def test_invalid_template(self): ) serializer = AuditLogEntrySerializer() - result = serialize(log, serializer) + result = serialize(log, serializer=serializer) assert result["note"] == "" diff --git a/tests/sentry/data_export/endpoints/test_data_export.py b/tests/sentry/data_export/endpoints/test_data_export.py index 3032689981c65d..f17e8ec7983e59 100644 --- a/tests/sentry/data_export/endpoints/test_data_export.py +++ b/tests/sentry/data_export/endpoints/test_data_export.py @@ -4,9 +4,11 @@ from sentry.data_export.models import ExportedData from sentry.search.utils import parse_datetime_string from sentry.testutils import APITestCase +from sentry.testutils.silo import region_silo_test from sentry.utils.snuba import MAX_FIELDS +@region_silo_test(stable=True) class DataExportTest(APITestCase): endpoint = "sentry-api-0-organization-data-export" method = "post" diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_comment_details.py b/tests/sentry/incidents/endpoints/test_organization_incident_comment_details.py index 1c37b83d2d1839..29dd8578a53976 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_comment_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_comment_details.py @@ -2,9 +2,10 @@ from sentry.incidents.models import IncidentActivity, IncidentActivityType from sentry.testutils import APITestCase -from sentry.testutils.silo import region_silo_test +from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test +@region_silo_test(stable=True) class BaseIncidentCommentDetailsTest(APITestCase): method = "put" endpoint = "sentry-api-0-organization-incident-comment-details" @@ -14,14 +15,14 @@ def setUp(self): user=self.user, organization=self.organization, role="owner", teams=[self.team] ) self.login_as(self.user) - self.activity = self.create_incident_comment(self.incident, user=self.user) + self.activity = self.create_incident_comment(self.incident, user_id=self.user.id) self.detected_activity = self.create_incident_activity( - self.incident, user=self.user, type=IncidentActivityType.CREATED.value + self.incident, user_id=self.user.id, type=IncidentActivityType.CREATED.value ) user2 = self.create_user() self.user2_activity = self.create_incident_comment( - incident=self.incident, user=user2, comment="hello from another user" + incident=self.incident, user_id=user2.id, comment="hello from another user" ) @cached_property @@ -63,7 +64,7 @@ def test_non_comment_type(self): ) -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentCommentUpdateEndpointTest(BaseIncidentCommentDetailsTest): method = "put" @@ -79,7 +80,7 @@ def test_simple(self): ) activity = IncidentActivity.objects.get(id=self.activity.id) assert activity.type == IncidentActivityType.COMMENT.value - assert activity.user == self.user + assert activity.user_id == self.user.id assert activity.comment == comment def test_cannot_edit_others_comment(self): @@ -94,7 +95,8 @@ def test_cannot_edit_others_comment(self): def test_superuser_can_edit(self): self.user.is_superuser = True - self.user.save() + with exempt_from_silo_limits(): + self.user.save() edited_comment = "this comment has been edited" @@ -107,11 +109,11 @@ def test_superuser_can_edit(self): status_code=200, ) activity = IncidentActivity.objects.get(id=self.user2_activity.id) - assert activity.user != self.user + assert activity.user_id != self.user.id assert activity.comment == edited_comment -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentCommentDeleteEndpointTest(BaseIncidentCommentDetailsTest): method = "delete" @@ -133,7 +135,8 @@ def test_cannot_delete_others_comments(self): def test_superuser_can_delete(self): self.user.is_superuser = True - self.user.save() + with exempt_from_silo_limits(): + self.user.save() with self.feature("organizations:incidents"): self.get_success_response( diff --git a/tests/sentry/receivers/test_sentry_apps.py b/tests/sentry/receivers/test_sentry_apps.py index f71452978b916c..db5657b44a2e9d 100644 --- a/tests/sentry/receivers/test_sentry_apps.py +++ b/tests/sentry/receivers/test_sentry_apps.py @@ -10,7 +10,7 @@ from sentry.testutils.helpers import Feature from sentry.testutils.helpers.faux import faux from sentry.testutils.helpers.features import with_feature -from sentry.testutils.silo import region_silo_test +from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test # This testcase needs to be an APITestCase because all of the logic to resolve # Issues and kick off side effects are just chillin in the endpoint code -_- @@ -18,7 +18,7 @@ @patch("sentry.tasks.sentry_apps.workflow_notification.delay") -@region_silo_test +@region_silo_test(stable=True) class TestIssueWorkflowNotifications(APITestCase): def setUp(self): self.issue = self.create_group(project=self.project) @@ -154,14 +154,15 @@ def test_notify_after_issue_ignored(self, delay): def test_notify_pending_installation(self, delay): self.install.status = SentryAppInstallationStatus.PENDING - self.install.save() + with exempt_from_silo_limits(): + self.install.save() self.update_issue() assert not delay.called @patch("sentry.tasks.sentry_functions.send_sentry_function_webhook.delay") -@region_silo_test +@region_silo_test(stable=True) class TestIssueWorkflowNotificationsSentryFunctions(APITestCase): def setUp(self): super().setUp() @@ -188,7 +189,8 @@ def test_notify_after_basic_resolved(self, delay): with Feature("organizations:sentry-functions"): self.update_issue() sub_data = {"resolution_type": "now"} - sub_data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + sub_data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "issue.resolved", @@ -205,7 +207,8 @@ def test_notify_after_resolve_in_commit(self, delay): {"statusDetails": {"inCommit": {"repository": repo.name, "commit": commit.key}}} ) sub_data = {"resolution_type": "in_commit"} - sub_data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + sub_data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "issue.resolved", @@ -219,7 +222,8 @@ def test_notify_after_resolve_in_specific_release(self, delay): release = self.create_release(project=self.project) self.update_issue({"statusDetails": {"inRelease": release.version}}) sub_data = {"resolution_type": "in_release"} - sub_data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + sub_data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "issue.resolved", @@ -234,7 +238,8 @@ def test_notify_after_resolve_in_latest_release(self, delay): self.update_issue({"statusDetails": {"inRelease": "latest"}}) sub_data = {"resolution_type": "in_release"} - sub_data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + sub_data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "issue.resolved", @@ -249,7 +254,9 @@ def test_notify_after_resolve_in_next_release(self, delay): self.update_issue({"statusDetails": {"inNextRelease": True}}) sub_data = {"resolution_type": "in_next_release"} - sub_data["user"] = serialize(self.user, UserSerializer()) + + with exempt_from_silo_limits(): + sub_data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "issue.resolved", @@ -299,7 +306,8 @@ def test_notify_after_issue_ignored(self, delay): with Feature("organizations:sentry-functions"): self.update_issue({"status": "ignored"}) sub_data = {} - sub_data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + sub_data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "issue.ignored", @@ -309,7 +317,7 @@ def test_notify_after_issue_ignored(self, delay): @patch("sentry.tasks.sentry_apps.workflow_notification.delay") -@region_silo_test +@region_silo_test(stable=True) class TestIssueAssigned(APITestCase): def setUp(self): self.issue = self.create_group(project=self.project) @@ -382,6 +390,7 @@ def test_after_issue_assigned_with_enhanced_privacy(self, delay): @with_feature("organizations:sentry-functions") @patch("sentry.tasks.sentry_functions.send_sentry_function_webhook.delay") +@region_silo_test(stable=True) class TestIssueAssignedSentryFunctions(APITestCase): def setUp(self): super().setUp() @@ -440,6 +449,7 @@ def test_after_issue_assigned_with_enhanced_privacy(self, delay): @patch("sentry.tasks.sentry_apps.build_comment_webhook.delay") +@region_silo_test(stable=True) class TestComments(APITestCase): def setUp(self): self.issue = self.create_group(project=self.project) @@ -512,6 +522,7 @@ def test_comment_deleted(self, delay): @patch("sentry.tasks.sentry_functions.send_sentry_function_webhook.delay") +@region_silo_test(stable=True) class TestCommentsSentryFunctions(APITestCase): def setUp(self): super().setUp() @@ -541,7 +552,8 @@ def test_comment_created(self, delay): "comment": "hello world", "project_slug": self.project.slug, } - data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "comment.created", @@ -562,7 +574,8 @@ def test_comment_updated(self, delay): "comment": "goodbye cruel world", "project_slug": self.project.slug, } - data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "comment.updated", @@ -581,7 +594,8 @@ def test_comment_deleted(self, delay): "comment": "hello world", "project_slug": self.project.slug, } - data["user"] = serialize(self.user, UserSerializer()) + with exempt_from_silo_limits(): + data["user"] = serialize(self.user) assert faux(delay).called_with( self.sentryFunction.external_id, "comment.deleted", From ddf085ac5091cc8426d7f8d5c1a9a5a8a4238bba Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Thu, 23 Feb 2023 08:49:54 -0800 Subject: [PATCH 10/11] Things --- src/sentry/api/serializers/models/group.py | 4 +-- src/sentry/search/snuba/backend.py | 28 +++++++++---------- src/sentry/testutils/factories.py | 4 ++- .../test_organization_incident_details.py | 11 ++++---- .../snuba/api/endpoints/test_group_details.py | 2 +- .../test_organization_group_index.py | 7 +++-- 6 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/sentry/api/serializers/models/group.py b/src/sentry/api/serializers/models/group.py index 772bbd439b40fc..77e3de67f8ffa6 100644 --- a/src/sentry/api/serializers/models/group.py +++ b/src/sentry/api/serializers/models/group.py @@ -196,8 +196,8 @@ def _serialize_assignees(self, item_list: Sequence[Group]) -> Mapping[int, Union for team in Team.objects.filter(id__in=all_team_ids.keys()): result[all_team_ids[team.id]] = team - for user in user_service.serialize_many(filter=dict(user_ids=list(all_user_ids.keys()))): - result[all_user_ids[user["id"]]] = user + for user in user_service.get_many(filter=dict(user_ids=list(all_user_ids.keys()))): + result[all_user_ids[user.id]] = user return result diff --git a/src/sentry/search/snuba/backend.py b/src/sentry/search/snuba/backend.py index d644c3d3fd1122..1706473752f372 100644 --- a/src/sentry/search/snuba/backend.py +++ b/src/sentry/search/snuba/backend.py @@ -53,23 +53,23 @@ def assigned_to_filter( for actor in actors: if actor is None: include_none = True - types_to_actors[type(actor) if not isinstance(actor, SimpleLazyObject) else User].append( - actor - ) + types_to_actors[ + (actor and actor.class_name()) if not isinstance(actor, SimpleLazyObject) else "User" + ].append(actor) query = Q() - if Team in types_to_actors: + if "Team" in types_to_actors: query |= Q( **{ f"{field_filter}__in": GroupAssignee.objects.filter( - team__in=types_to_actors[Team], project_id__in=[p.id for p in projects] + team__in=types_to_actors["Team"], project_id__in=[p.id for p in projects] ).values_list("group_id", flat=True) } ) - if User in types_to_actors: - users = types_to_actors[User] + if "User" in types_to_actors: + users = types_to_actors["User"] user_ids: List[int] = [u.id for u in users if u is not None] query |= Q( **{ @@ -200,14 +200,14 @@ def assigned_or_suggested_filter( for owner in owners: if owner is None: include_none = True - types_to_owners[type(owner) if not isinstance(owner, SimpleLazyObject) else User].append( - owner - ) + types_to_owners[ + (owner and owner.class_name()) if not isinstance(owner, SimpleLazyObject) else "User" + ].append(owner) query = Q() - if Team in types_to_owners: - teams = types_to_owners[Team] + if "Team" in types_to_owners: + teams = types_to_owners["Team"] query |= Q( **{ f"{field_filter}__in": GroupOwner.objects.filter( @@ -221,8 +221,8 @@ def assigned_or_suggested_filter( } ) | assigned_to_filter(teams, projects, field_filter=field_filter) - if User in types_to_owners: - users = types_to_owners[User] + if "User" in types_to_owners: + users = types_to_owners["User"] user_ids: List[int] = [u.id for u in users if u is not None] team_ids = list( Team.objects.filter( diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 5fc03900d6d78e..adc972a2eb7381 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -1141,7 +1141,9 @@ def create_incident( IncidentProject.objects.create(incident=incident, project=project) if seen_by: for user in seen_by: - IncidentSeen.objects.create(incident=incident, user=user, last_seen=timezone.now()) + IncidentSeen.objects.create( + incident=incident, user_id=user.id, last_seen=timezone.now() + ) return incident @staticmethod diff --git a/tests/sentry/incidents/endpoints/test_organization_incident_details.py b/tests/sentry/incidents/endpoints/test_organization_incident_details.py index 7cab8a493408a2..db1baa05281316 100644 --- a/tests/sentry/incidents/endpoints/test_organization_incident_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_incident_details.py @@ -5,7 +5,7 @@ from sentry.api.serializers import serialize from sentry.incidents.models import Incident, IncidentActivity, IncidentStatus from sentry.testutils import APITestCase -from sentry.testutils.silo import region_silo_test +from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test class BaseIncidentDetailsTest: @@ -40,7 +40,7 @@ def test_no_feature(self): assert resp.status_code == 404 -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentDetailsTest(BaseIncidentDetailsTest, APITestCase): @freeze_time() def test_simple(self): @@ -50,7 +50,8 @@ def test_simple(self): expected = serialize(incident) - user_data = serialize(self.user) + with exempt_from_silo_limits(): + user_data = serialize(self.user) seen_by = [user_data] assert resp.data["id"] == expected["id"] @@ -62,7 +63,7 @@ def test_simple(self): assert [item["id"] for item in resp.data["seenBy"]] == [item["id"] for item in seen_by] -@region_silo_test +@region_silo_test(stable=True) class OrganizationIncidentUpdateStatusTest(BaseIncidentDetailsTest, APITestCase): method = "put" @@ -103,7 +104,7 @@ def test_comment(self): activity = IncidentActivity.objects.filter(incident=incident).order_by("-id")[:1].get() assert activity.value == str(status) assert activity.comment == comment - assert activity.user == self.user + assert activity.user_id == self.user.id def test_invalid_status(self): incident = self.create_incident() diff --git a/tests/snuba/api/endpoints/test_group_details.py b/tests/snuba/api/endpoints/test_group_details.py index b24120c954ee2b..92e0c44c3576df 100644 --- a/tests/snuba/api/endpoints/test_group_details.py +++ b/tests/snuba/api/endpoints/test_group_details.py @@ -16,7 +16,7 @@ from sentry.testutils.silo import region_silo_test -@region_silo_test +@region_silo_test(stable=True) class GroupDetailsTest(APITestCase, SnubaTestCase): def test_multiple_environments(self): group = self.create_group() diff --git a/tests/snuba/api/endpoints/test_organization_group_index.py b/tests/snuba/api/endpoints/test_organization_group_index.py index 71f7ad5c35be9b..abf016f4d6c8db 100644 --- a/tests/snuba/api/endpoints/test_organization_group_index.py +++ b/tests/snuba/api/endpoints/test_organization_group_index.py @@ -51,12 +51,12 @@ from sentry.testutils import APITestCase, SnubaTestCase from sentry.testutils.helpers import parse_link_header from sentry.testutils.helpers.datetime import before_now, iso_format -from sentry.testutils.silo import region_silo_test +from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test from sentry.types.activity import ActivityType from sentry.utils import json -@region_silo_test +@region_silo_test(stable=True) class GroupListTest(APITestCase, SnubaTestCase): endpoint = "sentry-api-0-organization-group-index" @@ -744,7 +744,8 @@ def test_filters_based_on_retention(self): assert len(response.data) == 0 def test_token_auth(self): - token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) + with exempt_from_silo_limits(): + token = ApiToken.objects.create(user=self.user, scope_list=["event:read"]) response = self.client.get( reverse("sentry-api-0-organization-group-index", args=[self.project.organization.slug]), format="json", From b9fcf6e5751f6eea38f35700fae0f9b93acc43cf Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Fri, 24 Feb 2023 11:57:36 -0800 Subject: [PATCH 11/11] Fix --- .../slack/find_channel_id_for_rule.py | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py b/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py index ee76753e6e7200..54c6a332cec8bb 100644 --- a/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py +++ b/src/sentry/tasks/integrations/slack/find_channel_id_for_rule.py @@ -1,6 +1,8 @@ import logging from typing import Any, Optional, Sequence +from django.db import IntegrityError + from sentry.incidents.models import AlertRuleTriggerAction from sentry.integrations.slack.utils import ( SLACK_RATE_LIMITED_MESSAGE, @@ -9,7 +11,7 @@ strip_channel_name, ) from sentry.mediators import project_rules -from sentry.models import Project, Rule, RuleActivity, RuleActivityType, User +from sentry.models import Project, Rule, RuleActivity, RuleActivityType from sentry.services.hybrid_cloud.integration import RpcIntegration, integration_service from sentry.shared_integrations.exceptions import ApiRateLimitedError, DuplicateDisplayNameError from sentry.tasks.base import instrumented_task @@ -37,13 +39,6 @@ def find_channel_id_for_rule( redis_rule_status.set_value("failed") return - user = None - if user_id: - try: - user = User.objects.get(id=user_id) - except User.DoesNotExist: - pass - organization = project.organization integration_id: Optional[int] = None channel_name: Optional[str] = None @@ -98,10 +93,13 @@ def find_channel_id_for_rule( rule = project_rules.Updater.run(rule=rule, pending_save=False, **kwargs) else: rule = project_rules.Creator.run(pending_save=False, **kwargs) - if user: - RuleActivity.objects.create( - rule=rule, user=user, type=RuleActivityType.CREATED.value - ) + if user_id: + try: + RuleActivity.objects.create( + rule=rule, user_id=user_id, type=RuleActivityType.CREATED.value + ) + except IntegrityError: + pass redis_rule_status.set_value("success", rule.id) return