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.

Sign up for GitHub

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

Jump to bottom

Fix ChoiceField schema type with empty choices=[] #1240

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix ChoiceField schema type with empty choices=[] #1240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06112e5
Fix ChoiceField schema type with empty `choices=[]`
intgr May 16, 2024
3685aea
Additional testcase
intgr May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions drf_spectacular/plumbing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,13 @@ def build_parameter_type(
def build_choice_field(field) -> _SchemaType:
choices = list(OrderedDict.fromkeys(field.choices)) # preserve order and remove duplicates

if all(isinstance(choice, bool) for choice in choices):
type: Optional[str] = 'boolean'
if field.allow_blank and '' not in choices:
choices.append('')

if not choices:
type = None
elif all(isinstance(choice, bool) for choice in choices):
type = 'boolean'
elif all(isinstance(choice, int) for choice in choices):
type = 'integer'
elif all(isinstance(choice, (int, float, Decimal)) for choice in choices): # `number` includes `integer`
Expand All @@ -432,8 +437,6 @@ def build_choice_field(field) -> _SchemaType:
else:
type = None

if field.allow_blank and '' not in choices:
choices.append('')
if field.allow_null and None not in choices:
choices.append(None)

Expand Down
24 changes: 24 additions & 0 deletions tests/test_plumbing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,30 @@ def test_choicefield_choices_enum():
assert schema['enum'] == ['bluepill', 'redpill', '', None]
assert 'type' not in schema

schema = build_choice_field(serializers.ChoiceField(
choices=[1, 2], allow_blank=True
))
assert schema['enum'] == [1, 2, '']
assert 'type' not in schema
Comment on lines +401 to +405
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
  - string

But that's out-of-scope for this PR.



def test_choicefield_empty_choices():
schema = build_choice_field(serializers.ChoiceField(choices=[]))
assert schema['enum'] == []
assert 'type' not in schema

schema = build_choice_field(serializers.ChoiceField(choices=[], allow_null=True))
assert schema['enum'] == [None]
assert 'type' not in schema

schema = build_choice_field(serializers.ChoiceField(choices=[], allow_blank=True))
assert schema['enum'] == ['']
assert schema['type'] == 'string'

schema = build_choice_field(serializers.ChoiceField(choices=[], allow_blank=True, allow_null=True))
assert schema['enum'] == ['', None]
assert schema['type'] == 'string'


def test_safe_ref():
schema = build_basic_type(str)
Expand Down
Loading