From ec2ae9d1ef38d44cfeeb4abbe3bda035afe93e2b Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 4 Oct 2019 08:13:18 +0200 Subject: Add validation rules to Infraction serializer https://github.com/python-discord/site/issues/273 This commit adds validation rules to the Infraction serializer that validate if a given infraction should be accepted based on its status of being considered `active`. If the validation fails, the API will reject the request and return a 400 status. Specifically, this validator checks that: - infractions that can never be active do not have `active=True` set; - a user can never receive a second active infraction of the same type. Tests have been added to `test_infractions.py` to ensure that the validators work as expected. This commit implements the first part of #273 --- pydis_site/apps/api/models/bot/user.py | 2 +- pydis_site/apps/api/serializers.py | 13 ++++ pydis_site/apps/api/tests/test_infractions.py | 85 +++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/models/bot/user.py b/pydis_site/apps/api/models/bot/user.py index 21617dc4..5140d2bf 100644 --- a/pydis_site/apps/api/models/bot/user.py +++ b/pydis_site/apps/api/models/bot/user.py @@ -50,7 +50,7 @@ class User(ModelReprMixin, models.Model): def __str__(self): """Returns the name and discriminator for the current user, for display purposes.""" - return f"{self.name}#{self.discriminator}" + return f"{self.name}#{self.discriminator:0>4}" @property def top_role(self) -> Role: diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 326e20e1..784bb614 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -1,4 +1,5 @@ """Converters from Django models to data interchange formats and back.""" +import logging from rest_framework.serializers import ModelSerializer, PrimaryKeyRelatedField, ValidationError from rest_framework_bulk import BulkSerializerMixin @@ -12,6 +13,8 @@ from .models import ( Tag, User ) +log = logging.getLogger(__name__) + class BotSettingSerializer(ModelSerializer): """A class providing (de-)serialization of `BotSetting` instances.""" @@ -110,6 +113,16 @@ class InfractionSerializer(ModelSerializer): """Validate data constraints for the given data and abort if it is invalid.""" infr_type = attrs.get('type') + active = attrs.get('active') + if active and infr_type in ('note', 'warning', 'kick'): + raise ValidationError({'active': [f'{infr_type} infractions cannot be active.']}) + + user = attrs.get('user') + if active and Infraction.objects.filter(user=user, type=infr_type, active=True).exists(): + raise ValidationError( + {'active': [f'This user already has an active {infr_type} infraction']} + ) + expires_at = attrs.get('expires_at') if expires_at and infr_type in ('kick', 'warning'): raise ValidationError({'expires_at': [f'{infr_type} infractions cannot expire.']}) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index c58c32e2..8279b6a6 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -305,6 +305,91 @@ class CreationTests(APISubdomainTestCase): 'hidden': [f'{data["type"]} infractions must be hidden.'] }) + def test_returns_400_for_active_infraction_of_type_that_cannot_be_active(self): + """Test if the API rejects active infractions for types that cannot be active.""" + url = reverse('bot:infraction-list', host='api') + restricted_types = ('note', 'warning', 'kick') + + for infraction_type in restricted_types: + with self.subTest(infraction_type=infraction_type): + invalid_infraction = { + 'user': self.user.id, + 'actor': self.user.id, + 'type': infraction_type, + 'reason': 'Take me on!', + 'hidden': True, + 'active': True, + 'expires_at': '2019-10-04T12:52:00+00:00' + } + response = self.client.post(url, data=invalid_infraction) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'active': [f'{infraction_type} infractions cannot be active.'] + }) + + def test_returns_400_for_second_active_infraction_of_the_same_type(self): + """Test if the API rejects a second active infraction of the same type for a given user.""" + url = reverse('bot:infraction-list', host='api') + active_infraction_types = ('mute', 'ban', 'superstar') + + for infraction_type in active_infraction_types: + with self.subTest(infraction_type=infraction_type): + first_active_infraction = { + 'user': self.user.id, + 'actor': self.user.id, + 'type': infraction_type, + 'reason': 'Take me on!', + 'active': True, + 'expires_at': '2019-10-04T12:52:00+00:00' + } + + # Post the first active infraction of a type and confirm it's accepted. + first_response = self.client.post(url, data=first_active_infraction) + self.assertEqual(first_response.status_code, 201) + + second_active_infraction = { + 'user': self.user.id, + 'actor': self.user.id, + 'type': infraction_type, + 'reason': 'Take on me!', + 'active': True, + 'expires_at': '2019-10-04T12:52:00+00:00' + } + second_response = self.client.post(url, data=second_active_infraction) + self.assertEqual(second_response.status_code, 400) + self.assertEqual(second_response.json(), { + 'active': [f'This user already has an active {infraction_type} infraction'] + }) + + def test_returns_201_for_second_active_infraction_of_different_type(self): + """Test if the API accepts a second active infraction of a different type than the first.""" + url = reverse('bot:infraction-list', host='api') + first_active_infraction = { + 'user': self.user.id, + 'actor': self.user.id, + 'type': 'mute', + 'reason': 'Be silent!', + 'hidden': True, + 'active': True, + 'expires_at': '2019-10-04T12:52:00+00:00' + } + second_active_infraction = { + 'user': self.user.id, + 'actor': self.user.id, + 'type': 'ban', + 'reason': 'Be gone!', + 'hidden': True, + 'active': True, + 'expires_at': '2019-10-05T12:52:00+00:00' + } + # Post the first active infraction of a type and confirm it's accepted. + first_response = self.client.post(url, data=first_active_infraction) + self.assertEqual(first_response.status_code, 201) + + # Post the first active infraction of a type and confirm it's accepted. + second_response = self.client.post(url, data=second_active_infraction) + self.assertEqual(second_response.status_code, 201) + class ExpandedTests(APISubdomainTestCase): @classmethod -- cgit v1.2.3