ref(vercel): Assume one config per user/team (#26162)

Author: MeredithAnya

src/sentry/integrations/vercel/integration.py

@@ -17,7 +17,6 @@
 )
 from sentry.mediators.sentry_apps import InternalCreator
 from sentry.models import (
-    Integration,
     Organization,
     Project,
     ProjectKey,
@@ -116,9 +115,16 @@ def get_client(self):
 
         return VercelClient(access_token)
 
-    # note this could return a different integration if the user has multiple
-    # installations with the same organization
     def get_configuration_id(self):
+        # XXX(meredith): The "configurations" in the metadata is no longer
+        # needed since Vercel restricted installation on their end to be
+        # once per user/team. Eventually we should be able to just use
+        # `self.metadata["installation_id"]`
+        if not self.metadata.get("configurations"):
+            return self.metadata["installation_id"]
+
+        # note this could return a different integration if the user has multiple
+        # installations with the same organization
         for configuration_id, data in self.metadata["configurations"].items():
             if data["organization_id"] == self.organization_id:
                 return configuration_id
@@ -301,19 +307,6 @@ def get_pipeline_views(self):
 
         return [identity_pipeline_view]
 
-    def get_configuration_metadata(self, external_id):
-        # If a vercel team or user was already installed on another sentry org
-        # we want to make sure we don't overwrite the existing configurations. We
-        # keep all the configurations so that if one of them is deleted from vercel's
-        # side, the other sentry org will still have a working vercel integration.
-        try:
-            integration = Integration.objects.get(external_id=external_id, provider=self.key)
-        except Integration.DoesNotExist:
-            # first time setting up vercel team/user
-            return {}
-
-        return integration.metadata["configurations"]
-
     def build_integration(self, state):
         data = state["identity"]["data"]
         access_token = data["access_token"]
@@ -344,8 +337,6 @@ def build_integration(self, state):
             message = f"Could not create deployment webhook in Vercel: {details}"
             raise IntegrationError(message)
 
-        configurations = self.get_configuration_metadata(external_id)
-
         integration = {
             "name": name,
             "external_id": external_id,
@@ -354,25 +345,14 @@ def build_integration(self, state):
                 "installation_id": data["installation_id"],
                 "installation_type": installation_type,
                 "webhook_id": webhook["id"],
-                "configurations": configurations,
             },
             "post_install_data": {"user_id": state["user_id"]},
         }
 
         return integration
 
     def post_install(self, integration, organization, extra=None):
-        # add new configuration information to metadata
-        configurations = integration.metadata.get("configurations") or {}
-        configurations[integration.metadata["installation_id"]] = {
-            "access_token": integration.metadata["access_token"],
-            "webhook_id": integration.metadata["webhook_id"],
-            "organization_id": organization.id,
-        }
-        integration.metadata["configurations"] = configurations
-        integration.save()
-
-        # check if we have an installation already
+        # check if we have an Vercel internal installation already
         if SentryAppInstallationForProvider.objects.filter(
             organization=organization, provider="vercel"
         ).exists():

src/sentry/integrations/vercel/uninstall.py

@@ -18,6 +18,11 @@ def dispatch(self, request, *args, **kwargs):
         return super().dispatch(request, *args, **kwargs)
 
     def post(self, request):
+        # XXX(meredith): This is only handling the integration-configuration-removed
+        # webhook event, in the future will need to check the "type" attribute
+        # when handling more webhook event types
+        # https://vercel.com/docs/integrations?query=event%20paylo#webhooks/events
+
         # userId should always be present
         external_id = request.data.get("teamId") or request.data.get("userId")
         configuration_id = request.data["payload"]["configuration"]["id"]
@@ -49,6 +54,12 @@ def _delete(self, external_id, configuration_id, request):
             integration.delete()
             return self.respond(status=204)
 
+        # If we never set "configurations" in the integration, then we only have one
+        # and therefore can delete it.
+        if not integration.metadata.get("configurations"):
+            integration.delete()
+            return self.respond(status=204)
+
         configuration = integration.metadata["configurations"].pop(configuration_id)
         # one of two cases:
         #  1.) someone is deleting from vercel's end and we still need to delete the

tests/sentry/integrations/vercel/test_integration.py

@@ -90,25 +90,11 @@ def assert_setup_flow(self, is_team=False, multi_config_org=None, no_name=False)
 
         assert integration.external_id == external_id
         assert integration.name == name
-        configurations = {
-            "my_config_id": {
-                "access_token": "my_access_token",
-                "webhook_id": "webhook-id",
-                "organization_id": self.organization.id,
-            }
-        }
-        if multi_config_org:
-            configurations["orig_config_id"] = {
-                "access_token": "orig_access_token",
-                "webhook_id": "orig-webhook-id",
-                "organization_id": multi_config_org.id,
-            }
         assert integration.metadata == {
             "access_token": "my_access_token",
             "installation_id": "my_config_id",
             "installation_type": installation_type,
             "webhook_id": "webhook-id",
-            "configurations": configurations,
         }
         assert OrganizationIntegration.objects.get(
             integration=integration, organization=self.organization
@@ -145,28 +131,6 @@ def test_use_existing_installation(self):
         self.assert_setup_flow(is_team=False)
         assert SentryAppInstallation.objects.count() == 1
 
-    @responses.activate
-    def test_install_on_multiple_orgs(self):
-        orig_org = self.create_organization()
-        metadata = {
-            "access_token": "orig_access_token",
-            "installation_id": "orig_config_id",
-            "installation_type": "team",
-            "webhook_id": "orig-webhook-id",
-            "configurations": {
-                "orig_config_id": {
-                    "access_token": "orig_access_token",
-                    "webhook_id": "orig-webhook-id",
-                    "organization_id": orig_org.id,
-                }
-            },
-        }
-        Integration.objects.create(
-            provider="vercel", name="My Team Name", external_id="my_team_id", metadata=metadata
-        )
-
-        self.assert_setup_flow(is_team=True, multi_config_org=orig_org)
-
     @responses.activate
     def test_update_organization_config(self):
         """Test that Vercel environment variables are created"""

tests/sentry/integrations/vercel/test_uninstall.py

@@ -23,6 +23,50 @@
 
 
 class VercelUninstallTest(APITestCase):
+    def setUp(self):
+        self.url = "/extensions/vercel/delete/"
+        metadata = {
+            "access_token": "my_access_token",
+            "installation_id": "my_config_id",
+            "installation_type": "team",
+            "webhook_id": "my_webhook_id",
+        }
+        self.integration = Integration.objects.create(
+            provider="vercel",
+            external_id="vercel_team_id",
+            name="My Vercel Team",
+            metadata=metadata,
+        )
+        self.integration.add_organization(self.organization)
+
+    def _get_delete_response(self):
+        # https://vercel.com/docs/integrations?query=event%20paylo#webhooks/events/integration-configuration-removed
+        return """{
+            "payload": {
+                "configuration": {
+                    "id": "my_config_id",
+                    "projects": ["project_id1"]
+                }
+            },
+            "teamId": "vercel_team_id",
+            "userId": "vercel_user_id"
+        }"""
+
+    def test_uninstall(self):
+        response = self.client.post(
+            path=self.url,
+            data=self._get_delete_response(),
+            content_type="application/json",
+        )
+
+        assert response.status_code == 204
+        assert not Integration.objects.filter(id=self.integration.id).exists()
+        assert not OrganizationIntegration.objects.filter(
+            integration_id=self.integration.id, organization_id=self.organization.id
+        ).exists()
+
+
+class VercelUninstallWithConfigurationsTest(APITestCase):
     def setUp(self):
         self.url = "/extensions/vercel/delete/"
         self.second_org = self.create_organization(name="Blah", owner=self.user)