From 7df4b0579156022d3f1269de638a0d64a0f812a8 Mon Sep 17 00:00:00 2001 From: bast Date: Fri, 21 May 2021 06:11:59 -0700 Subject: Add permanent, types, and expires_at filters to bot/infractions endpoint --- pydis_site/apps/api/viewsets/bot/infraction.py | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index bd512ddd..b0c7d332 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -1,3 +1,6 @@ +from datetime import datetime + +from django.db.models import QuerySet from django.http.request import HttpRequest from django_filters.rest_framework import DjangoFilterBackend from rest_framework.decorators import action @@ -43,8 +46,12 @@ class InfractionViewSet( - **offset** `int`: the initial index from which to return the results (default 0) - **search** `str`: regular expression applied to the infraction's reason - **type** `str`: the type of the infraction + - **types** `str`: comma separated sequence of types to filter for - **user__id** `int`: snowflake of the user to which the infraction was applied - **ordering** `str`: comma-separated sequence of fields to order the returned results + - **permanent** `bool`: whether or not to retrieve permanent infractions (default True) + - **expires_after** `isodatetime`: the earliest expires_at time to return infractions for + - **expires_before** `isodatetime`: the latest expires_at time to return infractions for Invalid query parameters are ignored. @@ -156,6 +163,41 @@ class InfractionViewSet( return Response(serializer.data) + def get_queryset(self) -> QuerySet: + """ + Called to fetch the initial queryset, used to implement some of the more complex filters. + + This provides the `permanent` and the `expires_gte` and `expires_lte` options. + """ + filter_permanent = self.request.query_params.get('permanent') + additional_filters = {} + if filter_permanent is not None: + additional_filters['expires_at__isnull'] = filter_permanent.lower() == 'true' + + 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 + ) + except ValueError: + raise ValidationError({'expires_after': ['failed to convert to datetime']}) + + 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 + ) + except ValueError: + raise ValidationError({'expires_before': ['failed to convert to datetime']}) + + filter_types = self.request.query_params.get('types') + if filter_types: + additional_filters['type__in'] = [i.strip() for i in filter_types.split(",")] + + return self.queryset.filter(**additional_filters) + @action(url_path='expanded', detail=False) def list_expanded(self, *args, **kwargs) -> Response: """ -- cgit v1.2.3 From ea5a20ae5abb532edc2a852ff0a3bd94fde01a74 Mon Sep 17 00:00:00 2001 From: bast Date: Fri, 21 May 2021 06:48:18 -0700 Subject: Add tests for new filters on bot/infractions endpoint --- pydis_site/apps/api/tests/test_infractions.py | 76 ++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index 82b497aa..16c4d294 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -1,3 +1,4 @@ +import datetime from datetime import datetime as dt, timedelta, timezone from unittest.mock import patch from urllib.parse import quote @@ -16,7 +17,7 @@ class UnauthenticatedTests(APISubdomainTestCase): self.client.force_authenticate(user=None) def test_detail_lookup_returns_401(self): - url = reverse('bot:infraction-detail', args=(5,), host='api') + url = reverse('bot:infraction-detail', args=(6,), host='api') response = self.client.get(url) self.assertEqual(response.status_code, 401) @@ -34,7 +35,7 @@ class UnauthenticatedTests(APISubdomainTestCase): self.assertEqual(response.status_code, 401) def test_partial_update_returns_401(self): - url = reverse('bot:infraction-detail', args=(5,), host='api') + url = reverse('bot:infraction-detail', args=(6,), host='api') response = self.client.patch(url, data={'reason': 'Have a nice day.'}) self.assertEqual(response.status_code, 401) @@ -44,7 +45,7 @@ class InfractionTests(APISubdomainTestCase): @classmethod def setUpTestData(cls): cls.user = User.objects.create( - id=5, + id=6, name='james', discriminator=1, ) @@ -64,6 +65,22 @@ class InfractionTests(APISubdomainTestCase): reason='James is an ass, and we won\'t be working with him again.', active=False ) + cls.mute_permanent = Infraction.objects.create( + user_id=cls.user.id, + actor_id=cls.user.id, + type='mute', + reason='He has a filthy mouth and I am his soap.', + active=True, + expires_at=None + ) + cls.superstar_expires_soon = Infraction.objects.create( + user_id=cls.user.id, + actor_id=cls.user.id, + type='superstar', + reason='This one doesn\'t matter anymore.', + active=True, + expires_at=datetime.datetime.utcnow() + datetime.timedelta(hours=5) + ) def test_list_all(self): """Tests the list-view, which should be ordered by inserted_at (newest first).""" @@ -73,9 +90,11 @@ class InfractionTests(APISubdomainTestCase): self.assertEqual(response.status_code, 200) infractions = response.json() - self.assertEqual(len(infractions), 2) - self.assertEqual(infractions[0]['id'], self.ban_inactive.id) - self.assertEqual(infractions[1]['id'], self.ban_hidden.id) + self.assertEqual(len(infractions), 4) + self.assertEqual(infractions[0]['id'], self.superstar_expires_soon.id) + self.assertEqual(infractions[1]['id'], self.mute_permanent.id) + self.assertEqual(infractions[2]['id'], self.ban_inactive.id) + self.assertEqual(infractions[3]['id'], self.ban_hidden.id) def test_filter_search(self): url = reverse('bot:infraction-list', host='api') @@ -98,6 +117,51 @@ class InfractionTests(APISubdomainTestCase): self.assertEqual(len(infractions), 1) self.assertEqual(infractions[0]['id'], self.ban_hidden.id) + def test_filter_permanent_false(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?type=mute&permanent=false') + + self.assertEqual(response.status_code, 200) + infractions = response.json() + + self.assertEqual(len(infractions), 0) + + def test_filter_permanent_true(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?type=mute&permanent=true') + + self.assertEqual(response.status_code, 200) + infractions = response.json() + + self.assertEqual(infractions[0]['id'], self.mute_permanent.id) + + def test_filter_after(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) + response = self.client.get(f'{url}?type=superstar&expires_after={target_time.isoformat()}') + + self.assertEqual(response.status_code, 200) + infractions = response.json() + self.assertEqual(len(infractions), 0) + + def test_filter_before(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) + response = self.client.get(f'{url}?type=superstar&expires_before={target_time.isoformat()}') + + self.assertEqual(response.status_code, 200) + infractions = response.json() + self.assertEqual(len(infractions), 1) + self.assertEqual(infractions[0]['id'], self.superstar_expires_soon.id) + + def test_filter_manytypes(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?types=mute,ban') + + self.assertEqual(response.status_code, 200) + infractions = response.json() + self.assertEqual(len(infractions), 3) + def test_returns_empty_for_no_match(self): url = reverse('bot:infraction-list', host='api') response = self.client.get(f'{url}?type=ban&search=poop') -- cgit v1.2.3 From f53e275e42161b2cf3df49dd6171831edae1db1e Mon Sep 17 00:00:00 2001 From: bast Date: Sat, 22 May 2021 12:57:18 -0700 Subject: Add a test for ordering infractions returned from the api As this is used in the new bot infraction rescheduler --- pydis_site/apps/api/tests/test_infractions.py | 30 ++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index 16c4d294..fe51307d 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -81,6 +81,14 @@ class InfractionTests(APISubdomainTestCase): active=True, expires_at=datetime.datetime.utcnow() + datetime.timedelta(hours=5) ) + cls.voiceban_expires_later = Infraction.objects.create( + user_id=cls.user.id, + actor_id=cls.user.id, + type='voice_ban', + reason='Jet engine mic', + active=True, + expires_at=datetime.datetime.utcnow() + datetime.timedelta(days=5) + ) def test_list_all(self): """Tests the list-view, which should be ordered by inserted_at (newest first).""" @@ -90,11 +98,12 @@ class InfractionTests(APISubdomainTestCase): self.assertEqual(response.status_code, 200) infractions = response.json() - self.assertEqual(len(infractions), 4) - self.assertEqual(infractions[0]['id'], self.superstar_expires_soon.id) - self.assertEqual(infractions[1]['id'], self.mute_permanent.id) - self.assertEqual(infractions[2]['id'], self.ban_inactive.id) - self.assertEqual(infractions[3]['id'], self.ban_hidden.id) + self.assertEqual(len(infractions), 5) + self.assertEqual(infractions[0]['id'], self.voiceban_expires_later.id) + self.assertEqual(infractions[1]['id'], self.superstar_expires_soon.id) + self.assertEqual(infractions[2]['id'], self.mute_permanent.id) + self.assertEqual(infractions[3]['id'], self.ban_inactive.id) + self.assertEqual(infractions[4]['id'], self.ban_hidden.id) def test_filter_search(self): url = reverse('bot:infraction-list', host='api') @@ -162,6 +171,17 @@ class InfractionTests(APISubdomainTestCase): infractions = response.json() self.assertEqual(len(infractions), 3) + def test_sort_expiresby(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?ordering=expires_at&permanent=false') + self.assertEqual(response.status_code, 200) + infractions = response.json() + + self.assertEqual(len(infractions), 3) + self.assertEqual(infractions[0]['id'], self.superstar_expires_soon.id) + self.assertEqual(infractions[1]['id'], self.voiceban_expires_later.id) + self.assertEqual(infractions[2]['id'], self.ban_hidden.id) + def test_returns_empty_for_no_match(self): url = reverse('bot:infraction-list', host='api') response = self.client.get(f'{url}?type=ban&search=poop') -- cgit v1.2.3 From e3696a3b95204b452c18db910352df5617a7e77b Mon Sep 17 00:00:00 2001 From: bast Date: Sat, 22 May 2021 13:35:13 -0700 Subject: Add tests for failure states of `expires_before` and `expires_after` filters --- pydis_site/apps/api/tests/test_infractions.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index fe51307d..0f9a53c2 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -163,6 +163,20 @@ class InfractionTests(APISubdomainTestCase): self.assertEqual(len(infractions), 1) self.assertEqual(infractions[0]['id'], self.superstar_expires_soon.id) + def test_filter_after_invalid(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?expires_after=gibberish') + + self.assertEqual(response.status_code, 400) + self.assertEqual(list(response.json())[0], "expires_after") + + def test_filter_before_invalid(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?expires_before=000000000') + + self.assertEqual(response.status_code, 400) + self.assertEqual(list(response.json())[0], "expires_before") + def test_filter_manytypes(self): url = reverse('bot:infraction-list', host='api') response = self.client.get(f'{url}?types=mute,ban') -- cgit v1.2.3 From b9f2589c7b2b5f171c47f1a614229ff999341b29 Mon Sep 17 00:00:00 2001 From: bast Date: Sun, 30 May 2021 14:46:27 -0700 Subject: Raise ValidationError for new bot/infractions filter if the types are invalid If the before time is after the after time, or if both `type` and `types` are specified --- pydis_site/apps/api/tests/test_infractions.py | 22 ++++++++++++++++++++++ pydis_site/apps/api/viewsets/bot/infraction.py | 13 +++++++++++++ 2 files changed, 35 insertions(+) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index 0f9a53c2..f2641492 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -177,6 +177,20 @@ class InfractionTests(APISubdomainTestCase): self.assertEqual(response.status_code, 400) self.assertEqual(list(response.json())[0], "expires_before") + def test_after_before_before_invalid(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) + target_time_late = datetime.datetime.utcnow() + datetime.timedelta(hours=9) + response = self.client.get( + f'{url}?expires_before={target_time_late.isoformat()}' + f'&expires_after={target_time.isoformat()}' + ) + + self.assertEqual(response.status_code, 400) + errors = list(response.json()) + self.assertIn("expires_before", errors) + self.assertIn("expires_after", errors) + def test_filter_manytypes(self): url = reverse('bot:infraction-list', host='api') response = self.client.get(f'{url}?types=mute,ban') @@ -185,6 +199,14 @@ class InfractionTests(APISubdomainTestCase): infractions = response.json() self.assertEqual(len(infractions), 3) + def test_types_type_invalid(self): + url = reverse('bot:infraction-list', host='api') + response = self.client.get(f'{url}?types=mute,ban&type=superstar') + + self.assertEqual(response.status_code, 400) + errors = list(response.json()) + self.assertEqual("types", errors[0]) + def test_sort_expiresby(self): url = reverse('bot:infraction-list', host='api') response = self.client.get(f'{url}?ordering=expires_at&permanent=false') diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index b0c7d332..00dd05b8 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -54,6 +54,8 @@ class InfractionViewSet( - **expires_before** `isodatetime`: the latest expires_at time to return infractions for Invalid query parameters are ignored. + Only one of `type` and `types` may be provided. If both `expires_before` and `expires_after` + are provided, `expires_after` must come after `expires_before`. #### Response format Response is paginated but the result is returned without any pagination metadata. @@ -192,8 +194,19 @@ class InfractionViewSet( except ValueError: raise ValidationError({'expires_before': ['failed to convert to datetime']}) + 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']: + raise ValidationError({ + 'expires_before': ['cannot be after expires_after'], + 'expires_after': ['cannot be before expires_before'], + }) + filter_types = self.request.query_params.get('types') if filter_types: + if self.request.query_params.get('type'): + raise ValidationError({ + 'types': ['you must provide only one of "type" or "types"'], + }) additional_filters['type__in'] = [i.strip() for i in filter_types.split(",")] return self.queryset.filter(**additional_filters) -- cgit v1.2.3 From 45dfba505e04eec968a3be701e868753b8e4a849 Mon Sep 17 00:00:00 2001 From: bast Date: Sun, 30 May 2021 16:00:24 -0700 Subject: Fix bot/infractions after and before filter check being inverted The check was backwards, enforcing that no results must ever be returned Relevant tests have been added that actually fetch an infraction with the right times so that I'm sure it's not still broken --- pydis_site/apps/api/tests/test_infractions.py | 19 ++++++++++++++++--- pydis_site/apps/api/viewsets/bot/infraction.py | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index f2641492..d717ab48 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -177,15 +177,28 @@ class InfractionTests(APISubdomainTestCase): self.assertEqual(response.status_code, 400) self.assertEqual(list(response.json())[0], "expires_before") - def test_after_before_before_invalid(self): + def test_after_before_before(self): url = reverse('bot:infraction-list', host='api') - target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) - target_time_late = datetime.datetime.utcnow() + datetime.timedelta(hours=9) + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=4) + target_time_late = datetime.datetime.utcnow() + datetime.timedelta(hours=6) response = self.client.get( f'{url}?expires_before={target_time_late.isoformat()}' f'&expires_after={target_time.isoformat()}' ) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()), 1) + self.assertEqual(response.json()[0]["id"], self.superstar_expires_soon.id) + + def test_after_after_before_invalid(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) + target_time_late = datetime.datetime.utcnow() + datetime.timedelta(hours=9) + response = self.client.get( + f'{url}?expires_before={target_time.isoformat()}' + f'&expires_after={target_time_late.isoformat()}' + ) + self.assertEqual(response.status_code, 400) errors = list(response.json()) self.assertIn("expires_before", errors) diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index 00dd05b8..0baab6fd 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -195,7 +195,7 @@ class InfractionViewSet( raise ValidationError({'expires_before': ['failed to convert to datetime']}) 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']: + if additional_filters['expires_at__gte'] > additional_filters['expires_at__lte']: raise ValidationError({ 'expires_before': ['cannot be after expires_after'], 'expires_after': ['cannot be before expires_before'], -- cgit v1.2.3 From 7a5a0321d8638d22d58112d8976d18fb07d7ce4e Mon Sep 17 00:00:00 2001 From: bast Date: Sun, 30 May 2021 16:14:12 -0700 Subject: Ensure bot/infractions does not accept both expires and permanent filters Expires and permanent=false are permitted and tested for. Expires_before also filters the database for permanent=false explicitly --- pydis_site/apps/api/tests/test_infractions.py | 29 ++++++++++++++++++++++++++ pydis_site/apps/api/viewsets/bot/infraction.py | 18 ++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py index d717ab48..967698ff 100644 --- a/pydis_site/apps/api/tests/test_infractions.py +++ b/pydis_site/apps/api/tests/test_infractions.py @@ -204,6 +204,35 @@ class InfractionTests(APISubdomainTestCase): self.assertIn("expires_before", errors) self.assertIn("expires_after", errors) + def test_permanent_after_invalid(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) + response = self.client.get(f'{url}?permanent=true&expires_after={target_time.isoformat()}') + + self.assertEqual(response.status_code, 400) + errors = list(response.json()) + self.assertEqual("permanent", errors[0]) + + def test_permanent_before_invalid(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=5) + response = self.client.get(f'{url}?permanent=true&expires_before={target_time.isoformat()}') + + self.assertEqual(response.status_code, 400) + errors = list(response.json()) + self.assertEqual("permanent", errors[0]) + + def test_nonpermanent_before(self): + url = reverse('bot:infraction-list', host='api') + target_time = datetime.datetime.utcnow() + datetime.timedelta(hours=6) + response = self.client.get( + f'{url}?permanent=false&expires_before={target_time.isoformat()}' + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()), 1) + self.assertEqual(response.json()[0]["id"], self.superstar_expires_soon.id) + def test_filter_manytypes(self): url = reverse('bot:infraction-list', host='api') response = self.client.get(f'{url}?types=mute,ban') diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index 0baab6fd..f8b0cb9d 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -56,6 +56,7 @@ class InfractionViewSet( Invalid query parameters are ignored. Only one of `type` and `types` may be provided. If both `expires_before` and `expires_after` are provided, `expires_after` must come after `expires_before`. + If `permanent` is provided and true, `expires_before` and `expires_after` must not be provided. #### Response format Response is paginated but the result is returned without any pagination metadata. @@ -201,6 +202,23 @@ class InfractionViewSet( 'expires_after': ['cannot be before expires_before'], }) + if ( + ('expires_at__lte' in additional_filters or 'expires_at__gte' in additional_filters) + and 'expires_at__isnull' in additional_filters + and additional_filters['expires_at__isnull'] + ): + raise ValidationError({ + 'permanent': [ + 'cannot filter for permanent infractions at the' + ' same time as expires_at or expires_before', + ] + }) + + if filter_expires_before: + # Filter out permanent infractions specifically if we want ones that will expire + # before a given date + additional_filters['expires_at__isnull'] = False + filter_types = self.request.query_params.get('types') if filter_types: if self.request.query_params.get('type'): -- cgit v1.2.3