Skip to content

Modify ModelSerializer to support per-action field configuration #6842

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

Closed
wants to merge 4 commits into from
Closed

Modify ModelSerializer to support per-action field configuration #6842

wants to merge 4 commits into from

Conversation

gregschmit
Copy link
Contributor

Description

This loosely Refs #4632 but they asked for per-action serializer configuration in ViewSets, however this PR implements a per-action serializer, so the ViewSet would just include one serializer. But this would resolve the underlying problem there.

The general idea is that this PR modifies the ModelSerializer to use a new attribute on the Meta class, action_fields. action_fields is a dictionary of strings that represent actions, and under the action is a dictionary of fields, exclude, and extra_kwargs, that work just like the Meta attributes, except they override the default serializer behavior for that particular action. The previous workaround was to override the get_serializer_class method and use the action to decide which serializer you want to use.

The nice thing about this is that it is backward compatible (everything works the same if you don't have the action_fields attribute), and it means you don't have to build multiple serializers. I noticed that when building multiple serializers for the same model, a lot of logic would be duplicated and it wasn't very DRY.

I did put all this functionality into a 3rd party package at https://github.com/gregschmit/drf-action-serializer, but since it ended up being such a small and backwards compatible change, I decided to see if the DRF community wanted to just pull it into the project.

Also, I did write tests and added a section to the documentation, however this is my first PR for this project and I'm very new to contributing to big open source projects, so please let me know if any of the code/tests/docs that I wrote are crap.

@tomchristie
Copy link
Member

Hey @gregschmit great work on this - all very nicely done.

I don't think we want to add the action_fields API into core. Right now serializers are not coupled into the view behavior in any way, which I think is something we'd like to stick with.

You'd be very welcome to issue a pull request updating the docs to link to the package tho, that would be ace!

Thanks so much for the contribution!

@rpkilby
Copy link
Member

rpkilby commented Jul 29, 2019

Right now serializers are not coupled into the view behavior in any way, which I think is something we'd like to stick with.

Agreed. The view already configures the serializer. No need for the serializer to then derive additional configuration from the view. My recommendation would be to rework this as a view mixin. Instead of the serializer getting the action name from the view, the generic view's get_serializer method can provide the action name to the serializer.

@gregschmit
Copy link
Contributor Author

@rpkilby That's a good idea

@gregschmit gregschmit deleted the gns/action-serializer branch July 30, 2019 13:56
@jpic
Copy link

jpic commented Aug 11, 2019

Actually you can already:

> views.py(72)get_serializer_class()
     71     def get_serializer_class(self):
---> 72         import ipdb; ipdb.set_trace()
ipdb> self.action                                                                                                                                                             
'me'

Thanks for the heads up @rpkilby

BUT that doesn't work on the OPTIONS method:

views.py(75)get_serializer_class()
     74             return InvitationSerializer
---> 75         return super().get_serializer_class()
     76 

ipdb> self.action                                                                                                                                  
'metadata'

You can still use self.request.path.endswith() though

@thinker3
Copy link

thinker3 commented Sep 2, 2019

need swagger support

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