From 9ac477385d5ed26d2d8e4f711b2c927cfaf35461 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 08:21:18 +0200 Subject: Split nomination model to 2 tables and create migrations --- .../api/migrations/0068_split_nomination_tables.py | 60 ++++++++++++++++++++++ .../0069_change_nomination_entry_plural.py | 17 ++++++ pydis_site/apps/api/models/bot/nomination.py | 47 ++++++++++++----- 3 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 pydis_site/apps/api/migrations/0068_split_nomination_tables.py create mode 100644 pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py 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..2e2313ee --- /dev/null +++ b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py @@ -0,0 +1,60 @@ +# 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() + + +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.', null=True)), + ('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='Nomination to what this entry belongs.', + on_delete=django.db.models.deletion.CASCADE, to='api.Nomination')), + ], + bases=(pydis_site.apps.api.models.mixins.ModelReprMixin, models.Model), + ), + migrations.RunPython(migrate_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 voting message have been made.'), + ), + ] diff --git a/pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py b/pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py new file mode 100644 index 00000000..6bf4ac8c --- /dev/null +++ b/pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py @@ -0,0 +1,17 @@ +# Generated by Django 3.0.11 on 2021-02-21 16:44 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0068_split_nomination_tables'), + ] + + operations = [ + migrations.AlterModelOptions( + name='nominationentry', + options={'verbose_name_plural': 'nomination entries'}, + ), + ] diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 11b9e36e..ed6f7d81 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 voting message have been made." + ) def __str__(self): """Representation that makes the target and state of the nomination immediately evident.""" @@ -52,3 +45,33 @@ 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 single staff.""" + + nomination = models.ForeignKey( + Nomination, + on_delete=models.CASCADE, + help_text="Nomination to what this entry belongs." + ) + 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.", + null=True, + 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" -- cgit v1.2.3 From ff46d9f59b5da3aff5b8efbca3214ee0d3c064c1 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 08:21:54 +0200 Subject: Change nominations admin interface and add nomination entries interface --- pydis_site/apps/api/admin.py | 80 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 13 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()) + + +@admin.register(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.""" -- cgit v1.2.3 From 4bc55a5e606de8037d52e992dd308f52c86a8e5f Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 08:22:15 +0200 Subject: Change nominations serializer and add nomination entry serializer --- pydis_site/apps/api/serializers.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 10eb3839..65c69849 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -26,6 +26,7 @@ from .models import ( Role, User ) +from .models.bot.nomination import NominationEntry class BotSettingSerializer(ModelSerializer): @@ -338,6 +339,21 @@ class UserSerializer(ModelSerializer): raise ValidationError({"id": ["User with ID already present."]}) +class NominationEntrySerializer(ModelSerializer): + """A class providing (de-)serialization of `NominationEntry` instances.""" + + 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.""" @@ -346,8 +362,8 @@ class NominationSerializer(ModelSerializer): model = Nomination fields = ( - 'id', 'active', 'actor', 'reason', 'user', - 'inserted_at', 'end_reason', 'ended_at') + 'id', 'active', 'user', 'inserted_at', 'end_reason', 'ended_at' + ) class OffensiveMessageSerializer(ModelSerializer): -- cgit v1.2.3 From 51c57d3707c684bb908195f419e53f4ed164ba3b Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 08:23:04 +0200 Subject: Update nominations viewset GET and POST to make this working with 2-table system --- pydis_site/apps/api/viewsets/bot/nomination.py | 119 ++++++++++++++++++++----- 1 file changed, 96 insertions(+), 23 deletions(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index cf6e262f..8775515c 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -15,7 +15,8 @@ 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.nomination import NominationEntry +from pydis_site.apps.api.serializers import NominationEntrySerializer, NominationSerializer class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, GenericViewSet): @@ -29,7 +30,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 +40,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 +65,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 +87,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 have + active nomination, new nomination entry will be created assigned to + active nomination. #### Request body >>> { @@ -91,7 +104,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. @@ -148,10 +160,44 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 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') + filter_fields = ('user__id', 'active') + frozen_fields = ('id', 'inserted_at', 'user', 'ended_at') frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at') + def list(self, request: HttpRequest, *args, **kwargs) -> Response: + """ + DRF method for listing Nominations. + + Called by the Django Rest Framework in response to the corresponding HTTP request. + """ + queryset = self.filter_queryset(self.get_queryset()) + data = NominationSerializer(queryset, many=True).data + + for i, nomination in enumerate(data): + entries = NominationEntrySerializer( + NominationEntry.objects.filter(nomination_id=nomination["id"]), + many=True + ).data + data[i]["entries"] = entries + + return Response(data) + + def retrieve(self, request: HttpRequest, *args, **kwargs) -> Response: + """ + DRF method for retrieving a Nomination. + + Called by the Django Rest Framework in response to the corresponding HTTP request. + """ + nomination = self.get_object() + + data = NominationSerializer(nomination).data + data["entries"] = NominationEntrySerializer( + NominationEntry.objects.filter(nomination_id=nomination.id), + many=True + ).data + + return Response(data) + def create(self, request: HttpRequest, *args, **kwargs) -> Response: """ DRF method for creating a Nomination. @@ -163,19 +209,46 @@ 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} + # Serializer truncate unnecessary data away + entry_serializer = NominationEntrySerializer( + data=ChainMap(request.data, {"nomination": nomination.id}) ) + entry_serializer.is_valid(raise_exception=True) + + entry = NominationEntry.objects.create(**entry_serializer.validated_data) + + data = NominationSerializer(nomination).data + data["entries"] = NominationEntrySerializer([entry], many=True).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) + NominationEntry.objects.create(**entry_serializer.validated_data) + + data = NominationSerializer(nomination_filter[0]).data + data["entries"] = NominationEntrySerializer( + NominationEntry.objects.filter(nomination_id=nomination_filter[0].id), + many=True + ).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: """ -- cgit v1.2.3 From 71388fff77dcb2b074e02dfe33133d779c9afa2d Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:00:54 +0200 Subject: Add reviewed field to nomination serializer --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 65c69849..96f28aee 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -362,7 +362,7 @@ class NominationSerializer(ModelSerializer): model = Nomination fields = ( - 'id', 'active', 'user', 'inserted_at', 'end_reason', 'ended_at' + 'id', 'active', 'user', 'inserted_at', 'end_reason', 'ended_at', 'reviewed' ) -- cgit v1.2.3 From e17da1f2e991710beaabd37701ce06a57f7b4a77 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:01:18 +0200 Subject: Migrate PATCH request for 2-table nominations system --- pydis_site/apps/api/viewsets/bot/nomination.py | 70 ++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 8775515c..14dee9bc 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -112,18 +112,20 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 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. + 2. Updating `reviewed` field of `active` nomination. + 3. Ending an `active` nomination; + 4. 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 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. Actor field is required. #### Request body >>> { ... 'reason': 'He would make a great helper', + ... 'actor': 409107086526644234 ... } #### Response format @@ -134,7 +136,16 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge - 400: if a field in the request body is invalid or disallowed - 404: if an infraction with the given `id` could not be found - ### 2. Ending an `active` nomination + ### 2. Setting nomination `reviewed` + + #### Request body + >>> { + ... 'reviewed': True + ... } + + See operation 1 for the response format and status codes. + + ### 3. Ending an `active` nomination #### Request body >>> { @@ -144,11 +155,13 @@ 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. + ### 4. 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', ... } @@ -162,7 +175,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter) filter_fields = ('user__id', 'active') frozen_fields = ('id', 'inserted_at', 'user', 'ended_at') - frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at') + frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at', 'reviewed') def list(self, request: HttpRequest, *args, **kwargs) -> Response: """ @@ -274,8 +287,20 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'end_reason': ["An active nomination can't have an end reason."]} ) + elif 'reviewed' in data: + # 2. We're setting nomination reviewed + if not instance.active: + raise ValidationError( + {'reviewed': 'This field cannot be set if nomination is inactive.'} + ) + + if 'active' in data: + raise ValidationError( + {'active': 'This field cannot be set same time than ending nomination.'} + ) + elif instance.active and not data['active']: - # 2. We're ending an active nomination. + # 3. We're ending an active nomination. if 'reason' in data: raise ValidationError( {'reason': ['This field cannot be set when ending a nomination.']} @@ -289,11 +314,36 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge instance.ended_at = timezone.now() elif 'active' in data: - # 3. The `active` field is only allowed when ending a nomination. + # 4. The `active` field is only allowed when ending a nomination. raise ValidationError( {'active': ['This field can only be used to end a nomination']} ) - serializer.save() + if 'reason' in request.data: + if 'actor' not in request.data: + raise ValidationError( + {'actor': 'This field is required when editing reason.'} + ) + + entry_filter = NominationEntry.objects.filter( + nomination_id=instance.id, + actor__id=request.data['actor'] + ) + + if not entry_filter.exists(): + raise ValidationError( + {'actor': "Actor don't exist or have not nominated user."} + ) + + entry = entry_filter[0] + entry.reason = request.data['reason'] + entry.save() + + nomination = serializer.save() + return_data = NominationSerializer(nomination).data + return_data["entries"] = NominationEntrySerializer( + NominationEntry.objects.filter(nomination_id=nomination.id), + many=True + ).data - return Response(serializer.data) + return Response(return_data) -- cgit v1.2.3 From 0ae69d9892db73029336e7b9ca95164dd80f828f Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:05:54 +0200 Subject: Disable creating multiple nomination entries of one nomination for one actor --- pydis_site/apps/api/viewsets/bot/nomination.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 14dee9bc..81fb43f7 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -252,6 +252,16 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge data=ChainMap(request.data, {"nomination": nomination_filter[0].id}) ) entry_serializer.is_valid(raise_exception=True) + + # Don't allow user creating many nomination entries for one 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 have already created nomination entry for this nomination.'} + ) + NominationEntry.objects.create(**entry_serializer.validated_data) data = NominationSerializer(nomination_filter[0]).data -- cgit v1.2.3 From 2fb5bf5dbdc6cdfe0a62d54bc7b726eab0199a59 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:36:55 +0200 Subject: Add rollback to nominations table split migration --- .../apps/api/migrations/0068_split_nomination_tables.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py index 2e2313ee..1c392365 100644 --- a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py +++ b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py @@ -21,6 +21,19 @@ def migrate_nominations(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> 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 = [ @@ -43,7 +56,7 @@ class Migration(migrations.Migration): ], bases=(pydis_site.apps.api.models.mixins.ModelReprMixin, models.Model), ), - migrations.RunPython(migrate_nominations), + migrations.RunPython(migrate_nominations, unmigrate_nominations), migrations.RemoveField( model_name='nomination', name='actor', -- cgit v1.2.3 From 698cbc17405a49fe42669fedd6054ab6c9008a4a Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:37:12 +0200 Subject: Wrap validation errors to [] --- pydis_site/apps/api/viewsets/bot/nomination.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 81fb43f7..c4600425 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -259,7 +259,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge actor__id=entry_serializer.validated_data["actor"].id ).exists(): raise ValidationError( - {'actor': 'This actor have already created nomination entry for this nomination.'} + {'actor': ['This actor have already created nomination entry for this nomination.']} ) NominationEntry.objects.create(**entry_serializer.validated_data) @@ -301,12 +301,12 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge # 2. We're setting nomination reviewed if not instance.active: raise ValidationError( - {'reviewed': 'This field cannot be set if nomination is inactive.'} + {'reviewed': ['This field cannot be set if nomination is inactive.']} ) if 'active' in data: raise ValidationError( - {'active': 'This field cannot be set same time than ending nomination.'} + {'active': ['This field cannot be set same time than ending nomination.']} ) elif instance.active and not data['active']: @@ -332,7 +332,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge if 'reason' in request.data: if 'actor' not in request.data: raise ValidationError( - {'actor': 'This field is required when editing reason.'} + {'actor': ['This field is required when editing reason.']} ) entry_filter = NominationEntry.objects.filter( @@ -342,7 +342,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge if not entry_filter.exists(): raise ValidationError( - {'actor': "Actor don't exist or have not nominated user."} + {'actor': ["Actor don't exist or have not nominated user."]} ) entry = entry_filter[0] -- cgit v1.2.3 From 6591270f832e93a3eaf941aa1aeeddb6af7bce36 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 11:36:25 +0200 Subject: Import NominationEntry to models __init__.py --- pydis_site/apps/api/models/__init__.py | 1 + pydis_site/apps/api/models/bot/__init__.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) 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 -- cgit v1.2.3 From 126b5084f1f583790bcd1a386474230012d7cadb Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 11:36:47 +0200 Subject: Change NominationEntry import location in serializers --- pydis_site/apps/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 96f28aee..49c247b5 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -20,13 +20,13 @@ from .models import ( Infraction, MessageDeletionContext, Nomination, + NominationEntry, OffTopicChannelName, OffensiveMessage, Reminder, Role, User ) -from .models.bot.nomination import NominationEntry class BotSettingSerializer(ModelSerializer): -- cgit v1.2.3 From 3907122c9e76f78bd7bfd07028e9eb79b43d65b3 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 11:37:33 +0200 Subject: Small improvements in nomination viewset --- pydis_site/apps/api/viewsets/bot/nomination.py | 62 +++++++++++++------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index c4600425..7820ca0d 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -14,8 +14,7 @@ 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.models.bot.nomination import NominationEntry +from pydis_site.apps.api.models.bot import Nomination, NominationEntry from pydis_site.apps.api.serializers import NominationEntrySerializer, NominationSerializer @@ -112,9 +111,9 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge The PATCH route can be used for three distinct operations: 1. Updating the `reason` of `active` nomination; - 2. Updating `reviewed` field of `active` nomination. - 3. Ending an `active` nomination; - 4. Updating the `end_reason` or `reason` field of an `inactive` 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 @@ -136,16 +135,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge - 400: if a field in the request body is invalid or disallowed - 404: if an infraction with the given `id` could not be found - ### 2. Setting nomination `reviewed` - - #### Request body - >>> { - ... 'reviewed': True - ... } - - See operation 1 for the response format and status codes. - - ### 3. Ending an `active` nomination + ### 2. Ending an `active` nomination #### Request body >>> { @@ -155,7 +145,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge See operation 1 for the response format and status codes. - ### 4. Updating the `end_reason` or `reason` field of an `inactive` nomination. + ### 3. Updating the `end_reason` or `reason` field of an `inactive` nomination. Actor field is required when updating reason. #### Request body @@ -167,6 +157,15 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 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. """ @@ -297,21 +296,9 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge {'end_reason': ["An active nomination can't have an end reason."]} ) - elif 'reviewed' in data: - # 2. We're setting nomination reviewed - if not instance.active: - raise ValidationError( - {'reviewed': ['This field cannot be set if nomination is inactive.']} - ) - - if 'active' in data: - raise ValidationError( - {'active': ['This field cannot be set same time than ending nomination.']} - ) - elif instance.active and not data['active']: - # 3. We're ending an active nomination. - if 'reason' in data: + # 2. We're ending an active nomination. + if 'reason' in request.data: raise ValidationError( {'reason': ['This field cannot be set when ending a nomination.']} ) @@ -321,14 +308,27 @@ 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 same time than ending nomination.']} + ) + instance.ended_at = timezone.now() elif 'active' in data: - # 4. The `active` field is only allowed when ending a nomination. + # 3. The `active` field is only allowed when ending a nomination. raise ValidationError( {'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're setting nomination reviewed + if not instance.active: + raise ValidationError( + {'reviewed': ['This field cannot be set if nomination is inactive.']} + ) + if 'reason' in request.data: if 'actor' not in request.data: raise ValidationError( -- cgit v1.2.3 From 9af0579297e8f1b535099f24f41b4c7f85e2a0cc Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 22 Feb 2021 11:37:53 +0200 Subject: Update nomination-related tests to cover recent table-splitting changes --- pydis_site/apps/api/tests/test_models.py | 16 +-- pydis_site/apps/api/tests/test_nominations.py | 144 +++++++++++++++++++++++--- 2 files changed, 139 insertions(+), 21 deletions(-) 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..3892ec6e 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 have already created nomination entry for 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,76 @@ class NominationTests(APISubdomainTestCase): infractions = response.json() self.assertEqual(len(infractions), 2) + + def test_return_nomination_entries_get_single_nomination(self): + url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + data = response.json() + + self.assertEqual(len(data['entries']), 1) + self.assertEqual(data['entries'][0], { + "actor": self.user.id, + "reason": "He's pretty funky", + "inserted_at": self.active_nomination_entry.inserted_at.isoformat().replace( + "+00:00", "Z" + ) + }) + + def test_return_nomination_entries_get_all_nominations(self): + url = reverse('api:nomination-list', host='api') + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + data = response.json() + + self.assertEqual(len(data), 2) + self.assertEqual(len(data[0]["entries"]), 1) + self.assertEqual(len(data[1]["entries"]), 1) + + 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 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 same time than ending 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 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': ["Actor don't exist or have not nominated user."] + }) -- cgit v1.2.3 From f731e4dd31f9eba8e8c9916329a5c08578055077 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:27:36 +0200 Subject: Set related_name option of nomination field of NominationEntry In order to use entries in serializer without manually setting entries key we have to use related_name option to automatically fetch all related entries. --- pydis_site/apps/api/models/bot/nomination.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index ed6f7d81..a813855d 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -53,7 +53,8 @@ class NominationEntry(ModelReprMixin, models.Model): nomination = models.ForeignKey( Nomination, on_delete=models.CASCADE, - help_text="Nomination to what this entry belongs." + help_text="Nomination to what this entry belongs.", + related_name="entries" ) actor = models.ForeignKey( User, -- cgit v1.2.3 From 1fc5470a0af18aab427a48faf132bea2712330aa Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:28:54 +0200 Subject: Set default ordering of NominationEntry to inserted_at decreasing Set it here so we don't have to set it every place where we fetch entries. --- pydis_site/apps/api/models/bot/nomination.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index a813855d..e72a18ca 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -76,3 +76,7 @@ class NominationEntry(ModelReprMixin, models.Model): """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",) -- cgit v1.2.3 From bb88dc66eb4692eaf5a8ef276944ec13aebb97cb Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:29:53 +0200 Subject: Merge nomination migrations This doesn't make sense to have 3 small migrations for one PR, so I merged 2 existing migrations and 1 new, ordering and related_name adding migrations to one. --- .../apps/api/migrations/0068_split_nomination_tables.py | 4 +++- .../migrations/0069_change_nomination_entry_plural.py | 17 ----------------- 2 files changed, 3 insertions(+), 18 deletions(-) delete mode 100644 pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py diff --git a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py index 1c392365..107e3a56 100644 --- a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py +++ b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py @@ -52,9 +52,11 @@ class Migration(migrations.Migration): on_delete=django.db.models.deletion.CASCADE, related_name='nomination_set', to='api.User')), ('nomination', models.ForeignKey(help_text='Nomination to what this entry belongs.', - on_delete=django.db.models.deletion.CASCADE, to='api.Nomination')), + 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( diff --git a/pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py b/pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py deleted file mode 100644 index 6bf4ac8c..00000000 --- a/pydis_site/apps/api/migrations/0069_change_nomination_entry_plural.py +++ /dev/null @@ -1,17 +0,0 @@ -# Generated by Django 3.0.11 on 2021-02-21 16:44 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0068_split_nomination_tables'), - ] - - operations = [ - migrations.AlterModelOptions( - name='nominationentry', - options={'verbose_name_plural': 'nomination entries'}, - ), - ] -- cgit v1.2.3 From 7682b56fa881f809797e299bd829dfda6480b8d9 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:31:06 +0200 Subject: Add entries field to Nomination serializer After setting related_name in NominationEntry model nomination field, we can just provide serializer and DRF automatically fetch all related entries. --- pydis_site/apps/api/serializers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 49c247b5..17dd4b3d 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -357,12 +357,14 @@ class NominationEntrySerializer(ModelSerializer): 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', 'user', 'inserted_at', 'end_reason', 'ended_at', 'reviewed' + 'id', 'active', 'user', 'inserted_at', 'end_reason', 'ended_at', 'reviewed', 'entries' ) -- cgit v1.2.3 From 3edb416ba86c62d62c0258d4c69f967e47765186 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:38:13 +0200 Subject: Simplify nominations viewset After moving entries to nomination serializer we can get rid from GET request handlers and let DRF handle this. Also PATCH and POST handlers got some simplification by removing manual entries setting. --- pydis_site/apps/api/viewsets/bot/nomination.py | 51 ++------------------------ 1 file changed, 3 insertions(+), 48 deletions(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 7820ca0d..e3e71ca2 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -176,40 +176,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge frozen_fields = ('id', 'inserted_at', 'user', 'ended_at') frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at', 'reviewed') - def list(self, request: HttpRequest, *args, **kwargs) -> Response: - """ - DRF method for listing Nominations. - - Called by the Django Rest Framework in response to the corresponding HTTP request. - """ - queryset = self.filter_queryset(self.get_queryset()) - data = NominationSerializer(queryset, many=True).data - - for i, nomination in enumerate(data): - entries = NominationEntrySerializer( - NominationEntry.objects.filter(nomination_id=nomination["id"]), - many=True - ).data - data[i]["entries"] = entries - - return Response(data) - - def retrieve(self, request: HttpRequest, *args, **kwargs) -> Response: - """ - DRF method for retrieving a Nomination. - - Called by the Django Rest Framework in response to the corresponding HTTP request. - """ - nomination = self.get_object() - - data = NominationSerializer(nomination).data - data["entries"] = NominationEntrySerializer( - NominationEntry.objects.filter(nomination_id=nomination.id), - many=True - ).data - - return Response(data) - def create(self, request: HttpRequest, *args, **kwargs) -> Response: """ DRF method for creating a Nomination. @@ -238,11 +204,9 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge data=ChainMap(request.data, {"nomination": nomination.id}) ) entry_serializer.is_valid(raise_exception=True) - - entry = NominationEntry.objects.create(**entry_serializer.validated_data) + NominationEntry.objects.create(**entry_serializer.validated_data) data = NominationSerializer(nomination).data - data["entries"] = NominationEntrySerializer([entry], many=True).data headers = self.get_success_headers(data) return Response(data, status=status.HTTP_201_CREATED, headers=headers) @@ -264,10 +228,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge NominationEntry.objects.create(**entry_serializer.validated_data) data = NominationSerializer(nomination_filter[0]).data - data["entries"] = NominationEntrySerializer( - NominationEntry.objects.filter(nomination_id=nomination_filter[0].id), - many=True - ).data headers = self.get_success_headers(data) return Response(data, status=status.HTTP_201_CREATED, headers=headers) @@ -349,11 +309,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge entry.reason = request.data['reason'] entry.save() - nomination = serializer.save() - return_data = NominationSerializer(nomination).data - return_data["entries"] = NominationEntrySerializer( - NominationEntry.objects.filter(nomination_id=nomination.id), - many=True - ).data + serializer.save() - return Response(return_data) + return Response(serializer.data) -- cgit v1.2.3 From 774e7c09d6950bb74e83af456d20b8f72fdef8ed Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:41:40 +0200 Subject: Remove GET nomination test for entries Entries isn't handled manually anymore so these tests have no point. --- pydis_site/apps/api/tests/test_nominations.py | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 3892ec6e..1daa6f3f 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -484,32 +484,6 @@ class NominationTests(APISubdomainTestCase): self.assertEqual(len(infractions), 2) - def test_return_nomination_entries_get_single_nomination(self): - url = reverse('bot:nomination-detail', args=(self.active_nomination.id,), host='api') - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - data = response.json() - - self.assertEqual(len(data['entries']), 1) - self.assertEqual(data['entries'][0], { - "actor": self.user.id, - "reason": "He's pretty funky", - "inserted_at": self.active_nomination_entry.inserted_at.isoformat().replace( - "+00:00", "Z" - ) - }) - - def test_return_nomination_entries_get_all_nominations(self): - url = reverse('api:nomination-list', host='api') - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - - data = response.json() - - self.assertEqual(len(data), 2) - self.assertEqual(len(data[0]["entries"]), 1) - self.assertEqual(len(data[1]["entries"]), 1) - 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} -- cgit v1.2.3 From cd03f7448ae4c96c23a80c4c46344bc35b8c603a Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:44:22 +0200 Subject: Make default value of nomination entry reason to empty string For string fields NULL as default is not suggested, so use empty string instead. --- pydis_site/apps/api/migrations/0068_split_nomination_tables.py | 2 +- pydis_site/apps/api/models/bot/nomination.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py index 107e3a56..27c29017 100644 --- a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py +++ b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py @@ -45,7 +45,7 @@ class Migration(migrations.Migration): 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.', null=True)), + ('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.', diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index e72a18ca..443200ff 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -64,7 +64,7 @@ class NominationEntry(ModelReprMixin, models.Model): ) reason = models.TextField( help_text="Why the actor nominated this user.", - null=True, + default="", blank=True ) inserted_at = models.DateTimeField( -- cgit v1.2.3 From 98c60cf90f511f828d3e7bc3d7891b2809d27db6 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 24 Feb 2021 08:47:25 +0200 Subject: Add comment about manually defining nomination field of NominationEntry serializer --- pydis_site/apps/api/serializers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py index 17dd4b3d..f47bedca 100644 --- a/pydis_site/apps/api/serializers.py +++ b/pydis_site/apps/api/serializers.py @@ -342,6 +342,9 @@ class UserSerializer(ModelSerializer): 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 -- cgit v1.2.3 From 54eca6da46d83d8872746b89b95a22f7cf0c2b52 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:48:32 +0200 Subject: Fix grammar of nomination endpoints documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Leon Sandøy Co-authored-by: Joe Banks --- pydis_site/apps/api/viewsets/bot/nomination.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index e3e71ca2..c208df46 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -86,8 +86,8 @@ 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. If `user` already have - active nomination, new nomination entry will be created assigned to + is not allowed and invalid fields are ignored. If `user` already has an + active nomination, a new nomination entry will be created and assigned as the active nomination. #### Request body @@ -119,7 +119,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 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. Actor field is required. + ### 1. Updating the `reason` of `active` nomination. The `actor` field is required. #### Request body >>> { @@ -199,7 +199,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge serializer.is_valid(raise_exception=True) nomination = Nomination.objects.create(**serializer.validated_data) - # Serializer truncate unnecessary data away + # The serializer will truncate and get rid of excessive data entry_serializer = NominationEntrySerializer( data=ChainMap(request.data, {"nomination": nomination.id}) ) @@ -283,7 +283,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge # This is actually covered, but for some reason coverage don't think so. elif 'reviewed' in request.data: # pragma: no cover - # 4. We're setting nomination reviewed + # 4. We are altering the reviewed state of the nomination. if not instance.active: raise ValidationError( {'reviewed': ['This field cannot be set if nomination is inactive.']} -- cgit v1.2.3 From 72693a6b72332c1ca9411f73ae14d395c5dd8933 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:49:46 +0200 Subject: Replace double quotes with single quotes Co-authored-by: Joe Banks --- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index c208df46..ef7d1dab 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -302,7 +302,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge if not entry_filter.exists(): raise ValidationError( - {'actor': ["Actor don't exist or have not nominated user."]} + {'actor': ['Actor don't exist or have not nominated user.']} ) entry = entry_filter[0] -- cgit v1.2.3 From 9075231a5fd68986b0845327d6a2bffe70447fc8 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Fri, 5 Mar 2021 08:18:17 +0200 Subject: Use double quotes instead apostrophe because string contain "don't" --- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index ef7d1dab..c208df46 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -302,7 +302,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge if not entry_filter.exists(): raise ValidationError( - {'actor': ['Actor don't exist or have not nominated user.']} + {'actor': ["Actor don't exist or have not nominated user."]} ) entry = entry_filter[0] -- cgit v1.2.3 From d8751ad37ed5a493554575ea3adb264def342664 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Fri, 5 Mar 2021 08:23:28 +0200 Subject: Fix grammar of error messages and change tests to match with changes --- pydis_site/apps/api/tests/test_nominations.py | 8 ++++---- pydis_site/apps/api/viewsets/bot/nomination.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index 1daa6f3f..c07679c5 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -78,7 +78,7 @@ class CreationTests(APISubdomainTestCase): response2 = self.client.post(url, data=data) self.assertEqual(response2.status_code, 400) self.assertEqual(response2.json(), { - 'actor': ['This actor have already created nomination entry for this nomination.'] + 'actor': ['This actor has already endorsed this nomination.'] }) def test_returns_400_for_missing_user(self): @@ -498,7 +498,7 @@ class NominationTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'reviewed': ['This field cannot be set if nomination is inactive.'] + 'reviewed': ['This field cannot be set if the nomination is inactive.'] }) def test_patch_nomination_set_reviewed_and_end(self): @@ -508,7 +508,7 @@ class NominationTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'reviewed': ['This field cannot be set same time than ending nomination.'] + 'reviewed': ['This field cannot be set while you are ending a nomination.'] }) def test_modifying_reason_without_actor(self): @@ -518,7 +518,7 @@ class NominationTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'actor': ['This field is required when editing reason.'] + 'actor': ['This field is required when editing the reason.'] }) def test_modifying_reason_with_unknown_actor(self): diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index c208df46..451264f4 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -222,7 +222,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge actor__id=entry_serializer.validated_data["actor"].id ).exists(): raise ValidationError( - {'actor': ['This actor have already created nomination entry for this nomination.']} + {'actor': ['This actor has already endorsed this nomination.']} ) NominationEntry.objects.create(**entry_serializer.validated_data) @@ -270,7 +270,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge if 'reviewed' in request.data: raise ValidationError( - {'reviewed': ['This field cannot be set same time than ending nomination.']} + {'reviewed': ['This field cannot be set while you are ending a nomination.']} ) instance.ended_at = timezone.now() @@ -286,13 +286,13 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge # 4. We are altering the reviewed state of the nomination. if not instance.active: raise ValidationError( - {'reviewed': ['This field cannot be set if nomination is inactive.']} + {'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 reason.']} + {'actor': ['This field is required when editing the reason.']} ) entry_filter = NominationEntry.objects.filter( -- cgit v1.2.3 From 53a9f5282adfdd3a2a4ebc819fcc9cd568a632ea Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 6 Mar 2021 14:09:38 +0200 Subject: Fix grammar of nomination models --- pydis_site/apps/api/migrations/0068_split_nomination_tables.py | 4 ++-- pydis_site/apps/api/models/bot/nomination.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py index 27c29017..79825ed7 100644 --- a/pydis_site/apps/api/migrations/0068_split_nomination_tables.py +++ b/pydis_site/apps/api/migrations/0068_split_nomination_tables.py @@ -51,7 +51,7 @@ class Migration(migrations.Migration): ('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='Nomination to what this entry belongs.', + ('nomination', models.ForeignKey(help_text='The nomination this entry belongs to.', on_delete=django.db.models.deletion.CASCADE, to='api.Nomination', related_name='entries')), ], @@ -70,6 +70,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='nomination', name='reviewed', - field=models.BooleanField(default=False, help_text='Whether voting message have been made.'), + field=models.BooleanField(default=False, help_text='Whether a review was made.'), ), ] diff --git a/pydis_site/apps/api/models/bot/nomination.py b/pydis_site/apps/api/models/bot/nomination.py index 443200ff..221d8534 100644 --- a/pydis_site/apps/api/models/bot/nomination.py +++ b/pydis_site/apps/api/models/bot/nomination.py @@ -33,7 +33,7 @@ class Nomination(ModelReprMixin, models.Model): ) reviewed = models.BooleanField( default=False, - help_text="Whether voting message have been made." + help_text="Whether a review was made." ) def __str__(self): @@ -48,12 +48,12 @@ class Nomination(ModelReprMixin, models.Model): class NominationEntry(ModelReprMixin, models.Model): - """A nomination entry created by single staff.""" + """A nomination entry created by a single staff member.""" nomination = models.ForeignKey( Nomination, on_delete=models.CASCADE, - help_text="Nomination to what this entry belongs.", + help_text="The nomination this entry belongs to.", related_name="entries" ) actor = models.ForeignKey( -- cgit v1.2.3 From ce5a082458d8cc1433ce623d3b666fb6e0f82672 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 6 Mar 2021 14:11:56 +0200 Subject: Change as -> to in nomination viewset docs --- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 451264f4..9b9aa280 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -87,7 +87,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge 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. If `user` already has an - active nomination, a new nomination entry will be created and assigned as the + active nomination, a new nomination entry will be created and assigned to the active nomination. #### Request body -- cgit v1.2.3 From 9202d92d3cb27111eaf5d3cc1aee859f9cbe8918 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 6 Mar 2021 14:12:56 +0200 Subject: Fix grammar of nomination viewset command about single entry for user --- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydis_site/apps/api/viewsets/bot/nomination.py b/pydis_site/apps/api/viewsets/bot/nomination.py index 9b9aa280..0792a2a6 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -216,7 +216,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge ) entry_serializer.is_valid(raise_exception=True) - # Don't allow user creating many nomination entries for one nomination + # 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 -- cgit v1.2.3 From 48eff5d27715acd658f6284f4d4e1cdbbf71bee6 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 6 Mar 2021 14:13:50 +0200 Subject: Fix grammar of unknown actor error and change tests --- pydis_site/apps/api/tests/test_nominations.py | 2 +- pydis_site/apps/api/viewsets/bot/nomination.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pydis_site/apps/api/tests/test_nominations.py b/pydis_site/apps/api/tests/test_nominations.py index c07679c5..9cefbd8f 100644 --- a/pydis_site/apps/api/tests/test_nominations.py +++ b/pydis_site/apps/api/tests/test_nominations.py @@ -528,5 +528,5 @@ class NominationTests(APISubdomainTestCase): response = self.client.patch(url, data=data) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), { - 'actor': ["Actor don't exist or have not nominated user."] + '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 0792a2a6..144daab0 100644 --- a/pydis_site/apps/api/viewsets/bot/nomination.py +++ b/pydis_site/apps/api/viewsets/bot/nomination.py @@ -302,7 +302,7 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge if not entry_filter.exists(): raise ValidationError( - {'actor': ["Actor don't exist or have not nominated user."]} + {'actor': ["The actor doesn't exist or has not nominated the user."]} ) entry = entry_filter[0] -- cgit v1.2.3