Fix context being ignored in GenericAPIView.get_serializer

[mrtaalebi]

Fix GenericAPIView.get_serializer(self, *args, **kwargs) ignores context in kwargs and replaces it (which may be used by views) with its default context

[xordoquy]

I don't get why there should be some extra context passed as argument to get_serializer rather than override get_serializer_context as shown by the doc

[mrtaalebi]

Well, I have two reasons:

1. It adds more flexibility! For example, consider I've already calculated a variable in my view's get method because I need it there and I want to put it in serializer context, so accepting context helps me not to write the same code in get_serializer_context and avoids duplicate code.
2. I don't get why every other argument is directly passed to the serializer_class and context must be PURGED and removed where it can be passed along with other arguments and must be implemented using get_serializer_context.

[drsantos20]

Hi @mrtaalebi do we have any tests for the new condition?

[rpkilby]

I'm -1 to this change as-is (mixing context from the default with the values passed via kwargs is more confusing to me than not).

That, said, I can understand that it's confusing that the context passed to get_serializer is overwritten by the result of get_serializer_context. We should either do

kwargs.setdefault('context', self.get_serializer_context())

Or, we should raise an error/warning notifying the user that the supplied context is overwritten.

[mrtaalebi]

Hi @rpkilby,

• I think mixing is better than ignoring default context or passed one. As both were given for some reason.
• Also removing default context whenever the context is passed in the kwargs is against integrity.
• On the other hand, mixing them when some keys(exactly speaking of request, format, view) exist in both, may result in further problems.

So... I yield!

But your suggestion about raising an error is great. I've committed the new changes
and it raises an AssertionError whenever a context is passed to get_serializer

[mrtaalebi]

Hi @drsantos20 I am convinced that my previous change could make some problems whenever a developer wants to have request, format or view as a key in context!

And about the last change (AssertionError), it's a test itself and I think it doesn't need a test :D

[rpkilby]

Thanks. Adding to the milestone for review.

[johnnywell]

I would suggest the following:

kwargs.setdefault("context", {}).update(self.get_serializer_context())

[lovelydinosaur]

I really think we ought to use...

if 'context' not in kwargs:
    ...

I don't think we need to deal with "merge the context if one is passed" - if someone's passing an explicit context to the get_serializer method, then it's up to them to include any keys as appropriate.

[mrtaalebi]

Hey Tom!

What you're saying is what came to my mind in the first place and seemed to be very reasonable that I made these changes (and of course fixed typos later in the next commit :P).

But as @rpkilby commented that he's -1 to this change above, I've decided to follow him and changed it to be at least less confusing to anyone facing the behavior (context gets vanished).

So whatever you say! I can keep the last commit or revert it.

[lovelydinosaur]

Superseeded by #7298