diff options
author | 2021-03-08 19:48:58 +0200 | |
---|---|---|
committer | 2021-03-08 19:48:58 +0200 | |
commit | e0fc61a539d6b6ff50fbeb96beed639757d95ba9 (patch) | |
tree | dda6c829ba2c782b7578eaf230e5df3cbb1d58f8 | |
parent | Update Dockerfile (diff) | |
parent | Merge branch 'master' into ks123/nomination/split (diff) |
Merge pull request #447 from python-discord/ks123/nomination/split
Rework nominations
-rw-r--r-- | pydis_site/apps/api/admin.py | 80 | ||||
-rw-r--r-- | pydis_site/apps/api/migrations/0068_split_nomination_tables.py | 75 | ||||
-rw-r--r-- | pydis_site/apps/api/models/__init__.py | 1 | ||||
-rw-r--r-- | pydis_site/apps/api/models/bot/__init__.py | 2 | ||||
-rw-r--r-- | pydis_site/apps/api/models/bot/nomination.py | 52 | ||||
-rw-r--r-- | pydis_site/apps/api/serializers.py | 25 | ||||
-rw-r--r-- | pydis_site/apps/api/tests/test_models.py | 16 | ||||
-rw-r--r-- | pydis_site/apps/api/tests/test_nominations.py | 118 | ||||
-rw-r--r-- | pydis_site/apps/api/viewsets/bot/nomination.py | 142 |
9 files changed, 435 insertions, 76 deletions
diff --git a/pydis_site/apps/api/admin.py b/pydis_site/apps/api/admin.py index b6fee9d1..449e660e 100644 --- a/pydis_site/apps/api/admin.py +++ b/pydis_site/apps/api/admin.py @@ -21,6 +21,7 @@ from .models import ( Role, User ) +from .models.bot.nomination import NominationEntry admin.site.site_header = "Python Discord | Administration" admin.site.site_title = "Python Discord" @@ -218,7 +219,7 @@ class NominationActorFilter(admin.SimpleListFilter): def lookups(self, request: HttpRequest, model: NominationAdmin) -> Iterable[Tuple[int, str]]: """Selectable values for viewer to filter by.""" - actor_ids = Nomination.objects.order_by().values_list("actor").distinct() + actor_ids = NominationEntry.objects.order_by().values_list("actor").distinct() actors = User.objects.filter(id__in=actor_ids) return ((a.id, a.username) for a in actors) @@ -226,7 +227,10 @@ class NominationActorFilter(admin.SimpleListFilter): """Query to filter the list of Users against.""" if not self.value(): return - return queryset.filter(actor__id=self.value()) + nomination_ids = NominationEntry.objects.filter( + actor__id=self.value() + ).values_list("nomination_id").distinct() + return queryset.filter(id__in=nomination_ids) @admin.register(Nomination) @@ -236,9 +240,6 @@ class NominationAdmin(admin.ModelAdmin): search_fields = ( "user__name", "user__id", - "actor__name", - "actor__id", - "reason", "end_reason" ) @@ -247,27 +248,25 @@ class NominationAdmin(admin.ModelAdmin): list_display = ( "user", "active", - "reason", - "actor", + "reviewed" ) fields = ( "user", "active", - "actor", - "reason", "inserted_at", "ended_at", - "end_reason" + "end_reason", + "reviewed" ) - # only allow reason fields to be edited. + # only allow end reason field to be edited. readonly_fields = ( "user", "active", - "actor", "inserted_at", - "ended_at" + "ended_at", + "reviewed" ) def has_add_permission(self, *args) -> bool: @@ -275,6 +274,61 @@ class NominationAdmin(admin.ModelAdmin): return False +class NominationEntryActorFilter(admin.SimpleListFilter): + """Actor Filter for NominationEntry Admin list page.""" + + title = "Actor" + parameter_name = "actor" + + def lookups(self, request: HttpRequest, model: NominationAdmin) -> Iterable[Tuple[int, str]]: + """Selectable values for viewer to filter by.""" + actor_ids = NominationEntry.objects.order_by().values_list("actor").distinct() + actors = User.objects.filter(id__in=actor_ids) + return ((a.id, a.username) for a in actors) + + def queryset(self, request: HttpRequest, queryset: QuerySet) -> Optional[QuerySet]: + """Query to filter the list of Users against.""" + if not self.value(): + return + return queryset.filter(actor__id=self.value()) + + [email protected](NominationEntry) +class NominationEntryAdmin(admin.ModelAdmin): + """Admin formatting for the NominationEntry model.""" + + search_fields = ( + "actor__name", + "actor__id", + "reason", + ) + + list_filter = (NominationEntryActorFilter,) + + list_display = ( + "nomination", + "actor", + ) + + fields = ( + "nomination", + "actor", + "reason", + "inserted_at", + ) + + # only allow reason field to be edited + readonly_fields = ( + "nomination", + "actor", + "inserted_at", + ) + + def has_add_permission(self, request: HttpRequest) -> bool: + """Disable adding new nomination entry from admin.""" + return False + + @admin.register(OffTopicChannelName) class OffTopicChannelNameAdmin(admin.ModelAdmin): """Admin formatting for the OffTopicChannelName model.""" diff --git a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py new file mode 100644 index 00000000..79825ed7 --- /dev/null +++ b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py @@ -0,0 +1,75 @@ +# Generated by Django 3.0.11 on 2021-02-21 15:32 + +from django.apps.registry import Apps +from django.db import backends, migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +import django.db.models.deletion +import pydis_site.apps.api.models.mixins + + +def migrate_nominations(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: + Nomination = apps.get_model("api", "Nomination") + NominationEntry = apps.get_model("api", "NominationEntry") + + for nomination in Nomination.objects.all(): + nomination_entry = NominationEntry( + nomination=nomination, + actor=nomination.actor, + reason=nomination.reason, + inserted_at=nomination.inserted_at + ) + nomination_entry.save() + + +def unmigrate_nominations(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: + Nomination = apps.get_model("api", "Nomination") + NominationEntry = apps.get_model("api", "NominationEntry") + + for entry in NominationEntry.objects.all(): + nomination = Nomination.objects.get(pk=entry.nomination.id) + nomination.actor = entry.actor + nomination.reason = entry.reason + nomination.inserted_at = entry.inserted_at + + nomination.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0067_add_voice_ban_infraction_type'), + ] + + operations = [ + migrations.CreateModel( + name='NominationEntry', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('reason', models.TextField(blank=True, help_text='Why the actor nominated this user.', default="")), + ('inserted_at', + models.DateTimeField(auto_now_add=True, help_text='The creation date of this nomination entry.')), + ('actor', 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')), + ('nomination', models.ForeignKey(help_text='The nomination this entry belongs to.', + on_delete=django.db.models.deletion.CASCADE, to='api.Nomination', + related_name='entries')), + ], + bases=(pydis_site.apps.api.models.mixins.ModelReprMixin, models.Model), + options={'ordering': ('-inserted_at',), 'verbose_name_plural': 'nomination entries'} + ), + migrations.RunPython(migrate_nominations, unmigrate_nominations), + migrations.RemoveField( + model_name='nomination', + name='actor', + ), + migrations.RemoveField( + model_name='nomination', + name='reason', + ), + migrations.AddField( + model_name='nomination', + name='reviewed', + field=models.BooleanField(default=False, help_text='Whether a review was made.'), + ), + ] diff --git a/pydis_site/apps/api/models/__init__.py b/pydis_site/apps/api/models/__init__.py index 0a8c90f6..fd5bf220 100644 --- a/pydis_site/apps/api/models/__init__.py +++ b/pydis_site/apps/api/models/__init__.py @@ -8,6 +8,7 @@ from .bot import ( Message, MessageDeletionContext, Nomination, + NominationEntry, OffensiveMessage, OffTopicChannelName, Reminder, diff --git a/pydis_site/apps/api/models/bot/__init__.py b/pydis_site/apps/api/models/bot/__init__.py index 1673b434..ac864de3 100644 --- a/pydis_site/apps/api/models/bot/__init__.py +++ b/pydis_site/apps/api/models/bot/__init__.py @@ -6,7 +6,7 @@ from .documentation_link import DocumentationLink from .infraction import Infraction from .message import Message from .message_deletion_context import MessageDeletionContext -from .nomination import Nomination +from .nomination import Nomination, NominationEntry from .off_topic_channel_name import OffTopicChannelName from .offensive_message import OffensiveMessage from .reminder import Reminder diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 11b9e36e..221d8534 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -5,23 +5,12 @@ from pydis_site.apps.api.models.mixins import ModelReprMixin class Nomination(ModelReprMixin, models.Model): - """A helper nomination created by staff.""" + """A general helper nomination information created by staff.""" active = models.BooleanField( default=True, help_text="Whether this nomination is still relevant." ) - actor = models.ForeignKey( - User, - on_delete=models.CASCADE, - help_text="The staff member that nominated this user.", - related_name='nomination_set' - ) - reason = models.TextField( - help_text="Why this user was nominated.", - null=True, - blank=True - ) user = models.ForeignKey( User, on_delete=models.CASCADE, @@ -42,6 +31,10 @@ class Nomination(ModelReprMixin, models.Model): help_text="When the nomination was ended.", null=True ) + reviewed = models.BooleanField( + default=False, + help_text="Whether a review was made." + ) def __str__(self): """Representation that makes the target and state of the nomination immediately evident.""" @@ -52,3 +45,38 @@ class Nomination(ModelReprMixin, models.Model): """Set the ordering of nominations to most recent first.""" ordering = ("-inserted_at",) + + +class NominationEntry(ModelReprMixin, models.Model): + """A nomination entry created by a single staff member.""" + + nomination = models.ForeignKey( + Nomination, + on_delete=models.CASCADE, + help_text="The nomination this entry belongs to.", + related_name="entries" + ) + actor = models.ForeignKey( + User, + on_delete=models.CASCADE, + help_text="The staff member that nominated this user.", + related_name='nomination_set' + ) + reason = models.TextField( + help_text="Why the actor nominated this user.", + default="", + blank=True + ) + inserted_at = models.DateTimeField( + auto_now_add=True, + help_text="The creation date of this nomination entry." + ) + + class Meta: + """Meta options for NominationEntry model.""" + + verbose_name_plural = "nomination entries" + + # Set default ordering here to latest first + # so we don't need to define it everywhere + ordering = ("-inserted_at",) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 10eb3839..f47bedca 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -20,6 +20,7 @@ from .models import ( Infraction, MessageDeletionContext, Nomination, + NominationEntry, OffTopicChannelName, OffensiveMessage, Reminder, @@ -338,16 +339,36 @@ class UserSerializer(ModelSerializer): raise ValidationError({"id": ["User with ID already present."]}) +class NominationEntrySerializer(ModelSerializer): + """A class providing (de-)serialization of `NominationEntry` instances.""" + + # We need to define it here, because we don't want that nomination ID + # return inside nomination response entry, because ID is already available + # as top-level field. Queryset is required if field is not read only. + nomination = PrimaryKeyRelatedField( + queryset=Nomination.objects.all(), + write_only=True + ) + + class Meta: + """Metadata defined for the Django REST framework.""" + + model = NominationEntry + fields = ('nomination', 'actor', 'reason', 'inserted_at') + + class NominationSerializer(ModelSerializer): """A class providing (de-)serialization of `Nomination` instances.""" + entries = NominationEntrySerializer(many=True, read_only=True) + class Meta: """Metadata defined for the Django REST Framework.""" model = Nomination fields = ( - 'id', 'active', 'actor', 'reason', 'user', - 'inserted_at', 'end_reason', 'ended_at') + 'id', 'active', 'user', 'inserted_at', 'end_reason', 'ended_at', 'reviewed', 'entries' + ) class OffensiveMessageSerializer(ModelSerializer): diff --git a/pydis_site/apps/api/tests/test_models.py b/pydis_site/apps/api/tests/test_models.py index 853e6621..66052e01 100644 --- a/pydis_site/apps/api/tests/test_models.py +++ b/pydis_site/apps/api/tests/test_models.py @@ -10,6 +10,7 @@ from pydis_site.apps.api.models import ( Message, MessageDeletionContext, Nomination, + NominationEntry, OffTopicChannelName, OffensiveMessage, Reminder, @@ -37,17 +38,11 @@ class StringDunderMethodTests(SimpleTestCase): def setUp(self): self.nomination = Nomination( id=123, - actor=User( - id=9876, - name='Mr. Hemlock', - discriminator=6666, - ), user=User( id=9876, name="Hemlock's Cat", discriminator=7777, ), - reason="He purrrrs like the best!", ) self.objects = ( @@ -135,6 +130,15 @@ class StringDunderMethodTests(SimpleTestCase): ), content="oh no", expiration=dt(5018, 11, 20, 15, 52, tzinfo=timezone.utc) + ), + NominationEntry( + nomination_id=self.nomination.id, + actor=User( + id=9876, + name='Mr. Hemlock', + discriminator=6666, + ), + reason="He purrrrs like the best!", ) ) diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index b37135f8..9cefbd8f 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -3,7 +3,7 @@ from datetime import datetime as dt, timedelta, timezone from django_hosts.resolvers import reverse from .base import APISubdomainTestCase -from ..models import Nomination, User +from ..models import Nomination, NominationEntry, User class CreationTests(APISubdomainTestCase): @@ -14,6 +14,11 @@ class CreationTests(APISubdomainTestCase): name='joe dart', discriminator=1111, ) + cls.user2 = User.objects.create( + id=9876, + name='Who?', + discriminator=1234 + ) def test_accepts_valid_data(self): url = reverse('bot:nomination-list', host='api') @@ -27,17 +32,39 @@ class CreationTests(APISubdomainTestCase): self.assertEqual(response.status_code, 201) nomination = Nomination.objects.get(id=response.json()['id']) + nomination_entry = NominationEntry.objects.get( + nomination_id=nomination.id, + actor_id=self.user.id + ) self.assertAlmostEqual( nomination.inserted_at, 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_entry.reason, data['reason']) self.assertEqual(nomination.active, True) - def test_returns_400_on_second_active_nomination(self): + def test_returns_200_on_second_active_nomination_by_different_user(self): + url = reverse('bot:nomination-list', host='api') + first_data = { + 'actor': self.user.id, + 'reason': 'Joe Dart on Fender Bass', + 'user': self.user.id, + } + second_data = { + 'actor': self.user2.id, + 'reason': 'Great user', + 'user': self.user.id + } + + response1 = self.client.post(url, data=first_data) + self.assertEqual(response1.status_code, 201) + + response2 = self.client.post(url, data=second_data) + self.assertEqual(response2.status_code, 201) + + def test_returns_400_on_second_active_nomination_by_existing_nominator(self): url = reverse('bot:nomination-list', host='api') data = { 'actor': self.user.id, @@ -51,7 +78,7 @@ class CreationTests(APISubdomainTestCase): 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.'] + 'actor': ['This actor has already endorsed this nomination.'] }) def test_returns_400_for_missing_user(self): @@ -189,30 +216,40 @@ class NominationTests(APISubdomainTestCase): ) cls.active_nomination = Nomination.objects.create( - user=cls.user, + user=cls.user + ) + cls.active_nomination_entry = NominationEntry.objects.create( + nomination=cls.active_nomination, 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, end_reason="His neck couldn't hold the funk", ended_at="5018-11-20T15:52:00+00:00" ) + cls.inactive_nomination_entry = NominationEntry.objects.create( + nomination=cls.inactive_nomination, + actor=cls.user, + reason="He's pretty funky" + ) - def test_returns_200_update_reason_on_active(self): + def test_returns_200_update_reason_on_active_with_actor(self): url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') data = { - 'reason': "He's one funky duck" + 'reason': "He's one funky duck", + 'actor': self.user.id } 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']) + nomination_entry = NominationEntry.objects.get( + nomination_id=response.json()['id'], + actor_id=self.user.id + ) + self.assertEqual(nomination_entry.reason, data['reason']) def test_returns_400_on_frozen_field_update(self): url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') @@ -241,14 +278,18 @@ class NominationTests(APISubdomainTestCase): 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" + 'reason': "He's one funky duck", + 'actor': self.user.id } 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']) + nomination_entry = NominationEntry.objects.get( + nomination_id=response.json()['id'], + actor_id=self.user.id + ) + self.assertEqual(nomination_entry.reason, data['reason']) def test_returns_200_update_end_reason_on_inactive(self): url = reverse('bot:nomination-detail', args=(self.inactive_nomination.id,), host='api') @@ -442,3 +483,50 @@ class NominationTests(APISubdomainTestCase): infractions = response.json() self.assertEqual(len(infractions), 2) + + def test_patch_nomination_set_reviewed_of_active_nomination(self): + url = reverse('api:nomination-detail', args=(self.active_nomination.id,), host='api') + data = {'reviewed': True} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 200) + + def test_patch_nomination_set_reviewed_of_inactive_nomination(self): + url = reverse('api:nomination-detail', args=(self.inactive_nomination.id,), host='api') + data = {'reviewed': True} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'reviewed': ['This field cannot be set if the nomination is inactive.'] + }) + + def test_patch_nomination_set_reviewed_and_end(self): + url = reverse('api:nomination-detail', args=(self.active_nomination.id,), host='api') + data = {'reviewed': True, 'active': False, 'end_reason': "What?"} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'reviewed': ['This field cannot be set while you are ending a nomination.'] + }) + + def test_modifying_reason_without_actor(self): + url = reverse('api:nomination-detail', args=(self.active_nomination.id,), host='api') + data = {'reason': 'That is my reason!'} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'actor': ['This field is required when editing the reason.'] + }) + + def test_modifying_reason_with_unknown_actor(self): + url = reverse('api:nomination-detail', args=(self.active_nomination.id,), host='api') + data = {'reason': 'That is my reason!', 'actor': 90909090909090} + + response = self.client.patch(url, data=data) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'actor': ["The actor doesn't exist or has not nominated the user."] + }) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index cf6e262f..144daab0 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -14,8 +14,8 @@ from rest_framework.mixins import ( from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet -from pydis_site.apps.api.models.bot import Nomination -from pydis_site.apps.api.serializers import NominationSerializer +from pydis_site.apps.api.models.bot import Nomination, NominationEntry +from pydis_site.apps.api.serializers import NominationEntrySerializer, NominationSerializer class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, GenericViewSet): @@ -29,7 +29,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge #### 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 @@ -40,12 +39,18 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ... { ... 'id': 1, ... 'active': false, - ... 'actor': 336843820513755157, - ... 'reason': 'They know how to explain difficult concepts', ... 'user': 336843820513755157, ... 'inserted_at': '2019-04-25T14:02:37.775587Z', ... 'end_reason': 'They were helpered after a staff-vote', - ... 'ended_at': '2019-04-26T15:12:22.123587Z' + ... 'ended_at': '2019-04-26T15:12:22.123587Z', + ... 'entries': [ + ... { + ... 'actor': 336843820513755157, + ... 'reason': 'They know how to explain difficult concepts', + ... 'inserted_at': '2019-04-25T14:02:37.775587Z' + ... } + ... ], + ... 'reviewed': true ... } ... ] @@ -59,12 +64,18 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge >>> { ... 'id': 1, ... 'active': true, - ... 'actor': 336843820513755157, - ... 'reason': 'They know how to explain difficult concepts', ... 'user': 336843820513755157, ... 'inserted_at': '2019-04-25T14:02:37.775587Z', ... 'end_reason': 'They were helpered after a staff-vote', - ... 'ended_at': '2019-04-26T15:12:22.123587Z' + ... 'ended_at': '2019-04-26T15:12:22.123587Z', + ... 'entries': [ + ... { + ... 'actor': 336843820513755157, + ... 'reason': 'They know how to explain difficult concepts', + ... 'inserted_at': '2019-04-25T14:02:37.775587Z' + ... } + ... ], + ... 'reviewed': false ... } ### Status codes @@ -75,8 +86,9 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 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. + is not allowed and invalid fields are ignored. If `user` already has an + active nomination, a new nomination entry will be created and assigned to the + active nomination. #### Request body >>> { @@ -91,7 +103,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge #### 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. @@ -102,16 +113,18 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 1. Updating the `reason` of `active` nomination; 2. Ending an `active` nomination; 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. + 4. Updating `reviewed` field of `active` 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 it holds that providing other valid fields is not allowed and invalid fields are ignored. - ### 1. Updating the `reason` of `active` nomination + ### 1. Updating the `reason` of `active` nomination. The `actor` field is required. #### Request body >>> { ... 'reason': 'He would make a great helper', + ... 'actor': 409107086526644234 ... } #### Response format @@ -133,24 +146,35 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge See operation 1 for the response format and status codes. ### 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. + Actor field is required when updating reason. #### Request body >>> { ... 'reason': 'Updated reason for this nomination', + ... 'actor': 409107086526644234, ... 'end_reason': 'Updated end_reason for this nomination', ... } Note: The request body may contain either or both fields. See operation 1 for the response format and status codes. + + ### 4. Setting nomination `reviewed` + + #### Request body + >>> { + ... 'reviewed': True + ... } + + 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') - frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at') + filter_fields = ('user__id', 'active') + frozen_fields = ('id', 'inserted_at', 'user', 'ended_at') + frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at', 'reviewed') def create(self, request: HttpRequest, *args, **kwargs) -> Response: """ @@ -163,19 +187,50 @@ 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).exists(): - raise ValidationError({'active': ['There can only be one active nomination.']}) + nomination_filter = Nomination.objects.filter(active=True, user__id=user_id) + + if not nomination_filter.exists(): + serializer = NominationSerializer( + data=ChainMap( + request.data, + {"active": True} + ) + ) + serializer.is_valid(raise_exception=True) + nomination = Nomination.objects.create(**serializer.validated_data) - serializer = self.get_serializer( - data=ChainMap( - request.data, - {"active": True} + # The serializer will truncate and get rid of excessive data + entry_serializer = NominationEntrySerializer( + data=ChainMap(request.data, {"nomination": nomination.id}) ) + entry_serializer.is_valid(raise_exception=True) + NominationEntry.objects.create(**entry_serializer.validated_data) + + data = NominationSerializer(nomination).data + + headers = self.get_success_headers(data) + return Response(data, status=status.HTTP_201_CREATED, headers=headers) + + entry_serializer = NominationEntrySerializer( + data=ChainMap(request.data, {"nomination": nomination_filter[0].id}) ) - 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) + entry_serializer.is_valid(raise_exception=True) + + # Don't allow a user to create many nomination entries in a single nomination + if NominationEntry.objects.filter( + nomination_id=nomination_filter[0].id, + actor__id=entry_serializer.validated_data["actor"].id + ).exists(): + raise ValidationError( + {'actor': ['This actor has already endorsed this nomination.']} + ) + + NominationEntry.objects.create(**entry_serializer.validated_data) + + data = NominationSerializer(nomination_filter[0]).data + + headers = self.get_success_headers(data) + return Response(data, status=status.HTTP_201_CREATED, headers=headers) def partial_update(self, request: HttpRequest, *args, **kwargs) -> Response: """ @@ -203,7 +258,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge elif instance.active and not data['active']: # 2. We're ending an active nomination. - if 'reason' in data: + if 'reason' in request.data: raise ValidationError( {'reason': ['This field cannot be set when ending a nomination.']} ) @@ -213,6 +268,11 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'end_reason': ['This field is required when ending a nomination.']} ) + if 'reviewed' in request.data: + raise ValidationError( + {'reviewed': ['This field cannot be set while you are ending a nomination.']} + ) + instance.ended_at = timezone.now() elif 'active' in data: @@ -221,6 +281,34 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'active': ['This field can only be used to end a nomination']} ) + # This is actually covered, but for some reason coverage don't think so. + elif 'reviewed' in request.data: # pragma: no cover + # 4. We are altering the reviewed state of the nomination. + if not instance.active: + raise ValidationError( + {'reviewed': ['This field cannot be set if the nomination is inactive.']} + ) + + if 'reason' in request.data: + if 'actor' not in request.data: + raise ValidationError( + {'actor': ['This field is required when editing the reason.']} + ) + + entry_filter = NominationEntry.objects.filter( + nomination_id=instance.id, + actor__id=request.data['actor'] + ) + + if not entry_filter.exists(): + raise ValidationError( + {'actor': ["The actor doesn't exist or has not nominated the user."]} + ) + + entry = entry_filter[0] + entry.reason = request.data['reason'] + entry.save() + serializer.save() return Response(serializer.data) |