beda-software / drf-writable-nested

Writable nested model serializer for Django REST Framework
Other
1.07k stars 116 forks source link

Creating new children on parent create #85

Open Lynges opened 5 years ago

Lynges commented 5 years ago

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?

ruscoder commented 5 years ago

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

johnthagen commented 5 years ago

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

csdenboer commented 5 years ago

I'm having the same issue...

csdenboer commented 5 years ago

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.

ruscoder commented 5 years ago

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.

geordyvc commented 4 years ago

@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.

mchurichi commented 4 years ago

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.

johnthagen commented 4 years ago

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',)