Skip to content

Conversation

@feelixe
Copy link

@feelixe feelixe commented Oct 16, 2021

This adds support for initializing serializer with many=True. Works for create, not update.

The current exception when attempting many=True in the master branch is that initial_data is not set.
So basically, I've created a ListSerializer BaseNestedListSerializer. And assign this in BaseNestedModelSerializer in method many_init. The list serializer then iterates over self.get_initial() and sets initial_data on the child serializer. I've tried running this in my project and so far I've not experienced any problems.

I would love some help to review this and if there are some improvements that can be made and what steps need to be taken for a merge.

Example usage (Same as current usage, except if you want to use many=True, you have to instantiate the serializer with that kwarg):

serializers.py

from drf_writable_nested import WritableNestedModelSerializer

class MyModelSerializer(WritableNestedModelSerializer):
    owner = PersonSerializer() # Some nested serializer
    toys = ToySerializer(many=True) # Another nested serializer

    class Meta:
        model = MyModel
        fields = '__all__'

views.py

from rest_framework import viewsets
from .serializers import MyModelSerializer
from .models import MyModel

class MyModelViewset(viewsets.ModelViewSet):
    serializer_class = MyModelSerializer
    queryset = MyModel.objects.all()

    def get_serializer(self, *args, **kwargs):
        if self.action == 'create':
            kwargs['many'] = True
        return super().get_serializer(*args, **kwargs)

Or to allow for sending one or many:

from rest_framework import viewsets
from .serializers import MyModelSerializer
from .models import MyModel

class MyModelViewset(viewsets.ModelViewSet):
    serializer_class = MyModelSerializer
    queryset = MyModel.objects.all()

    def get_serializer(self, *args, **kwargs):
        if self.action == 'create' and isinstance(kwargs['data'], list):
            kwargs['many'] = True
        return super().get_serializer(*args, **kwargs)

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2021

Codecov Report

Merging #151 (9096e78) into master (f7d470f) will decrease coverage by 1.98%.
The diff coverage is 52.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
- Coverage   98.16%   96.18%   -1.99%     
==========================================
  Files           3        3              
  Lines         218      262      +44     
==========================================
+ Hits          214      252      +38     
- Misses          4       10       +6     
Impacted Files Coverage Δ
drf_writable_nested/mixins.py 96.00% <52.94%> (-2.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7d470f...9096e78. Read the comment docs.

@robd003
Copy link

robd003 commented Jun 4, 2022

Any chance @ruscoder or @ir4y could look at this?

@wael-kad
Copy link

wael-kad commented Sep 2, 2022

Does the self.get_initial() in the create of BaseNestedListSerializer work if our main serializer has a PrimaryKeyRelatedField through a Foreign Key? 🤔

Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not also add the code examples as tests to verify the changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants