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

Pass additional kwargs to nested serializers

Open philipstarkey opened this issue 2 years ago • 7 comments

I've run into a problem where some custom logic in a nested serializer is falling over because I can't provide custom kwargs (in this case, what I want to pass down is really just a subset of the outer serializers self.context).

I don't think this functionality exists at the moment (could be wrong) based on this: https://github.com/evenicoulddoit/django-rest-framework-serializer-extensions/blob/945e33dae10170e1dcca42c05636b77f300e67b6/rest_framework_serializer_extensions/serializers.py#L649-L658

I think the ideal solution would be to add an additional option to Meta.expandable_fields that is used as the base for the serializer kwargs if it's a dictionary, or if it's a callable, call it passing self as the only arg and use the result of the call as the base for kwargs (so that you can control what gets passed down from one serializer to another with custom, and possibly complex, logic). Does that sound feasible and a good idea?

philipstarkey avatar Oct 22 '21 09:10 philipstarkey

Hi @philipstarkey, sorry for the late reply, was on vacation. I must confess, having lived in Flask-land for such a period of time, I'm returning to this unsure as to what can be achieved through the serializer kwargs. Your proposal sounds completely reasonable though. When you say self, you mean the parent serializer, I suppose? Might you be able to give me an example, through which I'm happy to patch something together and add a test. Seems trivial enough :)

evenicoulddoit avatar Oct 31 '21 15:10 evenicoulddoit

Hi @evenicoulddoit, no worries!

Specifically in my case, I want to pass self.request into the nested serializers for some custom permissions logic, which is passed down through the context as per https://www.django-rest-framework.org/api-guide/serializers/#including-extra-context I'm guessing there are other reasons people would want to do it though.

And yep, my thought was to provide the parent serializer. Ideally I guess you would be able to specify:

class MySerializer(...):
    class Meta:
        expandable_fields = {
            "my_field": {
                ...
                "serializer_kwargs": lambda parent_serializer: {"context": {"request": parent_serializer.context["request"]}}
            }
        }

I'd then expect to be able to access the request from the __init__ (or other) method of any serializer.

Or, if you don't need something that fancy:

class MySerializer(...):
    class Meta:
        expandable_fields = {
            "my_field": {
                ...
                "serializer_kwargs": {"enable_custom_behaviour": False}
            }
        }

(The latter I guess you could achieve by using a different serializer class for your nested serializer, but it's really the first example I care about!)

philipstarkey avatar Nov 01 '21 09:11 philipstarkey

@philipstarkey I'm sorry it took me so long to look into this.

You mentioned specifically the issue of accessing the request context from within a nested serializer. In this branch, I've attempted to create a test to cover that exact situation. I created an expandable model field, and the nested model serializer has some additional fields which both pull from the context successfully.

Did I misinterpret your question?

evenicoulddoit avatar Dec 24 '21 18:12 evenicoulddoit

@evenicoulddoit Sorry for the delay in replying and thanks for creating that test case. That does appear to cover the cases I intended. I'm surprised it worked because I was sure I had tested something similar and it didn't. I've got a deadline for some other stuff coming up, but in a couple of weeks I'll be diving back into DRF/serializer extension things and can investigate more closely to see if the issue was my user error or if there's a bug when additional complexity is present.

philipstarkey avatar Feb 02 '22 04:02 philipstarkey

@philipstarkey perfect, thanks for the update - good to know I understood your issue correctly at least!

evenicoulddoit avatar Feb 02 '22 09:02 evenicoulddoit

I managed to look into this a bit more today. It seems the context is not available in __init__ (even after calling super().__init_). Does the context get attached to the nested serializer at some later point? I was able to access it within a nested serializer method as per your test case.

In addition I realised there may also be a distinction between context available in __init__ during queryset auto optimisation and during response serialization. It would be nice if both cases had access to the full context in __init__ (I'm looking to access request.user in order to do some permission filtering on field querysets...technically I only need this in the serializer instance that handles the response, but it might be nice to have it in the serializer instances created as part of the auto-optimisation routine)

philipstarkey avatar Mar 08 '22 08:03 philipstarkey

Further to this, it seems that DRF itself has the convention that the context is not available in nested serializers until the parent serializer calls field.bind() - and thus context is not generally available inside __init__ in nested serializers.

If you think my request is thus out of scope for this project as it's a DRF convention, I'd be happy to accept that and to close the issue (I have sufficient workarounds in place at the moment)

philipstarkey avatar Mar 09 '22 09:03 philipstarkey