Browse Source

feat(codeowners): Save integration_id if it exists (#25646)

Objective:
We know that there is an integration when creating/updating External User/Team Mappings through the UI. We should be passing in the integration_id and saving it.
NisanthanNanthakumar 3 years ago
parent
commit
5cf8e7fbd3

+ 20 - 13
src/sentry/api/bases/external_actor.py

@@ -1,4 +1,4 @@
-from typing import Any, MutableMapping
+from typing import Any, MutableMapping, Optional
 
 from django.db import IntegrityError
 from django.http import Http404
@@ -9,7 +9,7 @@ from rest_framework.request import Request
 from sentry import features
 from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
 from sentry.api.validators.integrations import validate_provider
-from sentry.models import ExternalActor, Organization, Team, User
+from sentry.models import ExternalActor, Organization, OrganizationIntegration, Team, User
 from sentry.types.integrations import ExternalProviders, get_provider_choices
 
 AVAILABLE_PROVIDERS = {
@@ -23,27 +23,35 @@ class ExternalActorSerializerBase(CamelSnakeModelSerializer):  # type: ignore
     external_id = serializers.CharField()
     external_name = serializers.CharField(required=True)
     provider = serializers.ChoiceField(choices=get_provider_choices(AVAILABLE_PROVIDERS))
+    integration_id = serializers.IntegerField(required=False, allow_null=True)
 
     @property
     def organization(self) -> Organization:
         return self.context["organization"]
 
-    def get_actor_id(self, validated_data: MutableMapping[str, Any]) -> int:
-        return int(validated_data.pop(self._actor_key).actor_id)
+    def validate_integration_id(self, integration_id: str) -> Optional[str]:
+        if not integration_id:
+            return None
 
-    def get_provider_id(self, validated_data: MutableMapping[str, Any]) -> int:
-        provider_name_option = validated_data.pop("provider", None)
+        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_provider(self, provider_name_option: str) -> int:
         provider = validate_provider(provider_name_option, available_providers=AVAILABLE_PROVIDERS)
         return int(provider.value)
 
+    def get_actor_id(self, validated_data: MutableMapping[str, Any]) -> int:
+        return int(validated_data.pop(self._actor_key).actor_id)
+
     def create(self, validated_data: MutableMapping[str, Any]) -> ExternalActor:
         actor_id = self.get_actor_id(validated_data)
-        provider = self.get_provider_id(validated_data)
-
         return ExternalActor.objects.get_or_create(
             **validated_data,
             actor_id=actor_id,
-            provider=provider,
             organization=self.organization,
         )
 
@@ -53,8 +61,7 @@ class ExternalActorSerializerBase(CamelSnakeModelSerializer):  # type: ignore
         # Discard the object ID passed by the API.
         if "id" in validated_data:
             validated_data.pop("id")
-        if "provider" in validated_data:
-            validated_data["provider"] = self.get_provider_id({**validated_data})
+
         if self._actor_key in validated_data:
             validated_data["actor_id"] = self.get_actor_id({**validated_data})
 
@@ -86,7 +93,7 @@ class ExternalUserSerializer(ExternalActorSerializerBase):
 
     class Meta:
         model = ExternalActor
-        fields = ["user_id", "external_name", "provider"]
+        fields = ["user_id", "external_name", "provider", "integration_id"]
 
 
 class ExternalTeamSerializer(ExternalActorSerializerBase):
@@ -103,7 +110,7 @@ class ExternalTeamSerializer(ExternalActorSerializerBase):
 
     class Meta:
         model = ExternalActor
-        fields = ["team_id", "external_name", "provider"]
+        fields = ["team_id", "external_name", "provider", "integration_id"]
 
 
 class ExternalActorEndpointMixin:

+ 1 - 0
src/sentry/api/endpoints/external_team.py

@@ -23,6 +23,7 @@ class ExternalTeamEndpoint(TeamEndpoint, ExternalActorEndpointMixin):  # type: i
         :pparam string team_slug: the slug of the team to get.
         :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.
         :auth: required
         """
         self.assert_has_feature(request, team.organization)

+ 1 - 0
static/app/components/integrationExternalMappingForm.tsx

@@ -28,6 +28,7 @@ export default class IntegrationExternalMappingForm extends React.Component<Prop
       teamId: '',
       sentryName: '',
       provider: integration.provider.key,
+      integrationId: integration.id,
       ...pick(mapping, ['externalName', 'userId', 'sentryName', 'teamId']),
     };
   }

+ 48 - 0
tests/sentry/api/endpoints/test_external_team.py

@@ -1,3 +1,4 @@
+from sentry.models import ExternalActor, Integration
 from sentry.testutils import APITestCase
 from sentry.types.integrations import get_provider_string
 
@@ -68,3 +69,50 @@ class ExternalTeamTest(APITestCase):
             "teamId": str(self.team.id),
             **data,
         }
+
+    def test_create_with_integration(self):
+        self.integration = Integration.objects.create(
+            provider="gitlab", name="Gitlab", external_id="gitlab:1"
+        )
+
+        self.integration.add_organization(self.organization, self.user)
+
+        data = {
+            "externalName": "@getsentry/ecosystem",
+            "provider": "github",
+            "integrationId": self.integration.id,
+        }
+        with self.feature({"organizations:import-codeowners": True}):
+            response = self.get_success_response(
+                self.organization.slug, self.team.slug, status_code=201, **data
+            )
+        assert response.data == {
+            "id": str(response.data["id"]),
+            "teamId": str(self.team.id),
+            "externalName": data["externalName"],
+            "provider": data["provider"],
+        }
+        assert (
+            ExternalActor.objects.get(id=response.data["id"]).integration_id == self.integration.id
+        )
+
+    def test_create_with_invalid_integration_id(self):
+        self.org2 = self.create_organization(owner=self.user, name="org2")
+        self.integration = Integration.objects.create(
+            provider="gitlab", name="Gitlab", external_id="gitlab:1"
+        )
+
+        self.integration.add_organization(self.org2, self.user)
+
+        data = {
+            "externalName": "@getsentry/ecosystem",
+            "provider": "github",
+            "integrationId": self.integration.id,
+        }
+        with self.feature({"organizations:import-codeowners": True}):
+            response = self.get_error_response(
+                self.organization.slug, self.team.slug, status_code=400, **data
+            )
+        assert response.data == {
+            "integrationId": ["Integration does not exist for this organization"]
+        }