Add missing ApiQuery decorators for Swagger
#1937
Conversation
Pull Request Test Coverage Report for Build 10936715946Details
💛 - Coveralls |
|
Question: How confident are we that these were the only methods with issues, and that other methods are unaffected? |
| @ApiQuery({ name: 'currency', required: true, type: String }) | ||
| @ApiQuery({ name: 'safes', required: true, type: String }) | ||
| @ApiQuery({ name: 'trusted', required: false, type: Boolean }) | ||
| @ApiQuery({ name: 'exclude_spam', required: false, type: Boolean }) | ||
| @Get('safes') | ||
| async getSafeOverview( | ||
| @Query('currency') currency: string, |
There was a problem hiding this comment.
TODO: When parameters are optional, mark them as optional in the code by appending a question mark (?) to the end of the parameter name. This way, Swagger may automatically recognize the parameter as optional. e.g.
| @Query('currency') currency: string, | |
| @Query('currency') currency?: string, |
Implementing this requires additional effort and is outside the scope of the current PR.
There was a problem hiding this comment.
Which parameters are not marked as optional? In this case, currency is required.
There was a problem hiding this comment.
@iamacook I have seen some instances, but here for example excludeSpam. It has a default value though.
There was a problem hiding this comment.
Perhaps we should reconsider usage of the default pipes, and instead assign a default value manually. This would negate the requirement of ApiQuery. I'll merge this for now and we can reconsider this later.
I manually looked through every |
Summary
Resolves #1747
Some optional queries are currently marked a required on Swagger. This adds the missing
ApiQuerydecorators for said queries.Changes
ApiQuerydecorators whereQuerys are present.