Consider de-coupling serializer from view
gregschmit opened this issue · 4 comments
Currently the ModelActionSerializer inspects the context->view->action to get the current action. We could create a ViewSet mixin that passes the action directly in the context in the get_serializer_context method, to prevent having ti inspect the view. Obviously since we are version 1.0 we can't break the current interface, but we could add this as a feature.
On second thought, why would the get_serializer_context pass the View as the view attribute of the context if the Serializer was never meant to look at it? I'm starting to think it's very reasonable for the serializer to inspect the action via the view if it's been configured to, so maybe this is a stupid "enhancement".
@rpkilby You initially advised that I write a Viewset mixin to pass the action to the serializer, however I noticed that the view is explicitly provided as context for the serializer. Given that, doesn't it seem sane to let the serializer check rather than having to modify the ViewSet also? It seems like more work for explicitly passing a value that is already passed as an attribute of the view.
doesn't it seem sane to let the serializer check rather than having to modify the ViewSet also?
Sure - it's a tradeoff. The upside to using the context is that the API solely exists on the serializer class. The downside is that your serializer is now coupled to the presence of the view object and specifically the viewset API. If I wanted to use this serializer outside of the request lifecycle (maybe schema generation?), or with generic views, it would be a little more difficult to do so. This is demonstrated a bit in the tests. Instead of just passing an action to the serializer (either as an argument or as part of the context), you have to mock out a view. But then again, it's not super complicated to dig in and figure out what you need to do to make it work. So, ¯\_(ツ)_/¯
Good point; I think I might add an optional action parameter, but still allow the serializer to inspect the view since that's 99% of the use case for this serializer. Also keep in mind that if you don't pass a view, everything works normally with the default serializer fields. I guess I see the per-action field config as a feature that is meant to work with the view, and since the view was already passed, it seemed natural.