feat(integration-platform):Prevent repeated publish requests (#22644)

Objective
When publishing a public integration, we want to create a new status to denote "Request Inprogress". And prevent further publish attempts while in progress. This should prevent repeated publish requests.

migrations_lockfile.txt

@@ -10,7 +10,7 @@ auth: 0008_alter_user_username_max_length
 contenttypes: 0002_remove_content_type_name
 jira_ac: 0001_initial
 nodestore: 0001_initial
-sentry: 0143_add_alerts_integrationfeature
+sentry: 0144_add_publish_request_inprogress_status
 sessions: 0001_initial
 sites: 0002_alter_domain_unique
 social_auth: 0001_initial

src/sentry/api/endpoints/sentry_app_publish_request.py

@@ -5,16 +5,27 @@ from rest_framework.response import Response
 from sentry import options
 from sentry.api.bases.sentryapps import SentryAppBaseEndpoint
 from sentry.utils import email
+from sentry.mediators.sentry_apps import Updater
+from sentry.constants import SentryAppStatus
 
 
 class SentryAppPublishRequestEndpoint(SentryAppBaseEndpoint):
     def post(self, request, sentry_app):
         # check status of app to make sure it is unpublished
         if sentry_app.is_published:
-            return Response({"detail": "Cannot publish already published integration"}, status=400)
+            return Response({"detail": "Cannot publish already published integration."}, status=400)
 
         if sentry_app.is_internal:
-            return Response({"detail": "Cannot publish internal integration"}, status=400)
+            return Response({"detail": "Cannot publish internal integration."}, status=400)
+
+        if sentry_app.is_publish_request_inprogress:
+            return Response({"detail": "Publish request in progress."}, status=400)
+
+        Updater.run(
+            user=request.user,
+            sentry_app=sentry_app,
+            status=SentryAppStatus.PUBLISH_REQUEST_INPROGRESS_STR,
+        )
 
         message = "User %s of organization %s wants to publish %s\n" % (
             request.user.email,

src/sentry/constants.py

@@ -423,9 +423,11 @@ class SentryAppStatus(object):
     UNPUBLISHED = 0
     PUBLISHED = 1
     INTERNAL = 2
+    PUBLISH_REQUEST_INPROGRESS = 3
     UNPUBLISHED_STR = "unpublished"
     PUBLISHED_STR = "published"
     INTERNAL_STR = "internal"
+    PUBLISH_REQUEST_INPROGRESS_STR = "publish_request_inprogress"
 
     @classmethod
     def as_choices(cls):
@@ -433,6 +435,7 @@ class SentryAppStatus(object):
             (cls.UNPUBLISHED, six.text_type(cls.UNPUBLISHED_STR)),
             (cls.PUBLISHED, six.text_type(cls.PUBLISHED_STR)),
             (cls.INTERNAL, six.text_type(cls.INTERNAL_STR)),
+            (cls.PUBLISH_REQUEST_INPROGRESS, six.text_type(cls.PUBLISH_REQUEST_INPROGRESS_STR)),
         )
 
     @classmethod
@@ -443,6 +446,8 @@ class SentryAppStatus(object):
             return cls.PUBLISHED_STR
         elif status == cls.INTERNAL:
             return cls.INTERNAL_STR
+        elif status == cls.PUBLISH_REQUEST_INPROGRESS:
+            return cls.PUBLISH_REQUEST_INPROGRESS_STR
 
 
 class SentryAppInstallationStatus(object):

src/sentry/mediators/sentry_apps/updater.py

@@ -64,6 +64,8 @@ class Updater(Mediator):
                 self.sentry_app.date_published = timezone.now()
             if self.status == SentryAppStatus.UNPUBLISHED_STR:
                 self.sentry_app.status = SentryAppStatus.UNPUBLISHED
+        if self.status == SentryAppStatus.PUBLISH_REQUEST_INPROGRESS_STR:
+            self.sentry_app.status = SentryAppStatus.PUBLISH_REQUEST_INPROGRESS
 
     @if_param("scopes")
     def _update_scopes(self):

src/sentry/migrations/0144_add_publish_request_inprogress_status.py

@@ -0,0 +1,38 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-12-15 02:28
+from __future__ import unicode_literals
+
+from django.db import migrations
+import sentry.db.models.fields.bounded
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = False
+
+    # This flag is used to decide whether to run this migration in a transaction or not.
+    # By default we prefer to run in a transaction, but for migrations where you want
+    # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
+    # want to create an index concurrently when adding one to an existing table.
+    atomic = True
+
+
+    dependencies = [
+        ('sentry', '0143_add_alerts_integrationfeature'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='sentryapp',
+            name='status',
+            field=sentry.db.models.fields.bounded.BoundedPositiveIntegerField(choices=[(0, 'unpublished'), (1, 'published'), (2, 'internal'), (3, 'publish_request_inprogress')], db_index=True, default=0),
+        ),
+    ]

src/sentry/models/sentryapp.py

@@ -158,6 +158,10 @@ class SentryApp(ParanoidModel, HasApiScopes):
     def is_internal(self):
         return self.status == SentryAppStatus.INTERNAL
 
+    @property
+    def is_publish_request_inprogress(self):
+        return self.status == SentryAppStatus.PUBLISH_REQUEST_INPROGRESS
+
     @property
     def slug_for_metrics(self):
         if self.is_internal:

src/sentry/static/sentry/app/components/modals/sentryAppPublishRequestModal.tsx

@@ -6,7 +6,7 @@ import PropTypes from 'prop-types';
 import {addErrorMessage, addSuccessMessage} from 'app/actionCreators/indicator';
 import {ModalRenderProps} from 'app/actionCreators/modal';
 import {PermissionChoice, SENTRY_APP_PERMISSIONS} from 'app/constants';
-import {t} from 'app/locale';
+import {t, tct} from 'app/locale';
 import space from 'app/styles/space';
 import {Scope, SentryApp} from 'app/types';
 import Form from 'app/views/settings/components/forms/form';
@@ -143,8 +143,13 @@ export default class SentryAppPublishRequestModal extends React.Component<Props>
     this.props.closeModal();
   };
 
-  handleSubmitError = () => {
-    addErrorMessage(t('Request to publish %s fails.', this.props.app.slug));
+  handleSubmitError = err => {
+    addErrorMessage(
+      tct('Request to publish [app] fails. [detail]', {
+        app: this.props.app.slug,
+        detail: err?.responseJSON?.detail,
+      })
+    );
   };
 
   render() {

tests/sentry/api/endpoints/test_sentry_app_publish_request.py

@@ -52,7 +52,7 @@ class SentryAppPublishRequestTest(APITestCase):
         self.login_as(user=self.user)
         response = self.client.post(self.url, format="json")
         assert response.status_code == 400
-        assert response.data["detail"] == "Cannot publish already published integration"
+        assert response.data["detail"] == "Cannot publish already published integration."
         send_mail.asssert_not_called()
 
     @mock.patch("sentry.utils.email.send_mail")
@@ -61,5 +61,5 @@ class SentryAppPublishRequestTest(APITestCase):
         self.login_as(user=self.user)
         response = self.client.post(self.url, format="json")
         assert response.status_code == 400
-        assert response.data["detail"] == "Cannot publish internal integration"
+        assert response.data["detail"] == "Cannot publish internal integration."
         send_mail.asssert_not_called()