django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

BookListSerializer example for create case is still flawed

Open mangelozzi opened this issue 1 year ago • 4 comments

This example at https://www.django-rest-framework.org/api-guide/serializers/#customizing-multiple-update

class BookListSerializer(serializers.ListSerializer):
    def update(self, instance, validated_data):
        # Maps for id->instance and id->data item.
        book_mapping = {book.id: book for book in instance}
        data_mapping = {item['id']: item for item in validated_data}

        # Perform creations and updates.
        ret = []
        for book_id, data in data_mapping.items():
            book = book_mapping.get(book_id, None)
            if book is None:
                ret.append(self.child.create(data))
            else:
                ret.append(self.child.update(book, data))

The update method handles update/insertions/deletes. In the insertion case, because an id key value pair will not exist in validated data, so this line:

 data_mapping = {item['id']: item for item in validated_data}

Will fail at item['id'], maybe it should be item.get('id'), but then maybe the rest of it might have to change too, e.g.:

         for book_id, data in data_mapping.items():
            if not book_id:
                ret.append(self.child.create(data))
            else:
                object = object_mapping.get(book_id, None)
                ret.append(self.child.update(object, data))

mangelozzi avatar Jul 14 '24 10:07 mangelozzi

do you have the proposed fix in mind to share?

auvipy avatar Dec 14 '24 09:12 auvipy

do you have the proposed fix in mind to share?

Yes it's at the end of the above post, is it unclear?

mangelozzi avatar Dec 16 '24 20:12 mangelozzi

i would like to work on this one.

@mangelozzi, we can't use item.get('id') directly, as the key of dict can't be null.

instead we can loop through the validated_data itself and try to get('id') if it is none create new child then and there only, else get from object_mapping and update.

AyushDharDubey avatar Jan 09 '25 18:01 AyushDharDubey

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 19 '25 15:07 stale[bot]