Browse Source

ref(app-platform): Handle slug validation (#12523)

* ref(app-platform): Handle slug validation
MeredithAnya 6 years ago
parent
commit
cea59b98d7

+ 21 - 2
src/sentry/api/serializers/rest_framework/sentry_app.py

@@ -5,11 +5,30 @@ from jsonschema.exceptions import ValidationError as SchemaValidationError
 from rest_framework import serializers
 from rest_framework.serializers import Serializer, ValidationError
 
+from django.template.defaultfilters import slugify
 from sentry.api.validators.sentry_apps.schema import validate as validate_schema
-from sentry.models import ApiScopes
+from sentry.models import ApiScopes, SentryApp
 from sentry.models.sentryapp import VALID_EVENT_RESOURCES, REQUIRED_EVENT_PERMISSIONS
 
 
+class NameField(serializers.CharField):
+    def from_native(self, data):
+        rv = super(NameField, self).from_native(data)
+        if not rv:
+            return
+        if not self.is_valid_slug(rv):
+            raise ValidationError(u'Name {} is already taken, please use another.'.format(data))
+        return rv
+
+    def is_valid_slug(self, value):
+        slug = slugify(value)
+
+        if SentryApp.with_deleted.filter(slug=slug).exists():
+            return False
+
+        return True
+
+
 class ApiScopesField(serializers.WritableField):
     def validate(self, data):
         valid_scopes = ApiScopes()
@@ -41,7 +60,7 @@ class SchemaField(serializers.WritableField):
 
 
 class SentryAppSerializer(Serializer):
-    name = serializers.CharField()
+    name = NameField()
     scopes = ApiScopesField()
     events = EventListField(required=False)
     schema = SchemaField(required=False)

+ 1 - 0
src/sentry/db/models/paranoia.py

@@ -33,6 +33,7 @@ class ParanoidModel(Model):
 
     date_deleted = models.DateTimeField(null=True, blank=True)
     objects = ParanoidManager()
+    with_deleted = BaseManager()
 
     def delete(self):
         self.update(date_deleted=timezone.now())

+ 0 - 1
src/sentry/models/sentryapp.py

@@ -10,7 +10,6 @@ from django.db.models import Q
 from django.utils import timezone
 from django.template.defaultfilters import slugify
 from hashlib import sha256
-
 from sentry.constants import SentryAppStatus, SENTRY_APP_SLUG_MAX_LENGTH
 from sentry.models import Organization
 from sentry.models.apiscopes import HasApiScopes

+ 18 - 0
tests/sentry/api/endpoints/test_sentry_app_details.py

@@ -147,6 +147,24 @@ class UpdateSentryAppDetailsTest(SentryAppDetailsTest):
         assert response.data['uuid'] == self.unpublished_app.uuid
         assert response.data['webhookUrl'] == 'https://newurl.com'
 
+    @with_feature('organizations:sentry-apps')
+    def test_cannot_update_name_with_non_unique_slug(self):
+        from sentry.mediators import sentry_apps
+        self.login_as(user=self.user)
+        sentry_app = self.create_sentry_app(
+            name='Foo Bar',
+            organization=self.org,
+        )
+        sentry_apps.Destroyer.run(sentry_app=sentry_app)
+        response = self.client.put(
+            self.url,
+            data={'name': sentry_app.name},
+            format='json',
+        )
+        assert response.status_code == 400
+        assert response.data == \
+            {"name": ["Name Foo Bar is already taken, please use another."]}
+
     @with_feature('organizations:sentry-apps')
     def test_cannot_update_events_without_permissions(self):
         self.login_as(user=self.user)

+ 14 - 0
tests/sentry/api/endpoints/test_sentry_apps.py

@@ -130,6 +130,20 @@ class PostSentryAppsTest(SentryAppsTest):
         assert response.status_code == 201, response.content
         assert six.viewitems(expected) <= six.viewitems(json.loads(response.content))
 
+    @with_feature('organizations:sentry-apps')
+    def test_non_unique_app_slug(self):
+        from sentry.mediators import sentry_apps
+        self.login_as(user=self.user)
+        sentry_app = self.create_sentry_app(
+            name='Foo Bar',
+            organization=self.org,
+        )
+        sentry_apps.Destroyer.run(sentry_app=sentry_app)
+        response = self._post(**{'name': sentry_app.name})
+        assert response.status_code == 422
+        assert response.data == \
+            {"name": ["Name Foo Bar is already taken, please use another."]}
+
     @with_feature('organizations:sentry-apps')
     def test_cannot_create_app_without_correct_permissions(self):
         self.login_as(user=self.user)