chore(hybrid-cloud): Allow Jira/Jira Server tests to be region by default (#63900)

This PR just tags the failing jira tests appropriately. There was quite
a bit a refactoring needed for Jira Server's tests, and we did uncover
one cross-silo query, so these tests are important 💪

---------

Co-authored-by: Ryan Skonnord <ryan.skonnord@sentry.io>

src/sentry/integrations/jira_server/integration.py

@@ -432,8 +432,9 @@ class JiraServerIntegration(IntegrationInstallation, IssueSyncMixin):
 
     def get_config_data(self):
         config = self.org_integration.config
-        project_mappings = IntegrationExternalProject.objects.filter(
-            organization_integration_id=self.org_integration.id
+        project_mappings = integration_service.get_integration_external_projects(
+            organization_id=self.org_integration.organization_id,
+            integration_id=self.org_integration.integration_id,
         )
         sync_status_forward = {}
         for pm in project_mappings:

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

@@ -48,8 +48,9 @@ class DatabaseBackedIntegrationService(IntegrationService):
     def send_message(
         self, *, integration_id: int, organization_id: int, channel: str, message: str
     ) -> bool:
-        integration = Integration.objects.filter(id=integration_id).first()
-        if integration is None:
+        try:
+            integration = Integration.objects.get(id=integration_id)
+        except Integration.DoesNotExist:
             return False
         install = integration.get_installation(organization_id=organization_id)
         if isinstance(install, NotifyBasicMixin):
@@ -341,8 +342,9 @@ class DatabaseBackedIntegrationService(IntegrationService):
     def add_organization(
         self, *, integration_id: int, org_ids: List[int]
     ) -> Optional[RpcIntegration]:
-        integration = Integration.objects.filter(id=integration_id).first()
-        if not integration:
+        try:
+            integration = Integration.objects.get(id=integration_id)
+        except Integration.DoesNotExist:
             return None
         for org_id in org_ids:
             integration.add_organization(organization_id=org_id)
@@ -421,24 +423,38 @@ class DatabaseBackedIntegrationService(IntegrationService):
         return False
 
     def delete_integration(self, *, integration_id: int) -> None:
-        integration = Integration.objects.filter(id=integration_id).first()
-        if integration is None:
+        try:
+            integration = Integration.objects.get(id=integration_id)
+        except Integration.DoesNotExist:
             return
         integration.delete()
 
     def get_integration_external_project(
         self, *, organization_id: int, integration_id: int, external_id: str
     ) -> RpcIntegrationExternalProject | None:
-        external_project = IntegrationExternalProject.objects.filter(
+        external_projects = self.get_integration_external_projects(
+            organization_id=organization_id,
+            integration_id=integration_id,
             external_id=external_id,
-            organization_integration_id__in=OrganizationIntegration.objects.filter(
+        )
+        return external_projects[0] if len(external_projects) > 0 else None
+
+    def get_integration_external_projects(
+        self, *, organization_id: int, integration_id: int, external_id: str | None = None
+    ) -> List[RpcIntegrationExternalProject]:
+        try:
+            oi = OrganizationIntegration.objects.get(
                 organization_id=organization_id,
                 integration_id=integration_id,
-            ),
-        ).first()
-        if external_project is None:
-            return None
-        return serialize_integration_external_project(external_project)
+            )
+        except OrganizationIntegration.DoesNotExist:
+            return []
+
+        iep_kwargs = {"organization_integration_id": oi.id}
+        if external_id is not None:
+            iep_kwargs["external_id"] = external_id
+        external_projects = IntegrationExternalProject.objects.filter(**iep_kwargs)
+        return [serialize_integration_external_project(iep) for iep in external_projects]
 
     def get_integration_identity_context(
         self,

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

@@ -274,6 +274,13 @@ class IntegrationService(RpcService):
     ) -> Optional[RpcIntegrationExternalProject]:
         pass
 
+    @rpc_method
+    @abstractmethod
+    def get_integration_external_projects(
+        self, *, organization_id: int, integration_id: int, external_id: str | None = None
+    ) -> List[RpcIntegrationExternalProject]:
+        pass
+
     @rpc_method
     @abstractmethod
     def get_integration_identity_context(

src/sentry/testutils/factories.py

@@ -69,6 +69,7 @@ from sentry.models.integrations.doc_integration import DocIntegration
 from sentry.models.integrations.external_actor import ExternalActor
 from sentry.models.integrations.external_issue import ExternalIssue
 from sentry.models.integrations.integration import Integration
+from sentry.models.integrations.integration_external_project import IntegrationExternalProject
 from sentry.models.integrations.integration_feature import (
     Feature,
     IntegrationFeature,
@@ -1354,6 +1355,18 @@ class Factories:
 
         return external_issue
 
+    @staticmethod
+    @assume_test_silo_mode(SiloMode.CONTROL)
+    def create_integration_external_project(
+        organization_id: int, integration_id: int, *args: Any, **kwargs: Any
+    ) -> IntegrationExternalProject:
+        oi = OrganizationIntegration.objects.get(
+            organization_id=organization_id, integration_id=integration_id
+        )
+        return IntegrationExternalProject.objects.create(
+            organization_integration_id=oi.id, *args, **kwargs
+        )
+
     @staticmethod
     @assume_test_silo_mode(SiloMode.REGION)
     def create_incident(

src/sentry/testutils/fixtures.py

@@ -345,6 +345,9 @@ class Fixtures:
     def create_integration_external_issue(self, *args, **kwargs):
         return Factories.create_integration_external_issue(*args, **kwargs)
 
+    def create_integration_external_project(self, *args, **kwargs):
+        return Factories.create_integration_external_project(*args, **kwargs)
+
     def create_incident(self, organization=None, projects=None, *args, **kwargs):
         if not organization:
             organization = self.organization

tests/sentry/integrations/jira/test_notify_action.py

@@ -5,8 +5,10 @@ from sentry.integrations.jira import JiraCreateTicketAction
 from sentry.models.grouplink import GroupLink
 from sentry.models.integrations.external_issue import ExternalIssue
 from sentry.models.rule import Rule
+from sentry.silo.base import SiloMode
 from sentry.testutils.cases import PerformanceIssueTestCase, RuleTestCase
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE
+from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
 from sentry.testutils.skips import requires_snuba
 from sentry.types.rules import RuleFuture
 from sentry.utils import json
@@ -14,11 +16,14 @@ from sentry.utils import json
 pytestmark = [requires_snuba]
 
 
+@region_silo_test
 class JiraCreateTicketActionTest(RuleTestCase, PerformanceIssueTestCase):
     rule_cls = JiraCreateTicketAction
 
     def setUp(self):
-        self.integration = self.create_provider_integration(
+        self.integration, _ = self.create_provider_integration_for(
+            organization=self.organization,
+            user=self.user,
             provider="jira",
             name="Jira Cloud",
             metadata={
@@ -28,7 +33,6 @@ class JiraCreateTicketActionTest(RuleTestCase, PerformanceIssueTestCase):
                 "domain_name": "example.atlassian.net",
             },
         )
-        self.integration.add_organization(self.organization, self.user)
         self.installation = self.integration.get_installation(self.organization.id)
 
         self.jira_rule = self.get_rule(
@@ -178,8 +182,9 @@ class JiraCreateTicketActionTest(RuleTestCase, PerformanceIssueTestCase):
         assert rule.render_label() == """Create a Jira issue in Jira Cloud with these """
 
     def test_render_label_without_integration(self):
-        deleted_id = self.integration.id
-        self.integration.delete()
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            deleted_id = self.integration.id
+            self.integration.delete()
 
         rule = self.get_rule(data={"integration": deleted_id})
 

tests/sentry/integrations/jira/test_sentry_installation.py

@@ -5,12 +5,14 @@ from jwt import ExpiredSignatureError
 from sentry.integrations.jira.views import UNABLE_TO_VERIFY_INSTALLATION
 from sentry.integrations.utils import AtlassianConnectValidationError
 from sentry.testutils.cases import APITestCase
+from sentry.testutils.silo import control_silo_test
 from sentry.utils.http import absolute_uri
 
 REFRESH_REQUIRED = b"This page has expired, please refresh to configure your Sentry integration"
 CLICK_TO_FINISH = b"Finish Installation in Sentry"
 
 
+@control_silo_test
 class JiraSentryInstallationViewTestCase(APITestCase):
     def setUp(self):
         super().setUp()

tests/sentry/integrations/jira/test_webhooks.py

@@ -16,17 +16,21 @@ from sentry.services.hybrid_cloud.integration.serial import serialize_integratio
 from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organization
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.testutils.cases import APITestCase, TestCase
+from sentry.testutils.silo import region_silo_test
 
 TOKEN = "JWT anexampletoken"
 
 
+@region_silo_test
 class JiraIssueUpdatedWebhookTest(APITestCase):
     endpoint = "sentry-extensions-jira-issue-updated"
     method = "post"
 
     def setUp(self):
         super().setUp()
-        integration = self.create_provider_integration(
+        integration, _ = self.create_provider_integration_for(
+            organization=self.organization,
+            user=self.user,
             provider="jira",
             name="Example Jira",
             metadata={
@@ -36,7 +40,7 @@ class JiraIssueUpdatedWebhookTest(APITestCase):
                 "domain_name": "example.atlassian.net",
             },
         )
-        integration.add_organization(self.organization, self.user)
+        # Ensure this is region safe, and doesn't require the ORM integration model
         self.integration = serialize_integration(integration=integration)
 
     @patch("sentry.integrations.jira.utils.api.sync_group_assignee_inbound")

tests/sentry/integrations/jira_server/test_integration.py

@@ -10,7 +10,6 @@ from django.urls import reverse
 
 from fixtures.integrations.jira.stub_client import StubJiraApiClient
 from fixtures.integrations.stub_service import StubService
-from sentry.integrations.jira import JiraIntegration
 from sentry.integrations.jira_server.integration import JiraServerIntegration
 from sentry.models.grouplink import GroupLink
 from sentry.models.groupmeta import GroupMeta
@@ -25,13 +24,13 @@ from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.factories import DEFAULT_EVENT_DATA
 from sentry.testutils.helpers.datetime import before_now, iso_format
-from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
+from sentry.testutils.silo import assume_test_silo_mode, control_silo_test, region_silo_test
 from sentry.testutils.skips import requires_snuba
 from sentry.utils import json
 from sentry.utils.http import absolute_uri
 from sentry_plugins.jira.plugin import JiraPlugin
 
-from . import get_integration
+from . import EXAMPLE_PRIVATE_KEY
 
 pytestmark = [requires_snuba]
 
@@ -43,55 +42,49 @@ def get_client():
     return StubJiraApiClient()
 
 
-@region_silo_test
-class RegionJiraServerIntegrationTest(APITestCase):
+class JiraServerIntegrationBaseTest(APITestCase):
     def setUp(self):
         super().setUp()
+        self.min_ago = iso_format(before_now(minutes=1))
+        (
+            self.integration,
+            self.org_integration,
+            self.identity,
+            self.identity_provider,
+        ) = self.create_identity_integration(
+            user=self.user,
+            organization=self.organization,
+            integration_params={
+                "provider": "jira_server",
+                "external_id": "jira.example.org:sentry-test",
+                "name": "Example Jira",
+                "metadata": {
+                    "verify_ssl": False,
+                    "webhook_secret": "a long secret value",
+                    "base_url": "https://jira.example.org",
+                },
+            },
+            identity_params={
+                "external_id": "jira.example.org:user_id",
+                "data": {
+                    "consumer_key": "sentry-test",
+                    "private_key": EXAMPLE_PRIVATE_KEY,
+                    "access_token": "access-token",
+                    "access_token_secret": "access-token-secret",
+                },
+            },
+        )
         with assume_test_silo_mode(SiloMode.CONTROL):
-            self.integration = get_integration(self.organization, self.user)
             self.user.name = "Sentry Admin"
             self.user.save()
-        self.login_as(self.user)
-        installation = self.integration.get_installation(self.organization.id)
-        assert isinstance(installation, JiraServerIntegration)
-        self.installation = installation
-
-    def test_create_comment(self):
-        group_note = mock.Mock()
-        comment = "hello world\nThis is a comment.\n\n\n    Glad it's quoted"
-        group_note.data = {"text": comment}
-        with mock.patch.object(StubJiraApiClient, "create_comment") as mock_create_comment:
-            with mock.patch.object(self.installation, "get_client", get_client):
-                self.installation.create_comment(1, self.user.id, group_note)
-                assert (
-                    mock_create_comment.call_args[0][1]
-                    == "Sentry Admin wrote:\n\n{quote}%s{quote}" % comment
-                )
-
-    def test_update_comment(self):
-        group_note = mock.Mock()
-        comment = "hello world\nThis is a comment.\n\n\n    I've changed it"
-        group_note.data = {"text": comment, "external_id": "123"}
-        with mock.patch.object(StubJiraApiClient, "update_comment") as mock_update_comment:
-            with mock.patch.object(self.installation, "get_client", get_client):
-                self.installation.update_comment(1, self.user.id, group_note)
-                assert mock_update_comment.call_args[0] == (
-                    1,
-                    "123",
-                    "Sentry Admin wrote:\n\n{quote}%s{quote}" % comment,
-                )
-
-
-class JiraServerIntegrationTest(APITestCase):
-    def setUp(self):
-        super().setUp()
-        self.min_ago = iso_format(before_now(minutes=1))
-        self.integration = get_integration(self.organization, self.user)
         installation = self.integration.get_installation(self.organization.id)
         assert isinstance(installation, JiraServerIntegration)
         self.installation = installation
         self.login_as(self.user)
 
+
+@region_silo_test
+class JiraServerRegionIntegrationTest(JiraServerIntegrationBaseTest):
     def test_get_create_issue_config(self):
         event = self.store_event(
             data={
@@ -736,24 +729,25 @@ class JiraServerIntegrationTest(APITestCase):
         assert result["key"] == "APP-123"
 
     def test_outbound_issue_sync(self):
-        integration = self.create_provider_integration(provider="jira", name="Example Jira")
-        integration.add_organization(self.organization, self.user)
+        integration, _ = self.create_provider_integration_for(
+            organization=self.organization,
+            user=self.user,
+            provider="jira_server",
+            name="Example Jira",
+        )
 
         external_issue = ExternalIssue.objects.create(
             organization_id=self.organization.id, integration_id=integration.id, key="SEN-5"
         )
-
-        IntegrationExternalProject.objects.create(
-            external_id="10100",
-            organization_integration_id=OrganizationIntegration.objects.get(
-                organization_id=self.organization.id, integration_id=integration.id
-            ).id,
+        self.create_integration_external_project(
+            organization_id=self.organization.id,
+            integration_id=integration.id,
             resolved_status="10101",
             unresolved_status="3",
+            external_id="10100",
         )
-
         installation = integration.get_installation(self.organization.id)
-        assert isinstance(installation, JiraIntegration)
+        assert isinstance(installation, JiraServerIntegration)
 
         with mock.patch.object(StubJiraApiClient, "transition_issue") as mock_transition_issue:
             with mock.patch.object(installation, "get_client", get_client):
@@ -810,8 +804,94 @@ class JiraServerIntegrationTest(APITestCase):
         # No sync made as jira users don't have email addresses
         assert len(responses.calls) == 1
 
+    def test_get_config_data(self):
+        integration = self.create_integration(
+            organization=self.organization,
+            provider="jira_server",
+            external_id="jira:1",
+            name="Example Jira",
+            oi_params={
+                "config": {
+                    "sync_comments": True,
+                    "sync_forward_assignment": True,
+                    "sync_reverse_assignment": True,
+                    "sync_status_reverse": True,
+                    "sync_status_forward": True,
+                }
+            },
+        )
+        self.create_integration_external_project(
+            organization_id=self.organization.id,
+            integration_id=integration.id,
+            external_id="12345",
+            unresolved_status="in_progress",
+            resolved_status="done",
+        )
+
+        installation = integration.get_installation(self.organization.id)
+
+        assert installation.get_config_data() == {
+            "sync_comments": True,
+            "sync_forward_assignment": True,
+            "sync_reverse_assignment": True,
+            "sync_status_reverse": True,
+            "sync_status_forward": {"12345": {"on_resolve": "done", "on_unresolve": "in_progress"}},
+            "issues_ignored_fields": "",
+        }
+
+    def test_get_config_data_issues_keys(self):
+        integration, org_integration = self.create_provider_integration_for(
+            organization=self.organization,
+            user=self.user,
+            provider="jira_server",
+            name="Example Jira",
+        )
+        installation = integration.get_installation(self.organization.id)
+
+        # If config has not be configured yet, uses empty string fallback
+        assert "issues_ignored_fields" not in org_integration.config
+        assert installation.get_config_data().get("issues_ignored_fields") == ""
+
+        # List is serialized as comma-separated list
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            org_integration.config["issues_ignored_fields"] = ["hello world", "goodnight", "moon"]
+            org_integration.save()
+        installation = integration.get_installation(self.organization.id)
+        assert (
+            installation.get_config_data().get("issues_ignored_fields")
+            == "hello world, goodnight, moon"
+        )
+
+    def test_create_comment(self):
+        group_note = mock.Mock()
+        comment = "hello world\nThis is a comment.\n\n\n    Glad it's quoted"
+        group_note.data = {"text": comment}
+        with mock.patch.object(StubJiraApiClient, "create_comment") as mock_create_comment:
+            with mock.patch.object(self.installation, "get_client", get_client):
+                self.installation.create_comment(1, self.user.id, group_note)
+                assert (
+                    mock_create_comment.call_args[0][1]
+                    == "Sentry Admin wrote:\n\n{quote}%s{quote}" % comment
+                )
+
+    def test_update_comment(self):
+        group_note = mock.Mock()
+        comment = "hello world\nThis is a comment.\n\n\n    I've changed it"
+        group_note.data = {"text": comment, "external_id": "123"}
+        with mock.patch.object(StubJiraApiClient, "update_comment") as mock_update_comment:
+            with mock.patch.object(self.installation, "get_client", get_client):
+                self.installation.update_comment(1, self.user.id, group_note)
+                assert mock_update_comment.call_args[0] == (
+                    1,
+                    "123",
+                    "Sentry Admin wrote:\n\n{quote}%s{quote}" % comment,
+                )
+
+
+@control_silo_test
+class JiraServerControlIntegrationTest(JiraServerIntegrationBaseTest):
     def test_update_organization_config_sync_keys(self):
-        integration = self.create_provider_integration(provider="jira", name="Example Jira")
+        integration = self.create_provider_integration(provider="jira_server", name="Example Jira")
         integration.add_organization(self.organization, self.user)
 
         installation = integration.get_installation(self.organization.id)
@@ -923,7 +1003,7 @@ class JiraServerIntegrationTest(APITestCase):
         )
 
     def test_update_organization_config_issues_keys(self):
-        integration = self.create_provider_integration(provider="jira", name="Example Jira")
+        integration = self.create_provider_integration(provider="jira_server", name="Example Jira")
         integration.add_organization(self.organization, self.user)
 
         installation = integration.get_installation(self.organization.id)
@@ -956,68 +1036,14 @@ class JiraServerIntegrationTest(APITestCase):
             "moon",
         ]
 
-    def test_get_config_data(self):
-        integration = self.create_provider_integration(provider="jira", name="Example Jira")
-        integration.add_organization(self.organization, self.user)
-
-        org_integration = OrganizationIntegration.objects.get(
-            organization_id=self.organization.id, integration_id=integration.id
-        )
-
-        org_integration.config = {
-            "sync_comments": True,
-            "sync_forward_assignment": True,
-            "sync_reverse_assignment": True,
-            "sync_status_reverse": True,
-            "sync_status_forward": True,
-        }
-        org_integration.save()
-
-        IntegrationExternalProject.objects.create(
-            organization_integration_id=org_integration.id,
-            external_id="12345",
-            unresolved_status="in_progress",
-            resolved_status="done",
-        )
-
-        installation = integration.get_installation(self.organization.id)
-
-        assert installation.get_config_data() == {
-            "sync_comments": True,
-            "sync_forward_assignment": True,
-            "sync_reverse_assignment": True,
-            "sync_status_reverse": True,
-            "sync_status_forward": {"12345": {"on_resolve": "done", "on_unresolve": "in_progress"}},
-            "issues_ignored_fields": "",
-        }
-
-    def test_get_config_data_issues_keys(self):
-        integration = self.create_provider_integration(provider="jira", name="Example Jira")
-        integration.add_organization(self.organization, self.user)
-
-        installation = integration.get_installation(self.organization.id)
-        org_integration = OrganizationIntegration.objects.get(
-            organization_id=self.organization.id, integration_id=integration.id
-        )
-
-        # If config has not be configured yet, uses empty string fallback
-        assert "issues_ignored_fields" not in org_integration.config
-        assert installation.get_config_data().get("issues_ignored_fields") == ""
-
-        # List is serialized as comma-separated list
-        org_integration.config["issues_ignored_fields"] = ["hello world", "goodnight", "moon"]
-        org_integration.save()
-        installation = integration.get_installation(self.organization.id)
-        assert (
-            installation.get_config_data().get("issues_ignored_fields")
-            == "hello world, goodnight, moon"
-        )
-
 
+@region_silo_test
 class JiraMigrationIntegrationTest(APITestCase):
     @cached_property
     def integration(self):
-        integration = self.create_provider_integration(
+        integration = self.create_integration(
+            organization=self.organization,
+            external_id="jira_server:1",
             provider="jira_server",
             name="Jira Server",
             metadata={
@@ -1027,7 +1053,6 @@ class JiraMigrationIntegrationTest(APITestCase):
                 "domain_name": "example.atlassian.net",
             },
         )
-        integration.add_organization(self.organization, self.user)
         return integration
 
     def setUp(self):
@@ -1062,10 +1087,13 @@ class JiraMigrationIntegrationTest(APITestCase):
         plugin2_issue = GroupMeta.objects.create(
             key=f"{self.plugin.slug}:tid", group_id=group2.id, value="BAR-1"
         )
-        org_integration = OrganizationIntegration.objects.get(integration_id=self.integration.id)
-        with unguarded_write(router.db_for_write(OrganizationIntegration)):
-            org_integration.config.update({"issues_ignored_fields": ["reporter", "test"]})
-        org_integration.save()
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            org_integration = OrganizationIntegration.objects.get(
+                integration_id=self.integration.id
+            )
+            with unguarded_write(router.db_for_write(OrganizationIntegration)):
+                org_integration.config.update({"issues_ignored_fields": ["reporter", "test"]})
+            org_integration.save()
 
         with self.tasks():
             self.installation.migrate_issues()
@@ -1087,8 +1115,9 @@ class JiraMigrationIntegrationTest(APITestCase):
             key=f"{self.plugin.slug}:tid", group_id=group.id, value="BAR-1"
         ).exists()
 
-        oi = OrganizationIntegration.objects.get(integration_id=self.integration.id)
-        assert len(oi.config["issues_ignored_fields"]) == 4
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            oi = OrganizationIntegration.objects.get(integration_id=self.integration.id)
+            assert len(oi.config["issues_ignored_fields"]) == 4
 
         assert self.plugin.get_option("enabled", self.project) is False
         assert plugin2.get_option("enabled", project2) is False