Merge pull request #737 from python-discord/embed-required-fields

Remove Embed validation

7 files changed, 23 insertions, 410 deletions

diff --git a/pydis_site/apps/api/migrations/0019_deletedmessage.py b/pydis_site/apps/api/migrations/0019_deletedmessage.py
index 6b848d64..25d04434 100644
--- a/pydis_site/apps/api/migrations/0019_deletedmessage.py
+++ b/pydis_site/apps/api/migrations/0019_deletedmessage.py
@@ -18,7 +18,7 @@ class Migration(migrations.Migration):
                 ('id', models.BigIntegerField(help_text='The message ID as taken from Discord.', primary_key=True, serialize=False, validators=[django.core.validators.MinValueValidator(limit_value=0, message='Message IDs cannot be negative.')])),
                 ('channel_id', models.BigIntegerField(help_text='The channel ID that this message was sent in, taken from Discord.', validators=[django.core.validators.MinValueValidator(limit_value=0, message='Channel IDs cannot be negative.')])),
                 ('content', models.CharField(help_text='The content of this message, taken from Discord.', max_length=2000)),
-                ('embeds', django.contrib.postgres.fields.ArrayField(base_field=django.contrib.postgres.fields.jsonb.JSONField(validators=[pydis_site.apps.api.models.utils.validate_embed]), help_text='Embeds attached to this message.', size=None)),
+                ('embeds', django.contrib.postgres.fields.ArrayField(base_field=django.contrib.postgres.fields.jsonb.JSONField(validators=[]), help_text='Embeds attached to this message.', size=None)),
                 ('author', models.ForeignKey(help_text='The author of this message.', on_delete=django.db.models.deletion.CASCADE, to='api.User')),
                 ('deletion_context', models.ForeignKey(help_text='The deletion context this message is part of.', on_delete=django.db.models.deletion.CASCADE, to='api.MessageDeletionContext')),
             ],
diff --git a/pydis_site/apps/api/migrations/0051_allow_blank_message_embeds.py b/pydis_site/apps/api/migrations/0051_allow_blank_message_embeds.py
index 124c6a57..622f21d1 100644
--- a/pydis_site/apps/api/migrations/0051_allow_blank_message_embeds.py
+++ b/pydis_site/apps/api/migrations/0051_allow_blank_message_embeds.py
@@ -3,7 +3,6 @@
 import django.contrib.postgres.fields
 import django.contrib.postgres.fields.jsonb
 from django.db import migrations
-import pydis_site.apps.api.models.utils
 
 
 class Migration(migrations.Migration):
@@ -16,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='deletedmessage',
             name='embeds',
-            field=django.contrib.postgres.fields.ArrayField(base_field=django.contrib.postgres.fields.jsonb.JSONField(validators=[pydis_site.apps.api.models.utils.validate_embed]), blank=True, help_text='Embeds attached to this message.', size=None),
+            field=django.contrib.postgres.fields.ArrayField(base_field=django.contrib.postgres.fields.jsonb.JSONField(validators=[]), blank=True, help_text='Embeds attached to this message.', size=None),
         ),
     ]
diff --git a/pydis_site/apps/api/migrations/0077_use_generic_jsonfield.py b/pydis_site/apps/api/migrations/0077_use_generic_jsonfield.py
index 9e8f2fb9..95ef5850 100644
--- a/pydis_site/apps/api/migrations/0077_use_generic_jsonfield.py
+++ b/pydis_site/apps/api/migrations/0077_use_generic_jsonfield.py
@@ -2,7 +2,6 @@
 
 import django.contrib.postgres.fields
 from django.db import migrations, models
-import pydis_site.apps.api.models.utils
 
 
 class Migration(migrations.Migration):
@@ -20,6 +19,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='deletedmessage',
             name='embeds',
-            field=django.contrib.postgres.fields.ArrayField(base_field=models.JSONField(validators=[pydis_site.apps.api.models.utils.validate_embed]), blank=True, help_text='Embeds attached to this message.', size=None),
+            field=django.contrib.postgres.fields.ArrayField(base_field=models.JSONField(validators=[]), blank=True, help_text='Embeds attached to this message.', size=None),
         ),
     ]
diff --git a/pydis_site/apps/api/migrations/0083_remove_embed_validation.py b/pydis_site/apps/api/migrations/0083_remove_embed_validation.py
new file mode 100644
index 00000000..e835bb66
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0083_remove_embed_validation.py
@@ -0,0 +1,19 @@
+# Generated by Django 3.1.14 on 2022-06-30 09:41
+
+import django.contrib.postgres.fields
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0082_otn_allow_big_solidus'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='deletedmessage',
+            name='embeds',
+            field=django.contrib.postgres.fields.ArrayField(base_field=models.JSONField(), blank=True, help_text='Embeds attached to this message.', size=None),
+        ),
+    ]
diff --git a/pydis_site/apps/api/models/bot/message.py b/pydis_site/apps/api/models/bot/message.py
index bab3368d..bfa54721 100644
--- a/pydis_site/apps/api/models/bot/message.py
+++ b/pydis_site/apps/api/models/bot/message.py
@@ -7,7 +7,6 @@ from django.utils import timezone
 
 from pydis_site.apps.api.models.bot.user import User
 from pydis_site.apps.api.models.mixins import ModelReprMixin
-from pydis_site.apps.api.models.utils import validate_embed
 
 
 class Message(ModelReprMixin, models.Model):
@@ -48,9 +47,7 @@ class Message(ModelReprMixin, models.Model):
         blank=True
     )
     embeds = pgfields.ArrayField(
-        models.JSONField(
-            validators=(validate_embed,)
-        ),
+        models.JSONField(),
         blank=True,
         help_text="Embeds attached to this message."
     )
diff --git a/pydis_site/apps/api/models/utils.py b/pydis_site/apps/api/models/utils.py
deleted file mode 100644
index 859394d2..00000000
--- a/pydis_site/apps/api/models/utils.py
+++ /dev/null
@@ -1,172 +0,0 @@
-from collections.abc import Mapping
-from typing import Any, Dict
-
-from django.core.exceptions import ValidationError
-from django.core.validators import MaxLengthValidator, MinLengthValidator
-
-
-def is_bool_validator(value: Any) -> None:
-    """Validates if a given value is of type bool."""
-    if not isinstance(value, bool):
-        raise ValidationError(f"This field must be of type bool, not {type(value)}.")
-
-
-def validate_embed_fields(fields: dict) -> None:
-    """Raises a ValidationError if any of the given embed fields is invalid."""
-    field_validators = {
-        'name': (MaxLengthValidator(limit_value=256),),
-        'value': (MaxLengthValidator(limit_value=1024),),
-        'inline': (is_bool_validator,),
-    }
-
-    required_fields = ('name', 'value')
-
-    for field in fields:
-        if not isinstance(field, Mapping):
-            raise ValidationError("Embed fields must be a mapping.")
-
-        if not all(required_field in field for required_field in required_fields):
-            raise ValidationError(
-                f"Embed fields must contain the following fields: {', '.join(required_fields)}."
-            )
-
-        for field_name, value in field.items():
-            if field_name not in field_validators:
-                raise ValidationError(f"Unknown embed field field: {field_name!r}.")
-
-            for validator in field_validators[field_name]:
-                validator(value)
-
-
-def validate_embed_footer(footer: Dict[str, str]) -> None:
-    """Raises a ValidationError if the given footer is invalid."""
-    field_validators = {
-        'text': (
-            MinLengthValidator(
-                limit_value=1,
-                message="Footer text must not be empty."
-            ),
-            MaxLengthValidator(limit_value=2048)
-        ),
-        'icon_url': (),
-        'proxy_icon_url': ()
-    }
-
-    if not isinstance(footer, Mapping):
-        raise ValidationError("Embed footer must be a mapping.")
-
-    for field_name, value in footer.items():
-        if field_name not in field_validators:
-            raise ValidationError(f"Unknown embed footer field: {field_name!r}.")
-
-        for validator in field_validators[field_name]:
-            validator(value)
-
-
-def validate_embed_author(author: Any) -> None:
-    """Raises a ValidationError if the given author is invalid."""
-    field_validators = {
-        'name': (
-            MinLengthValidator(
-                limit_value=1,
-                message="Embed author name must not be empty."
-            ),
-            MaxLengthValidator(limit_value=256)
-        ),
-        'url': (),
-        'icon_url': (),
-        'proxy_icon_url': ()
-    }
-
-    if not isinstance(author, Mapping):
-        raise ValidationError("Embed author must be a mapping.")
-
-    for field_name, value in author.items():
-        if field_name not in field_validators:
-            raise ValidationError(f"Unknown embed author field: {field_name!r}.")
-
-        for validator in field_validators[field_name]:
-            validator(value)
-
-
-def validate_embed(embed: Any) -> None:
-    """
-    Validate a JSON document containing an embed as possible to send on Discord.
-
-    This attempts to rebuild the validation used by Discord
-    as well as possible by checking for various embed limits so we can
-    ensure that any embed we store here will also be accepted as a
-    valid embed by the Discord API.
-
-    Using this directly is possible, although not intended - you usually
-    stick this onto the `validators` keyword argument of model fields.
-
-    Example:
-
-        >>> from django.db import models
-        >>> from pydis_site.apps.api.models.utils import validate_embed
-        >>> class MyMessage(models.Model):
-        ...     embed = models.JSONField(
-        ...         validators=(
-        ...             validate_embed,
-        ...         )
-        ...     )
-        ...     # ...
-        ...
-
-    Args:
-        embed (Any):
-            A dictionary describing the contents of this embed.
-            See the official documentation for a full reference
-            of accepted keys by this dictionary:
-                https://discordapp.com/developers/docs/resources/channel#embed-object
-
-    Raises:
-        ValidationError:
-            In case the given embed is deemed invalid, a `ValidationError`
-            is raised which in turn will allow Django to display errors
-            as appropriate.
-    """
-    all_keys = {
-        'title', 'type', 'description', 'url', 'timestamp',
-        'color', 'footer', 'image', 'thumbnail', 'video',
-        'provider', 'author', 'fields'
-    }
-    one_required_of = {'description', 'fields', 'image', 'title', 'video'}
-    field_validators = {
-        'title': (
-            MinLengthValidator(
-                limit_value=1,
-                message="Embed title must not be empty."
-            ),
-            MaxLengthValidator(limit_value=256)
-        ),
-        'description': (MaxLengthValidator(limit_value=4096),),
-        'fields': (
-            MaxLengthValidator(limit_value=25),
-            validate_embed_fields
-        ),
-        'footer': (validate_embed_footer,),
-        'author': (validate_embed_author,)
-    }
-
-    if not embed:
-        raise ValidationError("Tag embed must not be empty.")
-
-    elif not isinstance(embed, Mapping):
-        raise ValidationError("Tag embed must be a mapping.")
-
-    elif not any(field in embed for field in one_required_of):
-        raise ValidationError(f"Tag embed must contain one of the fields {one_required_of}.")
-
-    for required_key in one_required_of:
-        if required_key in embed and not embed[required_key]:
-            raise ValidationError(f"Key {required_key!r} must not be empty.")
-
-    for field_name, value in embed.items():
-        if field_name not in all_keys:
-            raise ValidationError(f"Unknown field name: {field_name!r}")
-
-        if field_name in field_validators:
-            for validator in field_validators[field_name]:
-                validator(value)
diff --git a/pydis_site/apps/api/tests/test_validators.py b/pydis_site/apps/api/tests/test_validators.py
index 551cc2aa..8c46fcbc 100644
--- a/pydis_site/apps/api/tests/test_validators.py
+++ b/pydis_site/apps/api/tests/test_validators.py
@@ -5,7 +5,6 @@ from django.test import TestCase
 
 from ..models.bot.bot_setting import validate_bot_setting_name
 from ..models.bot.offensive_message import future_date_validator
-from ..models.utils import validate_embed
 
 
 REQUIRED_KEYS = (
@@ -22,234 +21,6 @@ class BotSettingValidatorTests(TestCase):
             validate_bot_setting_name('bad name')
 
 
-class TagEmbedValidatorTests(TestCase):
-    def test_rejects_non_mapping(self):
-        with self.assertRaises(ValidationError):
-            validate_embed('non-empty non-mapping')
-
-    def test_rejects_missing_required_keys(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'unknown': "key"
-            })
-
-    def test_rejects_one_correct_one_incorrect(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'provider': "??",
-                'title': ""
-            })
-
-    def test_rejects_empty_required_key(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': ''
-            })
-
-    def test_rejects_list_as_embed(self):
-        with self.assertRaises(ValidationError):
-            validate_embed([])
-
-    def test_rejects_required_keys_and_unknown_keys(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "the duck walked up to the lemonade stand",
-                'and': "he said to the man running the stand"
-            })
-
-    def test_rejects_too_long_title(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': 'a' * 257
-            })
-
-    def test_rejects_too_many_fields(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': [{} for _ in range(26)]
-            })
-
-    def test_rejects_too_long_description(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'description': 'd' * 4097
-            })
-
-    def test_allows_valid_embed(self):
-        validate_embed({
-            'title': "My embed",
-            'description': "look at my embed, my embed is amazing"
-        })
-
-    def test_allows_unvalidated_fields(self):
-        validate_embed({
-            'title': "My embed",
-            'provider': "what am I??"
-        })
-
-    def test_rejects_fields_as_list_of_non_mappings(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': ['abc']
-            })
-
-    def test_rejects_fields_with_unknown_fields(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': [
-                    {
-                        'what': "is this field"
-                    }
-                ]
-            })
-
-    def test_rejects_fields_with_too_long_name(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': [
-                    {
-                        'name': "a" * 257
-                    }
-                ]
-            })
-
-    def test_rejects_one_correct_one_incorrect_field(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': [
-                    {
-                        'name': "Totally valid",
-                        'value': "LOOK AT ME"
-                    },
-                    {
-                        'name': "Totally valid",
-                        'value': "LOOK AT ME",
-                        'oh': "what is this key?"
-                    }
-                ]
-            })
-
-    def test_rejects_missing_required_field_field(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': [
-                    {
-                        'name': "Totally valid",
-                        'inline': True,
-                    }
-                ]
-            })
-
-    def test_rejects_invalid_inline_field_field(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'fields': [
-                    {
-                        'name': "Totally valid",
-                        'value': "LOOK AT ME",
-                        'inline': "Totally not a boolean",
-                    }
-                ]
-            })
-
-    def test_allows_valid_fields(self):
-        validate_embed({
-            'fields': [
-                {
-                    'name': "valid",
-                    'value': "field",
-                },
-                {
-                    'name': "valid",
-                    'value': "field",
-                    'inline': False,
-                },
-                {
-                    'name': "valid",
-                    'value': "field",
-                    'inline': True,
-                },
-            ]
-        })
-
-    def test_rejects_footer_as_non_mapping(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'footer': []
-            })
-
-    def test_rejects_footer_with_unknown_fields(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'footer': {
-                    'duck': "quack"
-                }
-            })
-
-    def test_rejects_footer_with_empty_text(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'footer': {
-                    'text': ""
-                }
-            })
-
-    def test_allows_footer_with_proper_values(self):
-        validate_embed({
-            'title': "whatever",
-            'footer': {
-                'text': "django good"
-            }
-        })
-
-    def test_rejects_author_as_non_mapping(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'author': []
-            })
-
-    def test_rejects_author_with_unknown_field(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'author': {
-                    'field': "that is unknown"
-                }
-            })
-
-    def test_rejects_author_with_empty_name(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'author': {
-                    'name': ""
-                }
-            })
-
-    def test_rejects_author_with_one_correct_one_incorrect(self):
-        with self.assertRaises(ValidationError):
-            validate_embed({
-                'title': "whatever",
-                'author': {
-                    # Relies on "dictionary insertion order remembering" (D.I.O.R.) behaviour
-                    'url': "bobswebsite.com",
-                    'name': ""
-                }
-            })
-
-    def test_allows_author_with_proper_values(self):
-        validate_embed({
-            'title': "whatever",
-            'author': {
-                'name': "Bob"
-            }
-        })
-
-
 class OffensiveMessageValidatorsTests(TestCase):
     def test_accepts_future_date(self):
         future_date_validator(datetime(3000, 1, 1, tzinfo=timezone.utc))