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(swagger.helper.ts): change query params meta to comply with swagger spec #198

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

DaVarga
Copy link
Contributor

@DaVarga DaVarga commented Aug 1, 2019

Make use of swagger array query params on getMany endpoints for fields filter or sort and
join. Also use integer as type for cache page offset and limit

fix #196

…er spec

make use of swagger array query params on getMany endpoints for 'fields' 'filter' 'or' 'sort' and
'join'. Also use integer as type for 'cache' 'page' 'offset' and 'limit'

fix #196
@michaelyali
Copy link
Member

@Diluka are you good with it?
We still need to change our swagger docs generation (regarding your complaints about English description and html tags) but we still can merge.
What do you think?

Copy link
Contributor

@Diluka Diluka left a comment

Choose a reason for hiding this comment

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

according to #196 (comment)
fields filter or sort there are not multi value params
we don't pass value like this &filter=a||eq||1,b||eq||2

packages/crud/src/crud/swagger.helper.ts Show resolved Hide resolved
@DaVarga
Copy link
Contributor Author

DaVarga commented Aug 6, 2019

@Diluka I think you missed the collectionFormat. The &filter=a||eq||1,b||eq|||2 you mentioned would be collectionFormat: csv used for the fields parameter.
But for join, sort, or and filter I used the format collectionFormat: multi. Which according to swagger translates to "Multiple parameter instances rather than multiple values." e.g. foo=value&foo=another_value

Look at the collectionFormat table at https://swagger.io/docs/specification/2-0/describing-parameters/#array

It also makes me wonder why the API has different approaches to use arrays in url params 🤔

Copy link
Contributor

@Diluka Diluka left a comment

Choose a reason for hiding this comment

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

oh, understood

@ignatvilesov
Copy link

@zMotivat0r @Diluka This PR is critical to generate a correct TS library. Could you please merge this one and share any release date?

@michaelyali michaelyali merged commit 01e9bf4 into nestjsx:master Oct 2, 2019
@ignatvilesov
Copy link

Thank you so much for merging this one @zMotivat0r
Any ideas when it's going to be released? Looking forward to using it in our application.

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.

Swagger getMany query params wrong type on "filter" "or" "sort" "join"
4 participants