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

Allow unexpected values for ChoiceField/MultipleChoiceField representations. #2839

Closed
shangxiao opened this issue Apr 19, 2015 · 6 comments · Fixed by #2940
Closed

Allow unexpected values for ChoiceField/MultipleChoiceField representations. #2839

shangxiao opened this issue Apr 19, 2015 · 6 comments · Fixed by #2940

Comments

@shangxiao
Copy link
Contributor

shangxiao commented Apr 19, 2015

When an invalid value is present in the database that isn't part of the list of choices (perhaps dealing with a legacy database) a KeyError will be raised in to_representation() of ChoiceField/MultipleChoiceField.

Personally I'd like to see the behaviour more in line with the Django ORM's choices behaviour where the invalid value is still readable, perhaps by simply returning it from to_representation()

In case this is disagreeable, I think the KeyError should be caught and a new exception raised with a more easily debuggable error message explaining which field has the invalid value.

@anatollix
Copy link

anatollix commented May 8, 2015

Totally agree!
It breaks down the system in such cases.
I think simply returning "invalid" value would be the best.

@maryokhin
Copy link
Contributor

maryokhin commented May 9, 2015

Additionally, the current behaviour does not allow to have one set of fields for the output and another for input. Eg. we have a 3 different values possible on the model layer, but I only want to accept 2 of them in the input of a specific serializer (yet return all 3 of them in the output if they're there). Solvable with a custom field, but still a pain.

@tomchristie
Copy link
Member

tomchristie commented May 11, 2015

I think the KeyError should be caught and a new exception raised with a more easily debuggable error message explaining which field has the invalid value.

Happy to accept an issue for this if it's presented as an example with:

  • The current behavior.
  • The suggested behavior.

As far as the remainder of the ticket - it's unclear to me what's being suggested, but it sounds like a case for a custom field, as does @maryokhin's different input and output requirement.

@anatollix
Copy link

anatollix commented May 11, 2015

The key of the current behaviour is difference from Django ORM behaviour.
Using Django ORM you can have in db field values that not presents in field choices.
And this is not rare case!
Your db can be legacy or in the middle of the data migrations or smth else.
But this is not rare.
Using Django you can safety deal with this case - those field still readable.
You just get value as-is.
Using DRF you get not operational api with strange and useless for the first time KeyError exception.

@tomchristie
Copy link
Member

tomchristie commented May 11, 2015

Ah, misunderstood the ticket here I think - so the request is that unexpected values should be passed through as-is to the representation. Sure I've no great problem with that. Happy to accept a pull request modifying the behavior there, and including an example test case.

@tomchristie tomchristie reopened this May 11, 2015
@tomchristie tomchristie changed the title ChoiceField, MultipleChoiceField to_representation() invalid values Allow unexpected values for ChoiceField/MultipleChoiceField representations. May 11, 2015
@anatollix
Copy link

anatollix commented May 14, 2015

Exactly!
Accepted.

@tomchristie tomchristie added this to the 3.1.3 Release milestone May 15, 2015
@xordoquy xordoquy changed the title Allow unexpected values for ChoiceField/MultipleChoiceField representations. Allow unexpected values for ChoiceField/MultipleChoiceField representations. Jun 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants