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

Set the action on a view when override_method regardless of its None-ness #2933

Merged
merged 1 commit into from
May 27, 2015

Conversation

cancan101
Copy link
Contributor

No description provided.

@tomchristie
Copy link
Member

Seems okay. Could you summarize the intent here, as #2062 is a pretty long thread. In what situation does your example get_serializer_class method (which is a pattern I'd advocate for) fail if we don't make this change?

@cancan101
Copy link
Contributor Author

The way the browsable API resolves the form to render is by using override_method the incoming request and changes the method to POST it then takes that requests and calls get_serializer_class on the view. The problem is for a given action if the view has POST support but not get support the override_method leaves the action passed to get_serializer_class as None.

This means that in the special case where the originally resolved action on the GET request was None the get_serializer_class doesn't see what would have been the resolved action when/if the POST request comes in,

@tomchristie tomchristie added this to the 3.1.3 Release milestone May 14, 2015
@tomchristie
Copy link
Member

Probably time for this to go in.

Is there any point in writing a test case for this, or do we consider that overkill, given that it's prob a bit awkward to test for?

Don't think I particularly care either way but happy enough for anyone else to chime in.

@cancan101
Copy link
Contributor Author

Thoughts?

tomchristie added a commit that referenced this pull request May 27, 2015
Set the action on a view when override_method regardless of its None-ness
@tomchristie tomchristie merged commit 95a27a1 into encode:master May 27, 2015
@tomchristie
Copy link
Member

Thoughts?

Merged! 😄

@cancan101 cancan101 deleted the overide_method_action branch January 26, 2016 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants