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

Enable setting swagger defaultModelsExpandDepth value #67

Conversation

PhracturedBlue
Copy link

@PhracturedBlue PhracturedBlue commented May 3, 2019

OpenAPI 3.0 by default displays the 'Models' section. This is configurable via the 'defaultModelsExpandDepth' settings.

This patch provides the ability to set this value via the OPENAPI_SWAGGER_UI_DEFAULT_MODELS_EXPAND_DEPTH config

setting OPENAPI_SWAGGER_UI_DEFAULT_MODELS_EXPAND_DEPTH = -1 will disable the 'Models' section entirely

setting OPENAPI_SWAGGER_UI_DEFAULT_MODELS_EXPAND_DEPTH = 0 will collapse the 'Models' section but still display it

setting OPENAPI_SWAGGER_UI_DEFAULT_MODELS_EXPAND_DEPTH = 1 is the default and will display the models section uncollapsed

@lafrech
Copy link
Member

lafrech commented May 3, 2019

Thanks. This looks pretty good!

A few comments:

  • We already expose the SUPPORTED_SUBMIT_METHODS parameter through OPENAPI_SWAGGER_UI_SUPPORTED_SUBMIT_METHODS. I'd like to always use the OPENAPI_SWAGGER_UI prefix for consistency (not just OPENAPI_SWAGGER). You wrote "OpenAPI 3.0". I suppose you meant "swagger-ui".

  • The new parameter should be added to the docs: https://flask-rest-api.readthedocs.io/en/stable/openapi.html.

Going further, rather than adding parameters one at a time each time a user needs one, maybe we could add the whole mapping from https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/configuration.md and write a more generic code. What do you think?

Care should be taken about retro-compatibility. I don't use swagger-ui and I don't know what happens when feeding an old version with a more recent parameter.

@coveralls
Copy link

coveralls commented May 3, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 3741825 on PhracturedBlue:defaultModelsExpandDepth into e4a0260 on Nobatek:master.

@PhracturedBlue
Copy link
Author

Thanks for the feedback. I have updated the code to have consistent variable names. I agree it would be nice to have a more flexible method to maintain swagger parameters, but I'm not sure of an easy way to do that in a backward compatible method. I'll think about it and submit a new patch with ideas on that, but I thin it is beyond the scope of this specific issue.

I'll take a look at the doc updates as well shortly

@lafrech
Copy link
Member

lafrech commented May 3, 2019

Agreed.

Regarding the generic method, we could maintain the mapping in here and just come up with a generic code to 1/ get parameters from config 2/ inject them in template. Ideally, we'd only inject parameters only if specified in config so that the default value is handled in swagger-ui rather than here.

This would be backward compatible.

@PhracturedBlue
Copy link
Author

I checked and this option seems to work with swagger-ui 3.0.20. with anything older than that flask_rest_api doesn't seem to work for me (regardless of whether this option is set)

@lafrech
Copy link
Member

lafrech commented May 7, 2019

This PR is almost good to go.

Could you please add yourself to AUTHORS.rst?

Also, you could squash the 3 first commits of this PR for a nicer history but it's alright if you don't.

I created a separate issue for a generic solution to expose all parameters: #69. If you want to take a stab at it, go for it.

with anything older than that flask_rest_api doesn't seem to work for me

Can you please be more explicit? I believe I checked with 2.x versions when I added the feature. If 2.x doesn't work, it should be fixed or documented.

@PhracturedBlue
Copy link
Author

Sorry for the long delay. I have made the requested changes, and have clarified that this function is available in OpenAPI 3.7.0 and newer. In older versions it is a no-op

@lafrech
Copy link
Member

lafrech commented Jul 16, 2020

Closing as this can now be achieved using OPENAPI_SWAGGER_UI_CONFIG thanks to #171.

@lafrech lafrech closed this Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants