fix(app-platform): Allow removal of Internal Apps (#13534)

src/sentry/api/endpoints/sentry_app_details.py

@@ -6,7 +6,6 @@
 from sentry.api.bases.sentryapps import SentryAppBaseEndpoint
 from sentry.api.serializers import serialize
 from sentry.api.serializers.rest_framework import SentryAppSerializer
-from sentry.constants import SentryAppStatus
 from sentry.mediators.sentry_apps import Updater, Destroyer
 
 
@@ -68,7 +67,7 @@ def delete(self, request, sentry_app):
                             actor=request.user):
             return Response(status=404)
 
-        if sentry_app.status == SentryAppStatus.UNPUBLISHED:
+        if sentry_app.is_unpublished or sentry_app.is_internal:
             Destroyer.run(
                 user=request.user,
                 sentry_app=sentry_app,

src/sentry/mediators/sentry_app_installations/destroyer.py

@@ -12,6 +12,7 @@ class Destroyer(Mediator):
     install = Param('sentry.models.SentryAppInstallation')
     user = Param('sentry.models.User')
     request = Param('rest_framework.request.Request', required=False)
+    notify = Param(bool, default=True)
 
     def call(self):
         self._destroy_grant()
@@ -32,11 +33,12 @@ def _destroy_service_hooks(self):
             service_hooks.Destroyer.run(service_hook=hook)
 
     def _destroy_installation(self):
-        InstallationNotifier.run(
-            install=self.install,
-            user=self.user,
-            action='deleted',
-        )
+        if self.notify:
+            InstallationNotifier.run(
+                install=self.install,
+                user=self.user,
+                action='deleted',
+            )
         self.install.delete()
 
     def audit(self):

src/sentry/mediators/sentry_apps/destroyer.py

@@ -10,19 +10,22 @@
 class Destroyer(Mediator):
     sentry_app = Param('sentry.models.SentryApp')
     request = Param('rest_framework.request.Request', required=False)
+    user = Param('sentry.models.User')
 
     def call(self):
-        self._destroy_sentry_app_installations()
         self._destroy_api_application()
+        self._destroy_sentry_app_installations()
         self._destroy_proxy_user()
         self._destroy_sentry_app()
         return self.sentry_app
 
     def _destroy_sentry_app_installations(self):
         for install in self.sentry_app.installations.all():
+            notify = False if self.sentry_app.is_internal else True
             sentry_app_installations.Destroyer.run(
                 install=install,
                 user=self.sentry_app.proxy_user,
+                notify=notify,
             )
 
     def _destroy_api_application(self):

src/sentry/static/sentry/app/views/organizationIntegrations/index.jsx

@@ -13,6 +13,7 @@ import LoadingIndicator from 'app/components/loadingIndicator';
 import MigrationWarnings from 'app/views/organizationIntegrations/migrationWarnings';
 import PermissionAlert from 'app/views/settings/organization/permissionAlert';
 import ProviderRow from 'app/views/organizationIntegrations/providerRow';
+import {removeSentryApp} from 'app/actionCreators/sentryApps';
 import SentryAppInstallations from 'app/views/organizationIntegrations/sentryAppInstallations';
 import SentryApplicationRow from 'app/views/settings/organizationDeveloperSettings/sentryApplicationRow';
 import SentryTypes from 'app/sentryTypes';
@@ -168,19 +169,31 @@ class OrganizationIntegrations extends AsyncComponent {
 
   renderInternalSentryApps(app, key) {
     const {organization} = this.props;
+
     return (
       <SentryApplicationRow
         key={`sentry-app-row-${key}`}
         data-test-id="internal-integration-row"
         api={this.api}
         showPublishStatus
         isInternal
+        onRemoveApp={() => this.onRemoveInternalApp(app)}
         organization={organization}
         app={app}
       />
     );
   }
 
+  onRemoveInternalApp = app => {
+    const apps = this.state.orgOwnedApps.filter(a => a.slug !== app.slug);
+    removeSentryApp(this.api, app).then(
+      () => {
+        this.setState({orgOwnedApps: apps});
+      },
+      () => {}
+    );
+  };
+
   renderBody() {
     const {reloading, orgOwnedApps, publishedApps, appInstalls} = this.state;
     const published = publishedApps || [];

tests/js/spec/views/settings/organizationIntegrations/index.spec.jsx

@@ -284,6 +284,32 @@ describe('OrganizationIntegrations', () => {
           wrapper.find('Panel [data-test-id="internal-integration-row"]').exists()
         ).toBe(true);
       });
+
+      it('removes an internal app', async function() {
+        const internalApp = {...sentryApp, status: 'internal'};
+        Client.addMockResponse({
+          url: `/organizations/${org.slug}/sentry-apps/`,
+          body: [internalApp],
+        });
+        Client.addMockResponse({
+          url: '/sentry-apps/',
+          body: [],
+        });
+        Client.addMockResponse({
+          url: `/sentry-apps/${internalApp.slug}/`,
+          method: 'DELETE',
+          statusCode: 200,
+        });
+
+        wrapper = mount(
+          <OrganizationIntegrations organization={org} params={params} />,
+          routerContext
+        );
+        wrapper.instance().onRemoveInternalApp(internalApp);
+        await tick();
+        wrapper.update();
+        expect(wrapper.instance().state.orgOwnedApps).toHaveLength(0);
+      });
     });
 
     describe('with installed integrations', () => {

tests/sentry/mediators/sentry_app_installations/test_destroyer.py

@@ -68,6 +68,33 @@ def test_deletes_service_hooks(self):
 
         assert not ServiceHook.objects.filter(pk=hook.id).exists()
 
+    @responses.activate
+    @patch('sentry.mediators.sentry_app_installations.InstallationNotifier.run')
+    def test_sends_notification(self, run):
+        with self.tasks():
+            responses.add(responses.POST, 'https://example.com/webhook')
+            request = self.make_request(user=self.user, method='GET')
+            Destroyer.run(
+                install=self.install,
+                user=self.user,
+                request=request,
+            )
+            run.assert_called_once_with(install=self.install, user=self.user, action='deleted')
+
+    @responses.activate
+    @patch('sentry.mediators.sentry_app_installations.InstallationNotifier.run')
+    def test_notify_false_does_not_send_notification(self, run):
+        with self.tasks():
+            responses.add(responses.POST, 'https://example.com/webhook')
+            request = self.make_request(user=self.user, method='GET')
+            Destroyer.run(
+                install=self.install,
+                user=self.user,
+                request=request,
+                notify=False,
+            )
+            assert not run.called
+
     @responses.activate
     def test_creates_audit_log_entry(self):
         responses.add(responses.POST, 'https://example.com/webhook')

tests/sentry/mediators/sentry_apps/test_destroyer.py

@@ -19,7 +19,7 @@ def setUp(self):
             scopes=('project:read',),
         )
 
-        self.destroyer = Destroyer(sentry_app=self.sentry_app)
+        self.destroyer = Destroyer(sentry_app=self.sentry_app, user=self.user)
 
     def test_deletes_app_installations(self):
         install = self.create_sentry_app_installation(
@@ -30,6 +30,17 @@ def test_deletes_app_installations(self):
         self.destroyer.call()
         assert not SentryAppInstallation.objects.filter(pk=install.id).exists()
 
+    @patch('sentry.mediators.sentry_app_installations.Destroyer.run')
+    def test_passes_notify_false_if_app_internal(self, run):
+        self.create_project(organization=self.org)
+        internal = self.create_internal_integration(organization=self.org)
+        Destroyer.run(sentry_app=internal, user=self.user)
+        run.assert_called_with(
+            install=internal.installations.first(),
+            user=internal.proxy_user,
+            notify=False,
+        )
+
     def test_deletes_api_application(self):
         application = self.sentry_app.application