Fix ChoiceField schema type with empty choices=[]
#1240
Conversation
| schema = build_choice_field(serializers.ChoiceField( | ||
| choices=[1, 2], allow_blank=True | ||
| )) | ||
| assert schema['enum'] == [1, 2, ''] | ||
| assert 'type' not in schema |
There was a problem hiding this comment.
Note that this is a change in behavior.
Previously this generated {"type": "integer"} because all choices= arguments are int and allow_blank wasn't considered.
I think this is more correct, but I'm open to restoring the old behavior for this.
There was a problem hiding this comment.
According to https://swagger.io/docs/specification/data-models/data-types/#mixed-type, type is expected to be a single value so this looks more correct to me too.
Would be nice to support oneOf someday that would help in this case to have:
oneOf:
- int
- stringBut that's out-of-scope for this PR.
|
Hey 👋 |
| schema = build_choice_field(serializers.ChoiceField( | ||
| choices=[1, 2], allow_blank=True | ||
| )) | ||
| assert schema['enum'] == [1, 2, ''] | ||
| assert 'type' not in schema |
There was a problem hiding this comment.
According to https://swagger.io/docs/specification/data-models/data-types/#mixed-type, type is expected to be a single value so this looks more correct to me too.
Would be nice to support oneOf someday that would help in this case to have:
oneOf:
- int
- stringBut that's out-of-scope for this PR.
|
will review this asap. quite pressed for time right now |
|
sorry guys this took so long to review. thanks for being patient with me. @intgr good change! this |
Currently
ChoiceField(choices=[])will get{"type": "boolean"}in schema. With this change,typewil be omitted from schema.Currently
ChoiceField(chocies=[], allow_blank=True)will generate schema{"type": "boolean", "enum": [""]}, that's a bit nonsensical, as the only enum listed value""is string, not boolean. With this change,{"type": "string", "enum": [""]}will be generated.This PR fixes these edge cases.