encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
28.32k stars 6.83k forks source link

fields of a `Serializer`class field are skipped if root serializer is partial #2585

Open nourspace opened 9 years ago

nourspace commented 9 years ago

I have this situation:


class LocationSerializer(serializers.Serializer):
    country = serializers.CharField(min_length=2, max_length=2)
    city = serializers.CharField(max_length=200)
    latitude = serializers.FloatField()
    longitude = serializers.FloatField()

class UserDetailSerializer(serializers.ModelSerializer):
    class Meta:
        model = User
        fields = ('location', )

    location = LocationSerializer(partial=False)

if I use UserDetailSerializer(partial=true) in a view and pass this in json:

{
    "location": {}
}

it will be validated because of this code part in fields.py:340

        if data is empty:
            if getattr(self.root, 'partial', False):
                raise SkipField()
            if self.required:
                self.fail('required')
            return (True, self.get_default())

I think it should be self.parent instead of self.root

tomchristie commented 9 years ago

location = LocationSerializer(partial=False)

Hrm. This isn't really the intended usage - so we may need to think about this. Partial is only intended as an argumet when instantiating a serializer instance, rather than declaring a nested serializer as a field.

nourspace commented 9 years ago

in this section: http://www.django-rest-framework.org/api-guide/serializers/#dealing-with-nested-objects

it only deals with normal cases where the entire hierarchy is required or not. however, there will be such cases where if nested serializer specified, all its fields will be required otherwise it is not valid. Like my case, where location missing all or one attribute is considered invalid, still got validated, at the same time where I don't want to force it along with other fields such as username, email, etc.

class UserDetailSerializer(serializers.ModelSerializer):
    class Meta:
        model = User
        fields = ('username', 'name', 'location', 'email')
tomchristie commented 9 years ago

Oh sure, I'm not saying that there might not be a valid case for this, just that it wasn't a use-case envisaged during the design.

patrick91 commented 8 years ago

I had a similar issue in which I wanted to validate nested resources that didn't have an id. So basically the nested resources with no id should have a full validation. I've resolved with this mixin:

class ValidatePartialWithoutIdMixin():
    def run_validation(self, data):
        # if root is partial we should validate everything if there is no id

        parent = self.parent

        if 'id' not in data:
            self.parent = None

        res = super(ValidatePartialWithoutIdMixin, self).run_validation(data)

        self.parent = parent

        return res
claytondaley commented 6 years ago

If deemed acceptable, the general idea of (re)configuring a nested serializer using kwargs could also be used to address nested validation issues like #2996 (related #2403).

That case is more complicated because it requires new behavior (orthogonal to partial), but it looks like the existing SkipField infrastructure could be coopted so it might not take much to add an appropriate flag/mode.