Revert "feat(hybridcloud) Move reply by email to outboxes (#58107)"

This reverts commit 14514f7.

Co-authored-by: markstory <24086+markstory@users.noreply.github.com>

src/sentry/models/activity.py

@@ -29,7 +29,7 @@
     from sentry.services.hybrid_cloud.user import RpcUser
 
 
-class ActivityManager(BaseManager["Activity"]):
+class ActivityManager(BaseManager):
     def get_activities_for_group(self, group: Group, num: int) -> Sequence[Group]:
         activities = []
         activity_qs = self.filter(group=group).order_by("-datetime")
@@ -100,7 +100,7 @@ class Activity(Model):
     datetime = models.DateTimeField(default=timezone.now)
     data: models.Field[dict[str, Any], dict[str, Any]] = GzippedDictField(null=True)
 
-    objects: ActivityManager = ActivityManager()
+    objects = ActivityManager()
 
     class Meta:
         app_label = "sentry"

src/sentry/models/outbox.py

@@ -110,7 +110,6 @@ class OutboxCategory(IntEnum):
     ACTOR_UPDATE = 31
     API_TOKEN_UPDATE = 32
     ORG_AUTH_TOKEN_UPDATE = 33
-    ISSUE_COMMENT_UPDATE = 34
 
     @classmethod
     def as_choices(cls):
@@ -306,7 +305,6 @@ class OutboxScope(IntEnum):
             OutboxCategory.ORG_AUTH_TOKEN_UPDATE,
             OutboxCategory.PARTNER_ACCOUNT_UPDATE,
             OutboxCategory.ACTOR_UPDATE,
-            OutboxCategory.ISSUE_COMMENT_UPDATE,
         },
     )
     USER_SCOPE = scope_categories(

src/sentry/receivers/outbox/control.py

@@ -17,7 +17,6 @@
 from sentry.models.integrations.sentry_app import SentryApp
 from sentry.models.outbox import OutboxCategory, process_control_outbox
 from sentry.receivers.outbox import maybe_process_tombstone
-from sentry.services.hybrid_cloud.issue import issue_service
 from sentry.services.hybrid_cloud.organization import RpcOrganizationSignal, organization_service
 from sentry.silo.base import SiloMode
 
@@ -114,13 +113,3 @@ def process_mark_invalid_sso(object_identifier: int, shard_identifier: int, **kw
     other_member.flags.sso__invalid = True
     other_member.flags.sso__linked = False
     organization_service.update_membership_flags(organization_member=other_member)
-
-
-@receiver(process_control_outbox, sender=OutboxCategory.ISSUE_COMMENT_UPDATE)
-def process_issue_email_reply(shard_identifier: int, payload: Any, **kwds):
-    issue_service.upsert_issue_email_reply(
-        organization_id=shard_identifier,
-        group_id=payload["group_id"],
-        from_email=payload["from_email"],
-        text=payload["text"],
-    )

src/sentry/services/hybrid_cloud/issue/__init__.py

@@ -1 +0,0 @@
-from .service import issue_service  # noqa

src/sentry/services/hybrid_cloud/issue/impl.py

@@ -38,12 +38,3 @@ def get_shared_for_region(
             return None
 
         return RpcGroupShareMetadata(title=group.title, message=group.message)
-
-    def upsert_issue_email_reply(
-        self, *, organization_id: int, group_id: int, from_email: str, text: str
-    ) -> None:
-        from sentry.tasks.email import process_inbound_email
-
-        # Call the task syncrhonously so that the outbox retry works
-        # correctly should this fail.
-        process_inbound_email(from_email, group_id, text)

src/sentry/services/hybrid_cloud/issue/service.py

@@ -8,7 +8,7 @@
 from typing import Optional
 
 from sentry.services.hybrid_cloud.issue.model import RpcGroupShareMetadata
-from sentry.services.hybrid_cloud.region import ByOrganizationId, ByOrganizationSlug, ByRegionName
+from sentry.services.hybrid_cloud.region import ByOrganizationSlug, ByRegionName
 from sentry.services.hybrid_cloud.rpc import RpcService, regional_rpc_method
 from sentry.silo.base import SiloMode
 
@@ -19,12 +19,9 @@ class IssueService(RpcService):
 
     We want as little access to issues and events in control as possible.
 
-    Unfortunately we have a handful of workflows that require
-    access to issues from control:
-
-    - The issue public share link view requires issue data to render the initial HTML so that open-graph
-      data can be included
-    - Replying to issue workflow notifications by email sends webhooks to control via mailgun.
+    Unfortunately the issue public share link view requires it as
+    we need issue data to render the initial HTML so that open-graph
+    data can be included.
     """
 
     key = "issue"
@@ -48,12 +45,5 @@ def get_shared_for_region(
     ) -> Optional[RpcGroupShareMetadata]:
         pass
 
-    @regional_rpc_method(resolve=ByOrganizationId(), return_none_if_mapping_not_found=True)
-    @abstractmethod
-    def upsert_issue_email_reply(
-        self, *, organization_id: int, group_id: int, from_email: str, text: str
-    ) -> None:
-        pass
-
 
 issue_service = IssueService.create_delegation()

src/sentry/tasks/email.py

@@ -1,27 +1,25 @@
 import logging
-from typing import Optional
 
 from sentry.auth import access
-from sentry.models.group import Group
-from sentry.services.hybrid_cloud.user.model import RpcUser
-from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.silo import SiloMode
 from sentry.tasks.base import instrumented_task
 from sentry.utils.email import send_messages
 
 logger = logging.getLogger(__name__)
 
 
-def _get_user_from_email(group: Group, email: str) -> Optional[RpcUser]:
-    for user in user_service.get_many_by_email(emails=[email]):
+def _get_user_from_email(group, email):
+    from sentry.models.user import User
+
+    # TODO(dcramer): we should encode the userid in emails so we can avoid this
+    for user in User.objects.filter(email__iexact=email):
         # Make sure that the user actually has access to this project
         context = access.from_user(user=user, organization=group.organization)
         if not any(context.has_team_access(t) for t in group.project.teams.all()):
             logger.warning("User %r does not have access to group %r", user, group)
             continue
 
         return user
-    return None
 
 
 @instrumented_task(
@@ -31,9 +29,8 @@ def _get_user_from_email(group: Group, email: str) -> Optional[RpcUser]:
     max_retries=None,
     silo_mode=SiloMode.REGION,
 )
-def process_inbound_email(mailfrom: str, group_id: int, payload: str):
-    # TODO(hybridcloud) Once we aren't invoking this with celery
-    # detach  this from celery and have a basic function instead.
+def process_inbound_email(mailfrom, group_id, payload):
+    """ """
     from sentry.models.group import Group
     from sentry.web.forms import NewNoteForm
 
@@ -48,9 +45,15 @@ def process_inbound_email(mailfrom: str, group_id: int, payload: str):
         logger.warning("Inbound email from unknown address: %s", mailfrom)
         return
 
+    event = group.get_latest_event()
+
+    if event:
+        event.group = group
+        event.project = group.project
+
     form = NewNoteForm({"text": payload})
     if form.is_valid():
-        form.save(group, user)
+        form.save(group, user, event=event)
 
 
 @instrumented_task(

src/sentry/utils/email/address.py

@@ -35,9 +35,7 @@ def email_to_group_id(address: str) -> tuple[int | None, int | None]:
         {group_id}.{org_id}+{signature}@example.com
 
     The form with org_id and group_id is newer
-    and required for multi-region sentry.
-
-    :return: Tuple of group_id, org_id
+    and required for multi-region sentry
     """
     address = address.split("@", 1)[0]
     signed_data = address.replace("+", ":")

src/sentry/web/forms/__init__.py

@@ -9,18 +9,7 @@ class NewNoteForm(forms.Form):
         widget=forms.Textarea(attrs={"rows": "1", "placeholder": "Type a note and press enter..."})
     )
 
-    def save(self, group, user):
-        qs = Activity.objects.filter(
-            group=group,
-            project_id=group.project_id,
-            user_id=user.id,
-            type=ActivityType.NOTE.value,
-            data=self.cleaned_data,
-        )
-        # Prevent duplicate comments, this is necessary for outbox based
-        # delivery to be idempotent
-        if qs.exists():
-            return
+    def save(self, group, user, event=None):
         return Activity.objects.create_group_activity(
             group, ActivityType.NOTE, user=user, data=self.cleaned_data
         )

src/sentry/web/frontend/mailgun_inbound_webhook.py

@@ -1,9 +1,7 @@
 import hmac
 import logging
 from hashlib import sha256
-from typing import Any
 
-from django.conf import settings
 from django.http import HttpRequest, HttpResponse
 from django.utils.crypto import constant_time_compare
 from django.utils.decorators import method_decorator
@@ -12,8 +10,7 @@
 from email_reply_parser import EmailReplyParser
 
 from sentry import options
-from sentry.models.organizationmapping import OrganizationMapping
-from sentry.models.outbox import ControlOutbox, OutboxCategory, OutboxScope
+from sentry.tasks.email import process_inbound_email
 from sentry.utils.email import email_to_group_id
 
 logger = logging.getLogger("sentry.mailgun")
@@ -66,23 +63,8 @@ def post(self, request: HttpRequest) -> HttpResponse:
             # If there's no body, we don't need to go any further
             return HttpResponse(status=200)
 
-        if org_id:
-            org_mapping = OrganizationMapping.objects.get(organization_id=org_id)
-            region_name = org_mapping.region_name
-        else:
-            region_name = settings.SENTRY_MONOLITH_REGION
-
-        # Email replies cannot be coaleseced so we
-        # need to generate unique object_identifier values.
-        outbox_payload: Any = {"from_email": from_email, "text": payload, "group_id": group_id}
-        outbox = ControlOutbox(
-            shard_scope=OutboxScope.ORGANIZATION_SCOPE,
-            shard_identifier=org_id or 0,
-            category=OutboxCategory.ISSUE_COMMENT_UPDATE,
-            object_identifier=ControlOutbox.next_object_identifier(),
-            region_name=region_name,
-            payload=outbox_payload,
-        )
-        outbox.save()
+        # TODO(hybridcloud) This needs to become an outbox message for the payload
+        # that is delivered to the correct region/org
+        process_inbound_email.delay(from_email, group_id, payload)
 
         return HttpResponse(status=201)

tests/sentry/receivers/test_sentry_apps.py

@@ -512,7 +512,7 @@ def test_comment_created(self, delay):
         note = Activity.objects.get(
             group=self.issue, project=self.project, type=ActivityType.NOTE.value
         )
-        comment_data = {
+        data = {
             "comment_id": note.id,
             "timestamp": note.datetime,
             "comment": "hello world",
@@ -523,7 +523,7 @@ def test_comment_created(self, delay):
             issue_id=self.issue.id,
             type="comment.created",
             user_id=self.user.id,
-            data=comment_data,
+            data=data,
         )
 
     def test_comment_updated(self, delay):
@@ -589,19 +589,19 @@ def test_comment_created(self, delay):
             note = Activity.objects.get(
                 group=self.issue, project=self.project, type=ActivityType.NOTE.value
             )
-            comment_data = {
+            data = {
                 "comment_id": note.id,
                 "timestamp": note.datetime,
                 "comment": "hello world",
                 "project_slug": self.project.slug,
             }
             with assume_test_silo_mode(SiloMode.CONTROL):
-                comment_data["user"] = serialize(self.user)
+                data["user"] = serialize(self.user)
             delay.assert_called_once_with(
                 self.sentryFunction.external_id,
                 "comment.created",
                 self.issue.id,
-                _as_serialized(comment_data),
+                _as_serialized(data),
             )
 
     def test_comment_updated(self, delay):

tests/sentry/tasks/test_email.py

@@ -1,14 +1,12 @@
 from sentry.models.activity import Activity
 from sentry.tasks.email import process_inbound_email
 from sentry.testutils.cases import TestCase
-from sentry.testutils.silo import region_silo_test
 from sentry.testutils.skips import requires_snuba
 from sentry.types.activity import ActivityType
 
 pytestmark = [requires_snuba]
 
 
-@region_silo_test(stable=True)
 class ProcessInboundEmailTest(TestCase):
     def test_task_persistent_name(self):
         assert process_inbound_email.name == "sentry.tasks.email.process_inbound_email"