Corrected OpenAPI schema type for DecimalField #7254
Conversation
The API renders DecimalFields as strings. The generated schema now renders decimal fields correctly as strings. Fixes #7253
rest_framework/schemas/openapi.py
Outdated
| 'type': 'number' | ||
| return { | ||
| 'type': 'string', | ||
| 'format': 'decimal' |
There was a problem hiding this comment.
The OpenAPI spec allows for specifying a format despite it not being "official". See OAI/OpenAPI-Specification#845 (comment) for additional discussion.
tests/schemas/test_openapi.py
Outdated
| assert properties['decimal1']['multipleOf'] == .01 | ||
| assert properties['decimal1']['maximum'] == 10000 | ||
| assert properties['decimal1']['minimum'] == -10000 | ||
| assert properties['decimal1'] == {'type': 'string', 'format': 'decimal'} |
There was a problem hiding this comment.
I opted for this style of validation to ensure there are no other properties being erroneously added to the spec, and breaking validation.
|
Hi @clintonb, coerce_decimal_to_string setting determines whether field will be serialized as My suggestion: Do not change to string for all cases. Based on |
|
Good catch @dhaval-mehta. Code updated. |
|
It's worth noting that I am reasonably certain the rendering of |
|
Please review, @carltongibson. |
@clintonb In #7161, I added code for My question is: |
|
Hi @dhaval-mehta: If you have time to prepare a PR for that, super! 🥇 |
Working on it :) |
|
@dhaval-mehta @carltongibson please review |
| if getattr(field, 'coerce_to_string', api_settings.COERCE_DECIMAL_TO_STRING): | ||
| content = { | ||
| 'type': 'string', | ||
| 'format': 'decimal', |
There was a problem hiding this comment.
I went through OAI/OpenAPI-Specification#845 (comment).
I found that the decimal format is supported for number datatype as well.
The rest of the code looks good to me!
There was a problem hiding this comment.
I found that the decimal format is supported for number datatype as well.
I constantly struggle to find answers spelled out clearly and canonically. Do we want to add this?
OAI/OpenAPI-Specification#845 is still WIP, is it actually part of the spec yet?
Inclined to take this PR as it is now, and add more later... Opinions?
| if getattr(field, 'coerce_to_string', api_settings.COERCE_DECIMAL_TO_STRING): | ||
| content = { | ||
| 'type': 'string', | ||
| 'format': 'decimal', |
There was a problem hiding this comment.
I found that the decimal format is supported for number datatype as well.
I constantly struggle to find answers spelled out clearly and canonically. Do we want to add this?
OAI/OpenAPI-Specification#845 is still WIP, is it actually part of the spec yet?
Inclined to take this PR as it is now, and add more later... Opinions?
There was a problem hiding this comment.
OK, let's have it. We can take further tweaks to decimal handling later if there's call for it. Thanks @clintonb. 🥇
Description
The API renders DecimalFields as strings. The generated schema now renders decimal fields correctly as strings.
Fixes #7253