Django-rest-framework: Writable nested serializers example is incomplete

[mhubig]

The Writable nested serializers example from the official documentation is incomplete. While it focus on creating an AlbumSerializer that can create nested tracks, it misses the fact that the used TrackSerializer can't be used to create tracks, because it lacks the needed album field! If you dare to add the missing album field to the TrackSerializer you go full circle, since now the AlbumSerializer can no longer create albums.

This leads to a whole lot of confusion as you can see when searching Stackoverflow for 'Writeable Nested Serializers'.

I think this is a very common scenario and there should be at least some information that this behavior is to be expected! A good solution would be to add some explanation why the serializer behaves like this and what can be done about it ...

Steps to reproduce

from django.db import models

class Album(models.Model):
    album_name = models.CharField(max_length=100)
    artist = models.CharField(max_length=100)

class Track(models.Model):
    album = models.ForeignKey(Album, related_name='tracks', on_delete=models.CASCADE)
    order = models.IntegerField()
    title = models.CharField(max_length=100)
    duration = models.IntegerField()

    class Meta:
        unique_together = ('album', 'order')
        ordering = ['order']

    def __unicode__(self):
        return f'{self.order} {self.title}'

from rest_framework import serializers
from models import Track, Album

class TrackSerializer(serializers.ModelSerializer):
    class Meta:
        model = Track
        fields = ('order', 'title', 'duration')

class TrackSerializer2(serializers.ModelSerializer):
    class Meta:
        model = Track
        fields = ('order', 'title', 'duration', 'album')

class AlbumSerializer(serializers.ModelSerializer):
    tracks = TrackSerializer(many=True)

    class Meta:
        model = Album
        fields = ('album_name', 'artist', 'tracks')

    def create(self, validated_data):
        tracks_data = validated_data.pop('tracks')
        album = Album.objects.create(**validated_data)
        for track_data in tracks_data:
            Track.objects.create(album=album, **track_data)
        return album

class AlbumSerializer2(serializers.ModelSerializer):
    tracks = TrackSerializer2(many=True)

    class Meta:
        model = Album
        fields = ('album_name', 'artist', 'tracks')

    def create(self, validated_data):
        tracks_data = validated_data.pop('tracks')
        album = Album.objects.create(**validated_data)
        for track_data in tracks_data:
            Track.objects.create(album=album, **track_data)
        return album

from django.test import TestCase
from django.db.utils import IntegrityError
from serializers import (TrackSerializer, TrackSerializer2,
                         AlbumSerializer, AlbumSerializer2)

class ChapterSerializerTests(TestCase):
    def setUp(self):
        self.album_data = {
            'album_name': 'The Grey Album',
            'artist': 'Danger Mouse',
            'tracks': [
                {
                    'order': 1,
                    'title': 'Public Service Announcement',
                    'duration': 245
                },
                {
                    'order': 2,
                    'title': 'What More Can I Say',
                    'duration': 264
                },
                {
                    'order': 3,
                    'title': 'Encore',
                    'duration': 159
                },
            ],
        }

    def test_create_album_with_nested_tracks(self):
        serializer = AlbumSerializer(data=self.album_data)
        is_valid = serializer.is_valid()
        self.assertDictEqual(serializer.errors, {})
        self.assertTrue(is_valid)
        self.assertTrue(serializer.save())

    def test_create_track_with_album_failes(self):
        album_serializer = AlbumSerializer(data=self.album_data)
        album_serializer.is_valid()
        album = album_serializer.save()

        data = {
            'order': 4,
            'title': 'December 4th',
            'duration': 220,
            'album': album.id
        }
        serializer = TrackSerializer(data=data)
        is_valid = serializer.is_valid()
        self.assertDictEqual(serializer.errors, {})
        self.assertTrue(is_valid)
        error_msg = 'NOT NULL constraint failed: example_track.album_id'
        with self.assertRaisesMessage(IntegrityError, error_msg):
            serializer.save()

    def test_create_track_with_album_using_serializer2(self):
        album_serializer = AlbumSerializer(data=self.album_data)
        album_serializer.is_valid()
        album = album_serializer.save()

        data = {
            'order': 4,
            'title': 'December 4th',
            'duration': 220,
            'album': album.id
        }
        serializer = TrackSerializer2(data=data)
        is_valid = serializer.is_valid()
        self.assertDictEqual(serializer.errors, {})
        self.assertTrue(is_valid)
        self.assertTrue(serializer.save())

    def test_create_album_with_nested_tracks_using_serializer2_fails(self):
        serializer = AlbumSerializer2(data=self.album_data)
        is_valid = serializer.is_valid()
        self.assertDictEqual(serializer.errors, {
            'tracks': [{
                'album': ['This field is required.']
            }, {
                'album': ['This field is required.']
            }, {
                'album': ['This field is required.']
            }]
        })

        self.assertFalse(is_valid)
        error_msg = 'You cannot call `.save()` on a serializer with invalid data.'
        with self.assertRaisesMessage(AssertionError, error_msg):
            serializer.save()

Repo with working code here: https://github.com/mhubig/DRF-Nested-Writeable-Serializer-Bug

[carltongibson]

Hi.

I'm not seeing the issue here. It looks to me as if everything is working as expected.

The example that matters is from test_create_track_with_album_failes(). The section that is relevant is the beginning section:

        album_serializer = AlbumSerializer(data=self.album_data)
        album_serializer.is_valid()
        album = album_serializer.save()

Here album will already have saved tracks. This is the point of overriding AlbumSerializer.create. (Inspect the object, add the assertions, check.) That's job done, no?

Subsequently going on to manually create the tracks is not necessary.

I think this is more a misunderstanding than an _Issue_.

[xordoquy]

A lot of confusion about nested serializer is that everybody would like its use case automagically covered and then has to really think about what needs to be done.
Nested serializers are deceptive. They look like an easy thing but really aren't.

[mhubig]

No sorry @xiaohanyu I disagree! I think the confusion is based on the fact that the documentation says just overwrite create and update and you good to go. But the real issues are coming from other parts of the serializer system like validation and .to_internal_value!

See the example: I overwrite create, than I may realize that I need the ability to add tracks to the existing albums so I try to create a working TrackSerializer and all of a sudden the TrackSerializer can no longer be used as a nested Serializer because I get an Validation error!

And even if I disable validation by overriding .validate() the validation error persists because it comes from .to_internal_value ...

[carltongibson]

@mhubig But in your test case, where you are manually instantiating the serializer, you are not performing a nested write: it's _parallel_, it's next-door. It's outside of the scope of AlbumSerializer.create, so it can't be performed by it.

The behaviour in your examples is exactly what's expected.

You've obviously got a use case where you're trying to do something different. I'm afraid the Issue Tracker isn't the place for that. The discussion group is the best place to take this discussion and other usage questions.

[mhubig]

@carltongibson No please look at the last test case (test_create_album_with_nested_tracks_using_serializer2_fails) this is a nested write, but it fails because of an validation error! But this is wrong! The data is valid, it is exactly the same as in the first test ...

[mhubig]

The only way to get working Track and Album Serializers is to duplicate the TrackSerializer. Which is no problem in the provided example, but in a real live application you may have some deeply nested serializers, which would led to an massive code duplication!

I think there should be a way to overwrite/disable the validation of the data provided to .create() and .update() but adding an .validate() method is not working here ...

(PS.: I try to avoid the discussion group because its not reachable via google search)

[carltongibson]

If you don't like the discussion group you need to try Stack Overflow. The issue tracker is for Issues. We have a strict policy on that, otherwise the project becomes unmaintainable. Sorry.

[mxmaxime]

In summary: in the TrackSerializer (the nested serializer), you have to exclude the field "album" (the primary key), because when you do the insertion its missing, so it's throwing a validation error.

{ "album_name": "The Grey Album", "artist": "Danger Mouse", "tracks": [ { // hmm, where's the "ablum" field? It's missing! -> Validation error. "order": 1, "title": "Public Service Announcement", "duration": 245 } ], }

[c4ys]

On create

Failed:

class TrackSerializer(serializers.ModelSerializer):
    class Meta:
        model = Track
        fields = '__all__'

Success:

class TrackSerializer(serializers.ModelSerializer):
    class Meta:
        model = Track
        exclude = ['album']