drf-writable-nested icon indicating copy to clipboard operation
drf-writable-nested copied to clipboard

Creating new children on parent create

Open Lynges opened this issue 5 years ago • 8 comments

Hi. Great package, everything seems to "just work". Very nice.

However, I am having some trouble with object creation.

Models:

class ParentModel(Model):
    name = CharField()

class ChildModel(Model):
    name = CharField()
    parent = ForeignKey('ParentModel', related_name='children')

Serializers:

class ChildSerializer(WritableNestedModelSerializer):
    class Meta:
        model = ChildModel
        fields = '__all__'

class ParentSerializer(WritableNestedModelSerializer):
    children = ChildSerializer(many=True)
    
    class Meta:
        model = ParentModel
        fields = '__all__'
        depth = 2


But if I try to create a new parent with the following payload:

{
   "name":"parentname",
   "children": [
      {
         "name":"child1name",
      },
     {
         "name":"child2name",
      }
   ]
}

I would get the error that the field parent is required. I am then required to provide a valid id for a parent instance in the db, but when I do so, it is overwritten and set to the id of the newly created parent. The data ends up looking like one would expect, but why is the parent pk required when it is not used?

And is there anything I can do to remedy this?

Lynges avatar Jul 02 '19 08:07 Lynges

Hello @Lynges! Could you please create a PR with failing test? It will help us to debug and fix your issue.

ruscoder avatar Jul 30 '19 17:07 ruscoder

For nested Serializers, you'll probably want to set exclude = (parent,) on the child Meta so as not to require it.

johnthagen avatar Jul 31 '19 00:07 johnthagen

I'm having the same issue...

csdenboer avatar Jul 31 '19 14:07 csdenboer

For nested Serializers, you'll probably want to set exclude = (parent,) on the child Meta so as not to require it.

Serializers should be re-usable stand-alone and nested.

csdenboer avatar Jul 31 '19 14:07 csdenboer

For nested Serializers, you'll probably want to set exclude = (parent,) on the child Meta so as not to require it.

I agree.

For nested Serializers, you'll probably want to set exclude = (parent,) on the child Meta so as not to require it.

Serializers should be re-usable stand-alone and nested.

I'm not sure about how difficult would be implementing automatic excluding for nested related fields, but if someone wants to implement it - welcome.

ruscoder avatar Aug 01 '19 04:08 ruscoder

@ruscoder Hey, pretty new to using your package. But I also encountered this issue. Upon reviewing the source code in a forked branch, I saw that the order of creating the instance and the related instances is actually right.

    def create(self, validated_data):
        relations, reverse_relations = self._extract_relations(validated_data)

        # Create or update direct relations (foreign key, one-to-one)
        self.update_or_create_direct_relations(
            validated_data,
            relations,
        )

        # Create instance
        instance = super(NestedCreateMixin, self).create(validated_data)

        self.update_or_create_reverse_relations(instance, reverse_relations)

        return instance

Related issue in forked branch: https://github.com/geordyvcErasmus/drf-writable-nested/issues/1
(short explanation: I want to create one-to-many reverse related instances when you don't supply a pk)

I am pretty new to creating reusable apps for Django. So if you have any tips on how to contribute, they are always welcome.

geordyvc avatar Dec 16 '19 12:12 geordyvc

For nested Serializers, you'll probably want to set exclude = (parent,) on the child Meta so as not to require it.

@ruscoder @johnthagen I spent a couple of hours debugging and googling until reach this comment. Probably worth it to have it documented on the repo's README as a "known problem with solution". I'll be happy to open a PR adding this if you guys agree.

mchurichi avatar Aug 20 '20 03:08 mchurichi

This is the best way I've found to address this issue and reduce code duplication. If this is ideal, I agree it should be added to the docs.

class Parent(Model):
    name = CharField()


class Child(Model):
    parent = ForeignKey(Parent, related_name='children')
    name = CharField()


class ParentSerializer(WritableNestedModelSerializer):
    children = ChildNestedSerializer(many=True)
    
    class Meta:
        model = Parent
        fields = '__all__'


class ChildSerializer(ModelSerializer):
    """Use this Serializer for an API endpoint directly writing to Child instances."""
    
    # Custom field assignments go here.

    class Meta:
        model = Child
        fields = '__all__'


class ChildNestedSerializer(ChildSerializer):
    """Use this Serializer when creating Child instances through a parent Serializer."""

    # In more complex Serializers, inherits any custom field assignments from 
    # ChildSerializer to avoid duplication.
    # Override custom fields as needed.

    class Meta:
        model = Child
        exclude = ('parent',)

johnthagen avatar Aug 20 '20 11:08 johnthagen