-
Notifications
You must be signed in to change notification settings - Fork 2
Add Swagger documentation setup and update route parameters in controllers #289
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
base: main
Are you sure you want to change the base?
Changes from all commits
d814809
a9d5753
0403237
a5aa0db
6b959af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1451,12 +1451,12 @@ export class OrganizationController { | |
| return res.status(HttpStatus.OK).send(userInfo); | ||
| } | ||
|
|
||
| @Get(':oid/get_users/:page?') | ||
| @Get(':oid/get_users') | ||
| @UseGuards(JwtAuthGuard, OrganizationRolesGuard, EmailVerifiedGuard) | ||
| @Roles(OrganizationRole.ADMIN) | ||
| async getUsers( | ||
| @Param('oid', ParseIntPipe) oid: number, | ||
| @Param('page', ParseIntPipe) page: number, | ||
| @Query('page', new DefaultValuePipe(1), ParseIntPipe) page: number, | ||
| @Query('search') search: string, | ||
| ): Promise<OrgUser[]> { | ||
| const pageSize = 50; | ||
|
Comment on lines
+1454
to
1462
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this new query syntax for some reason seems to be breaking the tests, I am not sure why but I would maybe try manually testing the endpoint first by opening localhost:3000/organization/users and seeing if the users load up
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I even tried the way listed on the path-to-regexp github, using this |
||
|
|
@@ -1468,12 +1468,12 @@ export class OrganizationController { | |
| return await this.organizationService.getUsers(oid, page, pageSize, search); | ||
| } | ||
|
|
||
| @Get(':oid/get_courses/:page?') | ||
| @Get(':oid/get_courses') | ||
| @UseGuards(JwtAuthGuard, OrganizationRolesGuard, EmailVerifiedGuard) | ||
| @Roles(OrganizationRole.ADMIN) | ||
| async getCourses( | ||
| @Param('oid', ParseIntPipe) oid: number, | ||
| @Param('page', new DefaultValuePipe(-1), ParseIntPipe) page: number, | ||
| @Query('page', new DefaultValuePipe(-1), ParseIntPipe) page: number, | ||
| @Query('search') search: string, | ||
| ): Promise<CourseResponse[]> { | ||
| const pageSize = 50; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use the !isProd() helper for this since iirc something weird is set up with production's setup but isProd() is known to work
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I guess I would wrap all of these new lines of code in the same if statement block, just so that no parts of swagger load in proud just an extra precaution, even though it probably doesn't matter