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

Fix nested serializer schema rendering #6568

Closed
wants to merge 14 commits into from
Closed

Fix nested serializer schema rendering #6568

wants to merge 14 commits into from

Conversation

Lucidiot
Copy link
Contributor

@Lucidiot Lucidiot commented Apr 5, 2019

PR on top of #6532 (OpenAPI generation) by @carltongibson

This fixes a bug with nested serializers where the properties and required properties of the nested Schema object would be nested in a properties property for no reason.

schema:
  properties:
    type: object
    properties: # <-- bad
      properties:
        something:
          type: string
      required:
      - something

This bug was noticed after rendering the ReDoc API docs using apistar, as the docs were showing strange properties inside response samples.

Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Hi @Lucidiot. Would you be able to add a small test example for this case? (Just to help us build the coverage and make sure we avoid a regression later.)

@Lucidiot
Copy link
Contributor Author

Lucidiot commented Apr 5, 2019

@carltongibson Yup, I pushed a test case!

Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Great. Thanks.

@carltongibson
Copy link
Collaborator

Merged in f74d3bf. Thanks @Lucidiot!

@Lucidiot Lucidiot deleted the fix-nested-serializers branch May 13, 2019 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants