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

Add tags to Swagger OpenAPI schemas #7104

Closed
wants to merge 1 commit into from
Closed

Add tags to Swagger OpenAPI schemas #7104

wants to merge 1 commit into from

Conversation

phuongtai
Copy link

@phuongtai phuongtai commented Dec 25, 2019

Description

My solution is find the first element of a specific path that proper with tags
Example:
PUT, PATCH, PUT, DELETE: /task/{id} -> tags=[task]
POST, GET: /task -> tags=[task]

Please read more at this issue #7103

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 @phuongtai. This looks like a nice improvement.

Can you rebase on master and add a test case asserting that the tags are generated correctly?

@@ -47,11 +47,13 @@ def get_paths(self, request=None):
operation = view.schema.get_operation(path, method)
# Normalise path for any provided mount url.
if path.startswith('/'):
tags = [path.split('/')[1]]
Copy link
Contributor

Choose a reason for hiding this comment

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

@phuongtai @carltongibson This will not work for nested resources.

Let's assume a restaurant management system. We have restaurants. Each restaurant has branches and each branch has inspections. To retrieve single inspection for a branch, following might be a URL:

/restaurants/{restaurant_id}/branches/{branch_id}/inspections/{inspection_id}

Ideally, tags for this path will be inspections but your code will generate restaurants

Realtime Example: Retrieve billing information for a sub-account by zoom.us
In case, the path is /accounts/{accountId}/billing and schema tag is billing not accounts

Copy link
Author

Choose a reason for hiding this comment

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

I understood your problem, I've just resolved the problem which is putting all of APIs by tags default. I removed it and get default tags is a top level of each APIs. So we have to find the better solutions which may be automatically or manually generated.

Copy link
Contributor

@dhaval-mehta dhaval-mehta Feb 2, 2020

Choose a reason for hiding this comment

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

My Idea is to extract the tags from the view-set. Spring Framework is doing the same.
@phuongtai Are you working on it? We also need this feature.

@bergran
Copy link

bergran commented Feb 3, 2020

Hello to everyone! I was looking for this feature either. We can try next:

  • Take from basename as reverse URL from Django function, basename is a required param on router register method.
  • From path Django function I'm not sure about this, but if we could take param name it would be nice!
  • other ways to register endpoints we can set default we discover other cases

** Edit:
can we maybe use docstrings to set it maybe

What do you think?

@phuongtai
Copy link
Author

Hello to everyone! I was looking for this feature either. We can try next:

  • Take from basename as reverse URL from Django function, basename is a required param on router register method.
  • From path Django function I'm not sure about this, but if we could take param name it would be nice!
  • other ways to register endpoints we can set default we discover other cases

** Edit:
can we maybe use docstrings to set it maybe

What do you think?

Thanks for your solution. That sound good but are you sure about working fine with ViewSet?

@carltongibson carltongibson mentioned this pull request Feb 5, 2020
5 tasks
@carltongibson
Copy link
Collaborator

This is a good start but isn't quite right.

I've added an outline of how I think it should go on the issue: #7103 (comment)

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.

4 participants