Ignore many=True serializer on POST during html form rendering by asedeno · Pull Request #3138 · encode/django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

I've been fighting issue #2918 all afternoon. After finding the (closed!) issue, I started poking at the rest framework code for a simple work-around.

I don't know if this is correct, though it does get me past my problems. This makes get_rendered_html_form ignore the serializer on hand for POST if many=True, and rediscovers what the serializer should be.

Existing tests still pass. Comments welcome.

Going to stay with the assessment here... #2918 (comment)

If this or a similar pull request came with a convincing case of what the behavioral change would be and why it's the right thing to do, then could reconsider.

My use case is a complicated search query where we want to POST the search criteria and return a serialized list of results. The request works just fine until you try to use the browsable API, in which case it crashes rendering the form, having already generated the correct search results.

I'm not sure what else you are looking for. All I'm trying to do is get a use case to not raise exceptions when the correct behavior is possible. The behavioral change here to dynamically generate a serializer in the case that we believe trying to use the existing serializer will raise.

I'm not sure what else you are looking for.

I appreciate that this change meets your particular use case, but it'll also negatively impact the browsable API for folks in other situations (by not including the form input).

Show me a more reasoned "this is what it changes, this is the set of cases it'd fix, this is why we're only doing this for POST" and we might be able to consider it valid, but right now it's just special casing 'POST' and it's not at all clear if or why this'd be the right thing to do.

So right now the exception being thrown is:
TypeError: 'ListSerializer' object is not iterable

How would you feel if instead of special-casing POST with many=True, I caught the TypeError on rendering the form and retried with a dynamically generated serializer. In that case, I'm only affecting broken cases.

Such a commit now exists on the many-post branch I was trying to merge.

That might be okay. No promises, but best thing to do would be to write the smallest possible test case that replicates the issue. Can you do so against a regular view, or does it need to be a viewset with a custom route added? At that point we'll have enough info to make a better judgement on this, and either:

• get the behavior right first time.
• catch and gracefully fallback on error.
• note that it's a case that can't be automatically handled and document it as a constraint, explaining how to work around.

2 participants