Browse Source

chore(HC): Adds organization action functions for Organization modifications (#50664)

Gabe Villalobos 1 year ago
parent
commit
43de083a4b

+ 0 - 9
pyproject.toml

@@ -642,7 +642,6 @@ module = [
     "sentry.integrations.msteams.actions.form",
     "sentry.integrations.msteams.actions.notification",
     "sentry.integrations.msteams.card_builder.base",
-    "sentry.integrations.msteams.card_builder.installation",
     "sentry.integrations.msteams.card_builder.issues",
     "sentry.integrations.msteams.card_builder.notifications",
     "sentry.integrations.msteams.client",
@@ -858,16 +857,10 @@ module = [
     "sentry.notifications.manager",
     "sentry.notifications.notifications.activity.base",
     "sentry.notifications.notifications.activity.new_processing_issues",
-    "sentry.notifications.notifications.activity.regression",
     "sentry.notifications.notifications.activity.release",
-    "sentry.notifications.notifications.activity.resolved_in_release",
     "sentry.notifications.notifications.base",
-    "sentry.notifications.notifications.codeowners_auto_sync",
     "sentry.notifications.notifications.integration_nudge",
-    "sentry.notifications.notifications.organization_request.abstract_invite_request",
-    "sentry.notifications.notifications.organization_request.integration_request",
     "sentry.notifications.notifications.rules",
-    "sentry.notifications.notifications.user_report",
     "sentry.notifications.utils",
     "sentry.notifications.utils.avatar",
     "sentry.notifications.utils.participants",
@@ -1019,7 +1012,6 @@ module = [
     "sentry.services.hybrid_cloud.organization.impl",
     "sentry.services.hybrid_cloud.organization.model",
     "sentry.services.hybrid_cloud.organization.serial",
-    "sentry.services.hybrid_cloud.organization_mapping.serial",
     "sentry.services.hybrid_cloud.organizationmember_mapping.impl",
     "sentry.services.hybrid_cloud.project_key.model",
     "sentry.services.hybrid_cloud.rpc",
@@ -1209,7 +1201,6 @@ module = [
     "sentry.web.frontend.debug.debug_mfa_added_email",
     "sentry.web.frontend.debug.debug_mfa_removed_email",
     "sentry.web.frontend.debug.debug_new_processing_issues_email",
-    "sentry.web.frontend.debug.debug_new_release_email",
     "sentry.web.frontend.debug.debug_oauth_authorize",
     "sentry.web.frontend.debug.debug_onboarding_continuation_email",
     "sentry.web.frontend.debug.debug_organization_integration_request",

+ 6 - 1
src/sentry/api/endpoints/organization_index.py

@@ -24,6 +24,9 @@ from sentry.models import (
 )
 from sentry.search.utils import tokenize_query
 from sentry.services.hybrid_cloud import IDEMPOTENCY_KEY_LENGTH
+from sentry.services.hybrid_cloud.organization_actions.impl import (
+    create_organization_with_outbox_message,
+)
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.signals import org_setup_complete, terms_accepted
 
@@ -208,7 +211,9 @@ class OrganizationIndexEndpoint(Endpoint):
             try:
 
                 with transaction.atomic():
-                    org = Organization.objects.create(name=result["name"], slug=result.get("slug"))
+                    org = create_organization_with_outbox_message(
+                        create_options={"name": result["name"], "slug": result.get("slug")}
+                    )
                     om = OrganizationMember.objects.create(
                         organization_id=org.id,
                         user_id=request.user.id,

+ 7 - 1
src/sentry/deletions/defaults/organization.py

@@ -1,4 +1,7 @@
 from sentry.models import OrganizationStatus
+from sentry.services.hybrid_cloud.organization_actions.impl import (
+    update_organization_with_outbox_message,
+)
 
 from ..base import ModelDeletionTask, ModelRelation
 
@@ -68,4 +71,7 @@ class OrganizationDeletionTask(ModelDeletionTask):
 
         for instance in instance_list:
             if instance.status != OrganizationStatus.DELETION_IN_PROGRESS:
-                instance.update(status=OrganizationStatus.DELETION_IN_PROGRESS)
+                update_organization_with_outbox_message(
+                    org_id=instance.id,
+                    update_data={"status": OrganizationStatus.DELETION_IN_PROGRESS},
+                )

+ 1 - 1
src/sentry/models/organization.py

@@ -165,7 +165,7 @@ class Organization(Model, OrganizationAbsoluteUrlMixin, SnowflakeIdMixin):
 
     __include_in_export__ = True
     name = models.CharField(max_length=64)
-    slug = models.SlugField(unique=True)
+    slug: models.SlugField[str, str] = models.SlugField(unique=True)
     status = BoundedPositiveIntegerField(
         choices=OrganizationStatus.as_choices(), default=OrganizationStatus.ACTIVE.value
     )

+ 44 - 0
src/sentry/services/hybrid_cloud/organization_actions/impl.py

@@ -0,0 +1,44 @@
+from typing import TypedDict
+
+from django.db import transaction
+from django.db.models.expressions import CombinedExpression
+
+from sentry.models import Organization, OrganizationStatus
+
+
+class OrganizationCreateAndUpdateOptions(TypedDict, total=False):
+    name: str
+    slug: str
+    status: OrganizationStatus
+    flags: CombinedExpression
+    default_role: int
+
+
+def create_organization_with_outbox_message(
+    *, create_options: OrganizationCreateAndUpdateOptions
+) -> Organization:
+    with transaction.atomic():
+        org: Organization = Organization.objects.create(**create_options)
+        Organization.outbox_for_update(org_id=org.id).save()
+    return org
+
+
+def update_organization_with_outbox_message(
+    *, org_id: int, update_data: OrganizationCreateAndUpdateOptions
+) -> Organization:
+    with transaction.atomic():
+        org: Organization = Organization.objects.get(id=org_id)
+        org.update(**update_data)
+        Organization.outbox_for_update(org_id=org.id).save()
+
+    org.refresh_from_db()
+    return org
+
+
+def upsert_organization_by_org_id_with_outbox_message(
+    *, org_id: int, upsert_data: OrganizationCreateAndUpdateOptions
+) -> Organization:
+    with transaction.atomic():
+        org, created = Organization.objects.update_or_create(id=org_id, defaults=upsert_data)
+        Organization.outbox_for_update(org_id=org_id).save()
+        return org

+ 133 - 0
tests/sentry/services/test_organization_actions.py

@@ -0,0 +1,133 @@
+import pytest
+
+from sentry.models import (
+    Organization,
+    OrganizationStatus,
+    OutboxCategory,
+    OutboxScope,
+    RegionOutbox,
+)
+from sentry.services.hybrid_cloud.organization_actions.impl import (
+    create_organization_with_outbox_message,
+    update_organization_with_outbox_message,
+    upsert_organization_by_org_id_with_outbox_message,
+)
+from sentry.testutils import TestCase
+from sentry.testutils.cases import BaseTestCase
+from sentry.testutils.outbox import outbox_runner
+from sentry.testutils.silo import region_silo_test
+
+
+def assert_outbox_update_message_exists(org: Organization, expected_count: int):
+    outbox_messages = RegionOutbox.objects.filter()
+
+    # TODO(HC): Remove this once we can ensure an expected count of 1 for every message
+    #  It's not essential since these messages will coallesce, but there's no reason we
+    #  should be queueing 2 outbox messages per create/update
+    assert outbox_messages.count() == expected_count
+    for org_update_outbox in outbox_messages:
+        assert org_update_outbox.shard_identifier == org.id
+        assert org_update_outbox.shard_scope == OutboxScope.ORGANIZATION_SCOPE
+        assert org_update_outbox.category == OutboxCategory.ORGANIZATION_UPDATE
+
+
+@region_silo_test(stable=True)
+class OrganizationUpdateTest(TestCase, BaseTestCase):
+    def setUp(self):
+        self.org: Organization = self.create_organization(slug="sluggy", name="barfoo")
+
+        with outbox_runner():
+            pass
+
+    def test_create_organization_with_outbox_message(self):
+        with outbox_runner():
+            pass
+
+        org: Organization = create_organization_with_outbox_message(
+            create_options={"slug": "santry", "name": "santry", "status": OrganizationStatus.ACTIVE}
+        )
+
+        assert org.id
+        assert org.slug == "santry"
+        assert org.name == "santry"
+        assert_outbox_update_message_exists(org=org, expected_count=2)
+
+
+@region_silo_test(stable=True)
+class OrganizationUpdateWithOutboxTest(TestCase, BaseTestCase):
+    def setUp(self):
+        self.org: Organization = self.create_organization(slug="sluggy", name="barfoo")
+
+        with outbox_runner():
+            pass
+
+    def test_update_organization_with_outbox_message(self):
+        update_organization_with_outbox_message(org_id=self.org.id, update_data={"name": "foobar"})
+
+        self.org.refresh_from_db()
+        assert self.org.name == "foobar"
+        assert self.org.slug == "sluggy"
+        assert_outbox_update_message_exists(org=self.org, expected_count=1)
+
+    def test_update_with_missing_org_id(self):
+        with pytest.raises(Organization.DoesNotExist):
+            update_organization_with_outbox_message(org_id=1234, update_data={"name": "foobar"})
+
+
+@region_silo_test(stable=True)
+class OrganizationUpsertWithOutboxTest(TestCase, BaseTestCase):
+    def setUp(self):
+        self.org: Organization = self.create_organization(slug="sluggy", name="barfoo")
+
+        with outbox_runner():
+            pass
+
+    def test_upsert_queues_outbox_message_and_updates_org(self):
+        # The test fixture creates at least 1 org so comparing count before
+        # and after the upsert is the safest way to assert we haven't created
+        # a new entry.
+        previous_org_count = Organization.objects.count()
+        org_before_modification = Organization.objects.get(id=self.org.id)
+        updated_org: Organization = upsert_organization_by_org_id_with_outbox_message(
+            org_id=self.org.id,
+            upsert_data={
+                "slug": "foobar",
+                "status": OrganizationStatus.DELETION_IN_PROGRESS,
+            },
+        )
+
+        assert Organization.objects.count() == previous_org_count
+        self.org.refresh_from_db()
+        assert updated_org.slug == self.org.slug == "foobar"
+        assert updated_org.name == self.org.name == "barfoo"
+        assert updated_org.status == self.org.status == OrganizationStatus.DELETION_IN_PROGRESS
+
+        assert (
+            updated_org.default_role
+            == self.org.default_role
+            == org_before_modification.default_role
+        )
+
+        assert_outbox_update_message_exists(org=self.org, expected_count=2)
+
+    def test_upsert_creates_organization_with_desired_id(self):
+        previous_org_count = Organization.objects.count()
+        org_before_modification = Organization.objects.get(id=self.org.id)
+        desired_org_id = 1234
+
+        created_org: Organization = upsert_organization_by_org_id_with_outbox_message(
+            org_id=desired_org_id,
+            upsert_data={"slug": "random", "name": "rando", "status": OrganizationStatus.ACTIVE},
+        )
+        assert Organization.objects.count() == previous_org_count + 1
+        db_created_org = Organization.objects.get(id=desired_org_id)
+        assert db_created_org.slug == created_org.slug == "random"
+        assert db_created_org.status == created_org.status == OrganizationStatus.ACTIVE
+        assert db_created_org.name == created_org.name == "rando"
+
+        # Probably overly cautious, but assert that previous org has not been modified
+        self.org.refresh_from_db()
+        assert org_before_modification.slug == self.org.slug
+        assert org_before_modification.name == self.org.name
+        assert org_before_modification.status == self.org.status
+        assert_outbox_update_message_exists(org=db_created_org, expected_count=2)