rsinger86 / drf-flex-fields

Dynamically set fields and expand nested resources in Django REST Framework serializers.
MIT License
740 stars 61 forks source link

fields arguement in Serializer constructor not working from 0.8.0 #58

Closed anindyamanna closed 3 years ago

anindyamanna commented 4 years ago

This works perfectly fine in 0.7.5, but breaks from 0.8 :worried:

from rest_flex_fields import FlexFieldsModelSerializer
from django.db import models

class MentorMentee(models.Model):
    mentee = models.ForeignKey('CustomUser', related_name="mentors", on_delete=models.CASCADE)
    skill = models.ForeignKey('Skill', on_delete=models.CASCADE)
    mentor = models.ForeignKey('CustomUser', related_name="mentees", on_delete=models.CASCADE)

class MentorReqSerializer(FlexFieldsModelSerializer):
    class Meta:
        model = MentorMentee
        fields = '__all__'

data = MentorReqSerializer(fields=('mentor', 'skill'), data={'mentor': 1, 'skill': 51518})
data.is_valid(raise_exception=True)

Raises

rest_framework.exceptions.ValidationError: {'mentee': [ErrorDetail(string='This field is required.', code='required')]}

rsinger86 commented 4 years ago

I think I know what happened here. In 0.8.0, field dynamicism was delayed until serialization, when to_representation is called, instead of happening immediately, when the serializer's __init__ method was called. So the 0.7.5 version would have immediately removed the mentee serializer field, allowing validation to pass.

It looks like the mentee field is required on the model, it cannot be null. To me, it seems odd that data would be validated that did not meet the constraints of the model? Just trying to understand the use case to see how common it might be.

anindyamanna commented 4 years ago

@rsinger86 Yes. The mentee field is required, but it gets manually populated as request.user in the viewset code before invoking object save. This manual population of one of the field in a model is a common thing. Maybe there's a better way to handle them. :thinking:

rsinger86 commented 4 years ago

Ah, I see. I've encountered this scenario myself a number of times -- needing to populate a serializer field with the current user of the request.

My solution has been to use a custom serializer field. As long as you are passing context={"request": request} when constructing the serializer, it will add a key = request.user to the serializer's validated_data.

class CurrentUserField(serializers.Field):
    def __init__(self, *args, **kwargs):
        self.create_only = kwargs.pop("create_only", False)
        super().__init__(*args, **kwargs)
        self.required = False
        self.allow_null = True

    def bind(self, field_name, parent):
        super().bind(field_name, parent)

        if self.create_only and self.parent and self.parent.instance is not None:
            self.read_only = True

    def get_current_user(self):
        if hasattr(self, "context") and "request" in self.context:
            user = self.context["request"].user
            return user
        else:
            return None

    def get_default(self):
        return self.get_current_user()

    def to_internal_value(self, data):
        return self.get_current_user()

    def to_representation(self, obj):
        return obj.username

    def validate_empty_values(self, data: dict):
        return (True, self.get_default())

Usage:


class MentorReqSerializer(FlexFieldsModelSerializer):
   mentee = CurrentUserField() # pass create_only=True if you only want this set when creating a new instance

    class Meta:
        model = MentorMentee
        fields = ("mentee", "mentor", "skill")