Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify frozen fields logic into serializer mixin #1169

Merged
merged 1 commit into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions pydis_site/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from datetime import timedelta
from typing import Any

from django.db import models
from django.db.models.query import QuerySet
from django.db.utils import IntegrityError
from rest_framework.exceptions import NotFound
Expand Down Expand Up @@ -35,6 +36,30 @@
User
)

class FrozenFieldsMixin:
"""
Serializer mixin that allows adding non-updateable fields to a serializer.

To use, inherit from the mixin and specify the fields that should only be
written to on creation in the `frozen_fields` attribute of the `Meta` class
in a serializer.

See also the DRF discussion for this feature at
https://github.com/encode/django-rest-framework/discussions/8606, which may
eventually provide an official way to implement this.
"""

def update(self, instance: models.Model, validated_data: dict) -> models.Model:
"""Validate that no frozen fields were changed and update the instance."""
for field_name in getattr(self.Meta, 'frozen_fields', ()):
if field_name in validated_data:
raise ValidationError(
{
field_name: ["This field cannot be updated."]
}
)
return super().update(instance, validated_data)


class BotSettingSerializer(ModelSerializer):
"""A class providing (de-)serialization of `BotSetting` instances."""
Expand Down Expand Up @@ -426,7 +451,7 @@ def to_representation(self, instance: FilterList) -> dict:
# endregion


class InfractionSerializer(ModelSerializer):
class InfractionSerializer(FrozenFieldsMixin, ModelSerializer):
"""A class providing (de-)serialization of `Infraction` instances."""

class Meta:
Expand All @@ -447,6 +472,7 @@ class Meta:
'dm_sent',
'jump_url'
)
frozen_fields = ('id', 'inserted_at', 'type', 'user', 'actor', 'hidden')

def validate(self, attrs: dict) -> dict:
"""Validate data constraints for the given data and abort if it is invalid."""
Expand Down Expand Up @@ -683,7 +709,7 @@ class Meta:
fields = ('nomination', 'actor', 'reason', 'inserted_at')


class NominationSerializer(ModelSerializer):
class NominationSerializer(FrozenFieldsMixin, ModelSerializer):
"""A class providing (de-)serialization of `Nomination` instances."""

entries = NominationEntrySerializer(many=True, read_only=True)
Expand All @@ -703,13 +729,15 @@ class Meta:
'entries',
'thread_id'
)
frozen_fields = ('id', 'inserted_at', 'user', 'ended_at')


class OffensiveMessageSerializer(ModelSerializer):
class OffensiveMessageSerializer(FrozenFieldsMixin, ModelSerializer):
"""A class providing (de-)serialization of `OffensiveMessage` instances."""

class Meta:
"""Metadata defined for the Django REST Framework."""

model = OffensiveMessage
fields = ('id', 'channel_id', 'delete_date')
frozen_fields = ('id', 'channel_id')
2 changes: 1 addition & 1 deletion pydis_site/apps/api/tests/test_nominations.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ def test_returns_200_update_reason_on_active_with_actor(self):
def test_returns_400_on_frozen_field_update(self):
url = reverse('api:bot:nomination-detail', args=(self.active_nomination.id,))
data = {
'user': "Theo Katzman"
'user': 1234
}

response = self.client.patch(url, data=data)
Expand Down
8 changes: 8 additions & 0 deletions pydis_site/apps/api/tests/test_offensive_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,14 @@ def test_updating_message(self):
delta=datetime.timedelta(seconds=1),
)

def test_updating_write_once_fields(self):
"""Fields such as the channel ID may not be updated."""
url = reverse('api:bot:offensivemessage-detail', args=(self.message.id,))
data = {'channel_id': self.message.channel_id + 1}
response = self.client.patch(url, data=data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {'channel_id': ["This field cannot be updated."]})

def test_updating_nonexistent_message(self):
url = reverse('api:bot:offensivemessage-detail', args=(self.message.id + 1,))
data = {'delete_date': self.in_one_week}
Expand Down
5 changes: 0 additions & 5 deletions pydis_site/apps/api/viewsets/bot/infraction.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,9 @@ class InfractionViewSet(
filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter)
filterset_fields = ('user__id', 'actor__id', 'active', 'hidden', 'type')
search_fields = ('$reason',)
frozen_fields = ('id', 'inserted_at', 'type', 'user', 'actor', 'hidden')

def partial_update(self, request: HttpRequest, *_args, **_kwargs) -> Response:
"""Method that handles the nuts and bolts of updating an Infraction."""
for field in request.data:
if field in self.frozen_fields:
raise ValidationError({field: ['This field cannot be updated.']})

instance = self.get_object()
serializer = self.get_serializer(instance, data=request.data, partial=True)
serializer.is_valid(raise_exception=True)
Expand Down
5 changes: 0 additions & 5 deletions pydis_site/apps/api/viewsets/bot/nomination.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ class NominationViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, Ge
queryset = Nomination.objects.all()
filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter)
filterset_fields = ('user__id', 'active')
frozen_fields = ('id', 'inserted_at', 'user', 'ended_at')
frozen_on_create = ('ended_at', 'end_reason', 'active', 'inserted_at', 'reviewed')

def create(self, request: HttpRequest, *args, **kwargs) -> Response:
Expand Down Expand Up @@ -238,10 +237,6 @@ def partial_update(self, request: HttpRequest, *args, **kwargs) -> Response:

Called by the Django Rest Framework in response to the corresponding HTTP request.
"""
for field in request.data:
if field in self.frozen_fields:
raise ValidationError({field: ['This field cannot be updated.']})

instance = self.get_object()
serializer = self.get_serializer(instance, data=request.data, partial=True)
serializer.is_valid(raise_exception=True)
Expand Down