From 6516eff584065ed121109b7874cb080f72c5e3cc Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 3 Aug 2020 22:16:27 +0200 Subject: Add a validator for package names. Package names are used for stats in the bot and are restricted to the a-z_ char set, a validator is added to accommodate this restriction at the site admin side. --- pydis_site/apps/api/models/bot/documentation_link.py | 7 +++++++ pydis_site/apps/api/tests/test_documentation_links.py | 11 +++++++++++ 2 files changed, 18 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/documentation_link.py b/pydis_site/apps/api/models/bot/documentation_link.py index 5a46460b..f77d6f38 100644 --- a/pydis_site/apps/api/models/bot/documentation_link.py +++ b/pydis_site/apps/api/models/bot/documentation_link.py @@ -1,3 +1,4 @@ +from django.core.validators import RegexValidator from django.db import models from pydis_site.apps.api.models.mixins import ModelReprMixin @@ -9,6 +10,12 @@ class DocumentationLink(ModelReprMixin, models.Model): package = models.CharField( primary_key=True, max_length=50, + validators=( + RegexValidator( + regex=r"^[a-z_]+$", + message="Package names can only consist of lowercase a-z letters and underscores." + ), + ), help_text="The Python package name that this documentation link belongs to." ) base_url = models.URLField( diff --git a/pydis_site/apps/api/tests/test_documentation_links.py b/pydis_site/apps/api/tests/test_documentation_links.py index e560a2fd..72e5cadb 100644 --- a/pydis_site/apps/api/tests/test_documentation_links.py +++ b/pydis_site/apps/api/tests/test_documentation_links.py @@ -108,6 +108,17 @@ class DetailLookupDocumentationLinkAPITests(APISubdomainTestCase): self.assertEqual(response.status_code, 400) + def test_create_invalid_package_name_returns_400(self): + test_cases = ("InvalidPackage", "invalid package", "i\u0150valid") + for case in test_cases: + with self.subTest(package_name=case): + body = self.doc_json.copy() + body['package'] = case + url = reverse('bot:documentationlink-list', host='api') + response = self.client.post(url, data=body) + + self.assertEqual(response.status_code, 400) + class DocumentationLinkCreationTests(APISubdomainTestCase): def setUp(self): -- cgit v1.2.3 From b7f302e4e7afefb16a652d3b0524f4cf4ee835e9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 4 Aug 2020 11:29:26 +0200 Subject: Add ascii digits to the validator. Some packages can contain them and are good for stats. --- pydis_site/apps/api/models/bot/documentation_link.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/documentation_link.py b/pydis_site/apps/api/models/bot/documentation_link.py index f77d6f38..e093af59 100644 --- a/pydis_site/apps/api/models/bot/documentation_link.py +++ b/pydis_site/apps/api/models/bot/documentation_link.py @@ -12,8 +12,8 @@ class DocumentationLink(ModelReprMixin, models.Model): max_length=50, validators=( RegexValidator( - regex=r"^[a-z_]+$", - message="Package names can only consist of lowercase a-z letters and underscores." + regex=r"^[a-z0-9_]+$", + message="Package names can only consist of lowercase a-z letters, digits, and underscores." ), ), help_text="The Python package name that this documentation link belongs to." -- cgit v1.2.3 From b242f6d4893dd47c8f07df7dd7bf97f8f1c6631c Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 4 Aug 2020 12:20:52 +0200 Subject: Move package name validator definition. The move prevents it going through the line limit and deeper nesting of parentheses from splitting up the string. --- pydis_site/apps/api/models/bot/documentation_link.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/documentation_link.py b/pydis_site/apps/api/models/bot/documentation_link.py index e093af59..56b47ae6 100644 --- a/pydis_site/apps/api/models/bot/documentation_link.py +++ b/pydis_site/apps/api/models/bot/documentation_link.py @@ -3,6 +3,11 @@ from django.db import models from pydis_site.apps.api.models.mixins import ModelReprMixin +package_name_validator = RegexValidator( + regex=r"^[a-z0-9_]+$", + message="Package names can only consist of lowercase a-z letters, digits, and underscores." +) + class DocumentationLink(ModelReprMixin, models.Model): """A documentation link used by the `!docs` command of the bot.""" @@ -10,12 +15,7 @@ class DocumentationLink(ModelReprMixin, models.Model): package = models.CharField( primary_key=True, max_length=50, - validators=( - RegexValidator( - regex=r"^[a-z0-9_]+$", - message="Package names can only consist of lowercase a-z letters, digits, and underscores." - ), - ), + validators=package_name_validator, help_text="The Python package name that this documentation link belongs to." ) base_url = models.URLField( -- cgit v1.2.3 From 62cf63427e51f2a03eb37d726ca9a6a12fed7374 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 4 Aug 2020 16:00:43 +0200 Subject: Fix package name validator definition. The validators kwarg expects an iterable of validators, while a validator directly was being supplied. --- pydis_site/apps/api/models/bot/documentation_link.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/documentation_link.py b/pydis_site/apps/api/models/bot/documentation_link.py index 56b47ae6..4f2bd2ab 100644 --- a/pydis_site/apps/api/models/bot/documentation_link.py +++ b/pydis_site/apps/api/models/bot/documentation_link.py @@ -3,9 +3,11 @@ from django.db import models from pydis_site.apps.api.models.mixins import ModelReprMixin -package_name_validator = RegexValidator( - regex=r"^[a-z0-9_]+$", - message="Package names can only consist of lowercase a-z letters, digits, and underscores." +package_name_validator = ( + RegexValidator( + regex=r"^[a-z0-9_]+$", + message="Package names can only consist of lowercase a-z letters, digits, and underscores." + ), ) -- cgit v1.2.3 From 29929b9771fde2179296b653109795da9857d281 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 4 Aug 2020 16:04:07 +0200 Subject: Create migration for doc package name validator. --- .../0061_documentationlink_packagename_validator.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 pydis_site/apps/api/migrations/0061_documentationlink_packagename_validator.py (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/migrations/0061_documentationlink_packagename_validator.py b/pydis_site/apps/api/migrations/0061_documentationlink_packagename_validator.py new file mode 100644 index 00000000..cc734c8a --- /dev/null +++ b/pydis_site/apps/api/migrations/0061_documentationlink_packagename_validator.py @@ -0,0 +1,19 @@ +# Generated by Django 3.0.8 on 2020-08-04 12:25 + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0060_populate_filterlists_fix'), + ] + + operations = [ + migrations.AlterField( + model_name='documentationlink', + name='package', + field=models.CharField(help_text='The Python package name that this documentation link belongs to.', max_length=50, primary_key=True, serialize=False, validators=[django.core.validators.RegexValidator(message='Package names can only consist of lowercase a-z letters, digits, and underscores.', regex='^[a-z0-9_]+$')]), + ), + ] -- cgit v1.2.3 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 (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(+) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(+) (limited to 'pydis_site/apps/api') 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 (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(+) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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(-) (limited to 'pydis_site/apps/api') 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 From 4f9c088f6b0458eb0ebb52ef899cdfdc57f2c43c Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Sun, 7 Mar 2021 00:59:41 +0200 Subject: Add route to get a member's data for helper review Added route for getting a user's join date, total messages, and top 3 channels by activity. This information will be used to auto-review nominees. --- postgres/init.sql | 18 ++++++++++++- pydis_site/apps/api/models/bot/metricity.py | 39 +++++++++++++++++++++++++++++ pydis_site/apps/api/viewsets/bot/user.py | 15 +++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/postgres/init.sql b/postgres/init.sql index 740063e7..ae86fca0 100644 --- a/postgres/init.sql +++ b/postgres/init.sql @@ -13,12 +13,28 @@ INSERT INTO users VALUES ( current_timestamp ); +CREATE TABLE channels ( + id varchar, + name varchar, + primary key(id) +); + +INSERT INTO channels VALUES( + '267659945086812160', + 'python-general' +); + +INSERT INTO channels VALUES( + '1234', + 'zebra' +); + CREATE TABLE messages ( id varchar, author_id varchar references users(id), is_deleted boolean, created_at timestamp, - channel_id varchar, + channel_id varchar references channels(id), primary key(id) ); diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index cae630f1..af5e1f3b 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -89,3 +89,42 @@ class Metricity: raise NotFound() return values[0] + + def top_channel_activity(self, user_id: str) -> int: + """ + Query the top three channels in which the user is most active. + + Help channels are grouped under "the help channels", + and off-topic channels are grouped under "off-topic". + """ + self.cursor.execute( + """ + SELECT + CASE + WHEN channels.name ILIKE 'help-%%' THEN 'the help channels' + WHEN channels.name ILIKE 'ot%%' THEN 'off-topic' + ELSE channels.name + END, + COUNT(1) + FROM + messages + LEFT JOIN channels ON channels.id = messages.channel_id + WHERE + author_id = '%s' + GROUP BY + 1 + ORDER BY + 2 DESC + LIMIT + 3; + """, + [user_id] + ) + + values = self.cursor.fetchall() + print(values) + + if not values: + raise NotFound() + + return values diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 829e2694..5e1f8775 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -262,3 +262,18 @@ class UserViewSet(ModelViewSet): except NotFound: return Response(dict(detail="User not found in metricity"), status=status.HTTP_404_NOT_FOUND) + + @action(detail=True) + def metricity_review_data(self, request: Request, pk: str = None) -> Response: + """Request handler for metricity_review_data endpoint.""" + user = self.get_object() + + with Metricity() as metricity: + try: + data = metricity.user(user.id) + data["total_messages"] = metricity.total_messages(user.id) + data["top_channel_activity"] = metricity.top_channel_activity(user.id) + return Response(data, status=status.HTTP_200_OK) + except NotFound: + return Response(dict(detail="User not found in metricity"), + status=status.HTTP_404_NOT_FOUND) -- cgit v1.2.3 From d2690bbedb5f5ef221cbfaa42ad78ff8fcc263f2 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Sun, 7 Mar 2021 01:05:09 +0200 Subject: Amend top_channel_activity return type --- pydis_site/apps/api/models/bot/metricity.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index af5e1f3b..29a43513 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -1,3 +1,5 @@ +from typing import List, Tuple + from django.db import connections BLOCK_INTERVAL = 10 * 60 # 10 minute blocks @@ -90,7 +92,7 @@ class Metricity: return values[0] - def top_channel_activity(self, user_id: str) -> int: + def top_channel_activity(self, user_id: str) -> List[Tuple[str, int]]: """ Query the top three channels in which the user is most active. -- cgit v1.2.3 From 3468d12b0c893bfa951e7baa85ada6a8716e5aef Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Sun, 7 Mar 2021 01:39:12 +0200 Subject: Added test for metricity-review-data --- pydis_site/apps/api/tests/test_users.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 69bbfefc..5851ac11 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -410,7 +410,7 @@ class UserMetricityTests(APISubdomainTestCase): joined_at = "foo" total_messages = 1 total_blocks = 1 - self.mock_metricity_user(joined_at, total_messages, total_blocks) + self.mock_metricity_user(joined_at, total_messages, total_blocks, []) # When url = reverse('bot:user-metricity-data', args=[0], host='api') @@ -442,7 +442,7 @@ class UserMetricityTests(APISubdomainTestCase): {'exception': ObjectDoesNotExist, 'voice_banned': False}, ] - self.mock_metricity_user("foo", 1, 1) + self.mock_metricity_user("foo", 1, 1, [["bar", 1]]) for case in cases: with self.subTest(exception=case['exception'], voice_banned=case['voice_banned']): @@ -455,7 +455,27 @@ class UserMetricityTests(APISubdomainTestCase): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["voice_banned"], case["voice_banned"]) - def mock_metricity_user(self, joined_at, total_messages, total_blocks): + def test_metricity_review_data(self): + # Given + joined_at = "foo" + total_messages = 10 + total_blocks = 1 + channel_activity = [["bar", 4], ["buzz", 6]] + self.mock_metricity_user(joined_at, total_messages, total_blocks, channel_activity) + + # When + url = reverse('bot:user-metricity-review-data', args=[0], host='api') + response = self.client.get(url) + + # Then + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), { + "joined_at": joined_at, + "top_channel_activity": channel_activity, + "total_messages": total_messages + }) + + def mock_metricity_user(self, joined_at, total_messages, total_blocks, top_channel_activity): patcher = patch("pydis_site.apps.api.viewsets.bot.user.Metricity") self.metricity = patcher.start() self.addCleanup(patcher.stop) @@ -463,6 +483,7 @@ class UserMetricityTests(APISubdomainTestCase): self.metricity.user.return_value = dict(joined_at=joined_at) self.metricity.total_messages.return_value = total_messages self.metricity.total_message_blocks.return_value = total_blocks + self.metricity.top_channel_activity.return_value = top_channel_activity def mock_no_metricity_user(self): patcher = patch("pydis_site.apps.api.viewsets.bot.user.Metricity") @@ -472,3 +493,4 @@ class UserMetricityTests(APISubdomainTestCase): self.metricity.user.side_effect = NotFound() self.metricity.total_messages.side_effect = NotFound() self.metricity.total_message_blocks.side_effect = NotFound() + self.metricity.top_channel_activity.side_effect = NotFound() -- cgit v1.2.3 From 66047f06e5ce060e79e49ef2dfab06475298b070 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Sun, 7 Mar 2021 01:46:43 +0200 Subject: Test metricity-review-data when user doesn't exist --- pydis_site/apps/api/tests/test_users.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 5851ac11..2ac06f6b 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -431,10 +431,13 @@ class UserMetricityTests(APISubdomainTestCase): # When url = reverse('bot:user-metricity-data', args=[0], host='api') - response = self.client.get(url) + response1 = self.client.get(url) + url = reverse('bot:user-metricity-review-data', args=[0], host='api') + response2 = self.client.get(url) # Then - self.assertEqual(response.status_code, 404) + self.assertEqual(response1.status_code, 404) + self.assertEqual(response2.status_code, 404) def test_metricity_voice_banned(self): cases = [ -- cgit v1.2.3 From 07847e959c9b2ac6a79ab38b900abc2d179d0478 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Sun, 7 Mar 2021 11:58:30 +0200 Subject: Get rid of stray print Oops. --- pydis_site/apps/api/models/bot/metricity.py | 1 - 1 file changed, 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index 29a43513..7e2a68f2 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -124,7 +124,6 @@ class Metricity: ) values = self.cursor.fetchall() - print(values) if not values: raise NotFound() -- cgit v1.2.3 From fdc1be68a90d6ebd9dfe29369bc8a974bcaa8214 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Thu, 11 Mar 2021 02:37:44 +0200 Subject: Ignore deleted messaages in message counts Co-authored-by: Joe Banks --- pydis_site/apps/api/models/bot/metricity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index 7e2a68f2..db975d4e 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -112,7 +112,7 @@ class Metricity: messages LEFT JOIN channels ON channels.id = messages.channel_id WHERE - author_id = '%s' + author_id = '%s' AND NOT messages.is_deleted GROUP BY 1 ORDER BY -- cgit v1.2.3 From f4a67489b81f95978912582189cb23afb2169e8e Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Fri, 12 Mar 2021 15:35:47 +0200 Subject: Document endpoint in viewset docstring --- pydis_site/apps/api/viewsets/bot/user.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index 5e1f8775..25722f5a 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -119,6 +119,22 @@ class UserViewSet(ModelViewSet): - 200: returned on success - 404: if a user with the given `snowflake` could not be found + ### GET /bot/users//metricity_review_data + Gets metricity data for a single user's review by ID. + + #### Response format + >>> { + ... 'joined_at': '2020-08-26T08:09:43.507000', + ... 'top_channel_activity': [['off-topic', 15], + ... ['talent-pool', 4], + ... ['defcon', 2]], + ... 'total_messages': 22 + ... } + + #### Status codes + - 200: returned on success + - 404: if a user with the given `snowflake` could not be found + ### POST /bot/users Adds a single or multiple new users. The roles attached to the user(s) must be roles known by the site. -- cgit v1.2.3 From 9b1b42a4899c0c3f47845def5d152e2bed6c8dd0 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Fri, 12 Mar 2021 15:59:11 +0200 Subject: Split test_no_metricity_user to two tests by endpoints --- pydis_site/apps/api/tests/test_users.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py index 2ac06f6b..c43b916a 100644 --- a/pydis_site/apps/api/tests/test_users.py +++ b/pydis_site/apps/api/tests/test_users.py @@ -431,13 +431,21 @@ class UserMetricityTests(APISubdomainTestCase): # When url = reverse('bot:user-metricity-data', args=[0], host='api') - response1 = self.client.get(url) + response = self.client.get(url) + + # Then + self.assertEqual(response.status_code, 404) + + def test_no_metricity_user_for_review(self): + # Given + self.mock_no_metricity_user() + + # When url = reverse('bot:user-metricity-review-data', args=[0], host='api') - response2 = self.client.get(url) + response = self.client.get(url) # Then - self.assertEqual(response1.status_code, 404) - self.assertEqual(response2.status_code, 404) + self.assertEqual(response.status_code, 404) def test_metricity_voice_banned(self): cases = [ -- cgit v1.2.3 From cd797f801abaff8874e75b1ed093e17f67572a37 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Fri, 12 Mar 2021 16:00:14 +0200 Subject: Add case in query for voice chat activity --- pydis_site/apps/api/models/bot/metricity.py | 1 + 1 file changed, 1 insertion(+) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/models/bot/metricity.py b/pydis_site/apps/api/models/bot/metricity.py index db975d4e..5daa5c66 100644 --- a/pydis_site/apps/api/models/bot/metricity.py +++ b/pydis_site/apps/api/models/bot/metricity.py @@ -105,6 +105,7 @@ class Metricity: CASE WHEN channels.name ILIKE 'help-%%' THEN 'the help channels' WHEN channels.name ILIKE 'ot%%' THEN 'off-topic' + WHEN channels.name ILIKE '%%voice%%' THEN 'voice chats' ELSE channels.name END, COUNT(1) -- cgit v1.2.3 From 163460967916463485a8193dde092eef639c9bea Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Fri, 26 Mar 2021 19:30:03 +0100 Subject: Ensure the base url ends with a slash URLs without a trailing slash won't get properly joined by the bot Adds compatibility with python-discord/bot@bc25bfdf42cdaaba924a7ad6de1dc06a9b381285's changes The styling of how the regex decorator is constructed was changed to be consisted with the function validator --- ...0069_documentationlink_packagename_validator.py | 19 ---------------- .../0069_documentationlink_validators.py | 25 ++++++++++++++++++++++ .../apps/api/models/bot/documentation_link.py | 20 +++++++++++------ 3 files changed, 38 insertions(+), 26 deletions(-) delete mode 100644 pydis_site/apps/api/migrations/0069_documentationlink_packagename_validator.py create mode 100644 pydis_site/apps/api/migrations/0069_documentationlink_validators.py (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/migrations/0069_documentationlink_packagename_validator.py b/pydis_site/apps/api/migrations/0069_documentationlink_packagename_validator.py deleted file mode 100644 index 4234e633..00000000 --- a/pydis_site/apps/api/migrations/0069_documentationlink_packagename_validator.py +++ /dev/null @@ -1,19 +0,0 @@ -# Generated by Django 3.0.11 on 2021-03-14 23:22 - -import django.core.validators -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0068_split_nomination_tables'), - ] - - operations = [ - migrations.AlterField( - model_name='documentationlink', - name='package', - field=models.CharField(help_text='The Python package name that this documentation link belongs to.', max_length=50, primary_key=True, serialize=False, validators=[django.core.validators.RegexValidator(message='Package names can only consist of lowercase a-z letters, digits, and underscores.', regex='^[a-z0-9_]+$')]), - ), - ] diff --git a/pydis_site/apps/api/migrations/0069_documentationlink_validators.py b/pydis_site/apps/api/migrations/0069_documentationlink_validators.py new file mode 100644 index 00000000..347c0e1a --- /dev/null +++ b/pydis_site/apps/api/migrations/0069_documentationlink_validators.py @@ -0,0 +1,25 @@ +# Generated by Django 3.0.11 on 2021-03-26 18:21 + +import django.core.validators +from django.db import migrations, models +import pydis_site.apps.api.models.bot.documentation_link + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0068_split_nomination_tables'), + ] + + operations = [ + migrations.AlterField( + model_name='documentationlink', + name='base_url', + field=models.URLField(help_text='The base URL from which documentation will be available for this project. Used to generate links to various symbols within this package.', validators=[pydis_site.apps.api.models.bot.documentation_link.ends_with_slash_validator]), + ), + migrations.AlterField( + model_name='documentationlink', + name='package', + field=models.CharField(help_text='The Python package name that this documentation link belongs to.', max_length=50, primary_key=True, serialize=False, validators=[django.core.validators.RegexValidator(message='Package names can only consist of lowercase a-z letters, digits, and underscores.', regex='^[a-z0-9_]+$')]), + ), + ] diff --git a/pydis_site/apps/api/models/bot/documentation_link.py b/pydis_site/apps/api/models/bot/documentation_link.py index 529d26d1..3dcc71fc 100644 --- a/pydis_site/apps/api/models/bot/documentation_link.py +++ b/pydis_site/apps/api/models/bot/documentation_link.py @@ -1,30 +1,36 @@ +from django.core.exceptions import ValidationError from django.core.validators import RegexValidator from django.db import models from pydis_site.apps.api.models.mixins import ModelReprMixin -package_name_validator = ( - RegexValidator( - regex=r"^[a-z0-9_]+$", - message="Package names can only consist of lowercase a-z letters, digits, and underscores." - ), +package_name_validator = RegexValidator( + regex=r"^[a-z0-9_]+$", + message="Package names can only consist of lowercase a-z letters, digits, and underscores." ) +def ends_with_slash_validator(string: str) -> None: + """Raise a ValidationError if `string` does not end with a slash.""" + if not string.endswith("/"): + raise ValidationError("The entered URL must end with a slash.") + + class DocumentationLink(ModelReprMixin, models.Model): """A documentation link used by the `!docs` command of the bot.""" package = models.CharField( primary_key=True, max_length=50, - validators=package_name_validator, + validators=(package_name_validator,), help_text="The Python package name that this documentation link belongs to." ) base_url = models.URLField( help_text=( "The base URL from which documentation will be available for this project. " "Used to generate links to various symbols within this package." - ) + ), + validators=(ends_with_slash_validator,) ) inventory_url = models.URLField( help_text="The URL at which the Sphinx inventory is available for this package." -- cgit v1.2.3 From 6fc8cdbd823abec4859a83662e50aefcaea84383 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Fri, 26 Mar 2021 19:47:20 +0100 Subject: Update tests to use trailing slashes on valid urls --- pydis_site/apps/api/tests/test_documentation_links.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pydis_site/apps/api') diff --git a/pydis_site/apps/api/tests/test_documentation_links.py b/pydis_site/apps/api/tests/test_documentation_links.py index 72e5cadb..39fb08f3 100644 --- a/pydis_site/apps/api/tests/test_documentation_links.py +++ b/pydis_site/apps/api/tests/test_documentation_links.py @@ -60,7 +60,7 @@ class DetailLookupDocumentationLinkAPITests(APISubdomainTestCase): def setUpTestData(cls): cls.doc_link = DocumentationLink.objects.create( package='testpackage', - base_url='https://example.com', + base_url='https://example.com/', inventory_url='https://example.com' ) @@ -126,7 +126,7 @@ class DocumentationLinkCreationTests(APISubdomainTestCase): self.body = { 'package': 'example', - 'base_url': 'https://example.com', + 'base_url': 'https://example.com/', 'inventory_url': 'https://docs.example.com' } -- cgit v1.2.3