Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass additional kwargs to nested serializers #48

Open
philipstarkey opened this issue Oct 22, 2021 · 7 comments
Open

Pass additional kwargs to nested serializers #48

philipstarkey opened this issue Oct 22, 2021 · 7 comments

Comments

@philipstarkey
Copy link
Contributor

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:

kwargs = dict()
if 'source' in field_definition:
kwargs.update(source=field_definition['source'])
if field_definition.get('many'):
kwargs.update(many=True)
expanded_fields[field_name] = (
field_definition['serializer'](**kwargs)
)

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?

@evenicoulddoit
Copy link
Owner

evenicoulddoit commented Oct 31, 2021

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 :)

@philipstarkey
Copy link
Contributor Author

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!)

@evenicoulddoit
Copy link
Owner

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

@philipstarkey
Copy link
Contributor Author

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

@evenicoulddoit
Copy link
Owner

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

@philipstarkey
Copy link
Contributor Author

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
Copy link
Contributor Author

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)

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

No branches or pull requests

2 participants