From 1b5e7eba8c00f357595b7dccb909a608c7df216f Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Thu, 27 Jan 2022 20:54:43 +0000 Subject: Update metricity tests to test new voice_gate_blocked behaviour --- pydis_site/apps/api/tests/test_users.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 9b91380b..e21bb32b 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -1,10 +1,9 @@ -from unittest.mock import patch +from unittest.mock import Mock, patch -from django.core.exceptions import ObjectDoesNotExist from django.urls import reverse from .base import AuthenticatedAPITestCase -from ..models import Role, User +from ..models import Infraction, Role, User from ..models.bot.metricity import NotFoundError from ..viewsets.bot.user import UserListPagination @@ -424,7 +423,7 @@ class UserMetricityTests(AuthenticatedAPITestCase): self.assertCountEqual(response.json(), { "joined_at": joined_at, "total_messages": total_messages, - "voice_banned": False, + "voice_gate_blocked": False, "activity_blocks": total_blocks }) @@ -451,23 +450,36 @@ class UserMetricityTests(AuthenticatedAPITestCase): self.assertEqual(response.status_code, 404) def test_metricity_voice_banned(self): + queryset_with_values = Mock(spec=Infraction.objects) + queryset_with_values.filter.return_value = queryset_with_values + queryset_with_values.exists.return_value = True + + queryset_without_values = Mock(spec=Infraction.objects) + queryset_without_values.filter.return_value = queryset_without_values + queryset_without_values.exists.return_value = False cases = [ - {'exception': None, 'voice_banned': True}, - {'exception': ObjectDoesNotExist, 'voice_banned': False}, + {'voice_infractions': queryset_with_values, 'voice_gate_blocked': True}, + {'voice_infractions': queryset_without_values, 'voice_gate_blocked': False}, ] self.mock_metricity_user("foo", 1, 1, [["bar", 1]]) for case in cases: - with self.subTest(exception=case['exception'], voice_banned=case['voice_banned']): - with patch("pydis_site.apps.api.viewsets.bot.user.Infraction.objects.get") as p: - p.side_effect = case['exception'] + with self.subTest( + voice_infractions=case['voice_infractions'], + voice_gate_blocked=case['voice_gate_blocked'] + ): + with patch("pydis_site.apps.api.viewsets.bot.user.Infraction.objects.filter") as p: + p.return_value = case['voice_infractions'] url = reverse('api:bot:user-metricity-data', args=[0]) response = self.client.get(url) self.assertEqual(response.status_code, 200) - self.assertEqual(response.json()["voice_banned"], case["voice_banned"]) + self.assertEqual( + response.json()["voice_gate_blocked"], + case["voice_gate_blocked"] + ) def test_metricity_review_data(self): # Given -- cgit v1.2.3 From 6c9fb075dc52d5674f90698cb74138712f7d99e1 Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Sun, 30 Jan 2022 15:34:04 +0100 Subject: Allow searching users by username and discriminator A test case is added to demonstrate this functionality. Closes #578. Co-authored-by: Boris Muratov <8bee278@gmail.com> --- pydis_site/apps/api/tests/test_users.py | 43 ++++++++++++++++++++++++++++++++ pydis_site/apps/api/viewsets/bot/user.py | 5 ++++ 2 files changed, 48 insertions(+) (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index e21bb32b..5d10069d 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -1,3 +1,4 @@ +import random from unittest.mock import Mock, patch from django.urls import reverse @@ -520,3 +521,45 @@ class UserMetricityTests(AuthenticatedAPITestCase): self.metricity.total_messages.side_effect = NotFoundError() self.metricity.total_message_blocks.side_effect = NotFoundError() self.metricity.top_channel_activity.side_effect = NotFoundError() + + +class UserViewSetTests(AuthenticatedAPITestCase): + @classmethod + def setUpTestData(cls): + cls.searched_user = User.objects.create( + id=12095219, + name=f"Test user {random.randint(100, 1000)}", + discriminator=random.randint(1, 9999), + in_guild=True, + ) + cls.other_user = User.objects.create( + id=18259125, + name=f"Test user {random.randint(100, 1000)}", + discriminator=random.randint(1, 9999), + in_guild=True, + ) + + def test_search_lookup_of_wanted_user(self) -> None: + """Searching a user by name and discriminator should return that user.""" + url = reverse('api:bot:user-list') + params = { + 'username': self.searched_user.name, + 'discriminator': self.searched_user.discriminator, + } + response = self.client.get(url, params) + result = response.json() + self.assertEqual(result['count'], 1) + [user] = result['results'] + self.assertEqual(user['id'], self.searched_user.id) + + def test_search_lookup_of_unknown_user(self) -> None: + """Searching an unknown user should return no results.""" + url = reverse('api:bot:user-list') + params = { + 'username': "f-string enjoyer", + 'discriminator': 1245, + } + response = self.client.get(url, params) + result = response.json() + self.assertEqual(result['count'], 0) + self.assertEqual(result['results'], []) diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index a867a80f..3318b2b9 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -2,6 +2,7 @@ import typing from collections import OrderedDict from django.db.models import Q +from django_filters.rest_framework import DjangoFilterBackend from rest_framework import status from rest_framework.decorators import action from rest_framework.pagination import PageNumberPagination @@ -77,6 +78,8 @@ class UserViewSet(ModelViewSet): ... } #### Optional Query Parameters + - username: username to search for + - discriminator: discriminator to search for - page_size: number of Users in one page, defaults to 10,000 - page: page number @@ -233,6 +236,8 @@ class UserViewSet(ModelViewSet): serializer_class = UserSerializer queryset = User.objects.all().order_by("id") pagination_class = UserListPagination + filter_backends = (DjangoFilterBackend,) + filter_fields = ('name', 'discriminator') def get_serializer(self, *args, **kwargs) -> ModelSerializer: """Set Serializer many attribute to True if request body contains a list.""" -- cgit v1.2.3 From 15cac33dd74219a762d3c066efca383552f666b0 Mon Sep 17 00:00:00 2001 From: Rohan Reddy Alleti Date: Thu, 17 Feb 2022 02:55:28 +0530 Subject: Return random off topic names which are Active only (#644) Co-authored-by: Xithrius <15021300+Xithrius@users.noreply.github.com> --- .../apps/api/tests/test_off_topic_channel_names.py | 35 +++++++++++++++++----- .../api/viewsets/bot/off_topic_channel_name.py | 3 +- 2 files changed, 29 insertions(+), 9 deletions(-) (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/test_off_topic_channel_names.py b/pydis_site/apps/api/tests/test_off_topic_channel_names.py index 2d273756..34098c92 100644 --- a/pydis_site/apps/api/tests/test_off_topic_channel_names.py +++ b/pydis_site/apps/api/tests/test_off_topic_channel_names.py @@ -74,6 +74,9 @@ class ListTests(AuthenticatedAPITestCase): cls.test_name_3 = OffTopicChannelName.objects.create( name="frozen-with-iceman", used=True, active=False ) + cls.test_name_4 = OffTopicChannelName.objects.create( + name="xith-is-cool", used=True, active=True + ) def test_returns_name_in_list(self): """Return all off-topic channel names.""" @@ -86,28 +89,46 @@ class ListTests(AuthenticatedAPITestCase): { self.test_name.name, self.test_name_2.name, - self.test_name_3.name + self.test_name_3.name, + self.test_name_4.name } ) - def test_returns_two_items_with_random_items_param_set_to_2(self): - """Return not-used name instead used.""" + def test_returns_two_active_items_with_random_items_param_set_to_2(self): + """Return not-used active names instead used.""" url = reverse('api:bot:offtopicchannelname-list') response = self.client.get(f'{url}?random_items=2') self.assertEqual(response.status_code, 200) self.assertEqual(len(response.json()), 2) - self.assertEqual(set(response.json()), {self.test_name.name, self.test_name_2.name}) + self.assertTrue( + all( + item in (self.test_name.name, self.test_name_2.name, self.test_name_4.name) + for item in response.json() + ) + ) + + def test_returns_three_active_items_with_random_items_param_set_to_3(self): + """Return not-used active names instead used.""" + url = reverse('api:bot:offtopicchannelname-list') + response = self.client.get(f'{url}?random_items=3') + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()), 3) + self.assertEqual( + set(response.json()), + {self.test_name.name, self.test_name_2.name, self.test_name_4.name} + ) def test_running_out_of_names_with_random_parameter(self): - """Reset names `used` parameter to `False` when running out of names.""" + """Reset names `used` parameter to `False` when running out of active names.""" url = reverse('api:bot:offtopicchannelname-list') response = self.client.get(f'{url}?random_items=3') self.assertEqual(response.status_code, 200) self.assertEqual( set(response.json()), - {self.test_name.name, self.test_name_2.name, self.test_name_3.name} + {self.test_name.name, self.test_name_2.name, self.test_name_4.name} ) def test_returns_inactive_ot_names(self): @@ -129,7 +150,7 @@ class ListTests(AuthenticatedAPITestCase): self.assertEqual(response.status_code, 200) self.assertEqual( set(response.json()), - {self.test_name.name, self.test_name_2.name} + {self.test_name.name, self.test_name_2.name, self.test_name_4.name} ) diff --git a/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py b/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py index 78f8c340..d0519e86 100644 --- a/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py +++ b/pydis_site/apps/api/viewsets/bot/off_topic_channel_name.py @@ -108,7 +108,7 @@ class OffTopicChannelNameViewSet(ModelViewSet): 'random_items': ["Must be a positive integer."] }) - queryset = self.queryset.order_by('used', '?')[:random_count] + queryset = self.queryset.filter(active=True).order_by('used', '?')[:random_count] # When any name is used in our listing then this means we reached end of round # and we need to reset all other names `used` to False @@ -133,7 +133,6 @@ class OffTopicChannelNameViewSet(ModelViewSet): return Response(serialized.data) params = {} - if active_param := request.query_params.get("active"): params["active"] = active_param.lower() == "true" -- cgit v1.2.3 From 26e4f518c874cafdee594c08c01d610e88528dc7 Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Sun, 20 Feb 2022 17:43:54 +0100 Subject: Prevent race condition with duplicate infractions DRF's `UniqueTogetherValidator` validates uniqueness by querying the database before running the actual insert. This is not, has not, and will never be valid, unless you happen to run a single worker, on a single thread, and your single worker running on a single thread is the only client for the database, in which case it may be valid. For any other cases, it's invalid, and it has never been valid. PostgreSQL spits out an `IntegrityError` for us if we have a duplicate entry, and PostgreSQL is the only valid and correct thing to trust here. The `UniqueTogetherValidator` is removed, and an existing test case calling into this validator to check for uniqueness is removed. Furthermore, to work around a Django quirk, `transaction.atomic()` is added to prevent one `subTest` from messing with another. Closes #665. --- pydis_site/apps/api/serializers.py | 7 --- pydis_site/apps/api/tests/test_infractions.py | 77 +++++++++++--------------- pydis_site/apps/api/viewsets/bot/infraction.py | 18 ++++++ 3 files changed, 50 insertions(+), 52 deletions(-) (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 4a702d61..745aff42 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -156,13 +156,6 @@ class InfractionSerializer(ModelSerializer): 'hidden', 'dm_sent' ) - validators = [ - UniqueTogetherValidator( - queryset=Infraction.objects.filter(active=True), - fields=['user', 'type', 'active'], - message='This user already has an active infraction of this type.', - ) - ] def validate(self, attrs: dict) -> dict: """Validate data constraints for the given data and abort if it is invalid.""" diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index b3dd16ee..aa0604f6 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -3,6 +3,7 @@ from datetime import datetime as dt, timedelta, timezone from unittest.mock import patch from urllib.parse import quote +from django.db import transaction from django.db.utils import IntegrityError from django.urls import reverse @@ -492,6 +493,7 @@ class CreationTests(AuthenticatedAPITestCase): ) for infraction_type, hidden in restricted_types: + # https://stackoverflow.com/a/23326971 with self.subTest(infraction_type=infraction_type): invalid_infraction = { 'user': self.user.id, @@ -516,37 +518,38 @@ class CreationTests(AuthenticatedAPITestCase): 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) + with transaction.atomic(): + 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' + } - 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(), - { - 'non_field_errors': [ - 'This user already has an active infraction of this type.' - ] + # 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(), + { + 'non_field_errors': [ + 'This user already has an active infraction of this type.' + ] + } + ) 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.""" @@ -811,22 +814,6 @@ class SerializerTests(AuthenticatedAPITestCase): self.assertTrue(serializer.is_valid(), msg=serializer.errors) - def test_validation_error_if_active_duplicate(self): - self.create_infraction('ban', active=True) - instance = self.create_infraction('ban', active=False) - - data = {'active': True} - serializer = InfractionSerializer(instance, data=data, partial=True) - - if not serializer.is_valid(): - self.assertIn('non_field_errors', serializer.errors) - - code = serializer.errors['non_field_errors'][0].code - msg = f'Expected failure on unique validator but got {serializer.errors}' - self.assertEqual(code, 'unique', msg=msg) - else: # pragma: no cover - self.fail('Validation unexpectedly succeeded.') - def test_is_valid_for_new_active_infraction(self): self.create_infraction('ban', active=False) diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index 8a48ed1f..31e8ba40 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -1,5 +1,6 @@ from datetime import datetime +from django.db import IntegrityError from django.db.models import QuerySet from django.http.request import HttpRequest from django_filters.rest_framework import DjangoFilterBackend @@ -271,3 +272,20 @@ class InfractionViewSet( """ self.serializer_class = ExpandedInfractionSerializer return self.partial_update(*args, **kwargs) + + def create(self, request: HttpRequest, *args, **kwargs) -> Response: + """ + Create an infraction for a target user. + + Called by the Django Rest Framework in response to the corresponding HTTP request. + """ + try: + return super().create(request, *args, **kwargs) + except IntegrityError: + raise ValidationError( + { + 'non_field_errors': [ + 'This user already has an active infraction of this type.', + ] + } + ) -- cgit v1.2.3 From 66061855ca2098fae7e80fa2e1abb29f810495f1 Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Sat, 26 Feb 2022 14:36:53 +0100 Subject: Fix timezone awareness warnings Add a `warnings.warnings` clause to prevent these from being raised again in the future, and raise a full traceback if they don't. --- pydis_site/apps/api/tests/test_infractions.py | 4 ++-- pydis_site/apps/api/tests/test_models.py | 7 +++---- pydis_site/apps/api/tests/test_reminders.py | 17 +++++++++-------- pydis_site/apps/api/viewsets/bot/infraction.py | 11 +++++------ pydis_site/settings.py | 17 +++++++++++++++++ 5 files changed, 36 insertions(+), 20 deletions(-) (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index aa0604f6..f1107734 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -80,7 +80,7 @@ class InfractionTests(AuthenticatedAPITestCase): type='superstar', reason='This one doesn\'t matter anymore.', active=True, - expires_at=datetime.datetime.utcnow() + datetime.timedelta(hours=5) + expires_at=dt.now(timezone.utc) + datetime.timedelta(hours=5) ) cls.voiceban_expires_later = Infraction.objects.create( user_id=cls.user.id, @@ -88,7 +88,7 @@ class InfractionTests(AuthenticatedAPITestCase): type='voice_ban', reason='Jet engine mic', active=True, - expires_at=datetime.datetime.utcnow() + datetime.timedelta(days=5) + expires_at=dt.now(timezone.utc) + datetime.timedelta(days=5) ) def test_list_all(self): diff --git a/pydis_site/apps/api/tests/test_models.py b/pydis_site/apps/api/tests/test_models.py index 5c9ddea4..0fad467c 100644 --- a/pydis_site/apps/api/tests/test_models.py +++ b/pydis_site/apps/api/tests/test_models.py @@ -1,8 +1,7 @@ -from datetime import datetime as dt +from datetime import datetime as dt, timezone from django.core.exceptions import ValidationError from django.test import SimpleTestCase, TestCase -from django.utils import timezone from pydis_site.apps.api.models import ( DeletedMessage, @@ -41,7 +40,7 @@ class NitroMessageLengthTest(TestCase): self.context = MessageDeletionContext.objects.create( id=50, actor=self.user, - creation=dt.utcnow() + creation=dt.now(timezone.utc) ) def test_create(self): @@ -99,7 +98,7 @@ class StringDunderMethodTests(SimpleTestCase): name='shawn', discriminator=555, ), - creation=dt.utcnow() + creation=dt.now(timezone.utc) ), embeds=[] ), diff --git a/pydis_site/apps/api/tests/test_reminders.py b/pydis_site/apps/api/tests/test_reminders.py index 709685bc..e17569f0 100644 --- a/pydis_site/apps/api/tests/test_reminders.py +++ b/pydis_site/apps/api/tests/test_reminders.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timezone from django.forms.models import model_to_dict from django.urls import reverse @@ -91,7 +91,7 @@ class ReminderDeletionTests(AuthenticatedAPITestCase): cls.reminder = Reminder.objects.create( author=cls.author, content="Don't forget to set yourself a reminder", - expiration=datetime.utcnow().isoformat(), + expiration=datetime.now(timezone.utc), jump_url="https://www.decliningmentalfaculties.com", channel_id=123 ) @@ -122,7 +122,7 @@ class ReminderListTests(AuthenticatedAPITestCase): cls.reminder_one = Reminder.objects.create( author=cls.author, content="We should take Bikini Bottom, and push it somewhere else!", - expiration=datetime.utcnow().isoformat(), + expiration=datetime.now(timezone.utc), jump_url="https://www.icantseemyforehead.com", channel_id=123 ) @@ -130,16 +130,17 @@ class ReminderListTests(AuthenticatedAPITestCase): cls.reminder_two = Reminder.objects.create( author=cls.author, content="Gahhh-I love being purple!", - expiration=datetime.utcnow().isoformat(), + expiration=datetime.now(timezone.utc), jump_url="https://www.goofygoobersicecreampartyboat.com", channel_id=123, active=False ) + drf_format = '%Y-%m-%dT%H:%M:%S.%fZ' cls.rem_dict_one = model_to_dict(cls.reminder_one) - cls.rem_dict_one['expiration'] += 'Z' # Massaging a quirk of the response time format + cls.rem_dict_one['expiration'] = cls.rem_dict_one['expiration'].strftime(drf_format) cls.rem_dict_two = model_to_dict(cls.reminder_two) - cls.rem_dict_two['expiration'] += 'Z' # Massaging a quirk of the response time format + cls.rem_dict_two['expiration'] = cls.rem_dict_two['expiration'].strftime(drf_format) def test_reminders_in_full_list(self): url = reverse('api:bot:reminder-list') @@ -175,7 +176,7 @@ class ReminderRetrieveTests(AuthenticatedAPITestCase): cls.reminder = Reminder.objects.create( author=cls.author, content="Reminder content", - expiration=datetime.utcnow().isoformat(), + expiration=datetime.now(timezone.utc), jump_url="http://example.com/", channel_id=123 ) @@ -203,7 +204,7 @@ class ReminderUpdateTests(AuthenticatedAPITestCase): cls.reminder = Reminder.objects.create( author=cls.author, content="Squash those do-gooders", - expiration=datetime.utcnow().isoformat(), + expiration=datetime.now(timezone.utc), jump_url="https://www.decliningmentalfaculties.com", channel_id=123 ) diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index 7e7adbca..d82090aa 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -3,6 +3,7 @@ from datetime import datetime from django.db import IntegrityError from django.db.models import QuerySet from django.http.request import HttpRequest +from django.utils import timezone from django_filters.rest_framework import DjangoFilterBackend from rest_framework.decorators import action from rest_framework.exceptions import ValidationError @@ -184,20 +185,18 @@ class InfractionViewSet( filter_expires_after = self.request.query_params.get('expires_after') if filter_expires_after: try: - additional_filters['expires_at__gte'] = datetime.fromisoformat( - filter_expires_after - ) + expires_after_parsed = datetime.fromisoformat(filter_expires_after) except ValueError: raise ValidationError({'expires_after': ['failed to convert to datetime']}) + additional_filters['expires_at__gte'] = timezone.make_aware(expires_after_parsed) filter_expires_before = self.request.query_params.get('expires_before') if filter_expires_before: try: - additional_filters['expires_at__lte'] = datetime.fromisoformat( - filter_expires_before - ) + expires_before_parsed = datetime.fromisoformat(filter_expires_before) except ValueError: raise ValidationError({'expires_before': ['failed to convert to datetime']}) + additional_filters['expires_at__lte'] = timezone.make_aware(expires_before_parsed) if 'expires_at__lte' in additional_filters and 'expires_at__gte' in additional_filters: if additional_filters['expires_at__gte'] > additional_filters['expires_at__lte']: diff --git a/pydis_site/settings.py b/pydis_site/settings.py index 4f2b4aef..17f220f3 100644 --- a/pydis_site/settings.py +++ b/pydis_site/settings.py @@ -13,6 +13,7 @@ https://docs.djangoproject.com/en/2.1/ref/settings/ import os import secrets import sys +import warnings from pathlib import Path from socket import gethostbyname, gethostname @@ -53,10 +54,26 @@ if DEBUG: ALLOWED_HOSTS = env.list('ALLOWED_HOSTS', default=['*']) SECRET_KEY = "yellow polkadot bikini" # noqa: S105 + # Prevent verbose warnings emitted when passing a non-timezone aware + # datetime object to the database, whilst we have time zone support + # active. See the Django documentation for more details: + # https://docs.djangoproject.com/en/dev/topics/i18n/timezones/ + warnings.filterwarnings( + 'error', r"DateTimeField .* received a naive datetime", + RuntimeWarning, r'django\.db\.models\.fields', + ) + elif 'CI' in os.environ: ALLOWED_HOSTS = ['*'] SECRET_KEY = secrets.token_urlsafe(32) + # See above. We run with `CI=true`, but debug unset in GitHub Actions, + # so we also want to filter it there. + warnings.filterwarnings( + 'error', r"DateTimeField .* received a naive datetime", + RuntimeWarning, r'django\.db\.models\.fields', + ) + else: ALLOWED_HOSTS = env.list( 'ALLOWED_HOSTS', -- cgit v1.2.3 From cb52e60a631041a08edb213f41cca8befba4bf7e Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 16 Mar 2022 23:17:46 +0000 Subject: Add tests for custom BumpedThread impl --- pydis_site/apps/api/tests/test_bumped_threads.py | 63 ++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 pydis_site/apps/api/tests/test_bumped_threads.py (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/test_bumped_threads.py b/pydis_site/apps/api/tests/test_bumped_threads.py new file mode 100644 index 00000000..316e3f0b --- /dev/null +++ b/pydis_site/apps/api/tests/test_bumped_threads.py @@ -0,0 +1,63 @@ +from django.urls import reverse + +from .base import AuthenticatedAPITestCase +from ..models import BumpedThread + + +class UnauthedBumpedThreadAPITests(AuthenticatedAPITestCase): + def setUp(self): + super().setUp() + self.client.force_authenticate(user=None) + + def test_detail_lookup_returns_401(self): + url = reverse('api:bot:bumpedthread-detail', args=(1,)) + response = self.client.get(url) + + self.assertEqual(response.status_code, 401) + + def test_list_returns_401(self): + url = reverse('api:bot:bumpedthread-list') + response = self.client.get(url) + + self.assertEqual(response.status_code, 401) + + def test_create_returns_401(self): + url = reverse('api:bot:bumpedthread-list') + response = self.client.post(url, {"thread_id": 3}) + + self.assertEqual(response.status_code, 401) + + def test_delete_returns_401(self): + url = reverse('api:bot:bumpedthread-detail', args=(1,)) + response = self.client.delete(url) + + self.assertEqual(response.status_code, 401) + + +class BumpedThreadAPITests(AuthenticatedAPITestCase): + @classmethod + def setUpTestData(cls): + cls.thread1 = BumpedThread.objects.create( + thread_id=1234, + ) + + def test_returns_bumped_threads_as_flat_list(self): + url = reverse('api:bot:bumpedthread-list') + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), [1234]) + + def test_returns_204_for_existing_data(self): + url = reverse('api:bot:bumpedthread-detail', args=(1234,)) + + response = self.client.get(url) + self.assertEqual(response.status_code, 204) + self.assertEqual(response.content, b"") + + def test_returns_404_for_non_existing_data(self): + url = reverse('api:bot:bumpedthread-detail', args=(42,)) + + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json(), {"detail": "Not found."}) -- cgit v1.2.3 From d4f717ec186ffeedf7bdeb4991868160f1540b83 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Thu, 30 Jun 2022 10:48:33 +0100 Subject: Remove embed validators for deleted messages These caused more harm than they were worth, as every time Discord updated a behaviour of an embed we would get errors and need ot update the validation. Instead we should just accept whatever discord gives us as correct --- .../api/migrations/0083_remove_embed_validation.py | 19 ++ pydis_site/apps/api/models/bot/message.py | 5 +- pydis_site/apps/api/models/utils.py | 172 ---------------- pydis_site/apps/api/tests/test_validators.py | 229 --------------------- 4 files changed, 20 insertions(+), 405 deletions(-) create mode 100644 pydis_site/apps/api/migrations/0083_remove_embed_validation.py delete mode 100644 pydis_site/apps/api/models/utils.py (limited to 'pydis_site/apps/api/tests') 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)) -- cgit v1.2.3 From 7b40cd8143fea0beb195c6940bf2356970fc6958 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 11 Jul 2022 04:27:16 +0400 Subject: Drop Migration Tests The migration test suite was not really used, and it doesn't entirely make sense to test a constant unchanging process either. Its behavior is also very coupled with django's internals, and locks us into the current version and setup. Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/tests/migrations/__init__.py | 1 - pydis_site/apps/api/tests/migrations/base.py | 102 ----- .../migrations/test_active_infraction_migration.py | 496 --------------------- pydis_site/apps/api/tests/migrations/test_base.py | 135 ------ 4 files changed, 734 deletions(-) delete mode 100644 pydis_site/apps/api/tests/migrations/__init__.py delete mode 100644 pydis_site/apps/api/tests/migrations/base.py delete mode 100644 pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py delete mode 100644 pydis_site/apps/api/tests/migrations/test_base.py (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/migrations/__init__.py b/pydis_site/apps/api/tests/migrations/__init__.py deleted file mode 100644 index 38e42ffc..00000000 --- a/pydis_site/apps/api/tests/migrations/__init__.py +++ /dev/null @@ -1 +0,0 @@ -"""This submodule contains tests for functions used in data migrations.""" diff --git a/pydis_site/apps/api/tests/migrations/base.py b/pydis_site/apps/api/tests/migrations/base.py deleted file mode 100644 index 0c0a5bd0..00000000 --- a/pydis_site/apps/api/tests/migrations/base.py +++ /dev/null @@ -1,102 +0,0 @@ -"""Includes utilities for testing migrations.""" -from django.db import connection -from django.db.migrations.executor import MigrationExecutor -from django.test import TestCase - - -class MigrationsTestCase(TestCase): - """ - A `TestCase` subclass to test migration files. - - To be able to properly test a migration, we will need to inject data into the test database - before the migrations we want to test are applied, but after the older migrations have been - applied. This makes sure that we are testing "as if" we were actually applying this migration - to a database in the state it was in before introducing the new migration. - - To set up a MigrationsTestCase, create a subclass of this class and set the following - class-level attributes: - - - app: The name of the app that contains the migrations (e.g., `'api'`) - - migration_prior: The name* of the last migration file before the migrations you want to test - - migration_target: The name* of the last migration file we want to test - - *) Specify the file names without a path or the `.py` file extension. - - Additionally, overwrite the `setUpMigrationData` in the subclass to inject data into the - database before the migrations we want to test are applied. Please read the docstring of the - method for more information. An optional hook, `setUpPostMigrationData` is also provided. - """ - - # These class-level attributes should be set in classes that inherit from this base class. - app = None - migration_prior = None - migration_target = None - - @classmethod - def setUpTestData(cls): - """ - Injects data into the test database prior to the migration we're trying to test. - - This class methods reverts the test database back to the state of the last migration file - prior to the migrations we want to test. It will then allow the user to inject data into the - test database by calling the `setUpMigrationData` hook. After the data has been injected, it - will apply the migrations we want to test and call the `setUpPostMigrationData` hook. The - user can now test if the migration correctly migrated the injected test data. - """ - if not cls.app: - raise ValueError("The `app` attribute was not set.") - - if not cls.migration_prior or not cls.migration_target: - raise ValueError("Both ` migration_prior` and `migration_target` need to be set.") - - cls.migrate_from = [(cls.app, cls.migration_prior)] - cls.migrate_to = [(cls.app, cls.migration_target)] - - # Reverse to database state prior to the migrations we want to test - executor = MigrationExecutor(connection) - executor.migrate(cls.migrate_from) - - # Call the data injection hook with the current state of the project - old_apps = executor.loader.project_state(cls.migrate_from).apps - cls.setUpMigrationData(old_apps) - - # Run the migrations we want to test - executor = MigrationExecutor(connection) - executor.loader.build_graph() - executor.migrate(cls.migrate_to) - - # Save the project state so we're able to work with the correct model states - cls.apps = executor.loader.project_state(cls.migrate_to).apps - - # Call `setUpPostMigrationData` to potentially set up post migration data used in testing - cls.setUpPostMigrationData(cls.apps) - - @classmethod - def setUpMigrationData(cls, apps): - """ - Override this method to inject data into the test database before the migration is applied. - - This method will be called after setting up the database according to the migrations that - come before the migration(s) we are trying to test, but before the to-be-tested migration(s) - are applied. This allows us to simulate a database state just prior to the migrations we are - trying to test. - - To make sure we're creating objects according to the state the models were in at this point - in the migration history, use `apps.get_model(app_name: str, model_name: str)` to get the - appropriate model, e.g.: - - >>> Infraction = apps.get_model('api', 'Infraction') - """ - pass - - @classmethod - def setUpPostMigrationData(cls, apps): - """ - Set up additional test data after the target migration has been applied. - - Use `apps.get_model(app_name: str, model_name: str)` to get the correct instances of the - model classes: - - >>> Infraction = apps.get_model('api', 'Infraction') - """ - pass diff --git a/pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py b/pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py deleted file mode 100644 index 8dc29b34..00000000 --- a/pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py +++ /dev/null @@ -1,496 +0,0 @@ -"""Tests for the data migration in `filename`.""" -import logging -from collections import ChainMap, namedtuple -from datetime import timedelta -from itertools import count -from typing import Dict, Iterable, Type, Union - -from django.db.models import Q -from django.forms.models import model_to_dict -from django.utils import timezone - -from pydis_site.apps.api.models import Infraction, User -from .base import MigrationsTestCase - -log = logging.getLogger(__name__) -log.setLevel(logging.DEBUG) - - -InfractionHistory = namedtuple('InfractionHistory', ("user_id", "infraction_history")) - - -class InfractionFactory: - """Factory that creates infractions for a User instance.""" - - infraction_id = count(1) - user_id = count(1) - default_values = { - 'active': True, - 'expires_at': None, - 'hidden': False, - } - - @classmethod - def create( - cls, - actor: User, - infractions: Iterable[Dict[str, Union[str, int, bool]]], - infraction_model: Type[Infraction] = Infraction, - user_model: Type[User] = User, - ) -> InfractionHistory: - """ - Creates `infractions` for the `user` with the given `actor`. - - The `infractions` dictionary can contain the following fields: - - `type` (required) - - `active` (default: True) - - `expires_at` (default: None; i.e, permanent) - - `hidden` (default: False). - - The parameters `infraction_model` and `user_model` can be used to pass in an instance of - both model classes from a different migration/project state. - """ - user_id = next(cls.user_id) - user = user_model.objects.create( - id=user_id, - name=f"Infracted user {user_id}", - discriminator=user_id, - avatar_hash=None, - ) - infraction_history = [] - - for infraction in infractions: - infraction = dict(infraction) - infraction["id"] = next(cls.infraction_id) - infraction = ChainMap(infraction, cls.default_values) - new_infraction = infraction_model.objects.create( - user=user, - actor=actor, - type=infraction["type"], - reason=f"`{infraction['type']}` infraction (ID: {infraction['id']} of {user}", - active=infraction['active'], - hidden=infraction['hidden'], - expires_at=infraction['expires_at'], - ) - infraction_history.append(new_infraction) - - return InfractionHistory(user_id=user_id, infraction_history=infraction_history) - - -class InfractionFactoryTests(MigrationsTestCase): - """Tests for the InfractionFactory.""" - - app = "api" - migration_prior = "0046_reminder_jump_url" - migration_target = "0046_reminder_jump_url" - - @classmethod - def setUpPostMigrationData(cls, apps): - """Create a default actor for all infractions.""" - cls.infraction_model = apps.get_model('api', 'Infraction') - cls.user_model = apps.get_model('api', 'User') - - cls.actor = cls.user_model.objects.create( - id=9999, - name="Unknown Moderator", - discriminator=1040, - avatar_hash=None, - ) - - def test_infraction_factory_total_count(self): - """Does the test database hold as many infractions as we tried to create?""" - InfractionFactory.create( - actor=self.actor, - infractions=( - {'type': 'kick', 'active': False, 'hidden': False}, - {'type': 'ban', 'active': True, 'hidden': False}, - {'type': 'note', 'active': False, 'hidden': True}, - ), - infraction_model=self.infraction_model, - user_model=self.user_model, - ) - database_count = Infraction.objects.all().count() - self.assertEqual(3, database_count) - - def test_infraction_factory_multiple_users(self): - """Does the test database hold as many infractions as we tried to create?""" - for _user in range(5): - InfractionFactory.create( - actor=self.actor, - infractions=( - {'type': 'kick', 'active': False, 'hidden': True}, - {'type': 'ban', 'active': True, 'hidden': False}, - ), - infraction_model=self.infraction_model, - user_model=self.user_model, - ) - - # Check if infractions and users are recorded properly in the database - database_count = Infraction.objects.all().count() - self.assertEqual(database_count, 10) - - user_count = User.objects.all().count() - self.assertEqual(user_count, 5 + 1) - - def test_infraction_factory_sets_correct_fields(self): - """Does the InfractionFactory set the correct attributes?""" - infractions = ( - { - 'type': 'note', - 'active': False, - 'hidden': True, - 'expires_at': timezone.now() - }, - {'type': 'warning', 'active': False, 'hidden': False, 'expires_at': None}, - {'type': 'watch', 'active': False, 'hidden': True, 'expires_at': None}, - {'type': 'mute', 'active': True, 'hidden': False, 'expires_at': None}, - {'type': 'kick', 'active': True, 'hidden': True, 'expires_at': None}, - {'type': 'ban', 'active': True, 'hidden': False, 'expires_at': None}, - { - 'type': 'superstar', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() - }, - ) - - InfractionFactory.create( - actor=self.actor, - infractions=infractions, - infraction_model=self.infraction_model, - user_model=self.user_model, - ) - - for infraction in infractions: - with self.subTest(**infraction): - self.assertTrue(Infraction.objects.filter(**infraction).exists()) - - -class ActiveInfractionMigrationTests(MigrationsTestCase): - """ - Tests the active infraction data migration. - - The active infraction data migration should do the following things: - - 1. migrates all active notes, warnings, and kicks to an inactive status; - 2. migrates all users with multiple active infractions of a single type to have only one active - infraction of that type. The infraction with the longest duration stays active. - """ - - app = "api" - migration_prior = "0046_reminder_jump_url" - migration_target = "0047_active_infractions_migration" - - @classmethod - def setUpMigrationData(cls, apps): - """Sets up an initial database state that contains the relevant test cases.""" - # Fetch the Infraction and User model in the current migration state - cls.infraction_model = apps.get_model('api', 'Infraction') - cls.user_model = apps.get_model('api', 'User') - - cls.created_infractions = {} - - # Moderator that serves as actor for all infractions - cls.user_moderator = cls.user_model.objects.create( - id=9999, - name="Olivier de Vienne", - discriminator=1040, - avatar_hash=None, - ) - - # User #1: clean user with no infractions - cls.created_infractions["no infractions"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=[], - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #2: One inactive note infraction - cls.created_infractions["one inactive note"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'note', 'active': False, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #3: One active note infraction - cls.created_infractions["one active note"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'note', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #4: One active and one inactive note infraction - cls.created_infractions["one active and one inactive note"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'note', 'active': False, 'hidden': True}, - {'type': 'note', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #5: Once active note, one active kick, once active warning - cls.created_infractions["active note, kick, warning"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'note', 'active': True, 'hidden': True}, - {'type': 'kick', 'active': True, 'hidden': True}, - {'type': 'warning', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #6: One inactive ban and one active ban - cls.created_infractions["one inactive and one active ban"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'ban', 'active': False, 'hidden': True}, - {'type': 'ban', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #7: Two active permanent bans - cls.created_infractions["two active perm bans"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'ban', 'active': True, 'hidden': True}, - {'type': 'ban', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #8: Multiple active temporary bans - cls.created_infractions["multiple active temp bans"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=1) - }, - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=10) - }, - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=20) - }, - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=5) - }, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #9: One active permanent ban, two active temporary bans - cls.created_infractions["active perm, two active temp bans"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=10) - }, - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': None, - }, - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=7) - }, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #10: One inactive permanent ban, two active temporary bans - cls.created_infractions["one inactive perm ban, two active temp bans"] = ( - InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=10) - }, - { - 'type': 'ban', - 'active': False, - 'hidden': True, - 'expires_at': None, - }, - { - 'type': 'ban', - 'active': True, - 'hidden': True, - 'expires_at': timezone.now() + timedelta(days=7) - }, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - ) - - # User #11: Active ban, active mute, active superstar - cls.created_infractions["active ban, mute, and superstar"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'ban', 'active': True, 'hidden': True}, - {'type': 'mute', 'active': True, 'hidden': True}, - {'type': 'superstar', 'active': True, 'hidden': True}, - {'type': 'watch', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - # User #12: Multiple active bans, active mutes, active superstars - cls.created_infractions["multiple active bans, mutes, stars"] = InfractionFactory.create( - actor=cls.user_moderator, - infractions=( - {'type': 'ban', 'active': True, 'hidden': True}, - {'type': 'ban', 'active': True, 'hidden': True}, - {'type': 'ban', 'active': True, 'hidden': True}, - {'type': 'mute', 'active': True, 'hidden': True}, - {'type': 'mute', 'active': True, 'hidden': True}, - {'type': 'mute', 'active': True, 'hidden': True}, - {'type': 'superstar', 'active': True, 'hidden': True}, - {'type': 'superstar', 'active': True, 'hidden': True}, - {'type': 'superstar', 'active': True, 'hidden': True}, - {'type': 'watch', 'active': True, 'hidden': True}, - {'type': 'watch', 'active': True, 'hidden': True}, - {'type': 'watch', 'active': True, 'hidden': True}, - ), - infraction_model=cls.infraction_model, - user_model=cls.user_model, - ) - - def test_all_never_active_types_became_inactive(self): - """Are all infractions of a non-active type inactive after the migration?""" - inactive_type_query = Q(type="note") | Q(type="warning") | Q(type="kick") - self.assertFalse( - self.infraction_model.objects.filter(inactive_type_query, active=True).exists() - ) - - def test_migration_left_clean_user_without_infractions(self): - """Do users without infractions have no infractions after the migration?""" - user_id, infraction_history = self.created_infractions["no infractions"] - self.assertFalse( - self.infraction_model.objects.filter(user__id=user_id).exists() - ) - - def test_migration_left_user_with_inactive_note_untouched(self): - """Did the migration leave users with only an inactive note untouched?""" - user_id, infraction_history = self.created_infractions["one inactive note"] - inactive_note = infraction_history[0] - self.assertTrue( - self.infraction_model.objects.filter(**model_to_dict(inactive_note)).exists() - ) - - def test_migration_only_touched_active_field_of_active_note(self): - """Does the migration only change the `active` field?""" - user_id, infraction_history = self.created_infractions["one active note"] - note = model_to_dict(infraction_history[0]) - note['active'] = False - self.assertTrue( - self.infraction_model.objects.filter(**note).exists() - ) - - def test_migration_only_touched_active_field_of_active_note_left_inactive_untouched(self): - """Does the migration only change the `active` field of active notes?""" - user_id, infraction_history = self.created_infractions["one active and one inactive note"] - for note in infraction_history: - with self.subTest(active=note.active): - note = model_to_dict(note) - note['active'] = False - self.assertTrue( - self.infraction_model.objects.filter(**note).exists() - ) - - def test_migration_migrates_all_nonactive_types_to_inactive(self): - """Do we set the `active` field of all non-active infractions to `False`?""" - user_id, infraction_history = self.created_infractions["active note, kick, warning"] - self.assertFalse( - self.infraction_model.objects.filter(user__id=user_id, active=True).exists() - ) - - def test_migration_leaves_user_with_one_active_ban_untouched(self): - """Do we leave a user with one active and one inactive ban untouched?""" - user_id, infraction_history = self.created_infractions["one inactive and one active ban"] - for infraction in infraction_history: - with self.subTest(active=infraction.active): - self.assertTrue( - self.infraction_model.objects.filter(**model_to_dict(infraction)).exists() - ) - - def test_migration_turns_double_active_perm_ban_into_single_active_perm_ban(self): - """Does the migration turn two active permanent bans into one active permanent ban?""" - user_id, infraction_history = self.created_infractions["two active perm bans"] - active_count = self.infraction_model.objects.filter(user__id=user_id, active=True).count() - self.assertEqual(active_count, 1) - - def test_migration_leaves_temporary_ban_with_longest_duration_active(self): - """Does the migration turn two active permanent bans into one active permanent ban?""" - user_id, infraction_history = self.created_infractions["multiple active temp bans"] - active_ban = self.infraction_model.objects.get(user__id=user_id, active=True) - self.assertEqual(active_ban.expires_at, infraction_history[2].expires_at) - - def test_migration_leaves_permanent_ban_active(self): - """Does the migration leave the permanent ban active?""" - user_id, infraction_history = self.created_infractions["active perm, two active temp bans"] - active_ban = self.infraction_model.objects.get(user__id=user_id, active=True) - self.assertIsNone(active_ban.expires_at) - - def test_migration_leaves_longest_temp_ban_active_with_inactive_permanent_ban(self): - """Does the longest temp ban stay active, even with an inactive perm ban present?""" - user_id, infraction_history = self.created_infractions[ - "one inactive perm ban, two active temp bans" - ] - active_ban = self.infraction_model.objects.get(user__id=user_id, active=True) - self.assertEqual(active_ban.expires_at, infraction_history[0].expires_at) - - def test_migration_leaves_all_active_types_active_if_one_of_each_exists(self): - """Do all active infractions stay active if only one of each is present?""" - user_id, infraction_history = self.created_infractions["active ban, mute, and superstar"] - active_count = self.infraction_model.objects.filter(user__id=user_id, active=True).count() - self.assertEqual(active_count, 4) - - def test_migration_reduces_all_active_types_to_a_single_active_infraction(self): - """Do we reduce all of the infraction types to one active infraction?""" - user_id, infraction_history = self.created_infractions["multiple active bans, mutes, stars"] - active_infractions = self.infraction_model.objects.filter(user__id=user_id, active=True) - self.assertEqual(len(active_infractions), 4) - types_observed = [infraction.type for infraction in active_infractions] - - for infraction_type in ('ban', 'mute', 'superstar', 'watch'): - with self.subTest(type=infraction_type): - self.assertIn(infraction_type, types_observed) diff --git a/pydis_site/apps/api/tests/migrations/test_base.py b/pydis_site/apps/api/tests/migrations/test_base.py deleted file mode 100644 index f69bc92c..00000000 --- a/pydis_site/apps/api/tests/migrations/test_base.py +++ /dev/null @@ -1,135 +0,0 @@ -import logging -from unittest.mock import call, patch - -from django.db.migrations.loader import MigrationLoader -from django.test import TestCase - -from .base import MigrationsTestCase, connection - -log = logging.getLogger(__name__) - - -class SpanishInquisition(MigrationsTestCase): - app = "api" - migration_prior = "scragly" - migration_target = "kosa" - - -@patch("pydis_site.apps.api.tests.migrations.base.MigrationExecutor") -class MigrationsTestCaseNoSideEffectsTests(TestCase): - """Tests the MigrationTestCase class with actual migration side effects disabled.""" - - def setUp(self): - """Set up an instance of MigrationsTestCase for use in tests.""" - self.test_case = SpanishInquisition() - - def test_missing_app_class_raises_value_error(self, _migration_executor): - """A MigrationsTestCase subclass should set the class-attribute `app`.""" - class Spam(MigrationsTestCase): - pass - - spam = Spam() - with self.assertRaises(ValueError, msg="The `app` attribute was not set."): - spam.setUpTestData() - - def test_missing_migration_class_attributes_raise_value_error(self, _migration_executor): - """A MigrationsTestCase subclass should set both `migration_prior` and `migration_target`""" - class Eggs(MigrationsTestCase): - app = "api" - migration_target = "lemon" - - class Bacon(MigrationsTestCase): - app = "api" - migration_prior = "mark" - - instances = (Eggs(), Bacon()) - - exception_message = "Both ` migration_prior` and `migration_target` need to be set." - for instance in instances: - with self.subTest( - migration_prior=instance.migration_prior, - migration_target=instance.migration_target, - ): - with self.assertRaises(ValueError, msg=exception_message): - instance.setUpTestData() - - @patch(f"{__name__}.SpanishInquisition.setUpMigrationData") - @patch(f"{__name__}.SpanishInquisition.setUpPostMigrationData") - def test_migration_data_hooks_are_called_once(self, pre_hook, post_hook, _migration_executor): - """The `setUpMigrationData` and `setUpPostMigrationData` hooks should be called once.""" - self.test_case.setUpTestData() - for hook in (pre_hook, post_hook): - with self.subTest(hook=repr(hook)): - hook.assert_called_once() - - def test_migration_executor_is_instantiated_twice(self, migration_executor): - """The `MigrationExecutor` should be instantiated with the database connection twice.""" - self.test_case.setUpTestData() - - expected_args = [call(connection), call(connection)] - self.assertEqual(migration_executor.call_args_list, expected_args) - - def test_project_state_is_loaded_for_correct_migration_files_twice(self, migration_executor): - """The `project_state` should first be loaded with `migrate_from`, then `migrate_to`.""" - self.test_case.setUpTestData() - - expected_args = [call(self.test_case.migrate_from), call(self.test_case.migrate_to)] - self.assertEqual(migration_executor().loader.project_state.call_args_list, expected_args) - - def test_loader_build_graph_gets_called_once(self, migration_executor): - """We should rebuild the migration graph before applying the second set of migrations.""" - self.test_case.setUpTestData() - - migration_executor().loader.build_graph.assert_called_once() - - def test_migration_executor_migrate_method_is_called_correctly_twice(self, migration_executor): - """The migrate method of the executor should be called twice with the correct arguments.""" - self.test_case.setUpTestData() - - self.assertEqual(migration_executor().migrate.call_count, 2) - calls = [call([('api', 'scragly')]), call([('api', 'kosa')])] - migration_executor().migrate.assert_has_calls(calls) - - -class LifeOfBrian(MigrationsTestCase): - app = "api" - migration_prior = "0046_reminder_jump_url" - migration_target = "0048_add_infractions_unique_constraints_active" - - @classmethod - def log_last_migration(cls): - """Parses the applied migrations dictionary to log the last applied migration.""" - loader = MigrationLoader(connection) - api_migrations = [ - migration for app, migration in loader.applied_migrations if app == cls.app - ] - last_migration = max(api_migrations, key=lambda name: int(name[:4])) - log.info(f"The last applied migration: {last_migration}") - - @classmethod - def setUpMigrationData(cls, apps): - """Method that logs the last applied migration at this point.""" - cls.log_last_migration() - - @classmethod - def setUpPostMigrationData(cls, apps): - """Method that logs the last applied migration at this point.""" - cls.log_last_migration() - - -class MigrationsTestCaseMigrationTest(TestCase): - """Tests if `MigrationsTestCase` travels to the right points in the migration history.""" - - def test_migrations_test_case_travels_to_correct_migrations_in_history(self): - """The test case should first revert to `migration_prior`, then go to `migration_target`.""" - brian = LifeOfBrian() - - with self.assertLogs(log, level=logging.INFO) as logs: - brian.setUpTestData() - - self.assertEqual(len(logs.records), 2) - - for time_point, record in zip(("migration_prior", "migration_target"), logs.records): - with self.subTest(time_point=time_point): - message = f"The last applied migration: {getattr(brian, time_point)}" - self.assertEqual(record.getMessage(), message) -- cgit v1.2.3 From ee25921da752d51215598bcd3eb5fd5ab74a4a46 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Mon, 11 Jul 2022 05:00:40 +0400 Subject: Remove Message Model Test The message model was tested by instantiating and confirming it has a string representation, but instantiating abstract models is undefined behavior, and can break with future versions of django. The behavior of the test is redundant anyway, since an abstract model wouldn't exist in isolation, and the desired behavior is confirmed by inheritors. Signed-off-by: Hassan Abouelela --- pydis_site/apps/api/tests/test_models.py | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'pydis_site/apps/api/tests') diff --git a/pydis_site/apps/api/tests/test_models.py b/pydis_site/apps/api/tests/test_models.py index 0fad467c..c07d59cd 100644 --- a/pydis_site/apps/api/tests/test_models.py +++ b/pydis_site/apps/api/tests/test_models.py @@ -7,7 +7,6 @@ from pydis_site.apps.api.models import ( DeletedMessage, DocumentationLink, Infraction, - Message, MessageDeletionContext, Nomination, NominationEntry, @@ -116,17 +115,6 @@ class StringDunderMethodTests(SimpleTestCase): colour=0x5, permissions=0, position=10, ), - Message( - id=45, - author=User( - id=444, - name='bill', - discriminator=5, - ), - channel_id=666, - content="wooey", - embeds=[] - ), MessageDeletionContext( actor=User( id=5555, -- cgit v1.2.3