From abd77fdaa67d1686df64f82a919ee283b8baec9b Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 24 Apr 2019 22:25:30 +0200 Subject: Adding migration solution --- pydis_site/apps/api/admin.py | 4 ++- .../api/migrations/0036_alter_nominations_api.py | 32 ++++++++++++++++++++++ pydis_site/apps/api/models/bot/nomination.py | 10 ++++++- pydis_site/apps/api/serializers.py | 4 ++- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 5 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 pydis_site/apps/api/migrations/0036_alter_nominations_api.py (limited to 'pydis_site') diff --git a/pydis_site/apps/api/admin.py b/pydis_site/apps/api/admin.py index 3ae7f3c5..1caa1d8b 100644 --- a/pydis_site/apps/api/admin.py +++ b/pydis_site/apps/api/admin.py @@ -3,7 +3,8 @@ from django.contrib import admin from .models import ( BotSetting, DeletedMessage, DocumentationLink, Infraction, - MessageDeletionContext, OffTopicChannelName, + MessageDeletionContext, Nomination, + OffTopicChannelName, Role, SnakeFact, SnakeIdiom, SnakeName, SpecialSnake, Tag, @@ -16,6 +17,7 @@ admin.site.register(DeletedMessage) admin.site.register(DocumentationLink) admin.site.register(Infraction) admin.site.register(MessageDeletionContext) +admin.site.register(Nomination) admin.site.register(OffTopicChannelName) admin.site.register(Role) admin.site.register(SnakeFact) diff --git a/pydis_site/apps/api/migrations/0036_alter_nominations_api.py b/pydis_site/apps/api/migrations/0036_alter_nominations_api.py new file mode 100644 index 00000000..6b50a2df --- /dev/null +++ b/pydis_site/apps/api/migrations/0036_alter_nominations_api.py @@ -0,0 +1,32 @@ +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0035_create_table_log_entry'), + ] + + operations = [ + migrations.AlterField( + model_name='nomination', + name='user', + field=models.OneToOneField(help_text='The nominated user.', on_delete=django.db.models.deletion.CASCADE, related_name='nomination', to='api.User'), + ), + migrations.AddField( + model_name='nomination', + name='id', + field=models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'), + ), + migrations.AddField( + model_name='nomination', + name='unnominate_reason', + field=models.TextField(default='', help_text='Why the nomination was ended.'), + ), + migrations.AddField( + model_name='nomination', + name='unwatched_at', + field=models.DateTimeField(help_text='When the nomination was ended.', null=True), + ), + ] diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 5ebb9759..9a9f31ff 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -24,10 +24,18 @@ class Nomination(ModelReprMixin, models.Model): User, on_delete=models.CASCADE, help_text="The nominated user.", - primary_key=True, related_name='nomination' ) inserted_at = models.DateTimeField( auto_now_add=True, help_text="The creation date of this nomination." ) + unnominate_reason = models.TextField( + help_text="Why the nomination was ended.", + default="" + ) + unwatched_at = models.DateTimeField( + auto_now_add=False, + help_text="When the nomination was ended.", + null=True + ) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 8f045044..54f26466 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -178,5 +178,7 @@ class NominationSerializer(ModelSerializer): class Meta: model = Nomination - fields = ('active', 'author', 'reason', 'user', 'inserted_at') + fields = ( + 'id', 'active', 'author', 'reason', 'user', + 'inserted_at', 'unnominate_reason', 'unwatched_at') depth = 1 diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 725ae176..9d3246c2 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -10,7 +10,7 @@ class NominationViewSet(ModelViewSet): # TODO: doc me serializer_class = NominationSerializer queryset = Nomination.objects.prefetch_related('author', 'user') - frozen_fields = ('author', 'inserted_at', 'user') + frozen_fields = ('id', 'author', 'inserted_at', 'user') def update(self, request, *args, **kwargs): for field in request.data: -- cgit v1.2.3 From f0d4000c3f7ee6399c2a03342e08c664ba2e8174 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 25 Apr 2019 20:29:03 +0200 Subject: Adding migrations to alter nominations API --- .../apps/api/migrations/0036_alter_nominations_api.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/migrations/0036_alter_nominations_api.py b/pydis_site/apps/api/migrations/0036_alter_nominations_api.py index 6b50a2df..f31be14c 100644 --- a/pydis_site/apps/api/migrations/0036_alter_nominations_api.py +++ b/pydis_site/apps/api/migrations/0036_alter_nominations_api.py @@ -1,5 +1,5 @@ -from django.db import migrations, models import django.db.models.deletion +from django.db import migrations, models class Migration(migrations.Migration): @@ -12,7 +12,17 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='nomination', name='user', - field=models.OneToOneField(help_text='The nominated user.', on_delete=django.db.models.deletion.CASCADE, related_name='nomination', to='api.User'), + field=models.ForeignKey(help_text='The nominated user.', on_delete=django.db.models.deletion.CASCADE, related_name='nomination', to='api.User'), + ), + migrations.AlterField( + model_name='nomination', + name='author', + field=models.ForeignKey(help_text='The staff member that nominated this user.', on_delete=django.db.models.deletion.CASCADE, related_name='nomination_set', to='api.User'), + ), + migrations.RenameField( + model_name='nomination', + old_name='author', + new_name='actor', ), migrations.AddField( model_name='nomination', -- cgit v1.2.3 From 620d529081540a189dccc81e93363b9e2dba130b Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 25 Apr 2019 20:30:01 +0200 Subject: Reworking nomination viewset and model --- pydis_site/apps/api/models/bot/nomination.py | 4 +-- pydis_site/apps/api/viewsets/bot/nomination.py | 47 ++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 4 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 9a9f31ff..227a40ec 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -11,7 +11,7 @@ class Nomination(ModelReprMixin, models.Model): default=True, help_text="Whether this nomination is still relevant." ) - author = models.ForeignKey( + actor = models.ForeignKey( User, on_delete=models.CASCADE, help_text="The staff member that nominated this user.", @@ -20,7 +20,7 @@ class Nomination(ModelReprMixin, models.Model): reason = models.TextField( help_text="Why this user was nominated." ) - user = models.OneToOneField( + user = models.ForeignKey( User, on_delete=models.CASCADE, help_text="The nominated user.", diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 9d3246c2..fe16bfa8 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -1,4 +1,9 @@ +from django.utils import timezone +from django_filters.rest_framework import DjangoFilterBackend +from rest_framework import status +from rest_framework.decorators import action from rest_framework.exceptions import ValidationError +from rest_framework.filters import OrderingFilter, SearchFilter from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet @@ -9,8 +14,22 @@ from pydis_site.apps.api.serializers import NominationSerializer class NominationViewSet(ModelViewSet): # TODO: doc me serializer_class = NominationSerializer - queryset = Nomination.objects.prefetch_related('author', 'user') - frozen_fields = ('id', 'author', 'inserted_at', 'user') + queryset = Nomination.objects.prefetch_related('actor', 'user') + filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) + filter_fields = ('user__id', 'actor__id', 'active') + frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'unwatched_at', 'active') + frozen_on_create = ('unwatched_at', 'unnominate_reason') + + def create(self, request, *args, **kwargs): + for field in request.data: + if field in self.frozen_on_create: + raise ValidationError({field: ['This field cannot be updated.']}) + + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + self.perform_create(serializer) + headers = self.get_success_headers(serializer.data) + return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) def update(self, request, *args, **kwargs): for field in request.data: @@ -23,3 +42,27 @@ class NominationViewSet(ModelViewSet): serializer.save() return Response(serializer.data) + + @action(detail=True, methods=['patch']) + def end_nomination(self, request, pk=None): + for field in request.data: + if field in self.frozen_fields: + raise ValidationError({field: ['This field cannot be updated.']}) + + if "unnominate_reason" not in request.data: + raise ValidationError( + {'unnominate_reason': ['This field is required when ending a nomination']} + ) + + instance = self.get_object() + if not instance.active: + raise ValidationError({'active': ['A nomination must be active to be ended.']}) + + instance.active = False + instance.unwatched_at = timezone.now() + serializer = self.get_serializer(instance, data=request.data, partial=True) + serializer.is_valid(raise_exception=True) + serializer.save() + instance.save() + + return Response(serializer.data) -- cgit v1.2.3 From 4a67a0646761802e5b6f1996f74ea7f7a9a1f2d9 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 25 Apr 2019 20:30:37 +0200 Subject: Adding validate to Nomination serializer --- pydis_site/apps/api/serializers.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 54f26466..6fb9d826 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -173,12 +173,22 @@ class UserSerializer(BulkSerializerMixin, ModelSerializer): class NominationSerializer(ModelSerializer): - author = PrimaryKeyRelatedField(queryset=User.objects.all()) + actor = PrimaryKeyRelatedField(queryset=User.objects.all()) user = PrimaryKeyRelatedField(queryset=User.objects.all()) class Meta: model = Nomination fields = ( - 'id', 'active', 'author', 'reason', 'user', + 'id', 'active', 'actor', 'reason', 'user', 'inserted_at', 'unnominate_reason', 'unwatched_at') depth = 1 + + def validate(self, attrs): + active = attrs.get("active") + + unnominate_reason = attrs.get("unnominate_reason") + if active and unnominate_reason: + raise ValidationError( + {'unnominate_reason': "An active nomination can't have an unnominate reason"} + ) + return attrs -- cgit v1.2.3 From ccc35f8cf496c8a50bc287605e9518c0eebb28a5 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 25 Apr 2019 20:31:33 +0200 Subject: Adding OrderingFilter to infraction ViewSet --- pydis_site/apps/api/viewsets/bot/infraction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index 8eacf5c1..70808278 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -1,7 +1,7 @@ from django_filters.rest_framework import DjangoFilterBackend from rest_framework.decorators import action from rest_framework.exceptions import ValidationError -from rest_framework.filters import SearchFilter +from rest_framework.filters import OrderingFilter, SearchFilter from rest_framework.mixins import ( CreateModelMixin, ListModelMixin, @@ -117,7 +117,7 @@ class InfractionViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge serializer_class = InfractionSerializer queryset = Infraction.objects.all() - filter_backends = (DjangoFilterBackend, SearchFilter) + filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active', 'hidden', 'type') search_fields = ('$reason',) frozen_fields = ('id', 'inserted_at', 'type', 'user', 'actor', 'hidden') -- cgit v1.2.3 From 0ebfb8e2c7b037ecbfabf1db7fe8803401ce38f6 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 25 Apr 2019 20:31:57 +0200 Subject: Adding initial tests for nomination endpoint --- pydis_site/apps/api/tests/test_nominations.py | 87 ++++++++++++++++++--------- 1 file changed, 59 insertions(+), 28 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 1f03d1b0..99a3fe69 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -1,41 +1,72 @@ +from datetime import datetime as dt, timedelta, timezone + from django_hosts.resolvers import reverse from .base import APISubdomainTestCase from ..models import Nomination, User -class NominationTests(APISubdomainTestCase): +# class NominationTests(APISubdomainTestCase): +# @classmethod +# def setUpTestData(cls): # noqa +# cls.actor = User.objects.create( +# id=5152, +# name='Ro Bert', +# discriminator=256, +# avatar_hash=None +# ) +# cls.user = cls.actor + +# cls.nomination = Nomination.objects.create( +# actor=cls.actor, +# reason="he's good", +# user=cls.actor +# ) + +# def test_returns_400_on_attempt_to_update_frozen_field(self): +# url = reverse('bot:nomination-detail', args=(self.user.id,), host='api') +# response = self.client.put( +# url, +# data={'inserted_at': 'something bad'} +# ) +# self.assertEqual(response.status_code, 400) +# self.assertEqual(response.json(), { +# 'inserted_at': ['This field cannot be updated.'] +# }) + +# def test_returns_200_on_successful_update(self): +# url = reverse('bot:nomination-detail', args=(self.user.id,), host='api') +# response = self.client.patch( +# url, +# data={'reason': 'there are many like it, but this test is mine'} +# ) +# self.assertEqual(response.status_code, 200) + + +class CreationTests(APISubdomainTestCase): @classmethod def setUpTestData(cls): # noqa - cls.author = User.objects.create( - id=5152, - name='Ro Bert', - discriminator=256, + cls.user = User.objects.create( + id=1234, + name='joe dart', + discriminator=1111, avatar_hash=None ) - cls.user = cls.author - cls.nomination = Nomination.objects.create( - author=cls.author, - reason="he's good", - user=cls.author - ) + def test_accepts_valid_data(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'actor': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'user': self.user.id, + } - def test_returns_400_on_attempt_to_update_frozen_field(self): - url = reverse('bot:nomination-detail', args=(self.user.id,), host='api') - response = self.client.put( - url, - data={'inserted_at': 'something bad'} - ) - self.assertEqual(response.status_code, 400) - self.assertEqual(response.json(), { - 'inserted_at': ['This field cannot be updated.'] - }) - - def test_returns_200_on_successful_update(self): - url = reverse('bot:nomination-detail', args=(self.user.id,), host='api') - response = self.client.patch( - url, - data={'reason': 'there are many like it, but this test is mine'} + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 201) + + nomination = Nomination.objects.get(id=response.json()['id']) + self.assertAlmostEqual( + nomination.inserted_at, + dt.now(timezone.utc), + delta=timedelta(seconds=2) ) - self.assertEqual(response.status_code, 200) -- cgit v1.2.3 From 3013eaa9db61472ddf52b8f75219e8a908f30f7e Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 25 Apr 2019 23:19:21 +0200 Subject: Adding CreationTests to test_nomination and making related changes in the ViewSet --- pydis_site/apps/api/tests/test_nominations.py | 131 +++++++++++++++++++++++++ pydis_site/apps/api/viewsets/bot/nomination.py | 8 +- 2 files changed, 135 insertions(+), 4 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 99a3fe69..258c2cd7 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -70,3 +70,134 @@ class CreationTests(APISubdomainTestCase): dt.now(timezone.utc), delta=timedelta(seconds=2) ) + self.assertEqual(nomination.user.id, data['user']) + self.assertEqual(nomination.actor.id, data['actor']) + self.assertEqual(nomination.reason, data['reason']) + self.assertEqual(nomination.active, True) + + def test_returns_400_for_missing_user(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'actor': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'user': ['This field is required.'] + }) + + def test_returns_400_for_missing_actor(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'actor': ['This field is required.'] + }) + + def test_returns_400_for_missing_reason(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'actor': self.user.id, + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'reason': ['This field is required.'] + }) + + def test_returns_400_for_bad_user(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': 1024, + 'reason': 'Joe Dart on Fender Bass', + 'actor': self.user.id, + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'user': ['Invalid pk "1024" - object does not exist.'] + }) + + def test_returns_400_for_bad_actor(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'actor': 1024, + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'actor': ['Invalid pk "1024" - object does not exist.'] + }) + + def test_returns_400_for_unnominate_reason_at_creation(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'actor': self.user.id, + 'unnominate_reason': "Joe Dart on the Joe Dart Bass" + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'unnominate_reason': ['This field cannot be set at creation.'] + }) + + def test_returns_400_for_unwatched_at_at_creation(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'actor': self.user.id, + 'unwatched_at': "Joe Dart on the Joe Dart Bass" + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'unwatched_at': ['This field cannot be set at creation.'] + }) + + def test_returns_400_for_inserted_at_at_creation(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'actor': self.user.id, + 'inserted_at': "Joe Dart on the Joe Dart Bass" + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'inserted_at': ['This field cannot be set at creation.'] + }) + + def test_returns_400_for_active_at_creation(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'user': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'actor': self.user.id, + 'active': False + } + + response = self.client.post(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'active': ['This field cannot be set at creation.'] + }) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index c3615dd9..1059ffcd 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -19,12 +19,12 @@ class NominationViewSet(ModelViewSet): filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active') frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'unwatched_at', 'active') - frozen_on_create = ('unwatched_at', 'unnominate_reason') + frozen_on_create = ('unwatched_at', 'unnominate_reason', 'active', 'inserted_at') def create(self, request, *args, **kwargs): for field in request.data: if field in self.frozen_on_create: - raise ValidationError({field: ['This field cannot be updated.']}) + raise ValidationError({field: ['This field cannot be set at creation.']}) serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) @@ -53,8 +53,8 @@ class NominationViewSet(ModelViewSet): @action(detail=True, methods=['patch']) def end_nomination(self, request, pk=None): for field in request.data: - if field in self.frozen_fields: - raise ValidationError({field: ['This field cannot be updated.']}) + if field != "unnominate_reason": + raise ValidationError({field: ['This field cannot be set at end_nomination']}) if "unnominate_reason" not in request.data: raise ValidationError( -- cgit v1.2.3 From b9db880d6229ddb7bf793bc8ca50b37e471956a7 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Sat, 27 Apr 2019 17:30:20 +0200 Subject: Finalizing and adding documentation to the nomination endpoint and simplifying the nomination serializer --- pydis_site/apps/api/serializers.py | 14 -- pydis_site/apps/api/viewsets/bot/nomination.py | 173 +++++++++++++++++++++++-- 2 files changed, 161 insertions(+), 26 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index d58f1fa7..abf49393 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -270,9 +270,6 @@ class UserSerializer(BulkSerializerMixin, ModelSerializer): class NominationSerializer(ModelSerializer): """A class providing (de-)serialization of `Nomination` instances.""" - actor = PrimaryKeyRelatedField(queryset=User.objects.all()) - user = PrimaryKeyRelatedField(queryset=User.objects.all()) - class Meta: """Metadata defined for the Django REST Framework.""" @@ -280,14 +277,3 @@ class NominationSerializer(ModelSerializer): fields = ( 'id', 'active', 'actor', 'reason', 'user', 'inserted_at', 'unnominate_reason', 'unwatched_at') - depth = 1 - - def validate(self, attrs): - active = attrs.get("active") - - unnominate_reason = attrs.get("unnominate_reason") - if active and unnominate_reason: - raise ValidationError( - {'unnominate_reason': "An active nomination can't have an unnominate reason"} - ) - return attrs diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 1059ffcd..888f049e 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -1,49 +1,192 @@ +from collections import ChainMap + from django.utils import timezone from django_filters.rest_framework import DjangoFilterBackend from rest_framework import status from rest_framework.decorators import action from rest_framework.exceptions import ValidationError from rest_framework.filters import OrderingFilter, SearchFilter +from rest_framework.mixins import ( + CreateModelMixin, + ListModelMixin, + RetrieveModelMixin, +) from rest_framework.response import Response -from rest_framework.viewsets import ModelViewSet +from rest_framework.viewsets import GenericViewSet from pydis_site.apps.api.models.bot import Nomination from pydis_site.apps.api.serializers import NominationSerializer -class NominationViewSet(ModelViewSet): - """View providing CRUD operations on helper nominations done through the bot.""" - +class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, GenericViewSet): + """ + View providing CRUD operations on helper nominations done through the bot. + + ## Routes + ### GET /bot/nominations + Retrieve all nominations. + May be filtered and ordered by the query parameters. + + #### Query parameters + - **active** `bool`: whether the nomination is still active + - **actor__id** `int`: snowflake of the user who nominated the user + - **user__id** `int`: snowflake of the user who received the nomination + - **ordering** `str`: comma-separated sequence of fields to order the returned results + + Invalid query parameters are ignored. + + #### Response format + [ + { + 'id': 1, + 'active': false, + 'actor': 336843820513755157, + 'reason': 'They know how to explain difficult concepts', + 'user': 336843820513755157, + 'inserted_at': '2019-04-25T14:02:37.775587Z', + 'unnominate_reason': 'They were helpered after a staff-vote', + 'unwatched_at': '2019-04-26T15:12:22.123587Z' + } + ] + + #### Status codes + - 200: returned on success + + ### GET /bot/nominations/ + Retrieve a single nomination by ID. + + ### Response format + { + 'id': 1, + 'active': true, + 'actor': 336843820513755157, + 'reason': 'They know how to explain difficult concepts', + 'user': 336843820513755157, + 'inserted_at': '2019-04-25T14:02:37.775587Z', + 'unnominate_reason': 'They were helpered after a staff-vote', + 'unwatched_at': '2019-04-26T15:12:22.123587Z' + } + + ### Status codes + - 200: returned on succes + - 404: returned if an nomination with the given `id` could not be found + + ### POST /bot/nominations + Create a new, active nomination returns the created nominations. + The `user`, `reason` and `actor` fields are required and the `user` + and `actor` need to know by the site. Providing other valid fields + is not allowed and invalid fields are ignored. A `user` is only + allowed one active nomination at a time. + + #### Request body + { + 'actor': 409107086526644234 + 'reason': 'He would make a great helper', + 'user': 409107086526644234 + } + + ### Response format + See `GET /bot/nominations/` + + #### Status codes + - 201: returned on success + - 400: returned on failure for one of the following reasons: + - A user already has an active nomination; + - The `user` or `actor` are unknown to the site; + - The request contained a field that cannot be set at creation. + + ### PATCH /bot/nominations/ + Update the nomination with the given `id` and return the updated nomination. + For active nominations, only the `reason` may be updated; for inactive + nominations, both the `reason` and `unnominate_reason` may be updated. + + #### Request body + { + 'reason': 'He would make a great helper', + 'unnominate_reason': 'He needs some time to mature his Python knowledge' + } + + ### Response format + See `GET /bot/nominations/` + + ## Status codes + - 200: returned on success + - 400: if a field in the request body is invalid or disallowed + - 404: if an infraction with the given `id` could not be found + + ### PATCH /bot/nominations//end_nomination + Ends an active nomination and returns the updated nomination. + + The `unnominate_reason` field is the only allowed and required field + for this operation. The nomination will automatically be marked as + `active = false` and the datetime of this operation will be added to + the `unwatched_at` field. + + #### Request body + { + 'unnominate_reason': 'He needs some time to mature his Python knowledge' + } + + ### Response format + See `GET /bot/nominations/` + + #### Status codes + - 200: returned on success + - 400: returned on failure for the following reasons: + - `unnominate_reason` is missing from the request body; + - Any other field is present in the request body; + - The nomination was already inactiive. + - 404: if an infraction with the given `id` could not be found + """ serializer_class = NominationSerializer - queryset = Nomination.objects.prefetch_related('actor', 'user') + queryset = Nomination.objects.all() filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active') frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'unwatched_at', 'active') frozen_on_create = ('unwatched_at', 'unnominate_reason', 'active', 'inserted_at') def create(self, request, *args, **kwargs): + """ + DRF method for creating a Nomination. + + Called by the Django Rest Framework in response to the corresponding HTTP request. + """ for field in request.data: if field in self.frozen_on_create: raise ValidationError({field: ['This field cannot be set at creation.']}) - serializer = self.get_serializer(data=request.data) + user_id = request.data.get("user") + if Nomination.objects.filter(active=True, user__id=user_id): + raise ValidationError({'active': ['There can only be one active nomination.']}) + + serializer = self.get_serializer( + data=ChainMap( + request.data, + {"active": True} + ) + ) serializer.is_valid(raise_exception=True) self.perform_create(serializer) headers = self.get_success_headers(serializer.data) return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) - def update(self, request, *args, **kwargs): + def partial_update(self, request, *args, **kwargs): """ DRF method for updating a Nomination. Called by the Django Rest Framework in response to the corresponding HTTP request. """ - for field in request.data: if field in self.frozen_fields: raise ValidationError({field: ['This field cannot be updated.']}) instance = self.get_object() + + if instance.active and request.data.get('unnominate_reason'): + raise ValidationError( + {'unnominate_reason': ["An active nomination can't have an unnominate reason."]} + ) + serializer = self.get_serializer(instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) serializer.save() @@ -52,23 +195,29 @@ class NominationViewSet(ModelViewSet): @action(detail=True, methods=['patch']) def end_nomination(self, request, pk=None): + """ + DRF action for ending an active nomination. + + Creates an API endpoint /bot/nominations/{id}/end_nomination to end a nomination. See + the class docstring for documentation. + """ for field in request.data: if field != "unnominate_reason": - raise ValidationError({field: ['This field cannot be set at end_nomination']}) + raise ValidationError({field: ['This field cannot be set at end_nomination.']}) if "unnominate_reason" not in request.data: raise ValidationError( - {'unnominate_reason': ['This field is required when ending a nomination']} + {'unnominate_reason': ['This field is required when ending a nomination.']} ) instance = self.get_object() if not instance.active: raise ValidationError({'active': ['A nomination must be active to be ended.']}) - instance.active = False - instance.unwatched_at = timezone.now() serializer = self.get_serializer(instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) + instance.active = False + instance.unwatched_at = timezone.now() serializer.save() instance.save() -- cgit v1.2.3 From 32c5307bfe4c3421d90891be53b4704de626e8a8 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Sat, 27 Apr 2019 17:33:39 +0200 Subject: Documentation: Clarifying infraction query parameters and adding ordering parameter --- pydis_site/apps/api/viewsets/bot/infraction.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/infraction.py b/pydis_site/apps/api/viewsets/bot/infraction.py index 8a279bc4..d12f0862 100644 --- a/pydis_site/apps/api/viewsets/bot/infraction.py +++ b/pydis_site/apps/api/viewsets/bot/infraction.py @@ -28,11 +28,12 @@ class InfractionViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge #### Query parameters - **active** `bool`: whether the infraction is still active - - **actor** `int`: snowflake of the user which applied the infraction + - **actor__id** `int`: snowflake of the user which applied the infraction - **hidden** `bool`: whether the infraction is a shadow infraction - **search** `str`: regular expression applied to the infraction's reason - **type** `str`: the type of the infraction - - **user** `int`: snowflake of the user to which the infraction was applied + - **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 Invalid query parameters are ignored. -- cgit v1.2.3 From ba9fbe428407127a72edb6b7cb5f2555f1be1642 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Sat, 27 Apr 2019 17:34:01 +0200 Subject: Adding complete test suite for nomination end point --- pydis_site/apps/api/tests/test_nominations.py | 355 +++++++++++++++++++++++--- 1 file changed, 318 insertions(+), 37 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 258c2cd7..cd6f1a90 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -6,43 +6,6 @@ from .base import APISubdomainTestCase from ..models import Nomination, User -# class NominationTests(APISubdomainTestCase): -# @classmethod -# def setUpTestData(cls): # noqa -# cls.actor = User.objects.create( -# id=5152, -# name='Ro Bert', -# discriminator=256, -# avatar_hash=None -# ) -# cls.user = cls.actor - -# cls.nomination = Nomination.objects.create( -# actor=cls.actor, -# reason="he's good", -# user=cls.actor -# ) - -# def test_returns_400_on_attempt_to_update_frozen_field(self): -# url = reverse('bot:nomination-detail', args=(self.user.id,), host='api') -# response = self.client.put( -# url, -# data={'inserted_at': 'something bad'} -# ) -# self.assertEqual(response.status_code, 400) -# self.assertEqual(response.json(), { -# 'inserted_at': ['This field cannot be updated.'] -# }) - -# def test_returns_200_on_successful_update(self): -# url = reverse('bot:nomination-detail', args=(self.user.id,), host='api') -# response = self.client.patch( -# url, -# data={'reason': 'there are many like it, but this test is mine'} -# ) -# self.assertEqual(response.status_code, 200) - - class CreationTests(APISubdomainTestCase): @classmethod def setUpTestData(cls): # noqa @@ -75,6 +38,23 @@ class CreationTests(APISubdomainTestCase): self.assertEqual(nomination.reason, data['reason']) self.assertEqual(nomination.active, True) + def test_returns_400_on_second_active_nomination(self): + url = reverse('bot:nomination-list', host='api') + data = { + 'actor': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'user': self.user.id, + } + + response1 = self.client.post(url, data=data) + self.assertEqual(response1.status_code, 201) + + response2 = self.client.post(url, data=data) + self.assertEqual(response2.status_code, 400) + self.assertEqual(response2.json(), { + 'active': ['There can only be one active nomination.'] + }) + def test_returns_400_for_missing_user(self): url = reverse('bot:nomination-list', host='api') data = { @@ -201,3 +181,304 @@ class CreationTests(APISubdomainTestCase): self.assertEqual(response.json(), { 'active': ['This field cannot be set at creation.'] }) + + +class NominationTests(APISubdomainTestCase): + @classmethod + def setUpTestData(cls): # noqa + cls.user = User.objects.create( + id=1234, + name='joe dart', + discriminator=1111, + avatar_hash=None + ) + + cls.active_nomination = Nomination.objects.create( + user=cls.user, + actor=cls.user, + reason="He's pretty funky" + ) + cls.inactive_nomination = Nomination.objects.create( + user=cls.user, + actor=cls.user, + reason="He's pretty funky", + active=False, + unnominate_reason="His neck couldn't hold the funk", + unwatched_at="5018-11-20T15:52:00+00:00" + ) + + def test_returns_200_update_reason_on_active(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + data = { + 'reason': "He's one funky duck" + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + nomination = Nomination.objects.get(id=response.json()['id']) + self.assertEqual(nomination.reason, data['reason']) + + def test_returns_400_on_frozen_field_update(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + data = { + 'user': "Theo Katzman" + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'user': ['This field cannot be updated.'] + }) + + def test_returns_400_update_unnominate_reason_on_active(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + data = { + 'unnominate_reason': 'He started playing jazz' + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'unnominate_reason': ["An active nomination can't have an unnominate reason."] + }) + + def test_returns_200_update_reason_on_inactive(self): + url = reverse('bot:nomination-detail', args=(self.inactive_nomination.id,), host='api') + data = { + 'reason': "He's one funky duck" + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + nomination = Nomination.objects.get(id=response.json()['id']) + self.assertEqual(nomination.reason, data['reason']) + + def test_returns_200_update_unnominate_reason_on_inactive(self): + url = reverse('bot:nomination-detail', args=(self.inactive_nomination.id,), host='api') + data = { + 'unnominate_reason': 'He started playing jazz' + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + nomination = Nomination.objects.get(id=response.json()['id']) + self.assertEqual(nomination.unnominate_reason, data['unnominate_reason']) + + def test_returns_200_on_valid_end_nomination(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.active_nomination.id,), + host='api' + ) + data = { + 'unnominate_reason': 'He started playing jazz' + } + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + nomination = Nomination.objects.get(id=response.json()['id']) + + self.assertAlmostEqual( + nomination.unwatched_at, + dt.now(timezone.utc), + delta=timedelta(seconds=2) + ) + self.assertFalse(nomination.active) + self.assertEqual(nomination.unnominate_reason, data['unnominate_reason']) + + def test_returns_400_on_invalid_field_end_nomination(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.active_nomination.id,), + host='api' + ) + data = { + 'reason': 'Why does a whale have feet?' + } + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'reason': ['This field cannot be set at end_nomination.'] + }) + + def test_returns_400_on_missing_unnominate_reason_end_nomination(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.active_nomination.id,), + host='api' + ) + data = {} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'unnominate_reason': ['This field is required when ending a nomination.'] + }) + + def test_returns_400_on_ending_inactive_nomination(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.inactive_nomination.id,), + host='api' + ) + data = { + 'unnominate_reason': 'He started playing jazz' + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'active': ['A nomination must be active to be ended.'] + }) + + def test_returns_404_on_get_unknown_nomination(self): + url = reverse( + 'bot:nomination-detail', + args=(9999,), + host='api' + ) + + response = self.client.get(url, data={}) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json(), { + "detail": "Not found." + }) + + def test_returns_404_on_patch_unknown_nomination(self): + url = reverse( + 'bot:nomination-detail', + args=(9999,), + host='api' + ) + + response = self.client.patch(url, data={}) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json(), { + "detail": "Not found." + }) + + def test_returns_404_on_end_unknown_nomination(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(9999,), + host='api' + ) + + data = { + 'unnominate_reason': 'He started playing jazz' + } + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json(), { + "detail": "Not found." + }) + + def test_returns_405_on_list_put(self): + url = reverse('bot:nomination-list', host='api') + + response = self.client.put(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"PUT\" not allowed." + }) + + def test_returns_405_on_list_patch(self): + url = reverse('bot:nomination-list', host='api') + + response = self.client.patch(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"PATCH\" not allowed." + }) + + def test_returns_405_on_list_delete(self): + url = reverse('bot:nomination-list', host='api') + + response = self.client.delete(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"DELETE\" not allowed." + }) + + def test_returns_405_on_detail_put(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + + response = self.client.put(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"PUT\" not allowed." + }) + + def test_returns_405_on_detail_post(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + + response = self.client.post(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"POST\" not allowed." + }) + + def test_returns_405_on_detail_delete(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + + response = self.client.delete(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"DELETE\" not allowed." + }) + + def test_returns_405_on_end_nomination_put(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.inactive_nomination.id,), + host='api' + ) + + response = self.client.put(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"PUT\" not allowed." + }) + + def test_returns_405_on_end_nomination_post(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.inactive_nomination.id,), + host='api' + ) + + response = self.client.post(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"POST\" not allowed." + }) + + def test_returns_405_on_end_nomination_delete(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.inactive_nomination.id,), + host='api' + ) + + response = self.client.delete(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"DELETE\" not allowed." + }) + + def test_returns_405_on_end_nomination_get(self): + url = reverse( + 'bot:nomination-end-nomination', + args=(self.inactive_nomination.id,), + host='api' + ) + + response = self.client.get(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"GET\" not allowed." + }) -- cgit v1.2.3 From 9bd2b6fd16e9867bee8a34661e649ead2aff27c6 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Sun, 28 Apr 2019 12:01:50 +0200 Subject: Changing 'end_nomination' endpoint to 'dismiss' --- pydis_site/apps/api/tests/test_nominations.py | 18 +++++++++--------- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index cd6f1a90..ba5ae26a 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -269,7 +269,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_200_on_valid_end_nomination(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.active_nomination.id,), host='api' ) @@ -291,7 +291,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_400_on_invalid_field_end_nomination(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.active_nomination.id,), host='api' ) @@ -306,7 +306,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_400_on_missing_unnominate_reason_end_nomination(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.active_nomination.id,), host='api' ) @@ -320,7 +320,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_400_on_ending_inactive_nomination(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.inactive_nomination.id,), host='api' ) @@ -362,7 +362,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_404_on_end_unknown_nomination(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(9999,), host='api' ) @@ -433,7 +433,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_put(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.inactive_nomination.id,), host='api' ) @@ -446,7 +446,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_post(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.inactive_nomination.id,), host='api' ) @@ -459,7 +459,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_delete(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.inactive_nomination.id,), host='api' ) @@ -472,7 +472,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_get(self): url = reverse( - 'bot:nomination-end-nomination', + 'bot:nomination-dismiss', args=(self.inactive_nomination.id,), host='api' ) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 888f049e..b42e53ff 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -194,7 +194,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge return Response(serializer.data) @action(detail=True, methods=['patch']) - def end_nomination(self, request, pk=None): + def dismiss(self, request, pk=None): """ DRF action for ending an active nomination. -- cgit v1.2.3 From 68589f4bab98fc37a341cfcbbd498d1c7fe44505 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Sun, 28 Apr 2019 12:04:12 +0200 Subject: Changing the docstring response style to REPL prefix style --- pydis_site/apps/api/viewsets/bot/nomination.py | 68 +++++++++++++------------- 1 file changed, 34 insertions(+), 34 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index b42e53ff..b4e0b463 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -36,18 +36,18 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge Invalid query parameters are ignored. #### Response format - [ - { - 'id': 1, - 'active': false, - 'actor': 336843820513755157, - 'reason': 'They know how to explain difficult concepts', - 'user': 336843820513755157, - 'inserted_at': '2019-04-25T14:02:37.775587Z', - 'unnominate_reason': 'They were helpered after a staff-vote', - 'unwatched_at': '2019-04-26T15:12:22.123587Z' - } - ] + >>> [ + ... { + ... 'id': 1, + ... 'active': false, + ... 'actor': 336843820513755157, + ... 'reason': 'They know how to explain difficult concepts', + ... 'user': 336843820513755157, + ... 'inserted_at': '2019-04-25T14:02:37.775587Z', + ... 'unnominate_reason': 'They were helpered after a staff-vote', + ... 'unwatched_at': '2019-04-26T15:12:22.123587Z' + ... } + ... ] #### Status codes - 200: returned on success @@ -56,16 +56,16 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge Retrieve a single nomination by ID. ### Response format - { - 'id': 1, - 'active': true, - 'actor': 336843820513755157, - 'reason': 'They know how to explain difficult concepts', - 'user': 336843820513755157, - 'inserted_at': '2019-04-25T14:02:37.775587Z', - 'unnominate_reason': 'They were helpered after a staff-vote', - 'unwatched_at': '2019-04-26T15:12:22.123587Z' - } + >>> { + ... 'id': 1, + ... 'active': true, + ... 'actor': 336843820513755157, + ... 'reason': 'They know how to explain difficult concepts', + ... 'user': 336843820513755157, + ... 'inserted_at': '2019-04-25T14:02:37.775587Z', + ... 'unnominate_reason': 'They were helpered after a staff-vote', + ... 'unwatched_at': '2019-04-26T15:12:22.123587Z' + ... } ### Status codes - 200: returned on succes @@ -79,11 +79,11 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge allowed one active nomination at a time. #### Request body - { - 'actor': 409107086526644234 - 'reason': 'He would make a great helper', - 'user': 409107086526644234 - } + >>> { + ... 'actor': 409107086526644234 + ... 'reason': 'He would make a great helper', + ... 'user': 409107086526644234 + ... } ### Response format See `GET /bot/nominations/` @@ -101,10 +101,10 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge nominations, both the `reason` and `unnominate_reason` may be updated. #### Request body - { - 'reason': 'He would make a great helper', - 'unnominate_reason': 'He needs some time to mature his Python knowledge' - } + >>> { + ... 'reason': 'He would make a great helper', + ... 'unnominate_reason': 'He needs some time to mature his Python knowledge' + ... } ### Response format See `GET /bot/nominations/` @@ -123,9 +123,9 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge the `unwatched_at` field. #### Request body - { - 'unnominate_reason': 'He needs some time to mature his Python knowledge' - } + >>> { + ... 'unnominate_reason': 'He needs some time to mature his Python knowledge' + ... } ### Response format See `GET /bot/nominations/` -- cgit v1.2.3 From 106dfff3c2156b1fbef25c338a19067e14fa4a8c Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Tue, 30 Apr 2019 11:32:00 +0200 Subject: Update pydis_site/apps/api/viewsets/bot/nomination.py Correcting a grammar mistake Co-Authored-By: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> --- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index b4e0b463..30eaaf6c 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -69,7 +69,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ### Status codes - 200: returned on succes - - 404: returned if an nomination with the given `id` could not be found + - 404: returned if a nomination with the given `id` could not be found ### POST /bot/nominations Create a new, active nomination returns the created nominations. -- cgit v1.2.3 From 6014e5dc1b4f33ef45d139762b56cf946a758d9b Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 3 May 2019 22:52:48 +0200 Subject: Changing import symbol structure to be consistent --- pydis_site/apps/api/admin.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/admin.py b/pydis_site/apps/api/admin.py index 1caa1d8b..a5b75fa9 100644 --- a/pydis_site/apps/api/admin.py +++ b/pydis_site/apps/api/admin.py @@ -4,11 +4,10 @@ from .models import ( BotSetting, DeletedMessage, DocumentationLink, Infraction, MessageDeletionContext, Nomination, - OffTopicChannelName, - Role, SnakeFact, - SnakeIdiom, SnakeName, - SpecialSnake, Tag, - User + OffTopicChannelName, Role, + SnakeFact, SnakeIdiom, + SnakeName, SpecialSnake, + Tag, User ) -- cgit v1.2.3 From 314939e35c6000c057254789848ada62854f0585 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 3 May 2019 23:04:35 +0200 Subject: Explicitely using exists() and changing endpoint back to end --- pydis_site/apps/api/viewsets/bot/nomination.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index b4e0b463..26b43f0f 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -156,7 +156,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge raise ValidationError({field: ['This field cannot be set at creation.']}) user_id = request.data.get("user") - if Nomination.objects.filter(active=True, user__id=user_id): + if Nomination.objects.filter(active=True, user__id=user_id).exists(): raise ValidationError({'active': ['There can only be one active nomination.']}) serializer = self.get_serializer( @@ -194,7 +194,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge return Response(serializer.data) @action(detail=True, methods=['patch']) - def dismiss(self, request, pk=None): + def end(self, request, pk=None): """ DRF action for ending an active nomination. -- cgit v1.2.3 From 8fa51d491b6fe201dc1da28253117fb88f9d00d1 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 3 May 2019 23:06:45 +0200 Subject: Changing tests back to 'end' endpoint and adding two tests to test filtering --- pydis_site/apps/api/tests/test_nominations.py | 48 ++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 9 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index ba5ae26a..aa6561c4 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -269,7 +269,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_200_on_valid_end_nomination(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.active_nomination.id,), host='api' ) @@ -291,7 +291,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_400_on_invalid_field_end_nomination(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.active_nomination.id,), host='api' ) @@ -306,7 +306,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_400_on_missing_unnominate_reason_end_nomination(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.active_nomination.id,), host='api' ) @@ -320,7 +320,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_400_on_ending_inactive_nomination(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.inactive_nomination.id,), host='api' ) @@ -362,7 +362,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_404_on_end_unknown_nomination(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(9999,), host='api' ) @@ -433,7 +433,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_put(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.inactive_nomination.id,), host='api' ) @@ -446,7 +446,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_post(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.inactive_nomination.id,), host='api' ) @@ -459,7 +459,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_delete(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.inactive_nomination.id,), host='api' ) @@ -472,7 +472,7 @@ class NominationTests(APISubdomainTestCase): def test_returns_405_on_end_nomination_get(self): url = reverse( - 'bot:nomination-dismiss', + 'bot:nomination-end', args=(self.inactive_nomination.id,), host='api' ) @@ -482,3 +482,33 @@ class NominationTests(APISubdomainTestCase): self.assertEqual(response.json(), { "detail": "Method \"GET\" not allowed." }) + + def test_filter_returns_0_objects_unknown_user__id(self): + url = reverse('bot:nomination-list', host='api') + + response = self.client.get( + url, + data={ + "user__id": 99998888 + } + ) + + self.assertEqual(response.status_code, 200) + infractions = response.json() + + self.assertEqual(len(infractions), 0) + + def test_filter_returns_2_objects_for_testdata(self): + url = reverse('bot:nomination-list', host='api') + + response = self.client.get( + url, + data={ + "user__id": self.user.id + } + ) + + self.assertEqual(response.status_code, 200) + infractions = response.json() + + self.assertEqual(len(infractions), 2) -- cgit v1.2.3 From e714fe3750181ac49c790f6724b566885fcc5a10 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 28 Jun 2019 20:34:32 +0200 Subject: Moving end nomination to PUT method and removing separate end point --- pydis_site/apps/api/models/bot/nomination.py | 2 +- pydis_site/apps/api/serializers.py | 2 +- pydis_site/apps/api/tests/test_nominations.py | 113 ++++++------------------- pydis_site/apps/api/viewsets/bot/nomination.py | 29 +++---- 4 files changed, 42 insertions(+), 104 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 227a40ec..9a80e956 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -30,7 +30,7 @@ class Nomination(ModelReprMixin, models.Model): auto_now_add=True, help_text="The creation date of this nomination." ) - unnominate_reason = models.TextField( + end_reason = models.TextField( help_text="Why the nomination was ended.", default="" ) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index abf49393..2a6d8fce 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -276,4 +276,4 @@ class NominationSerializer(ModelSerializer): model = Nomination fields = ( 'id', 'active', 'actor', 'reason', 'user', - 'inserted_at', 'unnominate_reason', 'unwatched_at') + 'inserted_at', 'end_reason', 'unwatched_at') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index aa6561c4..39b2d159 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -122,19 +122,19 @@ class CreationTests(APISubdomainTestCase): 'actor': ['Invalid pk "1024" - object does not exist.'] }) - def test_returns_400_for_unnominate_reason_at_creation(self): + def test_returns_400_for_end_reason_at_creation(self): url = reverse('bot:nomination-list', host='api') data = { 'user': self.user.id, 'reason': 'Joe Dart on Fender Bass', 'actor': self.user.id, - 'unnominate_reason': "Joe Dart on the Joe Dart Bass" + 'end_reason': "Joe Dart on the Joe Dart Bass" } response = self.client.post(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'unnominate_reason': ['This field cannot be set at creation.'] + 'end_reason': ['This field cannot be set at creation.'] }) def test_returns_400_for_unwatched_at_at_creation(self): @@ -203,7 +203,7 @@ class NominationTests(APISubdomainTestCase): actor=cls.user, reason="He's pretty funky", active=False, - unnominate_reason="His neck couldn't hold the funk", + end_reason="His neck couldn't hold the funk", unwatched_at="5018-11-20T15:52:00+00:00" ) @@ -231,16 +231,16 @@ class NominationTests(APISubdomainTestCase): 'user': ['This field cannot be updated.'] }) - def test_returns_400_update_unnominate_reason_on_active(self): + def test_returns_400_update_end_reason_on_active(self): url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') data = { - 'unnominate_reason': 'He started playing jazz' + 'end_reason': 'He started playing jazz' } response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'unnominate_reason': ["An active nomination can't have an unnominate reason."] + 'end_reason': ["An active nomination can't have an unnominate reason."] }) def test_returns_200_update_reason_on_inactive(self): @@ -255,28 +255,28 @@ class NominationTests(APISubdomainTestCase): nomination = Nomination.objects.get(id=response.json()['id']) self.assertEqual(nomination.reason, data['reason']) - def test_returns_200_update_unnominate_reason_on_inactive(self): + def test_returns_200_update_end_reason_on_inactive(self): url = reverse('bot:nomination-detail', args=(self.inactive_nomination.id,), host='api') data = { - 'unnominate_reason': 'He started playing jazz' + 'end_reason': 'He started playing jazz' } response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 200) nomination = Nomination.objects.get(id=response.json()['id']) - self.assertEqual(nomination.unnominate_reason, data['unnominate_reason']) + self.assertEqual(nomination.end_reason, data['end_reason']) def test_returns_200_on_valid_end_nomination(self): url = reverse( - 'bot:nomination-end', + 'bot:nomination-detail', args=(self.active_nomination.id,), host='api' ) data = { - 'unnominate_reason': 'He started playing jazz' + 'end_reason': 'He started playing jazz' } - response = self.client.patch(url, data=data) + response = self.client.put(url, data=data) self.assertEqual(response.status_code, 200) nomination = Nomination.objects.get(id=response.json()['id']) @@ -287,48 +287,48 @@ class NominationTests(APISubdomainTestCase): delta=timedelta(seconds=2) ) self.assertFalse(nomination.active) - self.assertEqual(nomination.unnominate_reason, data['unnominate_reason']) + self.assertEqual(nomination.end_reason, data['end_reason']) def test_returns_400_on_invalid_field_end_nomination(self): url = reverse( - 'bot:nomination-end', + 'bot:nomination-detail', args=(self.active_nomination.id,), host='api' ) data = { 'reason': 'Why does a whale have feet?' } - response = self.client.patch(url, data=data) + response = self.client.put(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { 'reason': ['This field cannot be set at end_nomination.'] }) - def test_returns_400_on_missing_unnominate_reason_end_nomination(self): + def test_returns_400_on_missing_end_reason_end_nomination(self): url = reverse( - 'bot:nomination-end', + 'bot:nomination-detail', args=(self.active_nomination.id,), host='api' ) data = {} - response = self.client.patch(url, data=data) + response = self.client.put(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'unnominate_reason': ['This field is required when ending a nomination.'] + 'end_reason': ['This field is required when ending a nomination.'] }) def test_returns_400_on_ending_inactive_nomination(self): url = reverse( - 'bot:nomination-end', + 'bot:nomination-detail', args=(self.inactive_nomination.id,), host='api' ) data = { - 'unnominate_reason': 'He started playing jazz' + 'end_reason': 'He started playing jazz' } - response = self.client.patch(url, data=data) + response = self.client.put(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { 'active': ['A nomination must be active to be ended.'] @@ -362,16 +362,16 @@ class NominationTests(APISubdomainTestCase): def test_returns_404_on_end_unknown_nomination(self): url = reverse( - 'bot:nomination-end', + 'bot:nomination-detail', args=(9999,), host='api' ) data = { - 'unnominate_reason': 'He started playing jazz' + 'end_reason': 'He started playing jazz' } - response = self.client.patch(url, data=data) + response = self.client.put(url, data=data) self.assertEqual(response.status_code, 404) self.assertEqual(response.json(), { "detail": "Not found." @@ -404,15 +404,6 @@ class NominationTests(APISubdomainTestCase): "detail": "Method \"DELETE\" not allowed." }) - def test_returns_405_on_detail_put(self): - url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') - - response = self.client.put(url, data={}) - self.assertEqual(response.status_code, 405) - self.assertEqual(response.json(), { - "detail": "Method \"PUT\" not allowed." - }) - def test_returns_405_on_detail_post(self): url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') @@ -431,58 +422,6 @@ class NominationTests(APISubdomainTestCase): "detail": "Method \"DELETE\" not allowed." }) - def test_returns_405_on_end_nomination_put(self): - url = reverse( - 'bot:nomination-end', - args=(self.inactive_nomination.id,), - host='api' - ) - - response = self.client.put(url, data={}) - self.assertEqual(response.status_code, 405) - self.assertEqual(response.json(), { - "detail": "Method \"PUT\" not allowed." - }) - - def test_returns_405_on_end_nomination_post(self): - url = reverse( - 'bot:nomination-end', - args=(self.inactive_nomination.id,), - host='api' - ) - - response = self.client.post(url, data={}) - self.assertEqual(response.status_code, 405) - self.assertEqual(response.json(), { - "detail": "Method \"POST\" not allowed." - }) - - def test_returns_405_on_end_nomination_delete(self): - url = reverse( - 'bot:nomination-end', - args=(self.inactive_nomination.id,), - host='api' - ) - - response = self.client.delete(url, data={}) - self.assertEqual(response.status_code, 405) - self.assertEqual(response.json(), { - "detail": "Method \"DELETE\" not allowed." - }) - - def test_returns_405_on_end_nomination_get(self): - url = reverse( - 'bot:nomination-end', - args=(self.inactive_nomination.id,), - host='api' - ) - - response = self.client.get(url, data={}) - self.assertEqual(response.status_code, 405) - self.assertEqual(response.json(), { - "detail": "Method \"GET\" not allowed." - }) - def test_filter_returns_0_objects_unknown_user__id(self): url = reverse('bot:nomination-list', host='api') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index e8d50728..3d311f72 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -44,7 +44,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... 'reason': 'They know how to explain difficult concepts', ... 'user': 336843820513755157, ... 'inserted_at': '2019-04-25T14:02:37.775587Z', - ... 'unnominate_reason': 'They were helpered after a staff-vote', + ... 'end_reason': 'They were helpered after a staff-vote', ... 'unwatched_at': '2019-04-26T15:12:22.123587Z' ... } ... ] @@ -63,7 +63,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... 'reason': 'They know how to explain difficult concepts', ... 'user': 336843820513755157, ... 'inserted_at': '2019-04-25T14:02:37.775587Z', - ... 'unnominate_reason': 'They were helpered after a staff-vote', + ... 'end_reason': 'They were helpered after a staff-vote', ... 'unwatched_at': '2019-04-26T15:12:22.123587Z' ... } @@ -98,12 +98,12 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ### PATCH /bot/nominations/ Update the nomination with the given `id` and return the updated nomination. For active nominations, only the `reason` may be updated; for inactive - nominations, both the `reason` and `unnominate_reason` may be updated. + nominations, both the `reason` and `end_reason` may be updated. #### Request body >>> { ... 'reason': 'He would make a great helper', - ... 'unnominate_reason': 'He needs some time to mature his Python knowledge' + ... 'end_reason': 'He needs some time to mature his Python knowledge' ... } ### Response format @@ -117,14 +117,14 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ### PATCH /bot/nominations//end_nomination Ends an active nomination and returns the updated nomination. - The `unnominate_reason` field is the only allowed and required field + The `end_reason` field is the only allowed and required field for this operation. The nomination will automatically be marked as `active = false` and the datetime of this operation will be added to the `unwatched_at` field. #### Request body >>> { - ... 'unnominate_reason': 'He needs some time to mature his Python knowledge' + ... 'end_reason': 'He needs some time to mature his Python knowledge' ... } ### Response format @@ -133,7 +133,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge #### Status codes - 200: returned on success - 400: returned on failure for the following reasons: - - `unnominate_reason` is missing from the request body; + - `end_reason` is missing from the request body; - Any other field is present in the request body; - The nomination was already inactiive. - 404: if an infraction with the given `id` could not be found @@ -143,7 +143,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active') frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'unwatched_at', 'active') - frozen_on_create = ('unwatched_at', 'unnominate_reason', 'active', 'inserted_at') + frozen_on_create = ('unwatched_at', 'end_reason', 'active', 'inserted_at') def create(self, request, *args, **kwargs): """ @@ -182,9 +182,9 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge instance = self.get_object() - if instance.active and request.data.get('unnominate_reason'): + if instance.active and request.data.get('end_reason'): raise ValidationError( - {'unnominate_reason': ["An active nomination can't have an unnominate reason."]} + {'end_reason': ["An active nomination can't have an unnominate reason."]} ) serializer = self.get_serializer(instance, data=request.data, partial=True) @@ -193,8 +193,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge return Response(serializer.data) - @action(detail=True, methods=['patch']) - def end(self, request, pk=None): + def update(self, request, *args, **kwargs): """ DRF action for ending an active nomination. @@ -202,12 +201,12 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge the class docstring for documentation. """ for field in request.data: - if field != "unnominate_reason": + if field != "end_reason": raise ValidationError({field: ['This field cannot be set at end_nomination.']}) - if "unnominate_reason" not in request.data: + if "end_reason" not in request.data: raise ValidationError( - {'unnominate_reason': ['This field is required when ending a nomination.']} + {'end_reason': ['This field is required when ending a nomination.']} ) instance = self.get_object() -- cgit v1.2.3 From 3fe8e904bc2f69fa315188334c301bea0d9494ed Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 28 Jun 2019 20:51:48 +0200 Subject: Nomination API: Changing 'unwatched_at' to 'ended_at' --- .../0037_nomination_field_name_change.py | 23 ++++++++++++++++++++++ pydis_site/apps/api/models/bot/nomination.py | 2 +- pydis_site/apps/api/serializers.py | 2 +- pydis_site/apps/api/tests/test_nominations.py | 10 +++++----- pydis_site/apps/api/viewsets/bot/nomination.py | 12 +++++------ 5 files changed, 36 insertions(+), 13 deletions(-) create mode 100644 pydis_site/apps/api/migrations/0037_nomination_field_name_change.py (limited to 'pydis_site') diff --git a/pydis_site/apps/api/migrations/0037_nomination_field_name_change.py b/pydis_site/apps/api/migrations/0037_nomination_field_name_change.py new file mode 100644 index 00000000..c5f2d0c5 --- /dev/null +++ b/pydis_site/apps/api/migrations/0037_nomination_field_name_change.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2 on 2019-06-28 18:09 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0036_alter_nominations_api'), + ] + + operations = [ + migrations.RenameField( + model_name='nomination', + old_name='unnominate_reason', + new_name='end_reason', + ), + migrations.RenameField( + model_name='nomination', + old_name='unwatched_at', + new_name='ended_at', + ), + ] diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 9a80e956..8a8f4d36 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -34,7 +34,7 @@ class Nomination(ModelReprMixin, models.Model): help_text="Why the nomination was ended.", default="" ) - unwatched_at = models.DateTimeField( + ended_at = models.DateTimeField( auto_now_add=False, help_text="When the nomination was ended.", null=True diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 2a6d8fce..f1200d34 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -276,4 +276,4 @@ class NominationSerializer(ModelSerializer): model = Nomination fields = ( 'id', 'active', 'actor', 'reason', 'user', - 'inserted_at', 'end_reason', 'unwatched_at') + 'inserted_at', 'end_reason', 'ended_at') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 39b2d159..206dc65b 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -137,19 +137,19 @@ class CreationTests(APISubdomainTestCase): 'end_reason': ['This field cannot be set at creation.'] }) - def test_returns_400_for_unwatched_at_at_creation(self): + def test_returns_400_for_ended_at_at_creation(self): url = reverse('bot:nomination-list', host='api') data = { 'user': self.user.id, 'reason': 'Joe Dart on Fender Bass', 'actor': self.user.id, - 'unwatched_at': "Joe Dart on the Joe Dart Bass" + 'ended_at': "Joe Dart on the Joe Dart Bass" } response = self.client.post(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'unwatched_at': ['This field cannot be set at creation.'] + 'ended_at': ['This field cannot be set at creation.'] }) def test_returns_400_for_inserted_at_at_creation(self): @@ -204,7 +204,7 @@ class NominationTests(APISubdomainTestCase): reason="He's pretty funky", active=False, end_reason="His neck couldn't hold the funk", - unwatched_at="5018-11-20T15:52:00+00:00" + ended_at="5018-11-20T15:52:00+00:00" ) def test_returns_200_update_reason_on_active(self): @@ -282,7 +282,7 @@ class NominationTests(APISubdomainTestCase): nomination = Nomination.objects.get(id=response.json()['id']) self.assertAlmostEqual( - nomination.unwatched_at, + nomination.ended_at, dt.now(timezone.utc), delta=timedelta(seconds=2) ) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 3d311f72..f8ad5017 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -45,7 +45,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... 'user': 336843820513755157, ... 'inserted_at': '2019-04-25T14:02:37.775587Z', ... 'end_reason': 'They were helpered after a staff-vote', - ... 'unwatched_at': '2019-04-26T15:12:22.123587Z' + ... 'ended_at': '2019-04-26T15:12:22.123587Z' ... } ... ] @@ -64,7 +64,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... 'user': 336843820513755157, ... 'inserted_at': '2019-04-25T14:02:37.775587Z', ... 'end_reason': 'They were helpered after a staff-vote', - ... 'unwatched_at': '2019-04-26T15:12:22.123587Z' + ... 'ended_at': '2019-04-26T15:12:22.123587Z' ... } ### Status codes @@ -120,7 +120,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge The `end_reason` field is the only allowed and required field for this operation. The nomination will automatically be marked as `active = false` and the datetime of this operation will be added to - the `unwatched_at` field. + the `ended_at` field. #### Request body >>> { @@ -142,8 +142,8 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge queryset = Nomination.objects.all() filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active') - frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'unwatched_at', 'active') - frozen_on_create = ('unwatched_at', 'end_reason', 'active', 'inserted_at') + frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'ended_at', 'active') + frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at') def create(self, request, *args, **kwargs): """ @@ -216,7 +216,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge serializer = self.get_serializer(instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) instance.active = False - instance.unwatched_at = timezone.now() + instance.ended_at = timezone.now() serializer.save() instance.save() -- cgit v1.2.3 From 8da441f2b7745b5d5b5f94a76bf4db8341c07e38 Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 28 Jun 2019 22:24:48 +0200 Subject: Removing unused import --- pydis_site/apps/api/viewsets/bot/nomination.py | 1 - 1 file changed, 1 deletion(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index f8ad5017..2eec6275 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -3,7 +3,6 @@ from collections import ChainMap from django.utils import timezone from django_filters.rest_framework import DjangoFilterBackend from rest_framework import status -from rest_framework.decorators import action from rest_framework.exceptions import ValidationError from rest_framework.filters import OrderingFilter, SearchFilter from rest_framework.mixins import ( -- cgit v1.2.3 From 300c912d108ae2e35cb90983844ae5b785505d2e Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 3 Jul 2019 21:25:43 +0200 Subject: Move end nomination operation to PATCH endpoint --- pydis_site/apps/api/tests/test_nominations.py | 52 ++++++------ pydis_site/apps/api/viewsets/bot/nomination.py | 108 +++++++++++++------------ 2 files changed, 79 insertions(+), 81 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 206dc65b..add5a7e4 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -240,7 +240,7 @@ class NominationTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'end_reason': ["An active nomination can't have an unnominate reason."] + 'end_reason': ["An active nomination can't have an end reason."] }) def test_returns_200_update_reason_on_inactive(self): @@ -274,9 +274,10 @@ class NominationTests(APISubdomainTestCase): host='api' ) data = { + 'active': False, 'end_reason': 'He started playing jazz' } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 200) nomination = Nomination.objects.get(id=response.json()['id']) @@ -296,12 +297,13 @@ class NominationTests(APISubdomainTestCase): host='api' ) data = { - 'reason': 'Why does a whale have feet?' + 'active': False, + 'reason': 'Why does a whale have feet?', } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'reason': ['This field cannot be set at end_nomination.'] + 'reason': ['This field cannot be set when ending a nomination.'] }) def test_returns_400_on_missing_end_reason_end_nomination(self): @@ -310,28 +312,30 @@ class NominationTests(APISubdomainTestCase): args=(self.active_nomination.id,), host='api' ) - data = {} + data = { + 'active': False, + } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { 'end_reason': ['This field is required when ending a nomination.'] }) - def test_returns_400_on_ending_inactive_nomination(self): + def test_returns_400_on_invalid_use_of_active(self): url = reverse( 'bot:nomination-detail', args=(self.inactive_nomination.id,), host='api' ) data = { - 'end_reason': 'He started playing jazz' + 'active': False, } - response = self.client.put(url, data=data) + response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'active': ['A nomination must be active to be ended.'] + 'active': ['This field can only be used to end a nomination'] }) def test_returns_404_on_get_unknown_nomination(self): @@ -360,23 +364,6 @@ class NominationTests(APISubdomainTestCase): "detail": "Not found." }) - def test_returns_404_on_end_unknown_nomination(self): - url = reverse( - 'bot:nomination-detail', - args=(9999,), - host='api' - ) - - data = { - 'end_reason': 'He started playing jazz' - } - - response = self.client.put(url, data=data) - self.assertEqual(response.status_code, 404) - self.assertEqual(response.json(), { - "detail": "Not found." - }) - def test_returns_405_on_list_put(self): url = reverse('bot:nomination-list', host='api') @@ -422,6 +409,15 @@ class NominationTests(APISubdomainTestCase): "detail": "Method \"DELETE\" not allowed." }) + def test_returns_405_on_detail_put(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + + response = self.client.put(url, data={}) + self.assertEqual(response.status_code, 405) + self.assertEqual(response.json(), { + "detail": "Method \"PUT\" not allowed." + }) + def test_filter_returns_0_objects_unknown_user__id(self): url = reverse('bot:nomination-list', host='api') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 2eec6275..fad16aa5 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -67,7 +67,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... } ### Status codes - - 200: returned on succes + - 200: returned on success - 404: returned if a nomination with the given `id` could not be found ### POST /bot/nominations @@ -84,7 +84,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... 'user': 409107086526644234 ... } - ### Response format + #### Response format See `GET /bot/nominations/` #### Status codes @@ -95,53 +95,59 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge - The request contained a field that cannot be set at creation. ### PATCH /bot/nominations/ - Update the nomination with the given `id` and return the updated nomination. - For active nominations, only the `reason` may be updated; for inactive - nominations, both the `reason` and `end_reason` may be updated. + Update or end the nomination with the given `id` and return the updated nomination. + + The PATCH route can be used for three distinct operations: + 1. Updating the `reason` of `active` nomination; + 2. Ending an `active` nomination; + 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. + + While the response format and status codes are the same for all three operations (see + below), the request bodies vary depending on the operation. For all operations holds + that providing other valid fields is not allowed and invalid fields are ignored. + + ### 1. Updating the `reason` of `active` nomination #### Request body >>> { ... 'reason': 'He would make a great helper', - ... 'end_reason': 'He needs some time to mature his Python knowledge' ... } - ### Response format + #### Response format See `GET /bot/nominations/` - ## Status codes + #### Status codes - 200: returned on success - 400: if a field in the request body is invalid or disallowed - 404: if an infraction with the given `id` could not be found - ### PATCH /bot/nominations//end_nomination - Ends an active nomination and returns the updated nomination. + ### 2. Ending an `active` nomination - The `end_reason` field is the only allowed and required field - for this operation. The nomination will automatically be marked as - `active = false` and the datetime of this operation will be added to - the `ended_at` field. + #### Request body + >>> { + ... 'active': False + ... 'end_reason': 'They've been added to the Helpers team', + ... } + + See operation 1 for the response format and status codes. + + ### 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. #### Request body >>> { - ... 'end_reason': 'He needs some time to mature his Python knowledge' + ... 'reason': 'Updated reason for this nomination', + ... 'end_reason': 'Updated end_reason for this nomination', ... } - ### Response format - See `GET /bot/nominations/` + Note: The request body may contain either or both fields. - #### Status codes - - 200: returned on success - - 400: returned on failure for the following reasons: - - `end_reason` is missing from the request body; - - Any other field is present in the request body; - - The nomination was already inactiive. - - 404: if an infraction with the given `id` could not be found + See operation 1 for the response format and status codes. """ serializer_class = NominationSerializer queryset = Nomination.objects.all() filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'actor__id', 'active') - frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'ended_at', 'active') + frozen_fields = ('id', 'actor', 'inserted_at', 'user', 'ended_at') frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at') def create(self, request, *args, **kwargs): @@ -180,43 +186,39 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge raise ValidationError({field: ['This field cannot be updated.']}) instance = self.get_object() - - if instance.active and request.data.get('end_reason'): - raise ValidationError( - {'end_reason': ["An active nomination can't have an unnominate reason."]} - ) - serializer = self.get_serializer(instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) - serializer.save() - return Response(serializer.data) + data = serializer.validated_data - def update(self, request, *args, **kwargs): - """ - DRF action for ending an active nomination. + # There are three distinct PATCH scenarios we need to validate. + if instance.active and 'active' not in data: + # 1. We're updating an active nomination without ending it. + if request.data.get('end_reason'): + raise ValidationError( + {'end_reason': ["An active nomination can't have an end reason."]} + ) - Creates an API endpoint /bot/nominations/{id}/end_nomination to end a nomination. See - the class docstring for documentation. - """ - for field in request.data: - if field != "end_reason": - raise ValidationError({field: ['This field cannot be set at end_nomination.']}) + elif instance.active and not data.get('active'): + # 2. We're ending an active nomination. + if 'reason' in data: + raise ValidationError( + {'reason': ['This field cannot be set when ending a nomination.']} + ) + + if 'end_reason' not in request.data: + raise ValidationError( + {'end_reason': ['This field is required when ending a nomination.']} + ) + + instance.ended_at = timezone.now() - if "end_reason" not in request.data: + elif 'active' in data: + # 3. The `active` field is only allowed when ending a nomination. raise ValidationError( - {'end_reason': ['This field is required when ending a nomination.']} + {'active': ['This field can only be used to end a nomination']} ) - instance = self.get_object() - if not instance.active: - raise ValidationError({'active': ['A nomination must be active to be ended.']}) - - serializer = self.get_serializer(instance, data=request.data, partial=True) - serializer.is_valid(raise_exception=True) - instance.active = False - instance.ended_at = timezone.now() serializer.save() - instance.save() return Response(serializer.data) -- cgit v1.2.3 From 2b6c47274717bfcf4bcffc0e4f3fb41c2a78ddfe Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 4 Jul 2019 11:20:48 +0200 Subject: Apply suggestions from code review Co-Authored-By: Mark --- pydis_site/apps/api/viewsets/bot/nomination.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index fad16aa5..3595bec3 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -103,7 +103,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. While the response format and status codes are the same for all three operations (see - below), the request bodies vary depending on the operation. For all operations holds + below), the request bodies vary depending on the operation. For all operations it holds that providing other valid fields is not allowed and invalid fields are ignored. ### 1. Updating the `reason` of `active` nomination @@ -199,7 +199,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'end_reason': ["An active nomination can't have an end reason."]} ) - elif instance.active and not data.get('active'): + elif instance.active and not data['active']: # 2. We're ending an active nomination. if 'reason' in data: raise ValidationError( -- cgit v1.2.3 From 115a181cd51f14cd9cf74168fdf5914cbcec51bc Mon Sep 17 00:00:00 2001 From: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 12 Jul 2019 10:18:55 +0200 Subject: Changing logic so an end_reason can never be specified when updating an active nomination --- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site') diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 3595bec3..a9dacd68 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -194,7 +194,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge # There are three distinct PATCH scenarios we need to validate. if instance.active and 'active' not in data: # 1. We're updating an active nomination without ending it. - if request.data.get('end_reason'): + if 'end_reason' in data: raise ValidationError( {'end_reason': ["An active nomination can't have an end reason."]} ) -- cgit v1.2.3