fix(notifications): Add `external_id` to `ExternalActorSerlializer` (#25678)

mypy.ini

@@ -13,6 +13,7 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/api/serializers/models/notification_setting.py,
         src/sentry/api/serializers/models/organization_member.py,
         src/sentry/api/serializers/models/team.py,
+        src/sentry/api/validators/external_actor.py,
         src/sentry/api/validators/notifications.py,
         src/sentry/notifications/**/*.py,
         src/sentry/snuba/outcomes.py,

src/sentry/api/bases/external_actor.py

@@ -8,8 +8,12 @@ from rest_framework.request import Request
 
 from sentry import features
 from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
+from sentry.api.validators.external_actor import (
+    validate_external_id_option,
+    validate_integration_id_option,
+)
 from sentry.api.validators.integrations import validate_provider
-from sentry.models import ExternalActor, Organization, OrganizationIntegration, Team, User
+from sentry.models import ExternalActor, Organization, Team, User
 from sentry.types.integrations import ExternalProviders, get_provider_choices
 
 AVAILABLE_PROVIDERS = {
@@ -20,7 +24,7 @@ AVAILABLE_PROVIDERS = {
 
 
 class ExternalActorSerializerBase(CamelSnakeModelSerializer):  # type: ignore
-    external_id = serializers.CharField()
+    external_id = serializers.CharField(required=False, allow_null=True)
     external_name = serializers.CharField(required=True)
     provider = serializers.ChoiceField(choices=get_provider_choices(AVAILABLE_PROVIDERS))
     integration_id = serializers.IntegerField(required=False, allow_null=True)
@@ -30,15 +34,10 @@ class ExternalActorSerializerBase(CamelSnakeModelSerializer):  # type: ignore
         return self.context["organization"]
 
     def validate_integration_id(self, integration_id: str) -> Optional[str]:
-        if not integration_id:
-            return None
+        return validate_integration_id_option(integration_id, self.organization)
 
-        integration_query = OrganizationIntegration.objects.filter(
-            organization=self.organization, integration_id=integration_id
-        )
-        if not integration_query.exists():
-            raise serializers.ValidationError("Integration does not exist for this organization")
-        return integration_id
+    def validate_external_id(self, external_id: str) -> Optional[str]:
+        return validate_external_id_option(external_id)
 
     def validate_provider(self, provider_name_option: str) -> int:
         provider = validate_provider(provider_name_option, available_providers=AVAILABLE_PROVIDERS)
@@ -93,7 +92,7 @@ class ExternalUserSerializer(ExternalActorSerializerBase):
 
     class Meta:
         model = ExternalActor
-        fields = ["user_id", "external_name", "provider", "integration_id"]
+        fields = ["user_id", "external_id", "external_name", "provider", "integration_id"]
 
 
 class ExternalTeamSerializer(ExternalActorSerializerBase):
@@ -110,7 +109,7 @@ class ExternalTeamSerializer(ExternalActorSerializerBase):
 
     class Meta:
         model = ExternalActor
-        fields = ["team_id", "external_name", "provider", "integration_id"]
+        fields = ["team_id", "external_id", "external_name", "provider", "integration_id"]
 
 
 class ExternalActorEndpointMixin:

src/sentry/api/endpoints/external_team.py

@@ -24,6 +24,7 @@ class ExternalTeamEndpoint(TeamEndpoint, ExternalActorEndpointMixin):  # type: i
         :param required string provider: enum("github", "gitlab")
         :param required string external_name: the associated Github/Gitlab team name.
         :param optional string integration_id: the id of the integration if it exists.
+        :param string external_id: the associated user ID for this provider
         :auth: required
         """
         self.assert_has_feature(request, team.organization)

src/sentry/api/endpoints/external_team_details.py

@@ -37,6 +37,7 @@ class ExternalTeamDetailsEndpoint(TeamEndpoint, ExternalActorEndpointMixin):  #
                                           team belongs to.
         :pparam string team_slug: the slug of the team to get.
         :pparam string external_team_id: id of external_team object
+        :param string external_id: the associated user ID for this provider
         :param string external_name: the Github/Gitlab team name.
         :param string provider: enum("github","gitlab")
         :auth: required

src/sentry/api/endpoints/external_user.py

@@ -21,8 +21,9 @@ class ExternalUserEndpoint(OrganizationEndpoint, ExternalActorEndpointMixin):  #
         :pparam string organization_slug: the slug of the organization the
                                           user belongs to.
         :param required string provider: enum("github", "gitlab", "slack")
-        :param required string external_name: the associated Github/Gitlab user name.
-        :param required int user_id: the User id.
+        :param required string external_name: the associated username for this provider.
+        :param required int user_id: the User ID in Sentry.
+        :param string external_id: the associated user ID for this provider
         :auth: required
         """
         self.assert_has_feature(request, organization)

src/sentry/api/endpoints/external_user_details.py

@@ -37,6 +37,7 @@ class ExternalUserDetailsEndpoint(OrganizationEndpoint, ExternalActorEndpointMix
                                           user belongs to.
         :pparam int user_id: the User id.
         :pparam string external_user_id: id of external_user object
+        :param string external_id: the associated user ID for this provider
         :param string external_name: the Github/Gitlab user name.
         :param string provider: enum("github","gitlab")
         :auth: required

src/sentry/api/validators/external_actor.py

@@ -0,0 +1,43 @@
+from typing import Optional
+
+from rest_framework import serializers
+
+from sentry.models import Organization, OrganizationIntegration
+
+EXTERNAL_ID_LENGTH_MIN = 1
+EXTERNAL_ID_LENGTH_MAX = 64
+
+
+def _out_of_range_string(param: str, minimum: int, maximum: int, actual: int) -> str:
+    return f"{param} has invalid length: {actual}. Length must be between {minimum} and {maximum}"
+
+
+def validate_external_id_option(external_id: Optional[str]) -> Optional[str]:
+    if not external_id:
+        return None
+
+    if not EXTERNAL_ID_LENGTH_MIN <= len(external_id) <= EXTERNAL_ID_LENGTH_MAX:
+        raise serializers.ValidationError(
+            _out_of_range_string(
+                "external_id",
+                EXTERNAL_ID_LENGTH_MIN,
+                EXTERNAL_ID_LENGTH_MAX,
+                len(external_id),
+            )
+        )
+
+    return external_id
+
+
+def validate_integration_id_option(
+    integration_id: Optional[str], organization: Organization
+) -> Optional[str]:
+    if not integration_id:
+        return None
+
+    integration_query = OrganizationIntegration.objects.filter(
+        organization=organization, integration_id=integration_id
+    )
+    if not integration_query.exists():
+        raise serializers.ValidationError("Integration does not exist for this organization")
+    return integration_id

tests/sentry/api/endpoints/test_external_team.py

@@ -116,3 +116,29 @@ class ExternalTeamTest(APITestCase):
         assert response.data == {
             "integrationId": ["Integration does not exist for this organization"]
         }
+
+    def test_create_with_external_id(self):
+        data = {
+            "externalId": "YU287RFO30",
+            "externalName": "@getsentry/ecosystem",
+            "provider": "slack",
+        }
+        with self.feature({"organizations:import-codeowners": True}):
+            response = self.get_success_response(self.organization.slug, self.team.slug, **data)
+        assert response.data == {
+            "id": str(response.data["id"]),
+            "teamId": str(self.team.id),
+            "externalName": data["externalName"],
+            "externalId": data["externalId"],
+            "provider": data["provider"],
+        }
+        assert ExternalActor.objects.get(id=response.data["id"]).external_id == "YU287RFO30"
+
+    def test_create_with_invalid_external_id(self):
+        data = {
+            "externalId": "",
+            "externalName": "@getsentry/ecosystem",
+            "provider": "slack",
+        }
+        with self.feature({"organizations:import-codeowners": True}):
+            self.get_error_response(self.organization.slug, self.team.slug, **data)