fix(relocation): Better organization fork endpoint errors (getsentry#…

…74784)

src/sentry/api/endpoints/organization_fork.py

@@ -18,6 +18,7 @@
 from sentry.api.permissions import SuperuserOrStaffFeatureFlaggedPermission
 from sentry.api.serializers import serialize
 from sentry.hybridcloud.services.organization_mapping import organization_mapping_service
+from sentry.models.organization import OrganizationStatus
 from sentry.models.relocation import Relocation
 from sentry.organizations.services.organization import organization_service
 from sentry.tasks.relocation import uploading_start
@@ -31,6 +32,9 @@
 ERR_ORGANIZATION_MAPPING_NOT_FOUND = Template(
     "The target organization `$slug` has no region mapping."
 )
+ERR_ORGANIZATION_INACTIVE = Template(
+    "The target organization `$slug` has status `$status`; status can only be `ACTIVE`."
+)
 ERR_CANNOT_FORK_INTO_SAME_REGION = Template(
     "The organization already lives in region `$region`, so it cannot be forked into that region."
 )
@@ -64,16 +68,11 @@ def post(self, request: Request, organization_id_or_slug) -> Response:
 
         logger.info("relocations.fork.post.start", extra={"caller": request.user.id})
 
-        org_retrieval_args = {
-            "only_visible": True,
-            "include_projects": False,
-            "include_teams": False,
-        }
         org_context = (
             organization_service.get_organization_by_id(id=organization_id_or_slug)
             if str(organization_id_or_slug).isdecimal()
             else organization_service.get_organization_by_slug(
-                slug=organization_id_or_slug, **org_retrieval_args
+                slug=organization_id_or_slug, only_visible=False  # Check for visibility below
             )
         )
         if not org_context:
@@ -88,11 +87,22 @@ def post(self, request: Request, organization_id_or_slug) -> Response:
 
         organization = org_context.organization
         org_slug = organization.slug
+        if org_context.organization.status != OrganizationStatus.ACTIVE:
+            return Response(
+                {
+                    "detail": ERR_ORGANIZATION_INACTIVE.substitute(
+                        slug=org_slug,
+                        status=str(OrganizationStatus(organization.status).name),
+                    )
+                },
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+
         org_mapping = organization_mapping_service.get(organization_id=organization.id)
         if not org_mapping:
             return Response(
                 {
-                    "detail": ERR_ORGANIZATION_NOT_FOUND.substitute(
+                    "detail": ERR_ORGANIZATION_MAPPING_NOT_FOUND.substitute(
                         slug=org_slug,
                     )
                 },

tests/sentry/api/endpoints/test_organization_fork.py

@@ -4,9 +4,12 @@
 from sentry.api.endpoints.organization_fork import (
     ERR_CANNOT_FORK_INTO_SAME_REGION,
     ERR_DUPLICATE_ORGANIZATION_FORK,
+    ERR_ORGANIZATION_INACTIVE,
+    ERR_ORGANIZATION_MAPPING_NOT_FOUND,
     ERR_ORGANIZATION_NOT_FOUND,
 )
 from sentry.models.organization import OrganizationStatus
+from sentry.models.organizationmapping import OrganizationMapping
 from sentry.models.relocation import Relocation, RelocationFile
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase
@@ -273,7 +276,6 @@ def test_bad_organization_not_found(
 
         response = response = self.get_error_response("does-not-exist", status_code=404)
 
-        assert response.data.get("detail") is not None
         assert response.data.get("detail") == ERR_ORGANIZATION_NOT_FOUND.substitute(
             pointer="does-not-exist"
         )
@@ -282,6 +284,29 @@ def test_bad_organization_not_found(
         assert Relocation.objects.count() == relocation_count
         assert RelocationFile.objects.count() == relocation_file_count
 
+    @override_options({"relocation.enabled": True, "relocation.daily-limit.small": 1})
+    @assume_test_silo_mode(SiloMode.REGION, region_name=REQUESTING_TEST_REGION)
+    def test_bad_organization_mapping_not_found(
+        self,
+        uploading_start_mock: Mock,
+        analytics_record_mock: Mock,
+    ):
+        self.login_as(user=self.superuser, superuser=True)
+        relocation_count = Relocation.objects.count()
+        relocation_file_count = RelocationFile.objects.count()
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            OrganizationMapping.objects.filter(slug=self.existing_org.slug).delete()
+
+        response = response = self.get_error_response(self.existing_org.slug, status_code=404)
+
+        assert response.data.get("detail") == ERR_ORGANIZATION_MAPPING_NOT_FOUND.substitute(
+            slug=self.existing_org.slug
+        )
+        assert uploading_start_mock.call_count == 0
+        assert analytics_record_mock.call_count == 0
+        assert Relocation.objects.count() == relocation_count
+        assert RelocationFile.objects.count() == relocation_file_count
+
     @override_options({"relocation.enabled": True, "relocation.daily-limit.small": 1})
     @assume_test_silo_mode(SiloMode.REGION, region_name=REQUESTING_TEST_REGION)
     def test_bad_cannot_fork_deleted_organization(
@@ -297,11 +322,12 @@ def test_bad_cannot_fork_deleted_organization(
         relocation_count = Relocation.objects.count()
         relocation_file_count = RelocationFile.objects.count()
 
-        response = response = self.get_error_response(self.existing_org.slug, status_code=404)
+        response = response = self.get_error_response(self.existing_org.slug, status_code=400)
 
         assert response.data.get("detail") is not None
-        assert response.data.get("detail") == ERR_ORGANIZATION_NOT_FOUND.substitute(
-            pointer=self.existing_org.slug
+        assert response.data.get("detail") == ERR_ORGANIZATION_INACTIVE.substitute(
+            slug=self.existing_org.slug,
+            status="DELETION_IN_PROGRESS",
         )
         assert uploading_start_mock.call_count == 0
         assert analytics_record_mock.call_count == 0